Opened 3 years ago

Closed 3 years ago

#25293 closed defect (wontfix)

Move CYTHON_CACHE_DIR to a Sage specific directory

Reported by: saraedum Owned by:
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: cython Keywords:
Cc: jdemeyer Merged in:
Authors: Julian Rüth Reviewers:
Report Upstream: N/A Work issues:
Branch: u/saraedum/cycache (Commits, GitHub, GitLab) Commit: 51d813a3c693cf2cdbe5235cf0bf83c256c5bdca
Dependencies: Stopgaps:

Status badges

Description

Currently the Cython cache (disabled) would be written to ~/.cycache and ~/.cython/inline. Since Sage ships its own (possibly patched) version of Cython, we should not mix our cache with the cache of the Cython that might be installed by the distribution or the user.

Change History (12)

comment:1 Changed 3 years ago by saraedum

  • Branch set to u/saraedum/cycache

comment:2 Changed 3 years ago by saraedum

  • Cc jdemeyer added
  • Commit set to 51d813a3c693cf2cdbe5235cf0bf83c256c5bdca
  • Status changed from new to needs_review

jdemeyer: I think you know well how sage-env is supposed to work. Do you think this is the right way of doing this?


New commits:

51d813aSetup CYTHON_CACHE_DIR for when we enable Cython caching again

comment:3 Changed 3 years ago by jdemeyer

I don't agree with this:

the version of Cython that
writes to CYTHON_CACHE_DIR might be incompatible with the (patched) version
shipped with Sage.

If Cython gives the same fingerprint for different Cython versions, that's clearly a bug in Cython.

comment:4 follow-up: Changed 3 years ago by saraedum

The version is part of the fingerprint but any patches that we add are not part of the fingerprint.

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

Replying to saraedum:

The version is part of the fingerprint but any patches that we add are not part of the fingerprint.

Well, then it's still a bug. Where in the upstream code is this check being done?

comment:6 follow-up: Changed 3 years ago by jdemeyer

  • Status changed from needs_review to needs_work

I don't see the need for a Sage-specific cycache directory. We don't do that for ccache either.

comment:7 in reply to: ↑ 5 ; follow-up: Changed 3 years ago by saraedum

Replying to jdemeyer:

Replying to saraedum:

The version is part of the fingerprint but any patches that we add are not part of the fingerprint.

Well, then it's still a bug. Where in the upstream code is this check being done?

I don't think it's a bug. Upstream makes sure that released versions of cython work properly. If you start to patch them, then you are on your own.

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

  • Milestone changed from sage-8.3 to sage-duplicate/invalid/wontfix
  • Status changed from needs_work to needs_review

Replying to jdemeyer:

I don't see the need for a Sage-specific cycache directory. We don't do that for ccache either.

I think we do that for ccache (see the line just above the one I am adding here.) But I am very happy not to introduce a sage-specific cycache. (It was mentioned in https://trac.sagemath.org/ticket/17851 that we should do that.)

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

Replying to saraedum:

I think we do that for ccache

That's CCACHE_BASEDIR which is specifically meant to share caches. It's basically setting the environment prefix such that ccache can ignore it. So it's rather the opposite of what you propose here.

comment:10 in reply to: ↑ 7 Changed 3 years ago by jdemeyer

Replying to saraedum:

I don't think it's a bug. Upstream makes sure that released versions of cython work properly. If you start to patch them, then you are on your own.

I'm not following you here... of course we will make sure that the Cython-in-Sage works correctly.

What I'm asking is the following: is it possible for two different Cython installations that produce different C code to accidentally re-use the same cache file?

  • If yes: it's a Cython bug
  • If no: no need for the Sage-specific cycache directory

I don't know exactly how ccache solves this problem, but it seems to work: you can mix GCC versions and ccache knows the difference.

comment:11 follow-up: Changed 3 years ago by saraedum

I'm not following you here... of course we will make sure that the Cython-in-Sage works correctly.

Say if we backported a patch (without changing Cython's version number - and it is unrealistic to assume that people will remember to change that version number) then it is likely that our Cython and the official Cython behave somewhat differently. If that difference in behaviour is "after" the cache lookup, then you "break" Cython caching in some sense.

But that's an academic discussion. In practice this won't happen.

Is it possible for two released versions of Cython to use the cache incorrectly, ideally not anymore; and sure, if it were then it would be a bug in Cython.

Let's close this issue.

comment:12 in reply to: ↑ 11 Changed 3 years ago by jdemeyer

  • Resolution set to wontfix
  • Status changed from needs_review to closed

Replying to saraedum:

if it were then it would be a bug in Cython.

Yes, that's basically what I have been saying all the time on this ticket.

Note: See TracTickets for help on using tickets.