Opened 6 years ago

Closed 3 years ago

Last modified 3 years ago

#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:

Status badges

Description (last modified by jdemeyer)

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 jdemeyer

  • Cc cremona added
  • Component changed from build to packages: standard

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.

comment:2 follow-up: Changed 6 years ago by cremona

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 jdemeyer

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: Changed 6 years ago by 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.

Last edited 3 years ago by slelievre (previous) (diff)

comment:5 in reply to: ↑ description Changed 6 years ago by leif

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 leif

  • 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 leif

  • 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 chapoton

  • 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 chapoton

  • 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:

c99b79dtrac 20717 update elliptic_curves spkg

comment:10 Changed 3 years ago by chapoton

  • Cc embray added

rather a trivial ticket, I would say

comment:11 Changed 3 years ago by embray

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 chapoton

  • Description modified (diff)
  • Milestone changed from sage-8.7 to sage-8.8

comment:13 Changed 3 years ago by git

  • Commit changed from c99b79d7782d7fa3ae095cb7e36dcdf125d4499a to c2c8550aa0862dd2a24a2ca1225db009c4735739

Branch pushed to git repo; I updated commit sha1. New commits:

1b4cccbMerge branch 'u/chapoton/20717' in 8.7.b7
c2c8550use version name 0.8.p1

comment:14 Changed 3 years ago by chapoton

https://upload.wikimedia.org/wikipedia/commons/thumb/a/a5/Emoji_u1f381.svg/128px-Emoji_u1f381.svg.png

Here it is ! Nice enough for you ?

comment:15 Changed 3 years ago by embray

  • Authors set to Frédéric Chapoton
  • 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:

3e95e2dTrac #20717: update elliptic_curves spkg

comment:16 Changed 3 years ago by jdemeyer

  • 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.
************************************************************************
Last edited 3 years ago by jdemeyer (previous) (diff)

comment:17 Changed 3 years ago by jdemeyer

Calling it 0.8p1 instead of 0.8.p1 would work.

comment:18 Changed 3 years ago by chapoton

How amusing.

comment:19 Changed 3 years ago by git

  • Commit changed from 3e95e2d59972644cabee589d062e0ea11100536d to 29a858da62e738c08d838af4265556ee5e5178fd

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

29a858dTrac #20717: update elliptic_curves spkg

comment:20 Changed 3 years ago by chapoton

  • Status changed from needs_work to needs_review

done and checked.

comment:21 Changed 3 years ago by chapoton

Anybody out there?

comment:22 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:23 Changed 3 years ago by chapoton

still nobody listening ?

comment:24 follow-up: Changed 3 years ago by 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?

comment:25 in reply to: ↑ 24 Changed 3 years ago by chapoton

  • 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 git

  • Commit changed from 29a858da62e738c08d838af4265556ee5e5178fd to 26fedcbc493d2d478f9d1c1609160bc00bb7fc14

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

26fedcbTrac #20717: update elliptic_curves spkg

comment:27 Changed 3 years ago by chapoton

  • Status changed from needs_work to needs_review

ok, should be good now

comment:28 Changed 3 years ago by chapoton

Please review, someone ?

comment:29 Changed 3 years ago by embray

  • Status changed from needs_review to positive_review

Looks like it's probably fine now, thanks.

comment:30 Changed 3 years ago by vbraun

  • Branch changed from public/ticket-20717 to 26fedcbc493d2d478f9d1c1609160bc00bb7fc14
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.