Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13681 closed defect (fixed)

fix issues with SAGE_TMP

Reported by: vbraun Owned by: leif
Priority: blocker Milestone: sage-5.4
Component: scripts Keywords:
Cc: jdemeyer Merged in: sage-5.4.rc4
Authors: Jeroen Demeyer Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #13631 Stopgaps:

Status badges

Description (last modified by vbraun)

#13579 changed SAGE_TMP. There are some places with less-than-ideal handling of SAGE_TMP that are broken in the process.

  • sage-cleaner hardcodes '.../temp/...' but thats not correct any more.
  • Many doctests use SAGE_TMP+'foo.bar'. The new SAGE_TMP doesn't end in a backslash any more. Using os.path.join would have saved the day.

Follow-up: #13147

Apply

Attachments (3)

13681_scripts.patch (1.3 KB) - added by jdemeyer 8 years ago.
13681_sagelib.patch (22.5 KB) - added by jdemeyer 8 years ago.
trac_13681_root.patch (714 bytes) - added by vbraun 8 years ago.
Initial patch

Download all attachments as: .zip

Change History (19)

comment:1 Changed 8 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Milestone changed from sage-5.5 to sage-5.4
  • Priority changed from major to blocker

Changed 8 years ago by jdemeyer

comment:2 follow-up: Changed 8 years ago by jhpalmieri

Would you consider merging #13147 into 5.4, and then rebasing this on top of that?

comment:3 in reply to: ↑ 2 ; follow-up: Changed 8 years ago by jdemeyer

Replying to jhpalmieri:

Would you consider merging #13147 into 5.4, and then rebasing this on top of that?

Why? I don't see the advantage.

comment:4 follow-up: Changed 8 years ago by jdemeyer

I could incorporate all the changes from #13147 except for the "lazy string" part.

Changed 8 years ago by jdemeyer

comment:5 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:6 Changed 8 years ago by jdemeyer

  • Status changed from new to needs_review

comment:7 in reply to: ↑ 4 Changed 8 years ago by jdemeyer

Replying to jdemeyer:

I could incorporate all the changes from #13147 except for the "lazy string" part.

Done.

comment:8 in reply to: ↑ 3 Changed 8 years ago by jhpalmieri

Replying to jdemeyer:

Replying to jhpalmieri:

Would you consider merging #13147 into 5.4, and then rebasing this on top of that?

Why? I don't see the advantage.

One advantage would have been so #13147 wouldn't need rebasing, and it also solved some of the same issues. But now I guess it will need rebasing anyway.

comment:9 follow-up: Changed 8 years ago by vbraun

Doctesting still litters files around than don't get deleted because SAGE_TESTDIR is not a the expected DOT_SAGE/tmp/hostname/pid/.... I'll attach a patch to fix this.

Positive review to all the other patches.

Changed 8 years ago by vbraun

Initial patch

comment:10 Changed 8 years ago by vbraun

  • Description modified (diff)
  • Reviewers set to Volker Braun

comment:11 in reply to: ↑ 9 Changed 8 years ago by jdemeyer

Replying to vbraun:

Doctesting still litters files around than don't get deleted because SAGE_TESTDIR is not a the expected DOT_SAGE/tmp/hostname/pid/....

Is this a regression due to #13579? If it's not, I prefer to leave this change for a different ticket.

comment:12 Changed 8 years ago by jdemeyer

Volker, could you agree with not merging your patch here? I feel that the chance of stuff breaking is too big to risk merging it.

comment:13 Changed 8 years ago by vbraun

  • Status changed from needs_review to positive_review

I'll attach the SAGE_TESTDIR stuff to the followup ticket, then.

comment:14 Changed 8 years ago by vbraun

  • Description modified (diff)

comment:15 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.4.rc4
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:16 Changed 8 years ago by jdemeyer

  • Dependencies set to #13631
Note: See TracTickets for help on using tickets.