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

yarnrc: respect XDG_CONFIG_HOME setting #2954

Closed
wants to merge 1 commit into from

Conversation

kevinburke
Copy link

If a user has a XDG_CONFIG_HOME environment variable, read and write
the yarnrc file from XDG_CONFIG_HOME/.yarnrc, not from $HOME/.yarnrc.

The specification can be found here:
https://wiki.archlinux.org/index.php/XDG_Base_Directory_support

Fixes #2334.

@kevinburke
Copy link
Author

One test failed when I ran them locally but it seems unrelated; I can add a separate issue for that if you like.

 FAIL  __tests__/commands/install/integration.js (224.941s)
  ● install should respect --no-bin-links flag

    Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.

      at ontimeout (timers.js:380:14)
      at tryOnTimeout (timers.js:244:5)
      at Timer.listOnTimeout (timers.js:214:5)

@kevinburke
Copy link
Author

Another option would be to remove the leading period if a user explicitly specifies a directory; the "hidden" nature of dotfiles was apparently an accident, see https://plus.google.com/101960720994009339267/posts/R58WgWwN9jp for more information.

@kevinburke
Copy link
Author

I just rebased against master and I believe this is failing now because of an unrelated third-party dependency problem.

@arcanis arcanis self-assigned this Apr 10, 2017
Copy link
Member

@arcanis arcanis left a comment

Choose a reason for hiding this comment

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

Thanks! Two things:

  • Could you also update the rc.js file (it's a new file, so you might need to pull)?
  • You probably also want to update getPossibleConfigLocations (otherwise, unless I'm mistaken, you will write to XDG_CONFIG_HOME, but not read from it)

Don't forget than in both case we want to read both ~/.yarnrc and ~/$XDG_CONFIG_HOME/.yarnrc (possibly with ~/.yarnrc being the most trustable authority and overriding any config inside the xdg folder).

If a user has a XDG_CONFIG_HOME environment variable, read and write
the yarnrc file from XDG_CONFIG_HOME/.yarnrc, not from $HOME/.yarnrc.

The specification can be found here:
https://wiki.archlinux.org/index.php/XDG_Base_Directory_support

Fixes yarnpkg#2334.
@jasonkarns
Copy link

jasonkarns commented Apr 21, 2017

This PR, while an improvement, does not bring yarn into compliance with XDG. The XDG spec defines a default value for XDG_CONFIG_HOME as ~/.config. This means XDG compliance would require ~/.config/yarn be used when XDG_CONFIG_HOME is not set.

Also, I would recommend not using a hidden file within the XDG location. Part of the reason for leveraging XDG is to not require the files be hidden individually. I would suggest:

$XDG_CONFIG_HOME/yarn or more conventionally: $XDG_CONFIG_HOME/yarn/config

It is obvious by the xdg-config location that the files contained therein are configuration files. Thus the "rc" suffix is redundant. And it seems to be conventional for xdg-compliant tools to create a self-named directory under the config directory in which to place all their files, thus allowing for additional files in the future.

@kevinburke kevinburke closed this Apr 21, 2017
@jasonkarns
Copy link

@kevinburke why closed? I very much want to see xdg support in yarn (raised my concerns above not to dissuade this PR but hopefully just tweak it a bit to be more spec compliant)

@felixsanz
Copy link

@kevinburke friendly ping!

@riri
Copy link

riri commented May 31, 2017

I'm a new yarn user, and I was happy to see the cache goes in $XDG_CACHE_HOME, but smiled when seeing the .yarnrc. @jasonkarns is right in his analysis, and the way it should go is rather simple finally:

  • if $XDG_CONFIG_HOME is set, use (and write to) $XDG_CONFIG_HOME/yarn/config
  • else if $HOME/.config exists, use $HOME/.config/yarn/config
  • else as a fallback, use $HOME/.yarnrc

@kevinburke
Copy link
Author

I don't have time to work on this, and there are enough different ideas about how this should be implemented, feel free to add your own patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants