xdg-utils: add missing dependencies #95675

erikarvstedt · 2020-08-17T10:54:35Z

Add coreutils to PATH, because the xdg scripts use other not yet provided coreutils like head.
This makes the custom 'cut' and 'sort' functions obsolete.
Remove double quotes around $out because $out contains no Bash field separators.
Replace all instances of 'which' with 'type -P'.
The previous sed command only replaced instances with a leading space.

Reproduce

firefox=$(nix-build --no-out-link '<nixpkgs>' -A firefox)
xdgUtils=$(nix-build --no-out-link '<nixpkgs>' -A xdg_utils)
env -i XDG_DATA_DIRS=$firefox/share PATH=$firefox/bin $xdgUtils/bin/xdg-mime query default x-scheme-handler/http
## Output before this patch:
# /nix/store/.../bin/xdg-mime: line 1031: head: command not found
# /nix/store/.../bin/xdg-mime: line 1031: head: command not found
# /nix/store/.../bin/xdg-mime: line 1031: head: command not found
# /nix/store/.../bin/xdg-mime: line 1031: head: command not found
# /nix/store/.../bin/xdg-mime: line 947: basename: command not found
## Output after this patch:
# firefox.desktop

- Add coreutils to PATH, because the xdg scripts use other not yet provided coreutils like head. This makes the custom 'cut' and 'sort' functions obsolete. Remove double quotes around $out because $out contains no Bash field separators. - Replace all instances of 'which' with 'type -P'. The previous sed command only replaced instances with a leading space.

erikarvstedt · 2020-09-01T07:38:37Z

/marvin opt-in
/status needs_reviewer

marvin-mk2 · 2020-09-01T07:38:39Z

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

raboof · 2020-10-19T19:56:20Z

pkgs/tools/X11/xdg-utils/default.nix

    xset()  { ${xset}/bin/xset      "$@"; }\
    perl()  { PERL5LIB=${perlPath} ${perlPackages.perl}/bin/perl "$@"; }\
    mimetype() { ${perlPackages.FileMimeInfo}/bin/mimetype "$@"; }\
-    PATH=$PATH:'"$out"'/bin\
+    PATH=$PATH:'$out'/bin:${coreutils}/bin\


