View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0002091OpenFOAMBugpublic2016-05-15 01:382016-10-12 18:27
Reporterwyldckat Assigned Tohenry  
PrioritylowSeverityminorReproducibilitysometimes
Status resolvedResolutionfixed 
Product Versiondev 
Summary0002091: Recent commits revealed that "Allwmake -update" is flawed for application updates
DescriptionThe recent commits that have been made for "foamToVTK" and the replacement of "execFlowFunctionObjects", revealed that using "./Allwmake -update" in the main project folder was not successful in sorting out all dependency files.

For example, in "foamToVTK", the file "vtkMesh.H" was moved to the new library "foamToVTK". Problem is that the "foamToVTK.C.dep" file still relied on this file and "wmake" would break the build because this dependency file depended on the moved file.

Similarly, the same happened to the solvers where "createControls.H" was renamed to "createControl.H".

After some debugging and studying how GNU Make 'gets things done', I figured out that the problem is that the pattern for "%.dep" is only used if there isn't already a respective rule for a "*.dep" file. Which lead me to the file "wmake/makefiles/general", which loads the "*.dep" files whenever they already exist (if the build target isn't "lnInclude"), therefore having priority over the pattern.
This was why the build would stop without updating the dependency files, given that it was already assuming the files were valid.

Attached is the tarball "update_dep_proposition_v1.tar.gz", which has the following modifications:

  - wmake/makefiles/files
    - It only has a typo fixed in one of the comments.

  - wmake/makefiles/general
    - Has a couple of typos fixed.
    - Has a new target named "updatedep", which depends on "dep".
    - Has an additional string lookup for "updatedep", so that it does not include the "*.dep" files if this target is used when calling "make".
      - The reason for this is because the "findstring" command will easily find "dep" in any other targets, since it's a common syllable for file names, in case make is called to build a specific file.

  - wmake/scripts/AllwmakeParseArguments
    - When the option "-update" is used, "WM_UPDATE_DEPENDENCIES" is exported with a value, so that it can be picked up later on by "wmake".

  - wmake/wmake
    - Added description for the new target "updatedep".
    - Added special handling of the "WM_UPDATE_DEPENDENCIES" environment variable, where it will call "make" with "updatedep", along with the default build options that are on the last call to "make".


The reason why I ended up proposing this structure is because calling "wmake updatedep" from within "wrmdep" was too prone to failure.


In addition, with this update convention, it might be possible to drop the option "-update" from "wrmdep", given that this new build option should be able to sort out these issues with the dependency files from now on. But I haven't finished testing to confirm if this is true or not.
TagsNo tags attached.

Activities

wyldckat

2016-05-15 01:38

updater  

update_dep_proposition_v1.tar.gz (6,246 bytes)

wyldckat

2016-05-15 01:55

updater   ~0006272

I've done a quick test with using OpenFOAM 3.0.x build structure from the "platforms" directory on the "platforms" in OpenFOAM-dev (namely by having the outdated object and dependency files) and the conclusion is that "wrmdep -update" is still needed, for example to catch that this symbolic link is no longer valid: "src/OpenFOAM/lnInclude/StaticAssert.H"

henry

2016-05-15 18:29

manager   ~0006276

Thanks, I am testing these improvements now and so far it is working well.

henry

2016-05-16 12:27

manager   ~0006277

Thanks for the your work on resolving this outstanding irritation with the handling of out-of-date '.dep' files, I think it fixes the problem.

I have pushed your changes:

commit 151631c5e42254252ffeee63dc52a7f159a07fbe

for further testing.

henry

2016-05-17 17:00

manager   ~0006284

Currently the 'updatedep' option is rather noisy:

Purging all dep files that relate to files that no longer exist...
Purging from 'src': OutputFilterFunctionObject.C
wrmdep error: could not find Make directory
Purging from 'applications': OutputFilterFunctionObject.C
wrmdep error: could not find Make directory
Purging from 'src': OutputFilterFunctionObject.H
wrmdep error: could not find Make directory
Purging from 'applications': OutputFilterFunctionObject.H
wrmdep error: could not find Make directory
Purging from 'src': OutputFilterFunctionObjectI.H
wrmdep error: could not find Make directory
Purging from 'applications': OutputFilterFunctionObjectI.H
wrmdep error: could not find Make directory
.
.
.

and not yet reliable:

+ wmake libso ptscotchDecomp
/shares/meredithk/OpenFOAM/OpenFOAM-dev/src/dummyThirdParty/ptscotchDecomp
make: Nothing to be done for `updatedep'.
make: *** No rule to make target `/shares/meredithk/OpenFOAM/OpenFOAM-dev/src/parallel/decompose/ptscotchDecomp/lnInclude/ptscotchDecompTemplates.C', needed by `/shares/meredithk/OpenFOAM/OpenFOAM-dev/platforms/linux64GccDPInt32Opt/src/dummyThirdParty/ptscotchDecomp/dummyPtscotchDecomp.C.dep'. Stop.

wyldckat

2016-05-17 17:11

updater   ~0006285

The initial noise is coming from "wrmdep -update"... the verbosity is useful for debugging, given that in this case it's trying to access deleted folders, but I guess it should be hidden behind a "-verbose" option for "wrmdep" and/or Allwmake.


As for the reliability issue, I didn't have at the time of the tests an example with these kinds of changes on a library and the tests were mostly oriented to changes made in applications (i.e. no "lnInclude" to use as reference). In this specific case, I'll have to take a look into it after you've made the push of the changes that seem to have been made to "ptscotchDecomp".

henry

2016-05-17 17:50

manager   ~0006286

The changes to "ptscotchDecomp" come from

commit 1441f8cab064a79b65e0dc7c80e86b8c45dc2f80

Date: Sun May 15 16:36:48 2016 +0100

henry

2016-05-17 17:55

manager   ~0006287

I think it is OK to have the

Purging from 'src': OutputFilterFunctionObject.C

messages but it would be good to avoid all the "error" messages

wrmdep error: could not find Make directory

which in this context are not errors and not relevant. The problem is that these messages are generated by a function which is also called when the missing Make directory would be considered an error so either the relevant functions would need to be duplicated without the message or some refactoring will be required.

MattijsJ

2016-05-20 11:33

reporter   ~0006298

Last edited: 2016-05-20 11:37

 Small one: wrmdep does not work for files that are qualified with $WM_MPLIB, e.g. ptscotchDecomp.H. The wrmdep (and I guess wmdep) do not search in that subdirectory in the platforms directory.

Apart from that really nice functionality!

2) wish: wrmdep print the name of the dep file it has deleted (instead of the search string)

henry

2016-05-21 17:21

manager   ~0006304

2) commit 57f3ac27735f3d6407d0a5f639ff4007d5a28731
   wrmdep: Now prints the full path of the .dep files removed

MattijsJ

2016-06-03 10:52

reporter   ~0006363

./Allwmake -update does not handle whole source directories being removed (I think). E.g. when the src/postProcessing directory disappeared it would not delete the dependencies (wrmdep) referring to those directories (specifically the forces.H)

wyldckat

2016-06-10 21:52

updater  

update_dep_proposition_v2.tar.gz (4,766 bytes)

wyldckat

2016-06-10 21:56

updater   ~0006428

The attached file 'update_dep_proposition_v2.tar.gz' provides the following files and respective updates:

  - src/parallel/decompose/Allwclean

     - This file is new and I've added it after thinking on what Mattijs commented about the variant folders '$WM_OPTIONS$WM_MPLIB'.

     - Without this script, "wclean all" will not find/clean these libraries up on its own.


  - wmake/scripts/AllwmakeParseArguments

     - The changes were made only to when using "-update":

       1. It's also necessary to call "wrmdep -old" for deleting "dep" files that no longer have corresponding source code.

       2. It's also necessary to call "wclean empty", for taking out empty folders that may be remnant, in case "git checkout" doesn't take them out on its own. And also because I had to upgrade the "wclean empty" feature.

       3. Added a few more comments to describe in more detail the various update steps.

       4. Updated "-help" information for the modified "-update" option.


  - wmake/scripts/wmakeFunctions

     - Based on one of Mattijs' comments, I've updated the "depToSource()" script function to also remove the "${WM_OPTIONS}${WM_MPLIB}" paths.


  - wmake/wclean

     - It now sources "wmakeFunctions", because it now needs a few functions from it.

     - The "empty" target has been upgraded to also search and remove object folders that no longer relate to source code, as well as taking out the respective binary file (application or library), based on the details written in the "variables" file.

     - This update to the "empty" target is now a bit more verbose, to given a better context and feedback to the user... which makes me wonder: should we start satisfying the "-s" option to be silent?


Hopefully this takes care of most of the "cleaning up when updating" issues. I've tested with a big jump between commits (17th of May vs earlier today), but this can still use more testing.


The only thing I can think of that can be considered as _missing_ is the ability to also take out object files that don't have the related source code files... but since these don't impair the build and only occupies a bit of disk space (er, as long it's not debug objects...), I haven't bothered hunting them as well (at least not yet).

henry

2016-06-11 16:32

manager   ~0006432

Thanks Bruno, I am testing this patch at the moment and so far it looks file.

See commit 07e5f2831b85ff7302b514e55ef128b58b322eec

Issue History

Date Modified Username Field Change
2016-05-15 01:38 wyldckat New Issue
2016-05-15 01:38 wyldckat Status new => assigned
2016-05-15 01:38 wyldckat Assigned To => henry
2016-05-15 01:38 wyldckat File Added: update_dep_proposition_v1.tar.gz
2016-05-15 01:55 wyldckat Note Added: 0006272
2016-05-15 18:29 henry Note Added: 0006276
2016-05-16 12:27 henry Note Added: 0006277
2016-05-17 17:00 henry Note Added: 0006284
2016-05-17 17:11 wyldckat Note Added: 0006285
2016-05-17 17:50 henry Note Added: 0006286
2016-05-17 17:55 henry Note Added: 0006287
2016-05-20 11:33 MattijsJ Note Added: 0006298
2016-05-20 11:37 MattijsJ Note Edited: 0006298
2016-05-21 17:21 henry Note Added: 0006304
2016-06-03 10:52 MattijsJ Note Added: 0006363
2016-06-10 21:52 wyldckat File Added: update_dep_proposition_v2.tar.gz
2016-06-10 21:56 wyldckat Note Added: 0006428
2016-06-11 16:32 henry Note Added: 0006432
2016-10-12 18:27 henry Status assigned => resolved
2016-10-12 18:27 henry Resolution open => fixed