Create check-dist.yml #227
Conversation
Co-authored-by: Konrad Pabjan <[email protected]>
|
Hi @brcrista! I recently set out to tackle this exact problem in my own Actions and started out by validating that the package was up to date (actions-is-packaged) but I eventually concluded that an automated build process is the right solution... so I set aside this approach (validating the Action is packaged) and focused on automating the build process. I encountered the problems you've mentioned (like triggering Actions) but I found solutions to all of them. I wrote a post about how + why ("Package GitHub Actions automatically with GitHub Actions") and created an example repository. I've also added this automated packaging to my own actions (e.g: actions-assert) and it has had a meaningful impact on the quality of my development experience. If there's an appetite to bring this solution into GitHub's first-party Actions, I'm happy to contribute, either by making Pull Requests myself or you're more than welcome to scavenge anything helpful from my work so far. ps. I noticed in your other PR actions/cache#604 that you're encountering some limitations of the package validation approach. During my own testing, I found that the least fragile approach is to use jobs:
is-packaged:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- id: dist
env:
BUILD_HASH: ${{ hashFiles('dist') }}
run: echo "::set-output name=hash::$BUILD_HASH"
- run: npm install
- run: npm run all
- name: Test Action package is up to date
uses: pr-mpt/actions-assert@v3
with:
assertion: npm://@assertions/is-equal:v1
expected: "${{ steps.dist.outputs.hash }}"
actual: "${{ hashFiles('dist') }}"
error-message: "Action package (`dist`) is not up to date with latest changes, please run `npm run all`" |
|
@shrink that's a nice action! I can see the value for a built-in - name: Test Action package is up to date
assert: ${{ hashFiles('dist') }} == ${{ steps.dist.outputs.hash }}
message: "Action package (`dist`) is not up to date with latest changes, please run `npm run all`"which would save a few lines in the YAML. I'm not sure we have the bandwidth now to fund this feature and some of the changes would be on the service side, but I'll keep an eye out now for where it would be useful. Also, I added a paragraph to the description to explain the choice of diffing vs. hashing. |
|
merge in |
This adds a workflow to check the contents of the checked-in
dist/index.jsagainst the expected version.Here are the bones of the issue:
index.jsis a generated fileindex.jsmust be checked into version controlindex.js == ncc_build(repo)in themainbranchEven though
index.jsis produced by the build, it must always be checked into version control. While we could do something more elaborate like having a workflow that runs and updatesindex.jsin the source branch for any PR, we would still need a check like this. I also don't think it really reduces developer friction:index.js, but it would affect every PR and not just ones whereindex.jshas an issue.Diff vs. hash
We originally thought of just checking the hash of the files (a checksum). However, since the files are human-readable, a diff will provide a more informative failure message than a hash. Also for this reason, I think the output of
git diffis easier to read than GNUdiff. With diffing, we can also ignore line endings. Computing a diff takes more time than computing a hash, but theindex.jsfiles should be small enough that this won't matter.Testing
index.js-> pass\r\nvs.\n-> passupload-artifact/.gitattributes
Line 1 in 27121b0