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

📢 Feedback/testing: Version 2.0.0 (next) with monorepo support! #164

Open
espy opened this issue Jun 14, 2018 · 26 comments
Open

📢 Feedback/testing: Version 2.0.0 (next) with monorepo support! #164

espy opened this issue Jun 14, 2018 · 26 comments

Comments

@espy
Copy link
Contributor

espy commented Jun 14, 2018

Hello everyone! 👋

🎊 We’ve just shipped preliminary monorepo support for greenkeeper-lockfile under the next tag with version number 2.0.0. To use it, you must explicitly install this version with

npm i greenkeeper-lockfile@2 // add -g if on a CI service

If you are using a default branch on Github that is not called master, please set an Environment Variable GK_LOCK_DEFAULT_BRANCH with the name of your default branch in your CI.

In other words, this isn’t in the regular release yet, because we’d like to give everyone the opportunity to test this on their various CI setups. 🔍

📢 Calling the Community

If any of the previous CI contributors are interested and have the time: it would be fantastic if you could take a look and confirm whether this all works on your respective platforms (pinging @ethanrubio, @zetaron , @justindowning, @selbyk, @cbothner, @tagoro9, @dbrockman and @donny-dont). Of course, this isn’t limited to them, any testing and feedback on the various CI services is extremely welcome 🙏 Thanks in advance to anyone who participates!

✅ Travis CI
✅ Circle CI
❓ Jenkins
❓ Wercker
❓ Bitrise
⚡️ Buildkite
❓ Codeship
❓ Semaphore
✅ TeamCity
❓ Drone.io
✅ AWS CodeBUild

Changes

  • Removed checking for first push in the individual ci-services, this is now in the core. Instead, lib/git-helpers.js has hasLockfileCommit() which checks the actual commit history for the same commit message the current run of greenkeeper-lockfile would generate, in the same branch it would use
  • lib/extract-dependency.js can now handle several dependencies and several package files at once, and makes use of the new Greenkeeper Monorepo Definitions to determine which modules belong to a single release and should therefore be grouped

Thanks everyone! 🤖 🌴

@dominics
Copy link
Contributor

dominics commented Jun 19, 2018

Hey @espy, I can confirm that this isn't working for us. The reason is we're relying on GH_TOKEN to authenticate against Github (as described in the README), rather than SSH authentication.

This is a problem because:

  • upload.js calls hasLockfileCommit (at line 40) before it configures the gk-origin remote URL (at line 56)
  • update.js doesn't support GH_TOKEN at all (because it previously wasn't doing anything against the remote; now it does a git fetch)

So, I'd describe this as: 2.0.0 has broken support for authenticating with GH_TOKEN. (Guess this also leads to the question of whether we're testing the GH_TOKEN support in the CI too.)

To fix it, probably the code in upload.js which configures the remote needs to be extracted to a git-helper, and be called before any call to hasLockfileCommit() (also, the git fetch should only fetch the configured gk-origin, not origin itself). Happy to open a separate issue if you like.

(Should mention that setting up SSH auth in our case would be quite painful; we're inside a container at that point (via Buildkite's docker-compose plugin). A token and env var are much easier to deal with.)

@tagoro9
Copy link
Contributor

tagoro9 commented Jun 20, 2018

@espy for us it is working fine (Team City) and we didn't have any authentication problem so our team city setup must also be authenticating with Github via SSH.

@patkub
Copy link
Contributor

patkub commented Jun 22, 2018

Hey @espy, I think the Team City shouldUpdate() lockfile regex needs to be changed to account for singular and plural "chore(package): update lockfile(s)"

@donny-dont
Copy link
Contributor

@espy is there an example that I can fork to make sure the plugin works?

@patkub
Copy link
Contributor

patkub commented Jul 3, 2018

@espy The commit lockfile(s) plural wording does not work on CircleCI nor AppVeyor. I have one package-lock.json file and it commits with plural chore(package): update lockfiles instead of singular.

Edit: Added logging and it says both package-lock.json and package.json are staged on the greenkeeper-lockfile-update step here https://circleci.com/gh/patkub/test-gk-lock-circleci/56

@Toolo
Copy link

Toolo commented Jul 6, 2018

@espy it's no longer working for us in TeamCity (@tagoro9 ): it's failing due to authentication issues. As @dominics say, we also rely on the GH_TOKEN parameter for authentication.

See a snippet of the error below:

[20:19:21]Error: Command failed: git fetch
[20:19:21]fatal: could not read Password for 'https://[email protected]': No such device or address
[20:19:21]
[20:19:21]    at checkExecSyncError (child_process.js:601:13)
[20:19:21]    at execSync (child_process.js:641:13)
[20:19:21]    at hasLockfileCommit (/home/ubuntu/.config/yarn/global/node_modules/greenkeeper-lockfile/lib/git-helpers.js:34:5)
[20:19:21]    at Module.upload [as exports] (/home/ubuntu/.config/yarn/global/node_modules/greenkeeper-lockfile/upload.js:40:7)
[20:19:21]    at Object.<anonymous> (/home/ubuntu/.config/yarn/global/node_modules/greenkeeper-lockfile/upload.js:66:37)
[20:19:21]    at Module._compile (module.js:652:30)
[20:19:21]    at Object.Module._extensions..js (module.js:663:10)
[20:19:21]    at Module.load (module.js:565:32)
[20:19:21]    at tryModuleLoad (module.js:505:12)
[20:19:21]    at Function.Module._load (module.js:497:3)
[20:19:21]Process exited with code 1

@janl
Copy link
Contributor

janl commented Jul 16, 2018

#183 should address the GH_TOKEN issue. @dominics @Toolo could you give this a try? Everything is on master now, but not yet on npm. Thanks! <3

