View Issue Details

IDProjectCategoryView StatusLast Update
0002283OpenFOAM[All Projects] Bugpublic2016-10-09 15:20
ReportermarkAssigned Tohenry 
Status resolvedResolutionfixed 
PlatformlinuxOSopensuseOS Version11.4
Product Version 
Fixed in Versiondev 
Summary0002283: absolute third-party paths in wmkdep, possible false positive on sed
DescriptionIn the wmake rules 'transform', the sed expression is missing a beginning anchor and a trailing slash , which makes it slightly susceptible to a false substitutions.

Additionally, any dependencies within ThirdParty retain their absolute path and are not resolved to $WM_THIRD_PARTY_DIR.

    @$(WMAKE_BIN)/wmkdep -I$(*D) $(LIB_HEADER_DIRS) $< | \
        sed -e 's,^$(WM_PROJECT_DIR)/,$$(WM_PROJECT_DIR)/,' \
            -e 's,^$(WM_THIRD_PARTY_DIR)/,$$(WM_THIRD_PARTY_DIR)/,' > $@

I guess that since these replacements are fixed strings, it would actually be possible to perform the substitution directly within the wmkdep and change the calling option accordingly:
    wmkdep -env WM_PROJECT_DIR -env WM_THIRD_PARTY_DIR ...
and obtain the appropriate values with corresponding getenv values directly within wmkdep. Probably doesn't save much time though.
TagsNo tags attached.



2016-10-05 22:45

updater   ~0006968

OK, I can see that the slash might be needed, because if there are two similar paths, e.g.:

  - .../OpenFOAM-dev
  - .../OpenFOAM-dev-mine

but if the code in the latter is some other custom code, this results in the incorrect filter of "$(WM_PROJECT_DIR)-mine".

As for the start of the path... ah, if we have something like this:

  - /opt/OpenFOAM-dev
  - /usr/local/opt/OpenFOAM-dev

and the custom code is at the latter path, then the filter results in "/usr/local$(WM_PROJECT_DIR)".

Both scenarios can lead to problems, when we share the code between two nearly identical machines, but with different working paths.

Applying the filter for the ThirdParty is matter of consistency, for the same reason: portability between compatible machines, which use different paths.

Furthermore, this can save up disk space and a bit of parsing time (when GNU Make processes the .dep files).

@Henry: Can this bug fix be applied to OpenFOAM-dev as-is, or is it preferable to make this built-in into wmkdep?


2016-10-05 23:17

manager   ~0006970

What exactly is the "bug" here? I have not reproduced the reported problem, it seems to work fine for me.


2016-10-08 21:19

updater   ~0006988

Let's see if I'm able to be concise and provide a few good practical examples, given that this can easily affect me sometime in the near future as well.

Problem 1: "WM_THIRD_PARTY_DIR" is not parsed and replaced for.

Steps for an example scenario to reproduce problem:

  1- I build "OpenFOAM-dev" at "/home/bruno/OpenFOAM/OpenFOAM-dev" on my workstation, along with "/home/bruno/OpenFOAM/ThirdParty-dev".

  2- Then I "rsync" or tarball the whole two folders with the existing builds onto another machine that has the same exact Linux Distribution and architecture, e.g. a Raspberry Pi or an Amazon EC2 free instance.

  3- There the default user-name is not the same, therefore I have to place it at for example "/home/pi/OpenFOAM/OpenFOAM-dev" and "/home/pi/OpenFOAM/ThirdParty-dev".

  4- Now, the objective was to send everything already pre-built, so that I could have relatively quick build times (takes 8x to 20x longer on those two types of machines). But I might need to do some additional tweaks and tests, so it would be easier to build only the modified files.

  5- Nonetheless, if I run "./Allwmake" at "OpenFOAM-dev" to do a 1-5 minute check to see if everything is ready for testing the build tweaks... for an Ubuntu/Debian distribution (several system packages already available), it will stop the build at "src/parallel/decompose/Allwmake" with the following error:

    + wmake scotchDecomp
    make: *** No rule to make target '/home/bruno/OpenFOAM/ThirdParty-dev/platforms/linux64GccDPInt64/scotch_6.0.3/include/scotch.h', needed by '/home/ofuser/OpenFOAM/OpenFOAM-dev/platforms/linux64GccDPInt64Opt/src/parallel/decompose/scotchDecomp/scotchDecomp.C.dep'. Stop.

  6- The reason for this was that the ".dep" file was hard-coded to the original "ThirdParty-dev" path, namely the one at "/home/bruno".

Solution for problem 1: Also handle parsing for "WM_THIRD_PARTY_DIR" in the file "wmake/rules/General/transform".


2016-10-08 21:41

updater   ~0006989

Problem 2: Similar paths, with identical prefixes.


  1- While using the installation environment from the scenario for Problem 1, I want to do some out-of-tree tests+development for a new library or utility. And for documentation/collaboration reasons back at the office, the name of the new path is "OpenFOAM-dev-testing", because "OpenFOAM-4.x-testing" is being done as well.

  2- If the installation is brought from the workstation where the path "/home/bruno/OpenFOAM-dev-testing" was used, it will work just fine on the test machine, because the ".dep" file has the replaced path "/home/bruno/OpenFOAM-dev-testing/..." as "$(WM_PROJECT_DIR)-testing/...".

  3- The problem comes up if we also have another test/development machine that has "OpenFOAM-dev" installed at "/opt/OpenFOAM/OpenFOAM-dev" as a pre-built installation, but since I have the folder at "/home/ofuser/OpenFOAM-dev-testing", given that I can't touch "/opt" on that machine... the result is that the ".dep" file looks for "$(WM_PROJECT_DIR)-testing", which equates to "/opt/OpenFOAM/OpenFOAM-dev-testing".

Solution for problem 2: Ensure that only the exact path is replaced, namely by using "^" and "/" in the sed command in the same "wmake/rules/General/transform" file, to enforce that only the correct path is found.

Caveat of this scenario: Sorry, it's not a very good scenario, because this still requires me to hack the ".dep" files for making the path neutral, if I don't want to have to fully rebuild again the testing source code on the 3rd machine.


2016-10-08 21:46

manager   ~0006990

So far I have been unable to reproduce the issues and see no reason to make this change. However if it does not break anything I can put it in if you think it is necessary.


2016-10-08 21:52

updater   ~0006992

If not for any other reason, then this is useful at least to keep the ".dep" files smaller and consistent.


2016-10-09 15:20

manager   ~0006995

Resolved by commit a8e62ec9db3b31cd95d6c26e48d99fa5cb29efe3

Issue History

Date Modified Username Field Change
2016-10-05 07:19 mark New Issue
2016-10-05 22:45 wyldckat Note Added: 0006968
2016-10-05 23:17 henry Note Added: 0006970
2016-10-08 21:19 wyldckat Note Added: 0006988
2016-10-08 21:41 wyldckat Note Added: 0006989
2016-10-08 21:46 henry Note Added: 0006990
2016-10-08 21:52 wyldckat Note Added: 0006992
2016-10-09 15:20 henry Assigned To => henry
2016-10-09 15:20 henry Status new => resolved
2016-10-09 15:20 henry Resolution open => fixed
2016-10-09 15:20 henry Fixed in Version => dev
2016-10-09 15:20 henry Note Added: 0006995