Skip to content

fix fetching from remote archives #455

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

Merged
merged 1 commit into from May 1, 2021
Merged

fix fetching from remote archives #455

merged 1 commit into from May 1, 2021

Conversation

ghost
Copy link

@ghost ghost 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.

@ghost ghost marked this pull request as draft May 5, 2020 15:42
@ghost ghost marked this pull request as ready for review May 5, 2020 16:14
@ghost
Copy link
Author

ghost 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.
@ghost
Copy link
Author

ghost commented Jun 8, 2020

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

@maralorn
Copy link
Member

maralorn commented May 1, 2021

I have tested that this works and doesn‘t break the other fetchers.

petis concerns seem to be addressed.

@maralorn maralorn merged commit 849a350 into NixOS:master May 1, 2021
@maralorn
Copy link
Member

maralorn commented May 1, 2021

Thank you @hyperfekt !

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

Successfully merging this pull request may close these issues.

Remote tarballs
2 participants