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

Implements --no-default-rc and --use-yarnrc #6007

Merged
merged 1 commit into from
Jun 27, 2018

Conversation

arcanis
Copy link
Member

@arcanis arcanis commented Jun 18, 2018

Summary

This PR implements the --no-default-rc and --use-yarnrc options.

  • --no-default-rc disable the rcfile lookup
  • --use-yarnrc <path> loads a specified yarnrc file

This allow to make more deterministic installs, since the exact rc files used are now known to the caller. I haven't implemented --use-npmrc as every settings should be configurable from a yarnrc file anyway.

Test plan

Added tests.

@buildsize
Copy link

buildsize bot commented Jun 18, 2018

File name Previous Size New Size Change
yarn-[version].noarch.rpm 925.42 KB 926.38 KB 987 bytes (0%)
yarn-[version].js 4 MB 4.01 MB 3.89 KB (0%)
yarn-legacy-[version].js 4.16 MB 4.17 MB 3.9 KB (0%)
yarn-v[version].tar.gz 930.83 KB 931.78 KB 969 bytes (0%)
yarn_[version]all.deb 685.28 KB 685.77 KB 502 bytes (0%)

@arcanis arcanis changed the title Implements --no-default-rc and --use-rc Implements --no-default-rc and --use-yarnrc Jun 18, 2018
@arcanis
Copy link
Member Author

arcanis commented Jun 19, 2018

AppVeyor is timeouting on the publish command, unrelated.

Copy link
Member

@bestander bestander left a comment

Choose a reason for hiding this comment

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

LGTM

@arcanis arcanis merged commit e946807 into yarnpkg:master Jun 27, 2018
@StephanBijzitter
Copy link

StephanBijzitter commented Jul 25, 2018

Little late to the party, but this pull request has added a feature that I did not yet know I needed. Thanks, --use-yarnrc will prove to be very useful!

@clintwood
Copy link
Contributor

@arcanis, @bestander please see issue #6174

I dug into this a bit and from my limited understanding of the code base it seems that the new options enableDefaultRc and extraneousYarnrcFiles are not being propagated from within updateCwd(...).

Manually changing the code to look like this seemed to correct the issue for me (but I have no idea if this is the correct fix):

async function updateCwd(config: Config): Promise<void> {
  await fs.mkdirp(config.globalFolder);

  await config.init({
    cwd: config.globalFolder,
    binLinks: true,
    globalFolder: config.globalFolder,
    cacheFolder: config._cacheRootFolder,
    linkFolder: config.linkFolder,
    // added missing config
    enableDefaultRc: config.enableDefaultRc,
    extraneousYarnrcFiles: config.extraneousYarnrcFiles,
  });
}

@greysteil
Copy link
Contributor

@clintwood - I've just run into the same issue on Dependabot (where we create our own config, and weren't passing through a value for enableDefaultRc). I think a better fix than the one you suggested is to set the default value for enableDefaultRc to true, which I've done in #6181.

@clintwood
Copy link
Contributor

@greyepoxy - That was the other option I thought of, however, it looks like the missing config settings are set to default values on initialisation if not set by this PR's command line switches by commander. So not passing them through and defaulting them in the Config class may cause these command line options to be ignored when actually set!?

BTW just skimming through the tests it seemed this specific issue did not have any test cases.

@fenduru
Copy link
Contributor

fenduru commented Aug 1, 2018

@arcanis I believe this PR should be reverted until #6174 is resolved, as it introduced a regression that I suspect will affect most enterprise users.

Originally it seemed like #6181 would be a quick fix, but the indication there is that it won't actually address the issue in #6174.

@clintwood clintwood mentioned this pull request Aug 2, 2018
@jonaskello
Copy link

jonaskello commented Aug 28, 2018

+1 for --use-npmrc. I don't think we can specify different registries and authentication in .yarnrc?

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.

7 participants