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 zipfile target. #1314

Merged
merged 2 commits into from Apr 24, 2017

Conversation

Projects
None yet
3 participants
@pillarsdotnet
Contributor

pillarsdotnet commented Mar 31, 2017

See #1313

@pillarsdotnet

This comment has been minimized.

Contributor

pillarsdotnet commented Mar 31, 2017

Dunno why travis-ci is failing, but a trivial change (adding a line to README.rst) also fails.

Bob Vincent
return nil
realpath = Pathname.new(output_path).realdirpath.to_s
::Dir.chdir(staging_path) do
safesystem("zip", "-9ry", realpath, ".")

This comment has been minimized.

@jordansissel

jordansissel Apr 10, 2017

Owner

This is a behavioral change (adding -r and -y flags). What is the expected impact on users? I imagine the -y flag to build the zip will not work when extracting the resulting zip on Windows?

This comment has been minimized.

@jordansissel

jordansissel Apr 10, 2017

Owner

The reason I mention this is that the PR and linked issue focus on fixing the directory path problem, and this is fixed by other parts of the patch (the two previous lines (realpath and Dir.chdir), and adding -9ry seems unrelated to the bug fix.

This comment has been minimized.

@pillarsdotnet

pillarsdotnet Apr 10, 2017

Contributor

the "-r" is necessary; the "-y" is helpful on unix but (...testing...) sadly unsupported on Windows. I'll re-roll the patch with out the "-y" flag.

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Apr 10, 2017

Travis is failing due to an integration problem between osx and fpm's test suite. I ignore this particular error.

@bazimov

This comment has been minimized.

bazimov commented Apr 24, 2017

Anything blocking this MR, we are seeing same issue on zip functionality. This MR fixes that issue.

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Apr 24, 2017

I'd like to see more tests (who wouldn't! hehe) for the zip output, but I won't block this PR on that.

Thank you for helping make fpm more awesome :)

@jordansissel jordansissel merged commit c364b2a into jordansissel:master Apr 24, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment