2017-06-25 06:17 BST

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0002424OpenFOAMPatchpublic2017-01-28 18:13
Reporterwyldckat 
Assigned Tohenry 
PrioritylowSeveritytrivialReproducibilityN/A
StatusresolvedResolutionfixed 
Product Versiondev 
Target VersionFixed in Versiondev 
Summary0002424: Proposition for a clearer message when OpenFOAM environment is not loaded in the shell
DescriptionFirst of all, I only picked up on this thanks to Alexey Matveichev who pointed it out on this post: https://www.cfd-online.com/Forums/openfoam-installation/182250-something-wrong-allwmake.html#post632112 - post #2

A somewhat common problem with new users is getting the following message when trying to build from source code:

   ./Allwmake:7:.:Can't open /wmake/scripts/AllwmakeParseArguments

Such examples were the aforementioned thread and the report #2202: https://bugs.openfoam.org/view.php?id=2202


This didn't use to happen before the introduction of using this 'AllwmakeParseArguments' script, because the following check would pretty much point out what's going on wrong:

  wmakeCheckPwd "$WM_PROJECT_DIR" || {
      echo "Allwmake error: Current directory is not \$WM_PROJECT_DIR"
      echo " The environment variables are inconsistent with the installation."
      echo " Check the OpenFOAM entries in your dot-files and source them."
      exit 1
  }


Therefore, there are various solutions to this situation, where:

The simplest solution
... is to change the sourcing of the script "AllwmakeParseArguments" to after the checks are complete in these files:

  - Allwmake
  - src/Allwmake
  - applications/Allwmake

The downside is that the script first fails with the message that the 'wmakeCheckPwd' script wasn't found.


Relative sourcing and checking for a loaded environment
After considering various scenarios and alternatives, I ended up with the attached "proposition_v1.tar.gz", which does the following changes:

  - Allwmake
  - src/Allwmake
  - applications/Allwmake

    - in all of these 3, given they mostly require hard-coded relative paths, then instead of sourcing with the full path, the relative path is used instead for sourcing "AllwmakeParseArguments".

    - Also added a comment for the checks.

  - wmake/scripts/AllwmakeParseArguments

    - It now checks if "$WM_PROJECT_DIR" is empty and if it is, then it gives the following output:

      Allwmake error: The OpenFOAM environment is not set.
          Check the OpenFOAM entries in your dot-files and source them.
          If in doubt, please read:
              http://openfoam.org/download/source/setting-environment


The only problem I can see with this is if the 'wmake' isn't where it's meant to be relative to the paths where is each of these 'Allwmake'.
Additional InformationOr overload 'wmakeCheckPwd'?
I also thought of overloading 'wmakeCheckPwd' with an additional option '-e' for environment checking, so that these 'Allwmake' scripts would delegate the current 2/3-step environment check into the script. This also means that the relative path would be used, e.g. in the root 'Allwmake':

   wmake/wmakeCheckPwd -e "$WM_PROJECT_DIR" || exit 1

But:
 - One minor problem with this is that it would turn 'wmakeCheckPwd' into a double-purpose script, which would require renaming it.

 - The other issue is that the relative path would fail and none of the informative messages would be emitted, as originally intended in the current 'Allwmake' scripts.
