-
Notifications
You must be signed in to change notification settings - Fork 288
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
Compilation differs on GitHub Actions vs locally #831
Comments
Are you using typescript? This sounds like it could be the same issue as #820 |
We are using Typescript, but I don't think it's the same as #820, as there is a Also, the build is succeeding, it just has different output when run on GitHub Actions vs when run locally. |
same here. I am not using Typescript.
|
Are you using the same version of npm locally and in CI? What about the same OS? |
Yes and same node version too. Deps are installed via "npm ci" and the lines in the compiled code are NOT in the package.json. I've confirmed this my echoing the contents of package.json before "npm ci", after, and after "ncc". The contents never change and never include the "_resolved" references in the compiled js. |
@sethvargo might it be this problem? #713 |
I don't think so, because the difference only occurs with |
Fair enough. Do you have a reproducing sample for this? Looks like the branch you linked in the issue description is gone? (I can see the workflow logs, but not the branch it was run from) |
@rethab sure - I spent some time to remove all irrelevant code in my fork: https://github.com/sethvargo/get-gke-credentials/runs/4888123729. The repo is on |
Hi @sethvargo , I have only very briefly checked your reproducer, but I have forked your reproducer here: https://github.com/rethab/get-gke-credentials All I did to make it green was to run
They don't generate a different package-lock.json in your project (in fact Perhaps I'm wrong and you are indeed facing a different issue, but if you are facing the same issue, then here are two options I am aware of:
|
Hi @rethab Thank you for looking into this. Running There is no
Regarding using Node 16, actions/runner#1439 was merged, and released in https://github.com/actions/runner/releases/tag/v2.285.0, but it will also likely be a few months until it's available in GitHub Enterprise installations. A lot of our users use self-hosted runners with GitHub Enterprise, so we can't switch until it's made available everywhere. |
Okay, got it. I'm not at a computer anymore, so I can't very what I'm going to say, but I believe GH is using npm 6 with node 12. From what I remember, there was a difference between npm versions and how node_modules are laid out.
Could you please try using npm 6 on your mac? (`npx ***@***.*** ci` would be the command I think).
…-------- Oorspronkelijk bericht --------
Aan 21 jan. 2022 15:33, Seth Vargo schreef:
Hi ***@***.***(https://github.com/rethab)
Thank you for looking into this. Running npm ci on my Mac produces no git diff. If I clone a fresh https://github.com/sethvargo/get-gke-credentials and run npm ci, there are no changes. Deleting the entire node_modules directory and running npm ci also doesn't generate a diff.
There is no npm run package step, so I'm not sure what you're referring to here. If you meant npm run build, deleting node_modules, running npm ci, and running npm run build also generates no diff. Per ***@***.***(rethab/get-gke-credentials@73ece2d), there has to be some diff that's generated.
- Mac OS 12.1 (21C52)
- Node v12.22.5
- NPM 8.3.0
Regarding using Node 16, [actions/runner#1439](actions/runner#1439) was merged, and released in https://github.com/actions/runner/releases/tag/v2.285.0, but it will also likely be a few months until it's available in GitHub Enterprise installations. A lot of our users use self-hosted runners with GitHub Enterprise, so we can't switch until it's made available everywhere.
—
Reply to this email directly, [view it on GitHub](#831 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAQ6PXGJ2HPTJJBCTEDO4DLUXFVD3ANCNFSM5KHGK3RA).
Triage notifications on the go with GitHub Mobile for [iOS](https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or [Android](https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Hi @rethab thanks for the reply. I think GitHub or your email client did something odd. The command came in as Don't worry about replying until you're back at a computer - this isn't a P0 since we have a workaround for now. |
GitHub thinks it's an email address and masks it 😅
All I wanted to say was try npm six. The command I showed was to use npx and specify the version of npm after the @.
npx npm[at]6 ci
…-------- Oorspronkelijk bericht --------
Aan 21 jan. 2022 15:48, Seth Vargo schreef:
Hi ***@***.***(https://github.com/rethab) thanks for the reply. I think GitHub or your email client did something odd. The command came in as npx ***@***.*** ci
[CleanShot 2022-01-21 at 09 47 ***@***.***(https://user-images.githubusercontent.com/408570/150547039-f172dcbe-48ae-4f0c-a884-56728baa7dd7.png)
Don't worry about replying until you're back at a computer - this isn't a P0 since we have a workaround for now.
—
Reply to this email directly, [view it on GitHub](#831 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAQ6PXADIOWYDYNMXIX7HWLUXFW35ANCNFSM5KHGK3RA).
Triage notifications on the go with GitHub Mobile for [iOS](https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or [Android](https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Welp, that seems to be the issue:
So is the best option here to wait until node16 is more broadly available on Actions? |
I'm not sure. I'm not involved with ncc or npm, I'm just a user who discovered this issue.
I can only tell you what we do: developers must use npm 6 and we have the above-mentioned "all" script (when I said package, I meant build). While this might not sound like a nice workaround, we haven't had issues since (and we have around 20 actions under active development).
To be frank, I don't remember if node 16 comes with npm 8, but from what I recall that is going to resolve the issue and I'm eager to upgrade. Just didn't get to it yet :)
…-------- Oorspronkelijk bericht --------
Aan 21 jan. 2022 16:10, Seth Vargo schreef:
Welp, that seems to be the issue:
$ rm -rf node_modules && npx ***@***.*** ci && npx ***@***.*** run build
added 46 packages in 1.028s
> ***@***.*** build /Users/sethvargo/get-gke-credentials
> ncc build src/main.ts
ncc: Version 0.33.1
ncc: Compiling file index.js into CJS
ncc: Using ***@***.*** (local user-provided)
1617kB dist/index.js
1617kB [3422ms] - ncc 0.33.1
➜ get-gke-credentials main ❃ git diff
diff --git a/dist/index.js b/dist/index.js
index 9e0f160..9535565 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -45624,7 +45624,7 @@ module.exports = require("zlib");
/***/ ((module) => {
"use strict";
-module.exports = JSON.parse('{"name":"google-auth-library","version":"7.11.0","author":"Google Inc.","description":"Google APIs Authentication Client Library for Node.js","engines":{"node":">=10"},"main":"./build/src/index.js","types":"./build/src/index.d.ts","repository":"googleapis/google-auth-library-nodejs.git","keywords":["google","api","google apis","client","client ***@***.******@***.******@***.******@***.******@***.******@***.******@***.******@***.******@***.******@***.******@***.***/tmp":"^0.2.0","assert-rejects":"^1.0.0","c8":"^7.0.0","chai":"^4.2.0","codecov":"^3.0.2","execa":"^5.0.0","gts":"^2.0.0","is-docker":"^2.0.0","karma":"^6.0.0","karma-chrome-launcher":"^3.0.0","karma-coverage":"^2.0.0","karma-firefox-launcher":"^2.0.0","karma-mocha":"^2.0.0","karma-remap-coverage":"^0.1.5","karma-sourcemap-loader":"^0.3.7","karma-webpack":"^5.0.0","keypair":"^1.0.4","linkinator":"^2.0.0","mocha":"^8.0.0","mv":"^2.1.1","ncp":"^2.0.0","nock":"^13.0.0","null-loader":"^4.0.0","puppeteer":"^13.0.0","sinon":"^12.0.0","tmp":"^0.2.0","ts-loader":"^8.0.0","typescript":"^3.8.3","webpack":"^5.21.2","webpack-cli":"^4.0.0"},"files":["build/src","!build/src/**/*.map"],"scripts":{"test":"c8 mocha build/test","clean":"gts clean","prepare":"npm run compile","lint":"gts check","compile":"tsc -p .","fix":"gts fix","pretest":"npm run compile","docs":"compodoc src/","samples-setup":"cd samples/ && npm link ../ && npm run setup && cd ../","samples-test":"cd samples/ && npm link ../ && npm test && cd ../","system-test":"mocha build/system-test --timeout 60000","presystem-test":"npm run compile","webpack":"webpack","browser-test":"karma start","docs-test":"linkinator docs","predocs-test":"npm run docs","prelint":"cd samples; npm link ../; npm install","precompile":"gts clean"},"license":"Apache-2.0"}');
+module.exports = JSON.parse('{"name":"google-auth-library","version":"7.11.0","author":"Google Inc.","description":"Google APIs Authentication Client Library for Node.js","engines":{"node":">=10"},"main":"./build/src/index.js","types":"./build/src/index.d.ts","repository":"googleapis/google-auth-library-nodejs.git","keywords":["google","api","google apis","client","client ***@***.******@***.******@***.******@***.******@***.******@***.******@***.******@***.******@***.******@***.******@***.***/tmp":"^0.2.0","assert-rejects":"^1.0.0","c8":"^7.0.0","chai":"^4.2.0","codecov":"^3.0.2","execa":"^5.0.0","gts":"^2.0.0","is-docker":"^2.0.0","karma":"^6.0.0","karma-chrome-launcher":"^3.0.0","karma-coverage":"^2.0.0","karma-firefox-launcher":"^2.0.0","karma-mocha":"^2.0.0","karma-remap-coverage":"^0.1.5","karma-sourcemap-loader":"^0.3.7","karma-webpack":"^5.0.0","keypair":"^1.0.4","linkinator":"^2.0.0","mocha":"^8.0.0","mv":"^2.1.1","ncp":"^2.0.0","nock":"^13.0.0","null-loader":"^4.0.0","puppeteer":"^13.0.0","sinon":"^12.0.0","tmp":"^0.2.0","ts-loader":"^8.0.0","typescript":"^3.8.3","webpack":"^5.21.2","webpack-cli":"^4.0.0"},"files":["build/src","!build/src/**/*.map"],"scripts":{"test":"c8 mocha build/test","clean":"gts clean","prepare":"npm run compile","lint":"gts check","compile":"tsc -p .","fix":"gts fix","pretest":"npm run compile","docs":"compodoc src/","samples-setup":"cd samples/ && npm link ../ && npm run setup && cd ../","samples-test":"cd samples/ && npm link ../ && npm test && cd ../","system-test":"mocha build/system-test --timeout 60000","presystem-test":"npm run compile","webpack":"webpack","browser-test":"karma start","docs-test":"linkinator docs","predocs-test":"npm run docs","prelint":"cd samples; npm link ../; npm install","precompile":"gts ***@***.***"}');
/***/ }),
So is the best option here to wait until node16 is more broadly available on Actions?
—
Reply to this email directly, [view it on GitHub](#831 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAQ6PXFKVJYMTD62CN74BP3UXFZNHANCNFSM5KHGK3RA).
Triage notifications on the go with GitHub Mobile for [iOS](https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or [Android](https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
I followed the steps provided by @rethab, but I am still experiencing #831 with node: What I already tried
|
Can you please change the |
@rethab Thanks for your fast answer. I now added a line showing the The only difference is between the |
In your logs, I also see But if the map files are indeed the only ones that differ, why are you generating them in the first place? Please note that I never generate them and this issue is only about |
Yea, in the diff in the action, the
They are generated because the template I used specified that. I now removed them as I do agree with you that, in this case, they should not be shipped in production. Local filesAction files |
After all, it seemed to be a line-ending problem, which was introduced by me working on both a Linux and Windows PC. After again changing the line endings and enabling |
@rickstaa wondering how your one works, I dev on Windows and run action in Ubuntu and it always failing with the git diff for index.js.map file. I did manual compare, the local windows one has extra \r\ in it compare to the one in Ubuntu, and the following doesn't seem to ignore it : |
I got the exact issue while dev on windows and run action on Linux I fixed by udpating package.json as following: so I added newLine option for tsc build "scripts": {
}, |
I had to add |
Hi, all 👋 , in https://github.com/actions/setup-java repo we faced pretty the same issue. The result output of the Runners specification:
If you open the We don't have such a problem in other repos where we use |
Thanks, but it didn't solve the issue for me: https://github.com/Rubilmax/foundry-gas-diff/actions/runs/5253244964/jobs/9490315554 (commit Rubilmax/foundry-gas-diff@69fea8e) |
It has to do something with GitHub actions runner OS because I haven't changed the version of ncc I use, and still my build went different |
I think I have got the problem. Running Of course as per actions/typescript-action#528 (comment), the diff command must use |
We ran into the same issue in ScribeMD/docker-cache and were able to address it with |
Hi, @Kurt-von-Laven, unfortunately, for us your solution didn't work out. For reference, #831 (comment) |
@IvanZosimov could you try my method as mentioned in #831 (comment) |
@SayakMukhopadhyay, it sounds like there are separate bugs since we aren't using Windows at all. |
If you have a |
There are definitely at least two different bugs here then, because we already had equivalent settings in our |
Thank you so much. This resolved my issue. |
This solution was suggested here: vercel/ncc#831 (comment)
This solution was suggested here: vercel/ncc#831 (comment)
When using the same version of
ncc
locally (OS X) and on GitHub Actions, the compiled file is slightly different (full diff):This causes a perpetual diff between what's built locally and what's built by CI.
The text was updated successfully, but these errors were encountered: