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

Jump to bottom

fetchgitlab: escape rev to support "+" in it #88810

Merged
merged 1 commit into from May 24, 2020
Merged

fetchgitlab: escape rev to support "+" in it #88810

merged 1 commit into from May 24, 2020

Conversation

flokli
Copy link
Contributor

@flokli flokli commented May 24, 2020

Use a URL to download from GitLab that works with both commit ids and
tags.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@flokli flokli requested review from picnoir and alyssais May 24, 2020 19:48
@flokli flokli mentioned this pull request May 24, 2020
Closed
11 tasks
@jtojnar
Copy link
Contributor

jtojnar commented May 24, 2020

The original URL works for tags too.IIRC we changed to this for compatibility reasons but would need to check the commit history.

@flokli
Copy link
Contributor Author

flokli commented May 24, 2020

@jtojnar doesn't seem to be the case: #88767 (comment)

@flokli
Copy link
Contributor Author

flokli commented May 24, 2020 •
edited

At least in the history of pkgs/build-support/fetchgitlab/default.nix, I can't find another URL.

Found it: 93e9b73

@flokli
Copy link
Contributor Author

flokli commented May 24, 2020

@typetetris as you opened #39200, any idea how to support tags as rev? That's the link the web interface redirects me to, when clicking manually.

@jtojnar
Copy link
Contributor

jtojnar commented May 24, 2020

I think it supports tags and the issue here is just escaping of special characters (+ is encoding of ). After all, we use tags with fetchFromGitLab all the time.

@jtojnar
Copy link
Contributor

jtojnar commented May 24, 2020

And the relevant commit is 94a420b

@flokli
Copy link
Contributor Author

flokli commented May 24, 2020

Alright - I pushed a new version escaping the + in a rev, which seems to work, too.

There's no urlencode function in nix or <nixpkgs/lib>, so I only escaped + for now.

@flokli flokli changed the title fetchgitlab: fix passing tags as rev fetchgitlab: escape rev to support "+" in it May 24, 2020
@flokli flokli merged commit 16c72ff into NixOS:master May 24, 2020
@flokli flokli deleted the fetchgitlab-rev-tag branch May 24, 2020 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar approved these changes

@picnoir picnoir Awaiting requested review from picnoir

@alyssais alyssais Awaiting requested review from alyssais

Assignees
No one assigned
Labels
6.topic: fetch 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@flokli @jtojnar