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 fetching from remote archives #455

Open
wants to merge 1 commit into
base: master
from

Conversation

@hyperfekt
Copy link

hyperfekt commented May 5, 2020

Does what it says.
Ideally a bigger rearchitecting would have been done at the point the assumption that the kind implies the command broke down (with 854695a), but I think that's beyond me.
Processing repositories may take longer than before if there is a big file at the same URL, since that is downloaded twice before even trying to use a version control client.
I did not add a test because I'm not sure how to do that in a way such that it can be disabled in nixpkgs (to remain in the build sandbox).
Closes #377.

@hyperfekt hyperfekt marked this pull request as draft May 5, 2020
@hyperfekt hyperfekt force-pushed the hyperfekt:fix-remote-archives branch from 2dfa43c to 4f4cd0f May 5, 2020
@hyperfekt hyperfekt marked this pull request as ready for review May 5, 2020
@hyperfekt hyperfekt force-pushed the hyperfekt:fix-remote-archives branch from 4f4cd0f to 017c690 May 5, 2020
@hyperfekt
Copy link
Author

hyperfekt commented May 23, 2020

Is there anything I can do to help this get merged?

@peti
Copy link
Member

peti commented Jun 4, 2020

It would be great to have a slightly more expressive description than "fix fetching from remote archives". Ideally, the commit message should say

  1. What exactly is broken and how can one reproduce that error,
  2. how does this commit fix it (i.e. I see that you introduced a Maybe that wasn't there before).
Commit 854695a removed attempting to use nix-prefetch-zip on the
downloaded artifact in favor of only nix-prefetch-url, which does not
unpack automatically. With that, fetching from remote archives became
broken as no handler would succeed.
This adds an optional `command` argument to fetchWith, with which the
command to use for fetching can be specified instead of it being derived
from the kind.
That `command` argument is then used by the reintroduced kind `zip` to
call nix-prefetch-url (with the `--unpack` flag) instead of
nix-prefetch-zip.
In addition, a bug where `fetchWith` would return the path of the
artifact instead of the output of the handler has been fixed.
@hyperfekt hyperfekt force-pushed the hyperfekt:fix-remote-archives branch from 017c690 to 7bf8ac5 Jun 8, 2020
@hyperfekt
Copy link
Author

hyperfekt commented Jun 8, 2020

Thank you, I amended the commit message with a detailed description of what the commit does.

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

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.