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

More structured settings in nix store URLs #11106

Open
Ericson2314 opened this issue Jul 15, 2024 · 2 comments
Open

More structured settings in nix store URLs #11106

Ericson2314 opened this issue Jul 15, 2024 · 2 comments
Labels
settings Settings, global flags, nix.conf store Issues and pull requests concerning the Nix store

Comments

@Ericson2314
Copy link
Member

Currently store settings are passed as query params, mapping the name of settings to strings that are encoded in the same way as nix.conf.

There are two things I don't like about this:

  • I don't like supporting arrays by splitting on spaces, that makes it impossible to have an array element that is a string maintaining a space.
  • I would like to move all the config code down to libmain, but if libstore relies on that (since store URL parsing, or at least StoreReference -> Store, probably should continue to live in libstore) then the config stuff can at most move from libutil to libstore.

A possible solution to get more structured information in the query param is as follows:

  1. Support tag[]=... as described in https://stackoverflow.com/questions/6243051/how-to-pass-an-array-within-a-query-string as a common idiom many web frameworks support (unfortunately not all in the same way). See also https://angular.io/api/common/http/HttpParamsOptions#fromObject. This gets us at least to std::map<std::string, std::string | std::vector<std::string>>, as long as no key contains []
  2. A foo.bar= flattening for nested objects, see also https://stackoverflow.com/questions/15872658/standardized-way-to-serialize-json-to-query-string. That gets us std::map<std::string, nlohmann::json> as long as no key contains . or [].
  3. A single ?__json=<percent-encoded-json> that is less good for humans, but easier for machines, as a complement to 2 and 3.

Supporting this in addition I think is definitely desirable, the real question is can we just support this, and immediately get rid of the old space-splitting logic, or would that be too big a breaking change?

Impact analysis

Here is a way to get the list of settings, bucketed by type

git grep -E -o '^ *(const )?Setting<.*> [a-zA-Z0-9]*' 'src/libstore/*store*' \
  | sed -E 's/: *(const )?Setting<(.*)> (.*)/\t\3\t\2/' \
  | jq -rRs 'split("\n")[1:-1] | map(split("\t")) | reduce .[] as $dot ({}; .[$dot.[2]] += [{"file": $dot.[0], "setting": $dot.[1] }])'

Currently, that gives:

{
  "bool": [
    {
      "file": "src/libstore/binary-cache-store.hh",
      "setting": "writeNARListing"
    },
    {
      "file": "src/libstore/binary-cache-store.hh",
      "setting": "writeDebugInfo"
    },
    {
      "file": "src/libstore/binary-cache-store.hh",
      "setting": "parallelCompression"
    },
    {
      "file": "src/libstore/local-overlay-store.hh",
      "setting": "checkMount"
    },
    {
      "file": "src/libstore/local-store.hh",
      "setting": "requireSigs"
    },
    {
      "file": "src/libstore/local-store.hh",
      "setting": "readOnly"
    },
    {
      "file": "src/libstore/s3-binary-cache-store.cc",
      "setting": "multipartUpload"
    },
    {
      "file": "src/libstore/ssh-store-config.hh",
      "setting": "compress"
    },
    {
      "file": "src/libstore/store-api.hh",
      "setting": "isTrusted"
    },
    {
      "file": "src/libstore/store-api.hh",
      "setting": "wantMassQuery"
    }
  ],
  "Path": [
    {
      "file": "src/libstore/binary-cache-store.hh",
      "setting": "secretKeyFile"
    },
    {
      "file": "src/libstore/binary-cache-store.hh",
      "setting": "localNarCache"
    },
    {
      "file": "src/libstore/ssh-store-config.hh",
      "setting": "sshKey"
    }
  ],
  "int": [
    {
      "file": "src/libstore/binary-cache-store.hh",
      "setting": "compressionLevel"
    },
    {
      "file": "src/libstore/legacy-ssh-store.hh",
      "setting": "maxConnections"
    },
    {
      "file": "src/libstore/legacy-ssh-store.hh",
      "setting": "logFD"
    },
    {
      "file": "src/libstore/remote-store.hh",
      "setting": "maxConnections"
    },
    {
      "file": "src/libstore/store-api.hh",
      "setting": "pathInfoCacheSize"
    },
    {
      "file": "src/libstore/store-api.hh",
      "setting": "priority"
    }
  ],
  "Strings": [
    {
      "file": "src/libstore/legacy-ssh-store.hh",
      "setting": "remoteProgram"
    },
    {
      "file": "src/libstore/ssh-store.cc",
      "setting": "remoteProgram"
    }
  ],
  "std::string": [
    {
      "file": "src/libstore/local-overlay-store.hh",
      "setting": "lowerStoreUri"
    },
    {
      "file": "src/libstore/s3-binary-cache-store.cc",
      "setting": "profile"
    },
    {
      "file": "src/libstore/s3-binary-cache-store.cc",
      "setting": "region"
    },
    {
      "file": "src/libstore/s3-binary-cache-store.cc",
      "setting": "scheme"
    },
    {
      "file": "src/libstore/s3-binary-cache-store.cc",
      "setting": "endpoint"
    },
    {
      "file": "src/libstore/s3-binary-cache-store.cc",
      "setting": "narinfoCompression"
    },
    {
      "file": "src/libstore/s3-binary-cache-store.cc",
      "setting": "lsCompression"
    },
    {
      "file": "src/libstore/s3-binary-cache-store.cc",
      "setting": "logCompression"
    },
    {
      "file": "src/libstore/ssh-store-config.hh",
      "setting": "sshPublicHostKey"
    },
    {
      "file": "src/libstore/ssh-store-config.hh",
      "setting": "remoteStore"
    }
  ],
  "unsigned int": [
    {
      "file": "src/libstore/remote-store.hh",
      "setting": "maxConnectionAge"
    }
  ],
  "uint64_t": [
    {
      "file": "src/libstore/s3-binary-cache-store.cc",
      "setting": "bufferSize"
    }
  ],
  "StringSet": [
    {
      "file": "src/libstore/store-api.hh",
      "setting": "systemFeatures"
    }
  ]
}

What this says is:

  • Already matches JSON encodings:

    • bool (bool)
    • std::string / Path` (string)
    • uint64_t / unsigned int / int (number)
  • Doesn't match, because array split

    • Strings / StringSet (array of strings)

The latter types are used by these settings:

  • remoteProgram
  • systemFeatures
@Ericson2314 Ericson2314 changed the title More structure settings in nix store URLs More structured settings in nix store URLs Jul 15, 2024
@Ericson2314 Ericson2314 added store Issues and pull requests concerning the Nix store settings Settings, global flags, nix.conf labels Jul 15, 2024
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2024-07-15-nix-team-meeting-minutes-161/49228/1

@teto
Copy link
Member

teto commented Jul 23, 2024

since you suggest a revamp, might be the opportunity to name/alias the builder #2766. So far I use shell variables that I pass to --option builders "$BUILDER1". The space delimiter was not convenient when defining the alias.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
settings Settings, global flags, nix.conf store Issues and pull requests concerning the Nix store
Projects
None yet
Development

No branches or pull requests

3 participants