pkgs: refactor needless quoting of homepage meta attribute #27809

infinisil · 2017-07-31T19:08:00Z

A lot of packages are needlessly quoting the homepage meta attribute
(about 1400, 22%), this commit refactors all of those instances.

Command used for refactoring (using ripgrep):

rg 'homepage = "[^\$\{#\(]*"' -g '!*-packages.nix' -g '!*-generated.nix' -l | xargs sed -i 's/homepage = "\([^\$\{#\(]*\)"/homepage = \1/g'

Notes: This excludes all the automatically generated files and the ones that contain an URL that wouldn't be valid unquoted (containing ${/#/(), which obviously shouldn't be refactored.

Motivation for this change

Of the 6280 instances of the homepage attribute (rg 'homepage = .*' -g '!*-packages.nix' -g '!*-generated.nix' -l | wc -l), there are 1396 instances where it's needlessly quoted (rg 'homepage = "[^\$\{#\(]*"' -g '!*-packages.nix' -g '!*-generated.nix' -l | wc -l). This PR makes all of these uniform.

Things done

Tested the changes with

nix-env -f . -qaP --meta --json --drv-path --show-trace

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
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 nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

A lot of packages are needlessly quoting the homepage meta attribute (about 1400, 22%), this commit refactors all of those instances.

commit

infinisil · 2017-07-31T20:09:10Z

I used this command to track down some links that shouldn't have been unquoted:

rg 'homepage = [a-z]+:.*;' --no-filename -o | while read e; do echo $e; nix-instantiate --eval -E "let $e in homepage"; done 2>&1 | rg 'error: ' -B 1

and fixed them.

infinisil · 2017-07-31T20:10:03Z

pkgs/tools/security/vulnix/requirements.nix

