Add an unzip tool that can pull files out of remote zip files

RESOLVED WONTFIX

Status

()

Core
Build Config
RESOLVED WONTFIX
9 months ago
9 months ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

9 months ago
The more or less immediate use case is to allow fuzzing builds to download the js shell debug info from the crashsymbols-full.zip file without having to download the whole file (which is > 800MB), and without having to create a new (separate) crashsymbols artifact just for that.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

9 months ago
mozreview-review
Comment on attachment 8848421 [details]
Bug 1348229 - Add an unzip py_action script based on the mozjar code.

https://reviewboard.mozilla.org/r/121324/#review123558

What are the downsides to creating a separate zip for the js shell debug info? That seems like a much simpler solution than creating our own unzip. I feel that we should try that first and do this only if necessary.

::: python/mozbuild/mozbuild/action/unzip.py:32
(Diff revision 1)
> +    args = parser.parse_args(args)
> +
> +    jar = JarReader(file=args.zip)
> +
> +    for entry in jar.entries:
> +        if not args.files or any(mozpath.match(entry, f)

Is there a way this can be reorganized so that it returns a failure code when one of the args.files isn't matched by any of the entries? Currently this py_action silently returns 0 if one of the arguments doesn't match, whereas regular unzip prints a message and returns an error code.
Attachment #8848421 - Flags: review?(mshal)

Comment 4

9 months ago
mozreview-review
Comment on attachment 8848422 [details]
Bug 1348229 - Add support to unzip directly from a remote URL.

https://reviewboard.mozilla.org/r/121326/#review123566

I think you'll want to get someone else to review this part if you intend to go forward with this approach, but the complexity here makes me think that a separate artifact is the way to go.

::: python/mozbuild/mozbuild/action/unzip.py:102
(Diff revision 1)
>      parser.add_argument("zip", help="Path to zip file")
>      parser.add_argument("files", nargs="*",
>                          help="Path to files to extract from zip")
>      args = parser.parse_args(args)
>  
> +    if '://' in args.zip:

This could also pick up a weird (but valid) Windows path, like c://Users/foo.
Attachment #8848422 - Flags: review?(mshal)
(Assignee)

Comment 5

9 months ago
(In reply to Michael Shal [:mshal] from comment #3)
> Comment on attachment 8848421 [details]
> Bug 1348229 - Add an unzip py_action script based on the mozjar code.
> 
> https://reviewboard.mozilla.org/r/121324/#review123558
> 
> What are the downsides to creating a separate zip for the js shell debug
> info?

Today it's a js shell debug info artifact. Tomorrow it's something else. etc. Don't you think we have enough artifacts already?

> That seems like a much simpler solution than creating our own unzip. I
> feel that we should try that first and do this only if necessary.

Independently of the "act on remote files" part, I think we should actually have our own unzip instead of having the system unzip program as a dependency. We have zip.py for the counterpart, too, instead of using the system zip program.

Comment 6

9 months ago
(In reply to Mike Hommey [:glandium] from comment #5)
> (In reply to Michael Shal [:mshal] from comment #3)
> > What are the downsides to creating a separate zip for the js shell debug
> > info?
> 
> Today it's a js shell debug info artifact. Tomorrow it's something else.
> etc. Don't you think we have enough artifacts already?

I'm not aware of a limit on the number of artifacts. Though it seems like it can be useful to treat the zip artifacts more like a collection of files rather than a single file. What are your thoughts on doing this server side instead? So allowing things like:

wget -r https://index.taskcluster.net/.../artifacts/public/build/target.crashreporter-symbols.zip/js

Then any client can grab things from inside artifacts instead of requiring the use of a specific python implementation.

> Independently of the "act on remote files" part, I think we should actually
> have our own unzip instead of having the system unzip program as a
> dependency. We have zip.py for the counterpart, too, instead of using the
> system zip program.

What problems do we have with the system zip?
(Assignee)

Comment 7

9 months ago
(In reply to Michael Shal [:mshal] from comment #6)
> (In reply to Mike Hommey [:glandium] from comment #5)
> > (In reply to Michael Shal [:mshal] from comment #3)
> > > What are the downsides to creating a separate zip for the js shell debug
> > > info?
> > 
> > Today it's a js shell debug info artifact. Tomorrow it's something else.
> > etc. Don't you think we have enough artifacts already?
> 
> I'm not aware of a limit on the number of artifacts. Though it seems like it
> can be useful to treat the zip artifacts more like a collection of files
> rather than a single file. What are your thoughts on doing this server side
> instead?

AFAIK, it's not possible to do anything beyond static-file hosting on the server side, because it's really hosted on S3.

> > Independently of the "act on remote files" part, I think we should actually
> > have our own unzip instead of having the system unzip program as a
> > dependency. We have zip.py for the counterpart, too, instead of using the
> > system zip program.
> 
> What problems do we have with the system zip?

That it's one more dependency for the build.

Comment 8

9 months ago
This bug seems like a lot of complexity for something that could be fixed by just uploading a one-off version of the symbols archive containing the files only needed by fuzzing.
(Assignee)

Updated

9 months ago
Status: NEW → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → WONTFIX
> for something that could be fixed by just uploading a one-off version
> of the symbols archive containing the files only needed by fuzzing.

Is there a bug for tracking this work?
Flags: needinfo?(mh+mozilla)
(Assignee)

Comment 10

9 months ago
If you haven't filed one, there isn't one.
Flags: needinfo?(mh+mozilla)
Filed bug 1351559.
You need to log in before you can comment on or make changes to this bug.