View Issue Details

IDProjectCategoryView StatusLast Update
0002424OpenFOAMPatchpublic2017-01-28 18:13
ReporterwyldckatAssigned Tohenry 
PrioritylowSeveritytrivialReproducibilityN/A
Status resolvedResolutionfixed 
Product Versiondev 
Fixed 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.

Activities

wyldckat

2017-01-08 20:54

updater  

proposition_v1.tar.gz (2,450 bytes)

wyldckat

2017-01-08 21:01

updater   ~0007606

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.

proposition_v1.patch (2,800 bytes)
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
proposition_v1.patch (2,800 bytes)

wyldckat

2017-01-28 17:09

updater   ~0007687

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.

henry

2017-01-28 18:13

manager   ~0007690

Thanks Bruno

Resolved by commit 84f63ba97930676ac9a13a9d45f49fbae1c25da1

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