Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#19487 closed enhancement (fixed)

Support .zip upstream/ files

Reported by: ncohen Owned by:
Priority: major Milestone: sage-7.0
Component: distribution Keywords:
Cc: mkoeppe Merged in:
Authors: Nathann Cohen Reviewers: Dima Pasechnik, Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: b8f4d6a (Commits) Commit:
Dependencies: #19484 Stopgaps:

Description (last modified by ncohen)

Here it is. It was not so tough, though the code is a bit clumsy.

Nathann

This ticket also updates our copy of bliss to use the original .zip file http://www.tcs.hut.fi/Software/bliss/bliss-0.73.zip

Change History (42)

comment:1 Changed 3 years ago by ncohen

  • Branch set to u/ncohen/19487
  • Commit set to 291bf3e6dd79cc18de67e9b213420fd66c98ff71
  • Status changed from new to needs_review

The mosst obvious use-case for this ticket would be #19483 (whose original tarball is .zip) but it is already reviewed. I'll do what the reviewers ask.

Nathann


New commits:

465d83ctrac #19484 - sage -unzip
291bf3etrac #19487: Support .zip upstream/ files

comment:2 follow-ups: Changed 3 years ago by vbraun

Ooh fugly, unzip and re-tar and pipe and untar? Of course its really the original script's fault for not being modular...

Can we at least put sage-unzip into build/bin/ and not expose it via a sage -unzip command line argument? It has no mathematical purpose, its just an internal tool for building Sage packages.

comment:3 in reply to: ↑ 2 Changed 3 years ago by dimpase

Replying to vbraun:

Ooh fugly, unzip and re-tar and pipe and untar? Of course its really the original script's fault for not being modular...

Can we at least put sage-unzip into build/bin/ and not expose it via a sage -unzip command line argument? It has no mathematical purpose, its just an internal tool for building Sage packages.

neither sage --git, nor sage --twisted have any maths purpose, yet they are still there...

comment:4 in reply to: ↑ 2 Changed 3 years ago by ncohen

Ooh fugly, unzip and re-tar and pipe and untar? Of course its really the original script's fault for not being modular...

Yeah well. I minimize my changes to the current scripts. So I adapted to what was already there. I loved that line in particular:

lbzip2 -d -c "$1" 2>/dev/null || bzip2 -d -c "$1" 2>/dev/null || gzip -d -c "$1" 2>/dev/null || cat "$1"

Can we at least put sage-unzip into build/bin/ and not expose it via a sage -unzip command line argument? It has no mathematical purpose, its just an internal tool for building Sage packages.

Can you make this comment on the right ticket, i.e. #19484?

Nathann

comment:5 Changed 3 years ago by vbraun

Discussion moved to #19484

comment:6 Changed 3 years ago by git

  • Commit changed from 291bf3e6dd79cc18de67e9b213420fd66c98ff71 to ba21cc01a8b39ecf546560c85d0b128b9eea10c7

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

727ac93trac #19484: sage-unzip
ba21cc0trac #19487: Support .zip upstream/ files

comment:7 Changed 3 years ago by dimpase

  • Cc mkoeppe added

I guess what I'd like to see is a zipfile-based example to use this patch to install stuff. How about making one on #18708 ?

comment:8 Changed 3 years ago by ncohen

I can update bliss, but that's a bit silly since it is the latest version since #19483 (which is why I wrote this branch). I don't know what 'normalize' is: can you add this ticket as a dependency of #18708? This way you can check that it works (should be totally straightforward) and that other package will be udpated.

Nathann

comment:9 Changed 3 years ago by mkoeppe

  • Status changed from needs_review to needs_work

This does not work on Mac OS X because "sed -s" is a GNU extension.

comment:10 Changed 3 years ago by ncohen

I don't have Mac OS X, and I do not know how it must be written to work there. Can you fix what must be for it to work on your platform ?

comment:11 Changed 3 years ago by mkoeppe

Sure, I'll try to make it portable

comment:12 Changed 3 years ago by git

  • Commit changed from ba21cc01a8b39ecf546560c85d0b128b9eea10c7 to 1312500851383e2eb45c4ea4f1da0663659a2d62

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

eeedb3btrac #19712: strongly_regular_graph() crashes when mu=0
f5d75abtrac #19487: Merged with mu=0
1312500trac #19487: remove -s

comment:13 Changed 3 years ago by git

  • Commit changed from 1312500851383e2eb45c4ea4f1da0663659a2d62 to a49b1e48d3bf4f673893637903974708325c53b9

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

ab197d3trac #19487: Support .zip upstream/ files
a49b1e4trac #19487: remove -s

comment:14 Changed 3 years ago by ncohen

Unless this does the trick? I removed the '-s', as I have no idea why it was there in the first place O_o

Nathann