It seems this was previously not done to avoid polluting $PATH (#25511 (comment)).

I think your argument that xdg-utils scripts will expect other coreutils commands, like head and basename, to be present is reasonable.

Another approach might be to process those xdg scripts to point to the specific tool implementations they expect (e.g. busybox or coreutils), but given that this derivation already pulls in coreutils it seems reasonable to just leverage those.

At least the following executables are used from coreutils: head, wc, tr, touch, test, tail. I think it is fine to pollute the $PATH with all those binaries or we wrap here for days.

Having xdg-utils contaminate the environment is bad because it can be confusing. Imagine opening a nix file in a text editor using xdg-open and then testing it in a terminal opened from the text editor. Now, arguably, users will likely have coreutils installed so this is more important when adding other programs to PATH, or when exporting other environment variables but we should still keep it in mind. Ideally we would use something like @abathur’s resholved.

resholved isn't in a production-ready state yet and it's also unvavailable in nixpkgs.
Do you propose to wait until resholved is released? This could take months, judging its current progress.

I haven't read context here so I'm commenting generally:

I don't want you to be stuck waiting for resholve to land in Nixpkgs (and hate me if it takes a bit).

But, if resholve does solve a problem here without needing any new features, I don't think it would take "months" and I'm open to discussing timelines/priorities.

It shouldn't be too hard to add resholve temporarily to test it out. It has a good core of functionality, and it works for a living in my own profile and in Graham's. I even considered it ready for inclusion earlier this year, but the override API's grown into a bit of a haphazard kitchen sink that I've decided to fix before calling an initial release.

It is basically at the top of my "priority" list, but some "urgent" projects are taking all of its sunlight.

marvin-mk2 · 2020-10-22T22:27:55Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be put back in the needs_reviewer queue in one day.

SuperSandro2000 · 2020-10-22T22:54:47Z

pkgs/tools/X11/xdg-utils/default.nix

    xset()  { ${xset}/bin/xset      "$@"; }\
    perl()  { PERL5LIB=${perlPath} ${perlPackages.perl}/bin/perl "$@"; }\
    mimetype() { ${perlPackages.FileMimeInfo}/bin/mimetype "$@"; }\
-    PATH=$PATH:'"$out"'/bin\
+    PATH=$PATH:'$out'/bin:${coreutils}/bin\


At least the following executables are used from coreutils: head, wc, tr, touch, test, tail. I think it is fine to pollute the $PATH with all those binaries or we wrap here for days.

marvin-mk2 · 2020-10-26T03:41:23Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be put back in the needs_reviewer queue in one day.

marvin-mk2 · 2020-10-29T21:17:54Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be put back in the needs_reviewer queue in one day.

SuperSandro2000 · 2020-10-30T04:18:40Z

Result of nixpkgs-review pr 95675 run on x86_64-darwin 1

2 packages marked as broken and skipped:

far2l
fff

2 packages failed to build:

eureka-editor
manim

4 packages built:

alacritty
gitAndTools.git-open
mkvtoolnix-cli
xdg_utils

marvin-mk2 · 2020-11-02T08:47:41Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be put back in the needs_reviewer queue in one day.

SuperSandro2000 · 2020-11-04T15:34:52Z

Result of nixpkgs-review pr 95675 run on x86_64-darwin 1

2 packages marked as broken and skipped:

far2l
fff

2 packages failed to build:

eureka-editor
manim

4 packages built:

alacritty
gitAndTools.git-open
mkvtoolnix-cli
xdg_utils

src/main.cc:55:10: fatal error: 'OSXCalls.h' file not found
#include "OSXCalls.h"
         ^~~~~~~~~~~~
5 warnings and 1 error generated.
make: *** [Makefile:120: obj_linux/main.o] Error 1
make: *** Waiting for unfinished jobs....

marvin-mk2 · 2020-11-07T17:02:57Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be put back in the needs_reviewer queue in one day.

marvin-mk2 · 2020-11-10T22:30:27Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be put back in the needs_reviewer queue in one day.

marvin-mk2 · 2020-11-14T00:11:05Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be put back in the needs_reviewer queue in one day.

erikarvstedt force-pushed the fix-sdg-utils branch 2 times, most recently from 80eb2c5 to 3565781 Compare August 17, 2020 11:01

erikarvstedt force-pushed the fix-sdg-utils branch from 3565781 to 68559e2 Compare August 17, 2020 11:04

ofborg bot requested a review from edolstra August 17, 2020 11:36

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels Aug 17, 2020

marvin-mk2 bot added the marvin label Sep 1, 2020

marvin-mk2 bot added the needs_reviewer label Sep 1, 2020

raboof approved these changes Oct 19, 2020

View reviewed changes

marvin-mk2 bot added awaiting_reviewer and removed needs_reviewer labels Oct 19, 2020

SuperSandro2000 approved these changes Oct 22, 2020

View reviewed changes

SuperSandro2000 mentioned this pull request Nov 4, 2020

eureka-editor: mark broken on darwin #102775

Merged

10 tasks

Mic92 merged commit d092541 into NixOS:master Nov 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xdg-utils: add missing dependencies #95675

xdg-utils: add missing dependencies #95675

erikarvstedt commented Aug 17, 2020 •

edited

erikarvstedt commented Sep 1, 2020

marvin-mk2 bot commented Sep 1, 2020

raboof Oct 19, 2020

SuperSandro2000 Oct 22, 2020

jtojnar Oct 26, 2020

erikarvstedt Oct 26, 2020

abathur Oct 26, 2020

marvin-mk2 bot commented Oct 22, 2020

SuperSandro2000 Oct 22, 2020

marvin-mk2 bot commented Oct 26, 2020

marvin-mk2 bot commented Oct 29, 2020

SuperSandro2000 commented Oct 30, 2020

marvin-mk2 bot commented Nov 2, 2020

SuperSandro2000 commented Nov 4, 2020 •

edited

marvin-mk2 bot commented Nov 7, 2020

marvin-mk2 bot commented Nov 10, 2020

marvin-mk2 bot commented Nov 14, 2020

xdg-utils: add missing dependencies #95675

xdg-utils: add missing dependencies #95675

Conversation

erikarvstedt commented Aug 17, 2020 • edited

Reproduce

erikarvstedt commented Sep 1, 2020

marvin-mk2 bot commented Sep 1, 2020

raboof Oct 19, 2020

Choose a reason for hiding this comment

SuperSandro2000 Oct 22, 2020

Choose a reason for hiding this comment

jtojnar Oct 26, 2020

Choose a reason for hiding this comment

erikarvstedt Oct 26, 2020

Choose a reason for hiding this comment

abathur Oct 26, 2020

Choose a reason for hiding this comment

marvin-mk2 bot commented Oct 22, 2020

SuperSandro2000 Oct 22, 2020

Choose a reason for hiding this comment

marvin-mk2 bot commented Oct 26, 2020

marvin-mk2 bot commented Oct 29, 2020

SuperSandro2000 commented Oct 30, 2020

marvin-mk2 bot commented Nov 2, 2020

SuperSandro2000 commented Nov 4, 2020 • edited

marvin-mk2 bot commented Nov 7, 2020

marvin-mk2 bot commented Nov 10, 2020

marvin-mk2 bot commented Nov 14, 2020

erikarvstedt commented Aug 17, 2020 •

edited

SuperSandro2000 commented Nov 4, 2020 •

edited