View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002077 | OpenFOAM | Bug | public | 2016-04-30 19:24 | 2016-05-08 17:15 |
Reporter | wyldckat | Assigned To | henry | ||
Priority | low | Severity | tweak | Reproducibility | have not tried |
Status | resolved | Resolution | fixed | ||
Product Version | dev | ||||
Fixed in Version | dev | ||||
Summary | 0002077: 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. | ||||
Tags | No tags attached. | ||||
|
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. |
|
(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. |
|
This does indeed appear to be the case and I will remove the backslash clutter. |
|
Resolved by commit a89cd81aff4193939a31697c463d50b3433a75d9 |
|
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. |
|
sorry, fixing closure... |
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 |