TagsNo tags attached.
Attached Files
  • gz file icon proposition_v1.tar.gz (2,450 bytes) 2017-01-08 20:54
  • patch file icon proposition_v1.patch (2,800 bytes) 2017-01-08 21:01 -
    diff --git a/Allwmake b/Allwmake
    index 67c26d0..3f6a752 100755
    --- a/Allwmake
    +++ b/Allwmake
    @@ -2,8 +2,9 @@
     cd ${0%/*} || exit 1    # Run from this directory
     
     # Parse arguments for library compilation
    -. $WM_PROJECT_DIR/wmake/scripts/AllwmakeParseArguments
    +. wmake/scripts/AllwmakeParseArguments
     
    +# Perform various checks
     wmakeCheckPwd "$WM_PROJECT_DIR" || {
         echo "Allwmake error: Current directory is not \$WM_PROJECT_DIR"
         echo "    The environment variables are inconsistent with the installation."
    diff --git a/applications/Allwmake b/applications/Allwmake
    index d851254..2b72583 100755
    --- a/applications/Allwmake
    +++ b/applications/Allwmake
    @@ -2,8 +2,9 @@
     cd ${0%/*} || exit 1    # Run from this directory
     
     # Parse arguments for library compilation
    -. $WM_PROJECT_DIR/wmake/scripts/AllwmakeParseArguments
    +. ../wmake/scripts/AllwmakeParseArguments
     
    +# Perform various checks
     wmakeCheckPwd "$WM_PROJECT_DIR/applications" || {
         echo "Allwmake error: Current directory is not \$WM_PROJECT_DIR/applications"
         echo "    The environment variables are inconsistent with the installation."
    diff --git a/src/Allwmake b/src/Allwmake
    index 5e6eedf..29d16a1 100755
    --- a/src/Allwmake
    +++ b/src/Allwmake
    @@ -2,8 +2,9 @@
     cd ${0%/*} || exit 1    # Run from this directory
     
     # Parse arguments for library compilation
    -. $WM_PROJECT_DIR/wmake/scripts/AllwmakeParseArguments
    +. ../wmake/scripts/AllwmakeParseArguments
     
    +# Perform various checks
     wmakeCheckPwd "$WM_PROJECT_DIR/src" || {
         echo "Allwmake error: Current directory is not \$WM_PROJECT_DIR/src"
         echo "    The environment variables are inconsistent with the installation."
    diff --git a/wmake/scripts/AllwmakeParseArguments b/wmake/scripts/AllwmakeParseArguments
    index a985064..0736876 100644
    --- a/wmake/scripts/AllwmakeParseArguments
    +++ b/wmake/scripts/AllwmakeParseArguments
    @@ -2,7 +2,7 @@
     # =========                 |
     # \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
     #  \\    /   O peration     |
    -#   \\  /    A nd           | Copyright (C) 2014-2016 OpenFOAM Foundation
    +#   \\  /    A nd           | Copyright (C) 2014-2017 OpenFOAM Foundation
     #    \\/     M anipulation  |
     #------------------------------------------------------------------------------
     # License
    @@ -34,6 +34,15 @@
     #------------------------------------------------------------------------------
     Script=${0##*/}
     
    +if [ -z "$WM_PROJECT_DIR" ]
    +then
    +    echo "$Script error: The OpenFOAM environment is not set."
    +    echo "    Check the OpenFOAM entries in your dot-files and source them."
    +    echo "    If in doubt, please read:"
    +    echo "       http://openfoam.org/download/source/setting-environment"
    +    exit 1
    +fi
    +
     usage() {
         exec 1>&2
         while [ "$#" -ge 1 ]; do echo "$1"; shift; done
    
    patch file icon proposition_v1.patch (2,800 bytes) 2017-01-08 21:01 +

-Relationships
+Relationships

-Notes

~0007606

wyldckat (updater)

Attached the patch file 'proposition_v1.patch' for easier inspection.

I didn't assign this immediately to @Henry, because I've gotten so confused with the various possibilities, namely:
 - the duplicate messages;
 - the possibility to move them to a single script that checks them all and other possible issues in the future;
 - the enforcement or not of the relative path structure;
 - and the logistics around them all...

... that it felt that it would be better leave this report easier to find for more people to discuss this issue.

~0007687

wyldckat (updater)

I've reviewed my perspectives on this and I believe the v1 patch is acceptable, because:
 - The relative paths are only used in the 3 main 'Allwmake' scripts, which have been as they are for several years now;
 - The changes are minimal and code-duplication that exists is also (mostly) only in these 3 main scripts, which have also been as-is for several years now;

Given it's a busy season right now for everyone, I'm assigning this to @Henry to be merged when possible and if acceptable.

~0007690

henry (manager)

Thanks Bruno

Resolved by commit 84f63ba97930676ac9a13a9d45f49fbae1c25da1
+Notes

-Issue History
Date Modified Username Field Change
2017-01-08 20:54 wyldckat New Issue
2017-01-08 20:54 wyldckat File Added: proposition_v1.tar.gz
2017-01-08 21:01 wyldckat File Added: proposition_v1.patch
2017-01-08 21:01 wyldckat Note Added: 0007606
2017-01-08 21:01 wyldckat Description Updated View Revisions
2017-01-08 21:01 wyldckat Additional Information Updated View Revisions
2017-01-28 17:02 wyldckat Assigned To => henry
2017-01-28 17:02 wyldckat Status new => assigned
2017-01-28 17:09 wyldckat Note Added: 0007687
2017-01-28 18:13 henry Status assigned => resolved
2017-01-28 18:13 henry Resolution open => fixed
2017-01-28 18:13 henry Fixed in Version => dev
2017-01-28 18:13 henry Note Added: 0007690
+Issue History