@tagoro9
Copy link
Contributor

tagoro9 commented Jul 16, 2018

@janl I just tested the new version and we are still getting the error.

In upload.js the code is configuring auth for the gk-orgin remote only:

exec(`git remote add gk-origin ${remote} || git remote set-url gk-origin ${remote}`)

But then in hasLockfileCommit the code is doing:

exec(`git fetch`)

Which is gonna fetch from the default remote if there is more than one (in our case is fetching from origin). I think the fix would be changing the fetch command to:

exec(`git fetch gk-origin`)

I just tried this on a new branch and seems to work. I'm gonna create a PR with the fix.

@travi
Copy link
Contributor

travi commented Jul 17, 2018

after updating to v2.3.1, i'm seeing the following error with a private repo:

fatal: 'gk-origin' does not appear to be a git repository
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
child_process.js:644
    throw err;
    ^
Error: Command failed: git fetch gk-origin
fatal: 'gk-origin' does not appear to be a git repository
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.

i assume that is related to the change directly above? looks like it needs a tweak of some sort

@mAAdhaTTah
Copy link

I'm not getting a lockfile upload on Greenkeeper PRs. The output says "master is not a greenkeeper branch". See this TravisCI run: https://travis-ci.org/valtech-nyc/brookjs/builds/404694533

@travi
Copy link
Contributor

travi commented Jul 18, 2018

@mAAdhaTTah i think you checked the wrong one of the two builds. looking at the other one, i see the same error as above.

@cormacrelf
Copy link

Same issue as @travi, but on a public repo.

@janl
Copy link
Contributor

janl commented Jul 19, 2018

Heya, @cormacrelf @travi @mAAdhaTTah fix should have been in here: #187

Sorry about the fuss.

@mAAdhaTTah
Copy link

Still getting the same error w/ 2.3.2: https://travis-ci.org/valtech-nyc/brookjs/builds/404693220

@janl janl mentioned this issue Jul 19, 2018
@janl
Copy link
Contributor

janl commented Jul 19, 2018

@mAAdhaTTah Thank you for your patience, a fix for this is incoming now, I just need to wait for a reviewer to come back from lunch 😂 🌯

@janl
Copy link
Contributor

janl commented Jul 19, 2018

This should do the trick, @2.3.3 or @next — Thanks again for your patience and help!

@mAAdhaTTah
Copy link

So it's not erroring, but it's not pushing a lockfile commit either: https://travis-ci.org/valtech-nyc/brookjs/builds/404693220

There are some error down at the end that may help.

@janl
Copy link
Contributor

janl commented Jul 19, 2018

@mAAdhaTTah Thanks, could you try @next with #190 and set GK_LOCK_DEBUG=1 in your travis config before your runs. That should give us a bit more details.

@cormacrelf
Copy link

Here's a run with GK_LOCK_DEBUG=1, v2.4.0

https://travis-ci.org/cormacrelf/angular-skyhook/jobs/405467762

Didn't produce a commit. I verified the branch myself, it should have.

@mAAdhaTTah
Copy link

@hassankhan
Copy link

hassankhan commented Jul 28, 2018

Hi there, I seem to be getting 'master' is not a Greenkeeper branch errors, any help would be appreciated: https://travis-ci.org/serverlize/serverlize/jobs/409380857

Related issue: serverlize/serverlize#25

@mAAdhaTTah
Copy link

@hassankhan See @travi's comment above; there are typically two CI runs, only which of which produces a lockfile commit (the "push" one; the "pr" one will not).

Also, just wanted to comment that the lockfile update is now happening successfully in my monorepo! 🎉 Thank you for all your help & work on this!

@travi
Copy link
Contributor

travi commented Aug 8, 2018

i think i finally dug into some odd behavior that i've been having far enough to report that there is an issue. i've had success using next on public projects, but have been seeing 404s on private packages. i recently added CIDR restrictions to our tokens, so i thought the odd behavior was somehow related to that. i think i have confirmed that the issue is instead with greenkeeper-lockfile.

when running builds for private projects on travis, we inject the npm token needed to pull our private packages into the .npmrc during the before_install step with the following command: echo "//registry.npmjs.org/:_authToken=${NPM_TOKEN}" >> .npmrc. even now, the npm install command successfully completes and does install our private packages.

then, in the before_script step, we run greenkeeper-lockfile-update through an npm script, which results in the following error:

Error: Command failed: npm install -S @gaincompliance/[email protected]
npm ERR! code E404
npm ERR! 404 Not Found: @gaincompliance/[email protected]

my best guess, without digging through the code yet, is that checking out the commit on gk-origin results in the .npmrc being in the state that is versioned in git, rather than the local working copy that had the token injected.

has something changed in v2 that could have resulted in this behavior?

@coderbyheart
Copy link
Contributor

I can confirm that v2 works as expected on AWS CodeBuild.

@janl
Copy link
Contributor

janl commented Sep 13, 2018

Thanks @coderbyheart!

@Koslun
Copy link

Koslun commented Nov 21, 2018

Everything was working for us with Codeship now with us getting an angular material (angular2 in monorepo definition) update containing the 3 dependencies in that monorepo. Everything except the lock files not updating that is. But notice that we were simply installing the default 1.15.1 version rather than version 2.x.y which actually has support for monorepos.

Will await to see if this fixes things but guess we won't know until we get the next monorepo update from Angular material.

We have also have at least two other monorepo dependencies but one isn't in the monorepo definition yet and the other, Angular, doesn't seem to be working yet. Made PRs for both, with the Angular PR merged but still seemingly something wrong with Angular.

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

No branches or pull requests