#20717 closed defect (fixed)
elliptic_curves upstream tarball contains OSX extended attribute files
Reported by: | embray | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | sage-8.8 |
Component: | packages: standard | Keywords: | upgrade |
Cc: | cremona, ohanar, spice, jdemeyer, embray | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | Erik Bray |
Report Upstream: | N/A | Work issues: | |
Branch: | 26fedcb (Commits, GitHub, GitLab) | Commit: | 26fedcbc493d2d478f9d1c1609160bc00bb7fc14 |
Dependencies: | Stopgaps: |
Description (last modified by )
Every file or directory in the upstream tarball for elliptic_curves contains a corresponding ._<filename>
file containing OSX-specific "extended attributes" for the file. Those should be stripped out when creating the tarball.
Is this just a Sage issue or a problem upstream as well? It's not always clear to me since I'm not sure if/when we're using unmodified source tarballs.
I have made a new tarball here:
http://chapoton.perso.math.cnrs.fr/elliptic_curves-0.8.1.tar.bz2
Change History (30)
comment:1 Changed 6 years ago by
- Cc cremona added
- Component changed from build to packages: standard
comment:2 follow-up: ↓ 3 Changed 6 years ago by
This package has nothing to do with me despite what it says in the SPKG.txt file.
I think the cremona_mini is the standard package which comes for free; again I did not create it and have never touched it. I only touch the real one called database_cremona_ellcurve*. The other part of it is something William put in years ago.
comment:3 in reply to: ↑ 2 Changed 6 years ago by
Replying to cremona:
This package has nothing to do with me despite what it says in the SPKG.txt file.
Fine. Let's fix the SPKG.txt
file then...
I only touch the real one called database_cremona_ellcurve*. The other part of it is something William put in years ago.
Still, the question of 1 remains: is this tarball available somewhere or is it created specifically for Sage?
comment:4 follow-up: ↓ 6 Changed 6 years ago by
The people who have edited the package most recently are Simon Spicer (recent PhD with William Stein, now working at Facebook or similar), 2014, and R. Andrew Ohana who is still active. I suggest asking the latter.
comment:5 in reply to: ↑ description Changed 6 years ago by
Replying to embray:
Every file or directory in the upstream tarball for elliptic_curves contains a corresponding
._<filename>
file containing OSX-specific "extended attributes" for the file. Those should be stripped out when creating the tarball.Is this just a Sage issue or a problem upstream as well? It's not always clear to me since I'm not sure if/when we're using unmodified source tarballs.
While I've worked on many (if not all) spkgs over the years, I've never seen such garbage in vanilla upstream tarballs. (Occasionally, there had been harmless leftovers like backup files from editors, or cache files from autotools which don't have to get shipped, but IIRC only in tarballs from projects closely related to Sage.)
Note that in the past, the upstream sources were part of the spkgs (now so-called legacy spkgs), and even to that time, it rarely happened that Sage developers accidentally included such files when creating an spkg, and it was part of the review process to make sure that a) the upstream sources are vanilla (modulo removed files we don't need, such as large prepared documentation, redundant sources of prerequisites, or e.g. Microsoft Visual Foo project files), and b) there are no additional files (like those mentioned above, including .DS_Store
etc.).
I personally would have made this ticket a blocker, rather than #20842, in addition to modifications to the release, dist and/or upload scripts.
comment:6 in reply to: ↑ 4 Changed 6 years ago by
- Cc ohanar spice added
Replying to cremona:
The people who have edited the package most recently are Simon Spicer (recent PhD with William Stein, now working at Facebook or similar), 2014 and R. Andrew Ohana who is still active. I suggest asking the latter.
CC'ing both...
comment:7 Changed 6 years ago by
- Priority changed from minor to critical
Ping.
I don't think it's minor if we have to work around that elsewhere. (In fact, doing so introduced other regressions in sage-uncompress-spkg
.)
On the other hand, I cannot judge how "dangerous" such files are on MacOS X; they don't hurt elsewhere at least (other than increasing disk usage, transfer times etc.)
When our elliptic_curves package is fixed, we can simply bail out in the future if a tarball contains such files, which is better for reviewing anyway, since we won't ever get that trouble again.
comment:8 Changed 3 years ago by
- Description modified (diff)
- Milestone changed from sage-7.3 to sage-8.7
- Status changed from new to needs_info
I have made a new clean tarball..
comment:9 Changed 3 years ago by
- Branch set to u/chapoton/20717
- Cc jdemeyer added
- Commit set to c99b79d7782d7fa3ae095cb7e36dcdf125d4499a
- Keywords upgrade added
- Status changed from needs_info to needs_review
New commits:
c99b79d | trac 20717 update elliptic_curves spkg
|
comment:11 Changed 3 years ago by
Can we just call the new "version" 0.8.p1 or something? The actual data files haven't changed at all.
comment:12 Changed 3 years ago by
- Description modified (diff)
- Milestone changed from sage-8.7 to sage-8.8
comment:13 Changed 3 years ago by
- Commit changed from c99b79d7782d7fa3ae095cb7e36dcdf125d4499a to c2c8550aa0862dd2a24a2ca1225db009c4735739
comment:14 Changed 3 years ago by
comment:15 Changed 3 years ago by
- Branch changed from u/chapoton/20717 to public/ticket-20717
- Commit changed from c2c8550aa0862dd2a24a2ca1225db009c4735739 to 3e95e2d59972644cabee589d062e0ea11100536d
- Reviewers set to Erik Bray
- Status changed from needs_review to positive_review
I squashed your changes, but otherwise yes. Wouldn't want to have both versions in the history; that would get confusing.
New commits:
3e95e2d | Trac #20717: update elliptic_curves spkg
|
comment:16 Changed 3 years ago by
- Status changed from positive_review to needs_work
You really need to change the version number if you change the tarball. Adding .p1
doesn't work since that's not considered part of the version number. The current version fails to build with
Found local metadata for elliptic_curves-0.8.p1 WARNING [tarball|download:149]: Invalid checksum; ignoring cached file /usr/local/src/sage-config/upstream/elliptic_curves-0.8.tar.bz2 Attempting to download package elliptic_curves-0.8.tar.bz2 from mirrors https://mirror.koddos.net/sagemath/spkg/upstream/elliptic_curves/elliptic_curves-0.8.tar.bz2 [......................................................................] ************************************************************************ Traceback (most recent call last): File "/usr/local/src/sage-config/build/bin/../sage_bootstrap/download/cmdline.py", line 118, in run_safe run() File "/usr/local/src/sage-config/build/bin/../sage_bootstrap/download/cmdline.py", line 100, in run app.download_tarball(args.url_or_tarball, args.destination) File "/usr/local/src/sage-config/build/bin/../sage_bootstrap/download/app.py", line 43, in download_tarball tarball.download() File "/usr/local/src/sage-config/build/bin/../sage_bootstrap/tarball.py", line 164, in download raise ChecksumError('checksum does not match') ChecksumError: checksum does not match ************************************************************************ ************************************************************************ Error downloading elliptic_curves-0.8.tar.bz2 ************************************************************************ Please email sage-devel (http://groups.google.com/group/sage-devel) explaining the problem and including the log file /usr/local/src/sage-config/logs/pkgs/elliptic_curves-0.8.p1.log Describe your computer, operating system, etc. ************************************************************************
comment:17 Changed 3 years ago by
Calling it 0.8p1
instead of 0.8.p1
would work.
comment:18 Changed 3 years ago by
How amusing.
comment:19 Changed 3 years ago by
- Commit changed from 3e95e2d59972644cabee589d062e0ea11100536d to 29a858da62e738c08d838af4265556ee5e5178fd
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
29a858d | Trac #20717: update elliptic_curves spkg
|
comment:21 Changed 3 years ago by
Anybody out there?
comment:22 Changed 3 years ago by
- Description modified (diff)
comment:23 Changed 3 years ago by
still nobody listening ?
comment:24 follow-up: ↓ 25 Changed 3 years ago by
I checked out the branch (from a successful build of 8.8.beta0) and triggered (with make) a totally unnecessarily rebuild of a lot of stuff, since the commit on this ticket was based on 8.7.beta?. So I killed that and rebased this onto 8.8.beta0 and am now building.
The tarball looks fine to me since the only diffs are the missing underscore files which were the problem in the first place.
I am happy with the changes to SPKG.txt *but* the tarball is called elliptic_curves-0.8.1.tar.bz2 yet unpacks into elliptic_curves-0.9 . Surely that is not intended?
comment:25 in reply to: ↑ 24 Changed 3 years ago by
- Status changed from needs_review to needs_work
Replying to cremona:
I checked out the branch (from a successful build of 8.8.beta0) and triggered (with make) a totally unnecessarily rebuild of a lot of stuff, since the commit on this ticket was based on 8.7.beta?. So I killed that and rebased this onto 8.8.beta0 and am now building.
The tarball looks fine to me since the only diffs are the missing underscore files which were the problem in the first place.
I am happy with the changes to SPKG.txt *but* the tarball is called elliptic_curves-0.8.1.tar.bz2 yet unpacks into elliptic_curves-0.9 . Surely that is not intended?
That's bad indeed. No time to fix that.
comment:26 Changed 3 years ago by
- Commit changed from 29a858da62e738c08d838af4265556ee5e5178fd to 26fedcbc493d2d478f9d1c1609160bc00bb7fc14
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
26fedcb | Trac #20717: update elliptic_curves spkg
|
comment:27 Changed 3 years ago by
- Status changed from needs_work to needs_review
ok, should be good now
comment:28 Changed 3 years ago by
Please review, someone ?
comment:29 Changed 3 years ago by
- Status changed from needs_review to positive_review
Looks like it's probably fine now, thanks.
comment:30 Changed 3 years ago by
- Branch changed from public/ticket-20717 to 26fedcbc493d2d478f9d1c1609160bc00bb7fc14
- Resolution set to fixed
- Status changed from positive_review to closed
John, are these tarballs actually hosted somewhere? The
SPKG.txt
file doesn't really say how to obtain the tarballs.I couldn't easily find them on http://johncremona.github.io/ecdata/ nor https://github.com/JohnCremona/ecdata but I might be missing something.