#17685 closed defect (fixed)
Wrong shell test for "sage -n=..."
Reported by: | tmonteil | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.5 |
Component: | scripts | Keywords: | |
Cc: | vbraun, fbissey, rbeezer | Merged in: | |
Authors: | Thierry Monteil | Reviewers: | François Bissey |
Report Upstream: | N/A | Work issues: | |
Branch: | 633f690 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description
As reported on this ask question, the command sage <filename>
is understood as sage -n=...
command if <filename>
contains -n
. This is due to a wrong use of shell regex.
$ sage file-name.py CRITICAL:root:unknown notebook: None Error, notebook must be one of default, ipython, sagenb but got None
Change History (18)
comment:1 Changed 6 years ago by
- Branch set to u/tmonteil/wrong_shell_test_for__sage__n_____
comment:2 Changed 6 years ago by
- Commit set to 633f690b9d5c354529ae60b252360877a07e7c20
- Status changed from new to needs_review
comment:3 Changed 6 years ago by
- Cc vbraun added
comment:4 follow-up: ↓ 16 Changed 6 years ago by
Hello,
Just by curiosity, why did you remove the "
?
Vincent
comment:5 Changed 6 years ago by
- Cc fbissey added
comment:6 Changed 6 years ago by
I have been getting that an awful lot with doctesting of test/cmdline.py. I wonder if I keep getting temporary filename that trigger this. I will eagerly test this.
comment:7 Changed 6 years ago by
- Status changed from needs_review to positive_review
Funky I have been complaining about that since sage 6.4 at least and no one could give me a lead on what was wrong. I give this a positive review.
comment:8 follow-up: ↓ 9 Changed 6 years ago by
- Reviewers set to François Bissey
author name + reviewer name!
comment:9 in reply to: ↑ 8 Changed 6 years ago by
Replying to vdelecroix:
author name + reviewer name!
Yes I should have filled my bits. I know why I always had the problem on my main machine. The host name is part of the temporary file name. My machine hostname is "qcd-nzi3", automatic fail.
I think you cannot have the "
because it would have to be matched with the rest of the expression, I could be wrong about that one.
comment:10 Changed 6 years ago by
Nice work! If you wanted to remove the =~
at the same time (see here) that would be swell, if no one would object to it (improves portability, I would think).
comment:11 Changed 6 years ago by
- Branch changed from u/tmonteil/wrong_shell_test_for__sage__n_____ to 633f690b9d5c354529ae60b252360877a07e7c20
- Resolution set to fixed
- Status changed from positive_review to closed
comment:12 Changed 6 years ago by
- Cc rbeezer added
- Commit 633f690b9d5c354529ae60b252360877a07e7c20 deleted
comment:13 Changed 6 years ago by
I added myself as a cc, and the changes say I deleted a commit? I hope I didn't make a mess here.
comment:14 Changed 6 years ago by
I don't think you deleted anything but now that it is merged it moved to a branch - I think. I won't try to comment on a closed ticket to check the behavior though.
comment:15 Changed 6 years ago by
This has happened to me often enough that I am pretty sure fbissey is right - the next comment even after a push sometimes makes those messages appear (also even on github).
comment:16 in reply to: ↑ 4 Changed 6 years ago by
Replying to vdelecroix:
Just by curiosity, why did you remove the
"
?
If i put a global quote "^-n=.*"
, then everything is escaped so ^
and .*
will not be recognized as special characters (it will recognize strings containing the substring ^-n=.*
). So i would have to do ^"-n=".*
to only escape -n=
. Now, looking at man 7 regex
, it seems that =
is special only if surrounded by some bracket, and -
is special only for intervals like A-Z
. So, since it is not the case, we should be safe without quotes (though i usually like to put lot of quotes in bash, just to be sure), hence i thought "if something goes wrong with that apparently less safe way, then it will be good because then we could help fixing bash."
Replying to kcrisman:
Nice work! If you wanted to remove the
=~
at the same time (see here) that would be swell, if no one would object to it (improves portability, I would think).
It seems that i am a bit late, i did not think the ticket will be merged so fast. Actually, even [[
is a bashism, not only =~
, so it is indeed a good proposition to get rid of this construction. See #17699 for a follow up.
comment:17 Changed 6 years ago by
The issue isn't really "bashisms": the file starts with #!/usr/bin/env bash
, so we're using bash. The issue is with compatibility with ancient versions of bash (pre version 3, I guess) which don't include things like =~
.
comment:18 Changed 6 years ago by
John P. is correct. That said, there probably are plenty of places we could increase portability - see all the Solaris and also other ports where this occasionally caused problems. Even Cygwin where dash
is sometimes the only thing available...
New commits:
#17685 better regex for sage ^-n=.*