Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a few more issues with sage-uncompress-spkg #20871

Closed
embray opened this issue Jun 24, 2016 · 21 comments
Closed

Fix a few more issues with sage-uncompress-spkg #20871

embray opened this issue Jun 24, 2016 · 21 comments

Comments

@embray
Copy link
Contributor

embray commented Jun 24, 2016

Fixes 2 issues:

  1. An outright bug following from Unpack all upstream tarballs into 'src' directory #20721: If the files in a tarball are not all under a top-level directory (shouldn't be the case, but possible), the -d flag did not work as advertised.

  2. Always unset setuid and setgid flags. There's no reason they should be set in a source tarball (I don't think?) and it will mitigate issues like nauty upstream source tarball has setgid on top-level directory #20870

Component: build

Author: Erik Bray

Branch/Commit: 0c2c9d4

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/20871

@embray embray added this to the sage-7.3 milestone Jun 24, 2016
@mkoeppe
Copy link
Member

mkoeppe commented Jun 27, 2016

Reviewer: Matthias Koeppe

@mkoeppe
Copy link
Member

mkoeppe commented Jun 28, 2016

comment:4

There is still an issue with zip files.

sage -i bliss gives:

[bliss-0.73] Found local metadata for bliss-0.73
[bliss-0.73] Using cached file /Users/mkoeppe/cvs/sage/upstream/bliss-0.73.zip
[bliss-0.73] bliss-0.73
[bliss-0.73] ====================================================
[bliss-0.73] Setting up build directory for bliss-0.73
[bliss-0.73] Traceback (most recent call last):
[bliss-0.73]   File "/Users/mkoeppe/cvs/sage/build/bin/sage-uncompress-spkg", line 260, in <module>
[bliss-0.73]     sys.exit(main())
[bliss-0.73]   File "/Users/mkoeppe/cvs/sage/build/bin/sage-uncompress-spkg", line 212, in main
[bliss-0.73]     archive = cls.open(filename)
[bliss-0.73] TypeError: unbound method open() must be called with SageZipFile instance as first argument (got str instance instead)
[bliss-0.73] Error: failed to extract /Users/mkoeppe/cvs/sage/upstream/bliss-0.73.zip

@embray
Copy link
Contributor Author

embray commented Jun 28, 2016

comment:5

There is still an issue with zip files.

Was this reported before?

Might as well fix it as part of this ticket too while I'm fixing bugs.

It looks like the ZipFile class is inconsistent with the TarFile class, where open is a classmethod. I think I knew that but forgot this time.

@mkoeppe
Copy link
Member

mkoeppe commented Jun 28, 2016

comment:6

Replying to @embray:

There is still an issue with zip files.

Was this reported before?

I didn't see a report; but zip files did work in the past. Perhaps introduced in #20721? I didn't catch it at this time.

Might as well fix it as part of this ticket too while I'm fixing bugs.

Thanks.

@vbraun
Copy link
Member

vbraun commented Jun 28, 2016

Changed branch from u/embray/unpack-tarball-src to 30ac0a6

@vbraun
Copy link
Member

vbraun commented Jun 28, 2016

Changed commit from 30ac0a6 to none

@vbraun vbraun reopened this Jun 28, 2016
@vbraun
Copy link
Member

vbraun commented Jun 28, 2016

New commits:

3ea4482Fix bug with -d option when there is no single top-level directory in the tarball--the script was not doing what it's supposed to in this case
30ac0a6Also unset SUID and SGID flags on extracted files, to prevent issues like https://github.com/sagemath/sage/issues/20870#

@vbraun
Copy link
Member

vbraun commented Jun 28, 2016

Changed branch from 30ac0a6 to u/embray/unpack-tarball-src

@vbraun
Copy link
Member

vbraun commented Jun 28, 2016

Commit: 30ac0a6

@embray
Copy link
Contributor Author

embray commented Jun 28, 2016

comment:10

What just happened here?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 28, 2016

Changed commit from 30ac0a6 to 40449bf

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 28, 2016

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

40449bfFix zip file opening.

@vbraun
Copy link
Member

vbraun commented Jun 28, 2016

comment:12

I merged the positively-reviewed ticket, all tests passed, and then I closed the ticket.

@embray
Copy link
Contributor Author

embray commented Jun 28, 2016

comment:13

The status was needs_work though. I just pushed another commit that should have been included.

@vbraun
Copy link
Member

vbraun commented Jun 28, 2016

comment:14

The status was positive review when I merged the ticket.

@mkoeppe
Copy link
Member

mkoeppe commented Jun 28, 2016

comment:16

Branch works now, but may need merge/rebase.

@vbraun
Copy link
Member

vbraun commented Jun 29, 2016

comment:17

Merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 30, 2016

Changed commit from 40449bf to 0c2c9d4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 30, 2016

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

c6ce4aaFix bug with -d option when there is no single top-level directory in the tarball--the script was not doing what it's supposed to in this case
66d67dcAlso unset SUID and SGID flags on extracted files, to prevent issues like https://github.com/sagemath/sage/issues/20870#
0c2c9d4Fix zip file opening.

@embray
Copy link
Contributor Author

embray commented Jun 30, 2016

comment:19

Rebased.

@vbraun
Copy link
Member

vbraun commented Jul 2, 2016

Changed branch from u/embray/unpack-tarball-src to 0c2c9d4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants