Allow relative paths in --store option #3736

meditans · 2020-06-23T15:16:19Z

In nix commands which accept --store options, we can now specify a
relative path, which will be canonicalized. The relative path must start
with ./ like in the nix language.

Fixes #3734

In nix commands which accept --store options, we can now specify a relative path, which will be canonicalized.

meditans · 2020-06-23T15:45:59Z

It works, but it's marked as WIP as some tests should be added for this.

thufschmitt · 2020-06-24T07:23:59Z

src/libstore/store-api.cc

@@ -864,7 +864,7 @@ StoreType getStoreType(const std::string & uri, const std::string & stateDir)
 {
    if (uri == "daemon") {
        return tDaemon;
-    } else if (uri == "local" || hasPrefix(uri, "/")) {
+    } else if (uri == "local" || hasPrefix(uri, "/") || hasPrefix(uri, "./")) {


Might make sense to also allow . as a path

The cost of that would be divergence from the nix expression language though, right?

this already happens with cli args. for instance:

nix build -f . hello

but does -f <url> work? Because we are overloading URL and relative path syntax, and URL syntax already is infamous for having parser mistakes/ambiguities/vulnerabilities, I rather move slowly. We can always add . later.

Of course, if @edolstra wants / there's concensus to go ahead and do ., we'll happily add it, but I rather advocate for the smallest change that gets the job done.

url's do work (although they have to be in tar format):

nix build -f https://github.com/NixOS/nixpkgs/archive/master.tar.gz hello

rules for it are:

nix/src/libexpr/common-eval-args.cc

Lines 74 to 85 in 3f01fa1

Path lookupFileArg(EvalState & state, string s)

{

if (isUri(s)) {

return state.store->toRealPath(

fetchers::downloadTarball(

state.store, resolveUri(s), "source", false).first.storePath);

} else if (s.size() > 2 && s.at(0) == '<' && s.at(s.size() - 1) == '>') {

Path p = s.substr(1, s.size() - 2);

return state.findFile(p);

} else

return absPath(s);

}

OK fair, but it as wrong of me to compare -f <url> in the first place, because this sort of "each store parses the thing" means having else cases is no good either---everything should be if (...) { ... } else return nullptr.

Now we could still do ., yes. My above does not work argument against that, I will totally grant.

@edolstra's weak preference is if it doesn't contain :// and does contain / then it is deemed a path and is turned into a LocalStore.

In meeting we got @edolstra's preference, and I just updated the PR accordingly.

…hs-in-store-option

The was Eelco's prefered logic, and it looks good to me!

matthewbauer · 2020-07-21T15:53:02Z

src/libstore/store-api.cc

@@ -914,12 +914,20 @@ ref<Store> openStore(const std::string & uri_,
    throw Error("don't know how to open Nix store '%s'", uri);
 }

+static bool isNonUriPath(const std::string & spec) {


Can you just use !isUri from file-transfer?

…te-regStore Remove storetype delegate reg store -- contains #3736

Enable the --store option to take relative paths

e197bc6

In nix commands which accept --store options, we can now specify a relative path, which will be canonicalized.

meditans changed the title ~~Allow relative paths in --store option~~ WIP Allow relative paths in --store option Jun 23, 2020

thufschmitt reviewed Jun 24, 2020

View reviewed changes

meditans added 3 commits June 26, 2020 10:40

Merge branch 'master' of github.com:NixOS/nix into allow-relative-pat…

278a679

…hs-in-store-option

Merge branch 'master' of github.com:NixOS/nix into allow-relative-pat…

fefd6c9

…hs-in-store-option

Test relative store paths

4178f36

meditans force-pushed the allow-relative-paths-in-store-option branch from 8e5be0c to 4178f36 Compare July 17, 2020 19:51

meditans changed the title ~~WIP Allow relative paths in --store option~~ Allow relative paths in --store option Jul 17, 2020

Change logic for deciding what is a relative path for the local store

6cce32c

The was Eelco's prefered logic, and it looks good to me!

matthewbauer reviewed Jul 21, 2020

View reviewed changes

edolstra merged commit ff314f1 into NixOS:master Jul 21, 2020

roberth mentioned this pull request Sep 15, 2020

Remove storetype delegate reg store -- contains #3736 #3829

Merged

edolstra added a commit that referenced this pull request Sep 17, 2020

Merge pull request #3829 from obsidiansystems/remove-storetype-delega…

649d3aa

…te-regStore Remove storetype delegate reg store -- contains #3736

ldesgoui mentioned this pull request Aug 1, 2021

Write Nix 2.4 release notes #4521

Closed

Ericson2314 deleted the allow-relative-paths-in-store-option branch April 19, 2023 17:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow relative paths in --store option #3736

Allow relative paths in --store option #3736

meditans commented Jun 23, 2020 •

edited

meditans commented Jun 23, 2020

thufschmitt Jun 24, 2020

meditans Jun 26, 2020

matthewbauer Jul 8, 2020

Ericson2314 Jul 17, 2020

matthewbauer Jul 17, 2020

matthewbauer Jul 17, 2020 •

edited

Ericson2314 Jul 18, 2020 •

edited

Ericson2314 Jul 20, 2020

Ericson2314 Jul 21, 2020

matthewbauer Jul 21, 2020

	Path lookupFileArg(EvalState & state, string s)
	{
	if (isUri(s)) {
	return state.store->toRealPath(
	fetchers::downloadTarball(
	state.store, resolveUri(s), "source", false).first.storePath);
	} else if (s.size() > 2 && s.at(0) == '<' && s.at(s.size() - 1) == '>') {
	Path p = s.substr(1, s.size() - 2);
	return state.findFile(p);
	} else
	return absPath(s);
	}

Allow relative paths in --store option #3736

Allow relative paths in --store option #3736

Conversation

meditans commented Jun 23, 2020 • edited

meditans commented Jun 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewbauer Jul 17, 2020 • edited

Choose a reason for hiding this comment

Ericson2314 Jul 18, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meditans commented Jun 23, 2020 •

edited

matthewbauer Jul 17, 2020 •

edited

Ericson2314 Jul 18, 2020 •

edited