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.

Sign up for GitHub

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

Jump to bottom

fetchers: Try to substitute if curl fails. #8184

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fetchers: Try to substitute if curl fails. #8184

wants to merge 2 commits into from

Conversation

jsoo1
Copy link

@jsoo1 jsoo1 commented Apr 7, 2023 •
edited
Loading

Motivation

Fetchers currently do not try to substitute if downloading via curl fails and there is no cached result.

This is important if using flakes or fetchers in an tightly controlled environment.

Context

Fixes #4313

For performance reasons - only try to substitute when there is an error during curl.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@jsoo1 jsoo1 requested a review from edolstra as a code owner April 7, 2023 17:14
@github-actions github-actions bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority store Issues and pull requests concerning the Nix store fetching Networking with the outside (non-Nix) world, input locking labels Jul 24, 2023
@github-actions github-actions bot removed new-cli Relating to the "nix" command store Issues and pull requests concerning the Nix store labels Jul 25, 2023
@jsoo1 jsoo1 force-pushed the substitute-download-file branch 2 times, most recently from 7c003f5 to fbc41fa Compare July 25, 2023 01:23
@jsoo1 jsoo1 mentioned this pull request Jul 25, 2023
Open
@edolstra edolstra self-assigned this Jul 31, 2023
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-07-31-nix-team-meeting-minutes-76/31486/1

@jsoo1
Copy link
Author

jsoo1 commented Aug 14, 2023

Looks like this needs a rebase. Woops! Sorry!

@jsoo1
Copy link
Author

jsoo1 commented Aug 25, 2023

Ok, rebased. I confirmed nix flake check completes on x86_64-linux.

@jsoo1
Copy link
Author

jsoo1 commented Sep 18, 2023

@edolstra Just a ping (sorry). Do you have thoughts about the urls returned when substitutes are tried?

jsoo1
jsoo1 commented Sep 18, 2023
View reviewed changes
src/libfetchers/tarball.cc
throw;
}

return {
Copy link
Author

Choose a reason for hiding this comment

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

This bit is what I know least about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edolstra edolstra Awaiting requested review from edolstra edolstra is a code owner

@thufschmitt thufschmitt Awaiting requested review from thufschmitt

@fricklerhandwerk fricklerhandwerk Awaiting requested review from fricklerhandwerk

Assignees

@edolstra edolstra

Labels
fetching Networking with the outside (non-Nix) world, input locking with-tests Issues related to testing. PRs with tests have some priority
Projects
Nix team
Status: 🏁 Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Substitute builtin url fetchers
3 participants
@jsoo1 @nixos-discourse @edolstra