@@ -22,7 +22,6 @@ rec {
    ]);

    meta = with stdenv.lib; {
-      homepage = ;


This package wasn't declaring any homepage anyways, so I removed the attribute.

FRidh · 2017-08-01T19:55:07Z

You can use the following command to check evaluation:

nix-env -f . -qaP --meta --json --drv-path --show-trace

infinisil · 2017-08-01T20:01:23Z

@FRidh Thanks a lot, used it to track down the last few errors.

copumpkin · 2017-08-01T20:56:35Z

Dammit, this is the opposite direction I was hoping we'd go 😦 Lots of us in the community think the magic URI/URL syntax in Nix is a wart and would like to move away from it, so I quote all URLs in my packages precisely because I want to make it easier for us to eventually ditch the special cases in the Nix grammar. The grammar is awkward today because it doesn't really parse full URIs and it can parse things as URIs that nobody intended to be. There's a Nix ticket on this but I can't find it right now.

copumpkin · 2017-08-01T20:59:06Z

Here's some links: NixOS/nix#1017 NixOS/nix#836 NixOS/nix#1019

infinisil · 2017-08-01T21:00:07Z

@copumpkin Didn't know about this, I also thought maybe this should get some more eyes before being merged. I don't really mind either way of doing it, as long as it's consistent, and since only 22% were using strings, I thought this would be the better way.

I would totally be willing to undo this PR and change every other occurence of non-quoted URL's to quoted ones.

copumpkin · 2017-08-01T21:02:14Z

I dunno, I suppose that now we have an RFC process, the right thing to do would be to put it to a vote to eliminate the magic syntax since it doesn't really buy us anything over strings, and complicates the grammar (and human model of syntax) considerably. Then as part of that, we'd mass-rewrite URLs to use quotes and slowly deprecate the syntax over the next few Nix releases. The whole "grassroots" thing I described above was probably not a very effective way of going about it 😄

infinisil · 2017-08-01T21:06:18Z

@copumpkin Do you think this PR should be reverted for now then? Having 22% packages that do it 'right' is better than 0% I'd say :)

cstrahan · 2017-08-03T01:44:26Z

Dammit, this is the opposite direction I was hoping we'd go 😦 Lots of us in the community think the magic URI/URL syntax in Nix is a wart and would like to move away from it, so I quote all URLs in my packages precisely because I want to make it easier for us to eventually ditch the special cases in the Nix grammar.

FWIW, that's exactly my stance as well. Nixpkgs has had some issues in the past where the ambiguity between functions (x:y: <body>) and unquoted URLs caused hard to track problems. Sadly, I can't recall which commits.

FRidh · 2017-08-03T07:07:40Z

I also prefer to use strings instead of some special type. But currently that's what we have so I'm of the opinion we should stick with it until we've actually decided otherwise. So, indeed, go for a RFC :)

globin · 2017-08-03T09:04:21Z

x:x vs x: x:

nix-repl
Welcome to Nix version 1.11.13. Type :? for help.

nix-repl> id = x:x

nix-repl> id' = x: x

nix-repl> id 1
error: attempt to call something which is not a function but a string, at (string):1:1

nix-repl> id
"x:x"

nix-repl> id' 1
1

nix-repl> id'
«lambda»

danbst · 2017-08-05T04:51:24Z

If this URL syntax removed, nix will become a language, where id implementation uses minimal amount of symbols.

oxij · 2018-02-11T15:57:29Z

From #34798. I see no policy behind this. What I see is @infinisil making a huge change to nixpkgs that goes backwards and then @FRidh merging it without discussion. And now you want people to make an RFC instead of just reverting this.

I don't like this personally but we have #27809 as a precedent.

pkgs: refactor needless quoting of homepage meta attribute

86a6bc4

A lot of packages are needlessly quoting the homepage meta attribute (about 1400, 22%), this commit refactors all of those instances.

infinisil requested a review from FRidh as a code owner July 31, 2017 19:08

pkgs: Fixing some links that were wrongfully unquoted in the previous

96df167

commit

infinisil commented Jul 31, 2017

View reviewed changes

Fixed some instances

052ec6b

FRidh merged commit f5fa5fa into NixOS:master Aug 1, 2017

vyp referenced this pull request Sep 24, 2017

guile-fibers: nitpicks

6786ad7

vyp mentioned this pull request Oct 16, 2017

Build Tensorflow from source #30434

Merged

8 tasks

rnhmjoj mentioned this pull request Jan 13, 2018

rewritefs: remove needless quotation #33818

Merged

oxij mentioned this pull request Feb 10, 2018

aspellDicts: add more dictionaries and some documentation #34798

Merged

6 tasks

abbradar added a commit that referenced this pull request Feb 25, 2018

treewide: unquote homepage in my packages

08f8ad2

I don't like this personally but we have #27809 as a precedent.

infinisil deleted the url-quote-refactor branch July 1, 2018 20:13

infinisil mentioned this pull request Jul 10, 2018

kail: init at 0.6.0 #43278

Merged

infinisil mentioned this pull request Sep 8, 2018

treewide: Unquote most URLs #46378

Closed

9 tasks

FRidh mentioned this pull request May 1, 2019

[RFC 0045] Deprecating unquoted URL syntax NixOS/rfcs#45

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkgs: refactor needless quoting of homepage meta attribute #27809

pkgs: refactor needless quoting of homepage meta attribute #27809

infinisil commented Jul 31, 2017 •

edited

infinisil commented Jul 31, 2017

infinisil Jul 31, 2017

FRidh commented Aug 1, 2017

infinisil commented Aug 1, 2017

copumpkin commented Aug 1, 2017

copumpkin commented Aug 1, 2017

infinisil commented Aug 1, 2017

copumpkin commented Aug 1, 2017

infinisil commented Aug 1, 2017

cstrahan commented Aug 3, 2017

FRidh commented Aug 3, 2017

globin commented Aug 3, 2017 •

edited

danbst commented Aug 5, 2017

oxij commented Feb 11, 2018

pkgs: refactor needless quoting of homepage meta attribute #27809

pkgs: refactor needless quoting of homepage meta attribute #27809

Conversation

infinisil commented Jul 31, 2017 • edited

Motivation for this change

Things done

infinisil commented Jul 31, 2017

infinisil Jul 31, 2017

Choose a reason for hiding this comment

FRidh commented Aug 1, 2017

infinisil commented Aug 1, 2017

copumpkin commented Aug 1, 2017

copumpkin commented Aug 1, 2017

infinisil commented Aug 1, 2017

copumpkin commented Aug 1, 2017

infinisil commented Aug 1, 2017

cstrahan commented Aug 3, 2017

FRidh commented Aug 3, 2017

globin commented Aug 3, 2017 • edited

danbst commented Aug 5, 2017

oxij commented Feb 11, 2018

infinisil commented Jul 31, 2017 •

edited

globin commented Aug 3, 2017 •

edited