comment:15 Changed 3 years ago by ncohen

  • Status changed from needs_work to needs_review

comment:16 Changed 3 years ago by mkoeppe

  • Branch changed from u/ncohen/19487 to u/mkoeppe/19487

comment:17 Changed 3 years ago by mkoeppe

  • Commit changed from a49b1e48d3bf4f673893637903974708325c53b9 to 7e464c1a8bf37422e30b36f5b99ff6a7748e6606

The regexp needed fixing also.


New commits:

7e464c1Make regular expression portable; add documentation

comment:18 Changed 3 years ago by ncohen

Err... You sure that there is no way to have a 'OR' in a regexp under MacOSX ?.... The readability has dramatically decreased, if I may say..

Also you seem to strip lz, but I do not think that our scripts can unpackage lz files.

Nathann

comment:19 follow-up: Changed 3 years ago by mkoeppe

The GNU sed implementation explains pretty well what are GNU extensions. And yes, if one always used the GNU version it's stunning how restrictive the classic UNIX versions were.

Perhaps best to rewrite the whole thing using a sequence of bash substitutions instead.

I stripped a few more file extensions to future-proof the script.

comment:20 Changed 3 years ago by dimpase

how about a python script instead?

comment:21 in reply to: ↑ 19 ; follow-up: Changed 3 years ago by ncohen

The GNU sed implementation explains pretty well what are GNU extensions. And yes, if one always used the GNU version it's stunning how restrictive the classic UNIX versions were.

I tried to rewrite this regexp in posix. Can you try whether this works in Mac OSX? The '--posix' flag is meant to disable whatever shouldn't be supported

~$ echo whatever.tar.2.0.zip.tar.gz.tar.tgz | sed --posix "s/[\.zip|\.tar|\.gz]*$//g" 
whatever.tar.2.0

Nathann

comment:22 follow-up: Changed 3 years ago by vbraun

The whole zip/re-tar stuff is hilarious. If anything, this ticket shows that the current sage-spkg script is just unmaintainable.

+1 to a python script...

comment:23 in reply to: ↑ 22 Changed 3 years ago by ncohen

The whole zip/re-tar stuff is hilarious.

+1. I couldn't believe when I first saw it :-P

If anything, this ticket shows that the current sage-spkg script is just unmaintainable.

+1 to a python script...

Well I don't mind if you want to write it, but given that the present ticket does not change it too much, honestly I don't think that it is very urgent to rewrite. But indeed, this script is... "hacky"? :-P

Nathann

comment:24 in reply to: ↑ 21 ; follow-up: Changed 3 years ago by mkoeppe

Replying to ncohen:

The GNU sed implementation explains pretty well what are GNU extensions. And yes, if one always used the GNU version it's stunning how restrictive the classic UNIX versions were.

I tried to rewrite this regexp in posix. Can you try whether this works in Mac OSX? The '--posix' flag is meant to disable whatever shouldn't be supported

~$ echo whatever.tar.2.0.zip.tar.gz.tar.tgz | sed --posix "s/[\.zip|\.tar|\.gz]*$//g" 
whatever.tar.2.0

It does not work. UNIX sed does not understand any command line option with double dashes.

And, by the way, your original regexp does not work because you're confusing [] and \( \).

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

Helloooo,

It does not work. UNIX sed does not understand any command line option with double dashes.

Do you mean the " ? Are those called "double dashes" ? Does it still fail if you use simple quotes instead, i.e. sed --posix 's/[\.zip|\.tar|\.gz]*$//g'

And, by the way, your original regexp does not work because you're confusing [] and \( \).

Yep yep sorry, I noticed that while working on this new version. Which seems to work reliably, on my distro at least.

Nathann

comment:26 follow-up: Changed 3 years ago by jhpalmieri

By double dashes, he meant the option --posix.

On OS X, use the -E flag for sed:

echo whatever.tar.2.0.zip.tar.gz.tar.tgz | sed -E "s/(\.zip|\.tar|\.gz|\.tgz)*$//g" 

But -E is not a posix option or a GNU sed option, so I guess we can't use it. A Python script would avoid these compatibility issues.

comment:27 in reply to: ↑ 26 Changed 3 years ago by ncohen

By double dashes, he meant the option --posix.

Oh, okay sorry. This being said, my command also works without the '--posix'. So if we can have

~$ echo whatever.tar.2.0.zip.tar.gz.tar.tgz | sed "s/[\.zip|\.tar|\.gz]*$//g"  
whatever.tar.2.0

and if it works in OSX too, then it's good?.. Apparently it is posix, for it works even when --posix is added (though it is not necessary).

Nathann

comment:28 follow-up: Changed 3 years ago by jhpalmieri

Your command works but is probably not what we want. The regular expression [\.zip|\.tar|\.gz]* matches any string formed by the characters between the brackets, so it is equivalent to [\.ziptarg|]*. So I think it matches too much: the sed invocation would convert whatever.tar.2.0.zig.zag.rat to whatever.tar.2.0. This was mkoeppe's comment about confusing [] with \(\).

On OS X,

echo whatever.tar.2.0.zip.tar.gz.tar.tgz | sed -E "s/(\.zip|\.tar|\.gz|\.tgz)*$//g"

works, while omitting -E

echo whatever.tar.2.0.zip.tar.gz.tar.tgz | sed "s/(\.zip|\.tar|\.gz|\.tgz)*$//g"

does not.

comment:29 in reply to: ↑ 28 Changed 3 years ago by ncohen

Your command works but is probably not what we want. The regular expression [\.zip|\.tar|\.gz]* matches any string formed by the characters between the brackets, so it is equivalent to [\.ziptarg|]*.

Arg... Yeah, it just doesn't work at all. Sorry :-/

I'll try to find something else :-/

Nathann

comment:30 Changed 3 years ago by ncohen

Okay okay.. Can't find much better. Bash substitution would be very verbose too so, well. Let's stick with the ugly/long sed command :-/

comment:31 Changed 3 years ago by ncohen

  • Description modified (diff)

comment:32 Changed 3 years ago by ncohen

  • Branch changed from u/mkoeppe/19487 to public/19487
  • Commit changed from 7e464c1a8bf37422e30b36f5b99ff6a7748e6606 to 81b0e6304a5f6374bcaf84481e07f66c1bf26d29

New commits:

7552aectrac #19487: Merged with 6.10.rc2
81b0e63trac #19487: Use bliss' original upstream .zip file

comment:33 Changed 3 years ago by ncohen

Hello !

I updated this ticket a bit in order to use the original .zip archive for the bliss package. It is the same version as the one we use, so it is not even a package update and there is no risk of anything. I used to test the ticket, and it works well: if you also agree with it, then we can get it merged.

Thanks,

Nathann

comment:34 Changed 3 years ago by dimpase

  • Milestone changed from sage-6.10 to sage-7.0
  • Reviewers set to Dima Pasechnik, Matthias Koeppe
  • Status changed from needs_review to positive_review

OK, good. I tested it with Sage 6.10 merged in, it works. I suppose the bliss zip file needs to be uploaded to the mirrors...

comment:35 Changed 3 years ago by dimpase

by the way, isn't the option to do checksum of just one file broken?

comment:36 Changed 3 years ago by dimpase

  • Status changed from positive_review to needs_work

also, no upstream/*bz2 files are checksum'ed anymore!

comment:37 Changed 3 years ago by dimpase

if seems to work if I apply

diff --git a/src/bin/sage-fix-pkg-checksums b/src/bin/sage-fix-pkg-checksums
index 149f68f..f16fab7 100755
--- a/src/bin/sage-fix-pkg-checksums
+++ b/src/bin/sage-fix-pkg-checksums
@@ -14,7 +14,7 @@ do
     pkg_name=${tarball%%-*}
     # Convert to lowercase for the directory name:
     pkg_name_lc=`echo $pkg_name | tr '[:upper:]' '[:lower:]'`
-    extension=$(echo $tarball | grep -o -E "(.tar|.gz|.zip)*$") # file extension (.tar,.tar.gz,.zip)
+    extension=$(echo $tarball | grep -o -E "(.tar|.gz|.bz2|.zip)*$") # file extension (.tar,.tar.gz,.zip)
     if [ -d "$SAGE_ROOT/build/pkgs/$pkg_name_lc" ]; then
         sage_version=`cat "$SAGE_ROOT/build/pkgs/$pkg_name_lc/package-version.txt" | sed 's/\.p[0-9][0-9]*$//'`
         if [ ${tarball%$extension*} = "$pkg_name-$sage_version" ]; then

comment:38 Changed 3 years ago by git

  • Commit changed from 81b0e6304a5f6374bcaf84481e07f66c1bf26d29 to ab96b271f093272132427b6c147c1704d2dc370f

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

ab96b27trac #19487: bz2 file extension

comment:39 Changed 3 years ago by git

  • Commit changed from ab96b271f093272132427b6c147c1704d2dc370f to b8f4d6af1650283791a53332e0b5a0eeacc46974

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

b8f4d6atrac #19487: bz2 file extension

comment:40 Changed 3 years ago by ncohen

  • Status changed from needs_work to positive_review

comment:41 Changed 3 years ago by vbraun

  • Branch changed from public/19487 to b8f4d6af1650283791a53332e0b5a0eeacc46974
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:42 Changed 3 years ago by jhpalmieri

  • Commit b8f4d6af1650283791a53332e0b5a0eeacc46974 deleted

See #20120 for a followup: replace the uncompressing stuff with a Python script.

Note: See TracTickets for help on using tickets.