View Issue Details

IDProjectCategoryView StatusLast Update
0002077OpenFOAM[All Projects] Bugpublic2016-05-08 17:15
ReporterwyldckatAssigned Tohenry 
PrioritylowSeveritytweakReproducibilityhave not tried
Status resolvedResolutionfixed 
Product Versiondev 
Fixed in Versiondev 
Summary0002077: Scripts in the tutorials folder: "\cp" + "\rm" vs "cp" + "rm" + override check
Description(This is partially related to report #2076.)
There are several Allrun and Allclean scripts in OpenFOAM's tutorials folder that rely on the escaped "\rm" and "\cp" commands... but not all of them use this.

AFAIK, this is because there can be script functions or aliases or some other command form that overrides the original intent of these commands. By using the backslash, the command line shell will forgo whatever override that exists and uses the standard "rm" or "cp" commands.

Therefore, we have a few options on this:

  1- Leave as-is, which is a way to continue to help isolate (debug) such scenarios, so that users can figure this out for themselves if they ever trip on this issue.

  2- Switch all "rm" and "cp" commands to using backslash.

  3- Switch all "rm" and "cp" commands to not using backslash and have a few detection lines in the "RunFunctions" and "CleanFunctions" that reports/resets the misuse of "rm" and/or "cp".


Whichever solution you prefer, I can propose the fix for the preferred solution.
TagsNo tags attached.

Activities

henry

2016-04-30 19:50

manager   ~0006210

Personally I think naming aliases or scripts "rm" or "cp" is very unwise and I don't see a particular need to have the backslashes. However it is only a minor inconvenience having them so I don't mind either way. But I agree that we should be consistent whichever approach we use.

wyldckat

2016-05-05 12:36

updater   ~0006235

(For future record: The following is a partial quote of what I sent Henry via email.)

Essentially one of the reasons why people override the default settings for "rm" and "cp" is so that it has the "-i" (interactive) option, to avoid accidental file removal or overwriting.

I need to do some tests, because most of the situations where "rm" and "cp" are overridden is through aliases, but some dash/bash versions drop aliases when going into a script, which would therefore no longer require this feature of using the backslash.

henry

2016-05-05 13:05

manager   ~0006236

This does indeed appear to be the case and I will remove the backslash clutter.

henry

2016-05-05 15:19

manager   ~0006237

Resolved by commit a89cd81aff4193939a31697c463d50b3433a75d9

wyldckat

2016-05-08 17:14

updater   ~0006247

I'm re-opening temporarily this issue only to add a few more details, after looking deeper into this:

  - The backslash was indeed used for overriding aliases: https://en.wikipedia.org/wiki/Alias_%28command%29#Overriding_aliases

  - As far as I can figure out, this manoeuvre was first introduced into OpenFOAM's shell scripts on the 10th of August 2010, on both OpenFOAM 1.7.x and internal development (e.g. seen in OpenFOAM-history): https://github.com/OpenCFD/OpenFOAM-1.7.x/commit/fcb3ae6c419a806f4e4ba0bee12c7abbd771d634

    - Unfortunately the commit comment did not reference why this was added in.

    - After that, all other commits seem to have been as a consequence of copy-paste-adapt.

  - I've done a few tests on Ubuntu 10.04 and CentOS 5.10 and I was not able to reproduce this need for overriding the aliases. If this was a problem, it was possibly only on older Linux Distributions or other OS' and older shells, e.g. Solaris.


I do have the vague memory of having needed this override feature a couple of times, but I can't remember the exact details of how it happened.

It is possible that:

  - There was some bug in the shells at the time and that the aliases were being inherited within the scripts somehow.

  - The only other possibility that I can figure out is if the shell scripts are sourced instead of executed, but that is a safety hazard, at least when taking into account OpenFOAM's current "All*" scripts.

wyldckat

2016-05-08 17:15

updater   ~0006248

sorry, fixing closure...

Issue History

Date Modified Username Field Change
2016-04-30 19:24 wyldckat New Issue
2016-04-30 19:50 henry Note Added: 0006210
2016-05-05 12:36 wyldckat Note Added: 0006235
2016-05-05 13:05 henry Note Added: 0006236
2016-05-05 15:19 henry Note Added: 0006237
2016-05-05 15:19 henry Status new => resolved
2016-05-05 15:19 henry Fixed in Version => dev
2016-05-05 15:19 henry Resolution open => fixed
2016-05-05 15:19 henry Assigned To => henry
2016-05-08 17:14 wyldckat Note Added: 0006247
2016-05-08 17:14 wyldckat Status resolved => feedback
2016-05-08 17:14 wyldckat Resolution fixed => reopened
2016-05-08 17:14 wyldckat Status feedback => closed
2016-05-08 17:14 wyldckat Resolution reopened => fixed
2016-05-08 17:15 wyldckat Note Added: 0006248
2016-05-08 17:15 wyldckat Status closed => feedback
2016-05-08 17:15 wyldckat Resolution fixed => reopened
2016-05-08 17:15 wyldckat Status feedback => resolved
2016-05-08 17:15 wyldckat Resolution reopened => fixed