changesets / action

684 stars 244 forks source link

[1.2.0] .npmrc detection depends on HOME process env #147

Open belgattitude opened 2 years ago

belgattitude commented 2 years ago

Not a big issue, but since latest release the .npmrc detection depends on process.env.HOME.

It broke our release (private repo with github private repositories). HOME was somehow changed in the previous step by our build scripts, thus .npmrc was wrongly recreated.

Workaround

To help if someone got hit.

      - name: Create Release Pull Request or Publish to GPR
        id: changesets
        uses: changesets/action@v1.2.0
        with:
          publish: yarn release
        env:
          HOME: ${{ github.workspace }}

Fix

I would gladly send a fix (or a doc), but I'm not sure how process.env.HOME and the new cwd param should behave. Otherwise as I read that .npmrc related code will be deprecated / removed, just a note is fine

Andarist commented 2 years ago

Otherwise as I read that .npmrc related code will be deprecated / removed, just a note is fine

This is what I plan to do. The "auto" behavior that we currently implement sounds nice on the surface but there are use cases that can't be covered with this. The "manual" solution that would roughly behave as the one included in the Changesets action is basically something like this:

env:
  NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

So if we consider this then including the logic in the action itself to create .npmrc seems like a huge overkill that falls short on a number of occasions.

belgattitude commented 2 years ago

Thanks for sharing, I agree with the reasoning. When we don't exactly fit in the "auto" it becomes harder to opt-out somehow.

For extra info another tricky thing:

https://github.com/changesets/action/blob/898d125cee6ba00c6a11b6cadca512752c6c910c/src/index.ts#L59-L62

If we don't actually use the npm registry, let's say only GPR instead, it still append the npm registry to the .npmrc. I don't know exactly what are the consequences of this. But that's difficult to opt-out.

Thanks for sharing your thoughts. I really appreciate the direction your're taking.

Andarist commented 2 years ago

If we don't actually use the npm registry, let's say only GPR instead, it still append the npm registry to the .npmrc. I don't know exactly what are the consequences of this. But that's difficult to opt-out.

In theory, this shouldn't have any consequences - or at least, that is the intention there. I agree though with the raised concerns about this and I plan to address this (like it has been mentioned in the thread).

TeemuKoivisto commented 2 years ago

I don't really understand what is going on, but adding that HOME environment variable fixed my changesets/action@v1 step. However, what really has annoyed me that the common workaround, generating .npmrc by yourself with eg echo "//registry.npmjs.org/:_authToken=${{ secrets.NPM_TOKEN }}" > .npmrc doesn't work because changeset happily commits it to the PR, thus making the token public (which is luckily quickly revoked by npm).

So instead of having to gitignore .npmrc I only have to ignore whatever GH action leaves behind, seems to be just .cache and .netrc. Would be nice if this was mentioned somewhere.

Andarist commented 2 years ago

Note that the echo~ recommendation is to create it in $HOME which usually should be outside of your repository. It looks like you have decided to create it within your repo and thus it got committed. I agree though that it shouldn't be committed though - we should only commit what we control/what we touch.

TeemuKoivisto commented 2 years ago

@Andarist okay. Well in the end I did end up gitignoring .npmrc because I just keep tripping on it time after time. And yes I used cat << EOF > "$HOME/.npmrc" but didn't seem to help.

belgattitude commented 2 years ago

@TeemuKoivisto FWIW

This is the config I use when publishing to GPR (.npmrc contains @xyz:registry=https://npm.pkg.github.com //npm.pkg.github.com/:_authToken=${GITHUB_PACKAGES_TOKEN}). I just ignored the netrc.

      # Automatic .npmrc generation will be removed, for now it's hard to debug
      # See https://github.com/changesets/action/issues/147#issuecomment-1030597823
      - name: Debug if .npmrc is present in working directory
        run: |
          cat ${{ github.workspace }}/.npmrc
      - name: Create Release Pull Request or Publish to GPR
        id: changesets
        uses: changesets/action@v1.2.1
        with:
          publish: yarn g:release
          cwd: ${{ github.workspace }}
          title: '[Release] Version packages'
        env:
          # See https://github.com/changesets/action/issues/147
          HOME: ${{ github.workspace }}
          # allows to download / query / comment packages
          GITHUB_TOKEN: ${{ secrets.CHANGESET_PAT_TOKEN }}
          # allows to publish packages
          GITHUB_PACKAGES_TOKEN: ${{ secrets.CHANGESET_PACKAGE_PUBLISH_TOKEN }}
Nils-Kolvenbach commented 8 months ago

I had a quite specific issue inside my GitHub action: Error: ENOENT: no such file or directory, open 'D:\a\client\client\undefined\.netrc' when trying to run electron-forge publish. This only happend inside the windows pipeline, macOS and linux worked without modifying the HOME environment variable.

Manually setting HOME to the github.workspace worked, thanks!

yashsway commented 3 weeks ago

I agree with OP here wholeheartedly.

Tried configuring the action today and ran into a couple of oddities that are really annoying to debug and opt out of:

I'll try to spend some time on the weekend to open a PR with suggested changes to fix these issues and see what you all think...

abetoots commented 13 hours ago

@yashsway just started down this rabbit hole and seems like you're further down than me. any progress updates?