Opened 8 years ago
Closed 8 years ago
#16827 closed defect (fixed)
Use atomic_write in sage-preparse
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-6.4 |
Component: | scripts | Keywords: | |
Cc: | Merged in: | ||
Authors: | Jeroen Demeyer | Reviewers: | Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | 8601fdd (Commits, GitHub, GitLab) | Commit: | 8601fddda9fd3526b7c68c9e1d07485e70e759db |
Dependencies: | Stopgaps: |
Description
To avoid race conditions, the script sage-preparse
should use atomic_write
.
Change History (7)
comment:1 Changed 8 years ago by
- Branch set to u/jdemeyer/ticket/16827
- Created changed from 08/14/14 11:50:12 to 08/14/14 11:50:12
- Modified changed from 08/14/14 11:50:12 to 08/14/14 11:50:12
comment:2 Changed 8 years ago by
- Commit set to 8601fddda9fd3526b7c68c9e1d07485e70e759db
- Status changed from new to needs_review
comment:3 Changed 8 years ago by
- Reviewers set to Volker Braun
- Status changed from needs_review to positive_review
comment:4 Changed 8 years ago by
I know this isn't part of this ticket, but there is a long-standing problem with preparsing that should be easy to fix: if you happen to give a file the same name as a Python module (which is not hard to do; I've seen people use "new.sage" and "socket.sage" before), then the preparsed file can shadow the actual Python module. I think a patch like this ought to fix it:
-
src/bin/sage-preparse
diff --git a/src/bin/sage-preparse b/src/bin/sage-preparse index e2d5e9c..dff9800 100755
a b def do_preparse(f, files_before=[]): 86 86 print "%s: Unknown file type %s"%(sys.argv[0], f) 87 87 sys.exit(1) 88 88 89 fname = '%s .py'%f[:-5]89 fname = '%s_preparsed.py'%f[:-5] 90 90 if os.path.exists(fname): 91 91 if AUTOGEN_MSG not in open(fname).read(): 92 92 print "Refusing to overwrite existing non-autogenerated file '%s'."%os.path.abspath(fname) -
src/bin/sage-run
diff --git a/src/bin/sage-run b/src/bin/sage-run index 3f2700b..638c770 100755
a b if fn.startswith('-'): 20 20 if fn.endswith('.sage'): 21 21 if call(['sage-preparse', fn]) != 0: 22 22 sys.exit(1) 23 os.execvp('sage-python', ['sage-python', fn[:-5] + ' .py'] + opts)23 os.execvp('sage-python', ['sage-python', fn[:-5] + '_preparsed.py'] + opts) 24 24 elif fn.endswith('.pyx') or fn.endswith('.spyx'): 25 25 os.execvp('sage-run-cython', ['sage-run-cython', fn] + opts) 26 26 else:
Alternatively, there is another approach at #11821, which also might prevent the race conditions discussed here: don't write the preparsed output to a file, just use standard output.
comment:5 Changed 8 years ago by
John, can you move you comment to a new ticket? I'm going to merge and close this ticket now.
comment:6 Changed 8 years ago by
New ticket: #16955
comment:7 Changed 8 years ago by
- Branch changed from u/jdemeyer/ticket/16827 to 8601fddda9fd3526b7c68c9e1d07485e70e759db
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Use atomic_write in sage-preparse