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

Revert "Update JS-DevTools/npm-publish action to v2 (#3336)" #3350

Merged
merged 1 commit into from
May 10, 2023

Conversation

andybalaam
Copy link
Contributor

@andybalaam andybalaam commented May 10, 2023

Attempting to fix failure to publish to NPM due to an "invalid token" error https://github.com/matrix-org/matrix-js-sdk/actions/runs/4926808655/jobs/8819822367 .

This reverts commit ca9263f.


This change is marked as an internal change (Task), so will not be included in the changelog.

@andybalaam andybalaam added T-Task Tasks for the team like planning backport staging Label to automatically backport PR to staging branch labels May 10, 2023
@t3chguy
Copy link
Member

t3chguy commented May 10, 2023

Indeed, due to the major update of npm-publish, the .npmrc file no longer contains the token after the run of the action

The user's ~/.npmrc file is no longer modified. Instead, a temporary .npmrc file is used. (JS-DevTools/npm-publish#15)

So the subsequent npm commands fail.

Regressed by #3336

@andybalaam andybalaam marked this pull request as ready for review May 10, 2023 10:22
@andybalaam andybalaam requested a review from a team as a code owner May 10, 2023 10:22
@andybalaam andybalaam requested a review from t3chguy May 10, 2023 10:22
@andybalaam andybalaam added this pull request to the merge queue May 10, 2023
@andybalaam
Copy link
Contributor Author

So the subsequent npm commands fail.

Which subsequent npm commands? I only see one in this workflow.

@t3chguy
Copy link
Member

t3chguy commented May 10, 2023

@andybalaam

https://github.com/matrix-org/matrix-js-sdk/blob/develop/.github/workflows/release-npm.yml#L25-L31 runs npm publish, with v1 it first sets up the .npmrc file - with v2 it sets up a temporary one that gets thrown away

https://github.com/matrix-org/matrix-js-sdk/blob/develop/.github/workflows/release-npm.yml#L37 is the subsequent (not publish) npm command.

@andybalaam
Copy link
Contributor Author

@t3chguy ah, thank you! I was not making the leap from release.yml to release-npm.yml

@andybalaam
Copy link
Contributor Author

Feels like we should be able to specify multiple tags to npm-publish. I'll look at that if I have time.

@t3chguy
Copy link
Member

t3chguy commented May 10, 2023

That would be a nice modification, but the underlying npm publish command doesn't support it

Merged via the queue into develop with commit 73ca9c9 May 10, 2023
@andybalaam andybalaam deleted the andybalaam/revert-npm-publish-upgrade branch May 10, 2023 10:43
@RiotRobot
Copy link
Contributor

The backport to staging failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-staging staging
# Navigate to the new working tree
cd .worktrees/backport-staging
# Create a new branch
git switch --create backport-3350-to-staging
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 73ca9c9ed28847454e13da358c581769e695ff42
# Push it to GitHub
git push --set-upstream origin backport-3350-to-staging
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-staging

Then, create a pull request where the base branch is staging and the compare/head branch is backport-3350-to-staging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport staging Label to automatically backport PR to staging branch T-Task Tasks for the team like planning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants