Skip to content

Fix query parsing for path-like flakes#9593

Merged
thufschmitt merged 2 commits intoNixOS:masterfrom
B4dM4n:fix-path-like-flake-query
Dec 12, 2023
Merged

Fix query parsing for path-like flakes#9593
thufschmitt merged 2 commits intoNixOS:masterfrom
B4dM4n:fix-path-like-flake-query

Conversation

@B4dM4n
Copy link
Contributor

@B4dM4n B4dM4n commented Dec 11, 2023

Motivation

Fix query string parsing in path-like flake URL's in v2.19:

nix run "github:NixOS/nix/2.19.2" -- eval '.?ref=HEAD#default.outPath'
fatal: ambiguous argument 'HEAD#d': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
error:
       … while fetching the input 'git+file:///home/user/nix?ref=HEAD%23d'

       error: program 'git' failed with exit code 128

Context

The length parameter of url.substr should not be counted from the beginning (which fragmentStart does), but from pathEnd+1. This leads to #d being included in the ref from the above example.

This change was introduced by 50e61f5.

Priorities

Add 👍 to pull requests you find important.

@B4dM4n B4dM4n requested a review from edolstra as a code owner December 11, 2023 15:37
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Dec 11, 2023
thufschmitt
thufschmitt approved these changes Dec 12, 2023
View reviewed changes
Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

Thanks!

For some reason I can't wrap my head around the fact that the second argument of substr is the lengh of the substring and not the last index 😬

@thufschmitt thufschmitt merged commit 0dfa66d into NixOS:master Dec 12, 2023
@edolstra
Copy link
Member

edolstra commented Dec 12, 2023

Oops, I fixed this on the lazy-trees branch a while ago but forgot to backport it...

@tomjnixon tomjnixon mentioned this pull request Dec 12, 2023
Open
@github-actions github-actions bot mentioned this pull request Dec 12, 2023
Merged
@github-actions
Copy link

github-actions bot commented Dec 12, 2023

Successfully created backport PR for 2.19-maintenance:

@Ericson2314 Ericson2314 mentioned this pull request Dec 12, 2023
Merged
@B4dM4n B4dM4n deleted the fix-path-like-flake-query branch December 15, 2023 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tomberek tomberek tomberek approved these changes

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

+1 more reviewer

@thufschmitt thufschmitt thufschmitt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@B4dM4n @edolstra @tomberek @thufschmitt @Ericson2314