Skip to content

Conversation

@jeremywiebe
Copy link
Contributor

@jeremywiebe jeremywiebe commented Nov 14, 2025

Summary:

When I cut the last release, it hit an error during the release. It looks like it was trying to copy a file/dir within node_modules to the dist/ folder for a package. This shouldn't be needed as we bundle our JS actions into a single dist/index.js file which has no external dependencies.

So, I've re-worked the build system a little bit. We copy the action.yml, package.json (if it exists), and then build the index.js to dist/index.js using ncc (if the source file exists).

This simplifies our build setup and will clean up what we publish (right now we publish the tags with a bunch of source artifacts that aren't needed).

node:fs:3035
  binding.copyFile(
          ^
Error: EISDIR: illegal operation on a directory, copyfile 'actions/check-for-changeset/node_modules/filter-files' -> 'actions/check-for-changeset/dist/node_modules/filter-files'
    at Object.copyFileSync (node:fs:3035:11)
    at copyDir (file:///home/runner/work/actions/actions/utils/build.mjs:24:16)
    at copyDir (file:///home/runner/work/actions/actions/utils/build.mjs:22:13)
    at buildPackage (file:///home/runner/work/actions/actions/utils/build.mjs:73:5)
    at file:///home/runner/work/actions/actions/utils/publish.mjs:119:30
    at Array.forEach (<anonymous>)
    at publishAsNeeded (file:///home/runner/work/actions/actions/utils/publish.mjs:113:18)
    at file:///home/runner/work/actions/actions/utils/run-publish.mjs:11:1
    at ModuleJob.run (node:internal/modules/esm/module_job:325:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:606:24) {
  errno: -21,
  code: 'EISDIR',
  syscall: 'copyfile',
  path: 'actions/check-for-changeset/node_modules/filter-files',
  dest: 'actions/check-for-changeset/dist/node_modules/filter-files'
}
Node.js v20.19.5

Issue: XXX-XXXX

Test plan:

I tested by running a publish in a dry-run:

Dry run output
Publishing (dry run)...
Processing apply-terraform-plan...
  Version apply-terraform-plan-v2.1.0 already exists. Nothing to do.

Processing check-for-changeset...
  Copying actions/check-for-changeset/package.json to actions/check-for-changeset/dist/package.json
  Copying actions/check-for-changeset/CHANGELOG.md to actions/check-for-changeset/dist/CHANGELOG.md
  Copying actions/check-for-changeset/package.json to actions/check-for-changeset/dist/package.json
  Processing action.yml for check-for-changeset
    Processing dependency: get-changed-files
      Replacing with: Khan/[email protected]
    Processing dependency: filter-files
      Replacing with: Khan/[email protected]
  Building actions/check-for-changeset/index.js
  Publishing check-for-changeset-v1.0.3 in actions/check-for-changeset/dist
  >> git init .
  >> git add .
  >> git config user.email "[email protected]"
  >> git config user.name "Khan Actions Bot"
  >> git commit -m publish
  >> git remote add origin [email protected]:Khan/actions.git
  >> git fetch origin --tags
  >> git tag check-for-changeset-v1.0.3
  >> git tag -f check-for-changeset-v1
🏁  Finished publishing check-for-changeset-v1.0.3

Processing cypress-install...
  Version cypress-install-v0.0.1 already exists. Nothing to do.

Processing filter-files...
  Version filter-files-v2.1.0 already exists. Nothing to do.

Processing full-or-limited...
  Version full-or-limited-v0.0.2 already exists. Nothing to do.

Processing generate-terraform-plan...
  Version generate-terraform-plan-v2.2.1 already exists. Nothing to do.

Processing gerald-pr...
  Copying actions/gerald-pr/package.json to actions/gerald-pr/dist/package.json
  Copying actions/gerald-pr/CHANGELOG.md to actions/gerald-pr/dist/CHANGELOG.md
  Copying actions/gerald-pr/package.json to actions/gerald-pr/dist/package.json
  Processing action.yml for gerald-pr
    Processing dependency: get-changed-files
      Replacing with: Khan/[email protected]
  Publishing gerald-pr-v3.1.4 in actions/gerald-pr/dist
  >> git init .
  >> git add .
  >> git config user.email "[email protected]"
  >> git config user.name "Khan Actions Bot"
  >> git commit -m publish
  >> git remote add origin [email protected]:Khan/actions.git
  >> git fetch origin --tags
  >> git tag gerald-pr-v3.1.4
  >> git tag -f gerald-pr-v3
🏁  Finished publishing gerald-pr-v3.1.4

Processing get-changed-files...
  Copying actions/get-changed-files/package.json to actions/get-changed-files/dist/package.json
  Copying actions/get-changed-files/CHANGELOG.md to actions/get-changed-files/dist/CHANGELOG.md
  Copying actions/get-changed-files/package.json to actions/get-changed-files/dist/package.json
  Processing action.yml for get-changed-files
  Building actions/get-changed-files/index.js
  Publishing get-changed-files-v2.1.1 in actions/get-changed-files/dist
  >> git init .
  >> git add .
  >> git config user.email "[email protected]"
  >> git config user.name "Khan Actions Bot"
  >> git commit -m publish
  >> git remote add origin [email protected]:Khan/actions.git
  >> git fetch origin --tags
  >> git tag get-changed-files-v2.1.1
  >> git tag -f get-changed-files-v2
🏁  Finished publishing get-changed-files-v2.1.1

Processing json-args...
  Version json-args-v1.0.0 already exists. Nothing to do.

Processing shared-node-cache...
  Version shared-node-cache-v3.0.0 already exists. Nothing to do.
dist/ folder contents
actions/check-for-changeset/dist
actions/check-for-changeset/dist/sourcemap-register.js
actions/check-for-changeset/dist/index.js
actions/check-for-changeset/dist/package.json
actions/check-for-changeset/dist/index.js.map
actions/check-for-changeset/dist/action.yml

actions/filter-files/dist
actions/filter-files/dist/CHANGELOG.md
actions/filter-files/dist/sourcemap-register.js
actions/filter-files/dist/index.js
actions/filter-files/dist/package.json
actions/filter-files/dist/index.js.map
actions/filter-files/dist/action.yml

actions/full-or-limited/dist
actions/full-or-limited/dist/CHANGELOG.md
actions/full-or-limited/dist/package.json
actions/full-or-limited/dist/action.yml

actions/gerald-pr/dist
actions/gerald-pr/dist/package.json
actions/gerald-pr/dist/action.yml

actions/get-changed-files/dist
actions/get-changed-files/dist/sourcemap-register.js
actions/get-changed-files/dist/index.js
actions/get-changed-files/dist/package.json
actions/get-changed-files/dist/index.js.map
actions/get-changed-files/dist/action.yml

actions/json-args/dist
actions/json-args/dist/CHANGELOG.md
actions/json-args/dist/package.json
actions/json-args/dist/action.yml

actions/shared-node-cache/dist
actions/shared-node-cache/dist/CHANGELOG.md
actions/shared-node-cache/dist/package.json
actions/shared-node-cache/dist/action.yml

@changeset-bot
Copy link

changeset-bot bot commented Nov 14, 2025

🦋 Changeset detected

Latest commit: 7465a42

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
get-changed-files Patch
check-for-changeset Patch
gerald-pr Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jeremywiebe jeremywiebe marked this pull request as ready for review November 14, 2025 02:13
@khan-actions-bot khan-actions-bot requested review from a team, kevinb-khan and somewhatabstract and removed request for a team November 14, 2025 02:14
@khan-actions-bot
Copy link
Contributor

khan-actions-bot commented Nov 14, 2025

Gerald

Required Reviewers
  • @Khan/github-actions for changes to README.md, package.json, pnpm-lock.yaml, .changeset/short-keys-brush.md, utils/build.mjs, utils/publish.mjs

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

@jeremywiebe jeremywiebe requested a review from jaredly November 14, 2025 05:09
@khan-actions-bot khan-actions-bot requested a review from a team November 14, 2025 16:37
Comment on lines +81 to +82
bundleIfExists(`${base}/package.json`);
bundleIfExists(`${base}/*.md`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This means that we "opt in" to the files we bundle instead of copying them all. The original code was copying the node_modules dirs over which isn't needed because ncc creates a single-file bundle that doesn't need any external libs.

Copy link
Member

@somewhatabstract somewhatabstract left a comment

Choose a reason for hiding this comment

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

This looks good however it's concerning that there are no tests for this code. It seems pretty complex and tests would help avoid issues.

"get-changed-files": patch
---

Fixed build
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: It would be helpful if this were a little more descriptive about the change.

Copy link
Contributor

Choose a reason for hiding this comment

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

++

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll expand, however, I view Changesets notes as end-user facing info. As a result, I don't think the description here should describe infrastructure changes that went into fixing the build - that's for the PR description. Someone consuming get-changed-files isn't going to know or understand the changes in this PR that led to a new release. The only reason I'm bumping get-changed-files is because the last release failed.

I'd love to hear your thoughts.

Copy link
Member

@somewhatabstract somewhatabstract Nov 14, 2025

Choose a reason for hiding this comment

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

Agreed, they are end-user facing. I think that understanding what was changed impacts both decisions around picking up the new changes, as well as helping figure out why things are going wrong. I think being clearer about how this affects the build is important for that second step as it leaves a breadcrumb that may help folks troubleshoot problems.

monorepoName,
) => {
console.log("Processing action.yml for", name);
console.log(" Processing action.yml for", name);
Copy link
Member

@somewhatabstract somewhatabstract Nov 14, 2025

Choose a reason for hiding this comment

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

suggestion[nit]: You don't need to do anything for this, just noting it. All these indent shifts indicate to me that we should be using console.group so indentation is handled for us. Or at least tracking indentation level via a variable so spacing is just n * level and things will automatically self-adjust if we increase or decrease the indentation of a parent.

Copy link
Member

@somewhatabstract somewhatabstract left a comment

Choose a reason for hiding this comment

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

Meant to check Approve on that last review.

If you add tests, that would be great 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants