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

feat(essentials)!: In a zero-install CI environment, --check-cache or --no-check-cache needs to be explicitly defined #4857

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jj811208
Copy link
Contributor

@jj811208 jj811208 commented Sep 17, 2022

What's the problem this PR addresses?

Related #3591

This addresses the task:

Make Yarn detect whether it's running inside a public repository (GitHub Actions) and, if it is AND the repository uses zero-installs, exit and recommend adding either --check-cache or --no-check-cache.

Some Thoughts:

  • All CI environments require users to explicitly use --check-cache or --no-check-cache
    1. I checked github action document and ci-info package but I didn't get any help to determine if the current environment is a public repository.
    2. I think this is very worthwhile to use in any CI (including private repositories)

  • Not sure how to accurately determine if zero-install mode is enabled

    I checked zero-install's documentation and read the related code, but I can't seem to tell if zero-install is enabled or not.

    So, finally, I assume that the following conditions are met, representing that the zero-install mode is on
    1. in a CI environment
    2. pnp strict mode is enabled
    3. the .yarn/cache directory existed before the install command execute.

  • I can't fix the netlify deployment error, if I'm going in the right direction, we should add --no-check-cache on that side

...

How did you fix it?

  1. If the install command is in a CI environment and zero-install mode is enabled, but not explicitly defined --check-cache or --no-check-cache, throws an error
    d43d137
  2. Add a test about the feature
    a9933f2
  3. Avoid all about yarn install tests to concerns this
    68d2392
    0fb04d3

...

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@jj811208 jj811208 changed the title Feat/install detect environment feat(essentials):make install detect CI environment Sep 17, 2022
@jj811208 jj811208 changed the title feat(essentials):make install detect CI environment feat(essentials): make install detect CI environment Sep 17, 2022
@jj811208 jj811208 force-pushed the feat/install-detect-environment branch from 3bf704a to d8377ce Compare September 17, 2022 18:15
@jj811208
Copy link
Contributor Author

jj811208 commented Sep 17, 2022

I found out how to get whether the project is a github public repository in Configuration.ts#L29-L31

but I want to know why we only focus on these

  1. only for Github repository
  2. only for public repository
  3. only for Github actions

I think making all CI environments explicitly define --check-cache or --no-check-cache ensures that users understand the security implications of zero-install is quite meaningful

I like yarn, but I am new at this project, maybe I missed something.

@jj811208 jj811208 changed the title feat(essentials): make install detect CI environment feat(essentials): In a zero-install CI environment, --check-cache' or --no-check-cache' needs to be explicitly defined. Sep 17, 2022
@jj811208 jj811208 changed the title feat(essentials): In a zero-install CI environment, --check-cache' or --no-check-cache' needs to be explicitly defined. feat(essentials): In a zero-install CI environment, --check-cache' or --no-check-cache' needs to be explicitly defined Sep 17, 2022
@jj811208 jj811208 force-pushed the feat/install-detect-environment branch 2 times, most recently from e1adcb1 to 1008f91 Compare September 18, 2022 06:04
@jj811208 jj811208 changed the title feat(essentials): In a zero-install CI environment, --check-cache' or --no-check-cache' needs to be explicitly defined feat(essentials): In a zero-install CI environment, --check-cache or --no-check-cache needs to be explicitly defined Sep 18, 2022
@jj811208 jj811208 marked this pull request as ready for review September 18, 2022 06:28
@jj811208 jj811208 requested a review from arcanis as a code owner September 18, 2022 06:28
@jj811208 jj811208 force-pushed the feat/install-detect-environment branch from 1008f91 to b5bb7c1 Compare September 18, 2022 15:29
@jj811208 jj811208 changed the title feat(essentials): In a zero-install CI environment, --check-cache or --no-check-cache needs to be explicitly defined feat(essentials)!: In a zero-install CI environment, --check-cache or --no-check-cache needs to be explicitly defined Sep 18, 2022
@jj811208 jj811208 force-pushed the feat/install-detect-environment branch 2 times, most recently from da2feac to 90790ad Compare September 22, 2022 15:50
@jj811208 jj811208 force-pushed the feat/install-detect-environment branch from 90790ad to 92de7eb Compare October 5, 2022 12:16
@merceyz merceyz self-requested a review October 5, 2022 15:40
@jj811208 jj811208 force-pushed the feat/install-detect-environment branch from 92de7eb to cd8939a Compare October 22, 2022 11:20
`yarn install` should be explicitly defining check-cache option,  if run in a CI environment and zero-install mode is enabled
`yarn install` should be explicitly defining check-cache option,  if run in a CI environment and zero-install mode is enabled
@jj811208 jj811208 force-pushed the feat/install-detect-environment branch from cd8939a to 3b2a718 Compare October 29, 2022 07:04
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.

1 participant