Skip to content
This repository has been archived by the owner on Nov 27, 2019. It is now read-only.

! Fix error "file already exists" when using command "sign" with XPI file #521

Conversation

PikachuEXE
Copy link
Contributor

Use another unzip library decompress-zip to replace unzip due to a potential bug in unzip

Fixes #458
Tested command locally

Cause of bug:
A library used unzip contains a pontential bug: when unzipping a folder in zip, a file is created instead.
So the file is created multiple times causing the error.

@PikachuEXE PikachuEXE force-pushed the fix/cmd/sign/file-already-exists branch 3 times, most recently from 3b6ad8e to e31f0e2 Compare April 3, 2016 04:23
});
unzipper.on("error", function(err) {
cleanUpAndReject(err);
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can shorten this to unzipper.on("error", cleanUpAndReject)

@johannhof
Copy link

Hey, thanks for the pull request. I've had some really minor comments, but this LGTM otherwise.

Not sure if we can cover this problem with a test, was there ever a way to reliably reproduce it?

@PikachuEXE
Copy link
Contributor Author

There is no test for passing in an XPI (path), and I don't know how to set it up :S

@johannhof
Copy link

Ah don't worry about it, if it's a problem with a third-party lib I think we can do without a test. Do you have time to fix the small nits above? :)

@PikachuEXE PikachuEXE force-pushed the fix/cmd/sign/file-already-exists branch from e31f0e2 to 1358a03 Compare April 6, 2016 05:12
…file

Use another unzip library `decompress-zip` to replace `unzip` due to a potential bug in `unzip`
@PikachuEXE PikachuEXE force-pushed the fix/cmd/sign/file-already-exists branch from 1358a03 to 22df306 Compare April 6, 2016 05:15
@PikachuEXE
Copy link
Contributor Author

Fixed and all green

@johannhof
Copy link

👍 Awesome, thanks!

@johannhof johannhof merged commit eb4f945 into mozilla-jetpack:master Apr 6, 2016
@PikachuEXE PikachuEXE deleted the fix/cmd/sign/file-already-exists branch April 6, 2016 05:33
@kumar303
Copy link
Contributor

kumar303 commented Apr 8, 2016

thanks for fixing this!

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

Successfully merging this pull request may close these issues.

3 participants