View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002283 | OpenFOAM | Bug | public | 2016-10-05 07:19 | 2016-10-09 15:20 |
| Reporter | Assigned To | henry | |||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Platform | linux | OS | opensuse | OS Version | 11.4 |
| Fixed in Version | dev | ||||
| Summary | 0002283: absolute third-party paths in wmkdep, possible false positive on sed | ||||
| Description | In 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. | ||||
| Tags | No tags attached. | ||||
|
|
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? |
|
|
What exactly is the "bug" here? I have not reproduced the reported problem, it seems to work fine for me. |
|
|
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 /home/ofuser/OpenFOAM/OpenFOAM-dev/src/parallel/decompose/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". |
|
|
Problem 2: Similar paths, with identical prefixes. Scenario: 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. |
|
|
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. |
|
|
If not for any other reason, then this is useful at least to keep the ".dep" files smaller and consistent. |
|
|
Resolved by commit a8e62ec9db3b31cd95d6c26e48d99fa5cb29efe3 |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-10-05 07:19 |
|
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 |
