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

AppVeyor CI Integration #80

Closed
wants to merge 10 commits into from
Closed

Conversation

patkub
Copy link
Contributor

@patkub patkub commented Oct 25, 2017

An attempt at getting greenkeeper-lockfile working in AppVeyor CI.
AppVeyor Build that updated lockfile: https://ci.appveyor.com/project/patkub/test-greenkeeper-lockfile-appveyor/build/1.0.20
Greenkeeper PR where lockfile was updated: patkub/test-greenkeeper-lockfile-appveyor#2

Issues:

  • AppVeyor runs Windows, so the existing git commands won't work. How should I go about detecting AppVeyor CI in update-lockfile.js?
  • May break in other places where exec runs linux commands

@patkub patkub changed the title Appveyor CI Integration AppVeyor CI Integration Oct 25, 2017
@janl
Copy link
Contributor

janl commented Oct 25, 2017

AppVeyor runs Windows, so the existing git commands won't work. How should I go about detecting AppVeyor CI in update-lockfile.js?

Why wouldn’t these commands work on Windows?

@patkub
Copy link
Contributor Author

patkub commented Oct 25, 2017

@janl

The changes to the commands in update-lockfile.js make it work on Windows, but we need to add a check to choose when to use the existing Linux commands vs the Windows commands.

Failed build log on AppVeyor before changes to update-lockfile.js:
https://ci.appveyor.com/project/patkub/test-greenkeeper-lockfile-appveyor/build/1.0.14

The system cannot find the path specified.
The system cannot find the path specified.
The system cannot find the path specified.
child_process.js:511
    throw err;
    ^

This is cause by https://github.com/greenkeeperio/greenkeeper-lockfile/blob/master/lib/update-lockfile.js#L58,L60 because /dev/null does not exist on Windows, and true is not recognized as a command.

Error: Command failed: git commit -m "chore(package): update lockfile
https://npm.im/greenkeeper-lockfile"

This seems to be caused by ${updateMessage}.

Edit: Clarified Linux/Windows detection issue, added build log

@patkub
Copy link
Contributor Author

patkub commented Oct 25, 2017

@chgibb
Copy link

chgibb commented Nov 17, 2017

Would it not be possible to run through MSYS on Appveyor? With some minor tweaking, shell scripts written for Travis will work fine on Appveyor under MSYS.

@patkub
Copy link
Contributor Author

patkub commented Nov 17, 2017

@chgibb Thanks for the suggestion of using MSYS. I tried running just the ci-services implementation through MSYS on Appveyor and the git commit command still fails.
https://ci.appveyor.com/project/patkub/test-greenkeeper-lockfile-appveyor/build/1.0.58

I installed greenkeeper-lockfile globally with npm install -g patkub/greenkeeper-lockfile#appveyor-msys. However, running greenkeeper-lockfile through msys looks for update-lockfile.js in the local node_modules directory instead of the global location.

Installing greenkeeper-lockfile locally without modifying package.json gets it to run, but still fails on the git commit -m command.

appveyor.yml: https://github.com/patkub/test-greenkeeper-lockfile-appveyor/blob/master/appveyor.yml

@chgibb
Copy link

chgibb commented Nov 17, 2017

@patkub Can you send me a link to patkub/greenkeeper-lockfile#appveyor-msys? Wherever the repo for that package is.

@patkub
Copy link
Contributor Author

patkub commented Nov 17, 2017

@chgibb Yes. I just cherry-picked the add appveyor to ci-services commit from here.
https://github.com/patkub/greenkeeper-lockfile/tree/appveyor-msys

@chgibb
Copy link

chgibb commented Nov 18, 2017

This

  exec('git add npm-shrinkwrap.json 2>/dev/null || true')
  exec('git add package-lock.json 2>/dev/null || true')
  exec('git add yarn.lock 2>/dev/null || true')
  exec('git config user.email "support@greenkeeper.io"')
  exec('git config user.name "greenkeeper[bot]"')

will fail if run through node directly.

Instead of C:\MinGW\msys\1.0\bin\sh greenkeeper-lockfile-update, something like bash node ./node_modules/.bin/greenkeeper-lockfile-update should work. As long as you can get bash to invoke node to invoke greenkeeper-lockfile-update, as opposed to cmd or ps(which is the default Appveyor uses in its config).

@patkub
Copy link
Contributor Author

patkub commented Nov 19, 2017

@chgibb It looks like even though I get bash to invoke node to invoke greenkeeper-lockfile-update, the commands still fail to run in a bash environment. I tried two approaches:

  1. patkub/test-greenkeeper-lockfile-appveyor@20641a3

I used the regular bash command available in Appveyor to run node greenkeeper-lockfile/update.js. The command passes on master, but fails to commit on a greenkeeper branch.
master: https://ci.appveyor.com/project/patkub/test-greenkeeper-lockfile-appveyor/build/1.0.97
greenkeeper: https://ci.appveyor.com/project/patkub/test-greenkeeper-lockfile-appveyor/build/1.0.98

  1. patkub/test-greenkeeper-lockfile-appveyor@d3f509e

Using msys bash to invoke node to invoke greenkeeper-lockfile/update.js also passes on master, but fails to commit on a greenkeeper branch.
master: https://ci.appveyor.com/project/patkub/test-greenkeeper-lockfile-appveyor/build/1.0.99
greenkeeper: https://ci.appveyor.com/project/patkub/test-greenkeeper-lockfile-appveyor/build/1.0.100

Could this be because exec spawns a new shell, which probably isn't in a bash environment?
https://nodejs.org/api/child_process.html#child_process_child_process_exec_command_options_callback

@chgibb
Copy link

chgibb commented Nov 19, 2017

@patkub you're probably correct. I was hoping exec would spawn a new instance of the current shell (hence, trying to invoke through bash). You might want to look at invoking git through cp.spawnSync and then just discarding the buffered stdio output to get the same behaviour as exec with the redirect.

@patkub
Copy link
Contributor Author

patkub commented Nov 19, 2017

@chgibb @janl How about checking if running on windows, and using exec to run windows compatible git commands? I added a check for Windows and git commands to commit the updated lockfile here. This works, but could make the package more complex, and other commands run though exec might also need to be changed to work on Windows.

patkub@378ed90
Using cp.spawnSync, greenkeeper-lockfile says the Lockfile updated, but no changes are committed.
git status after greenkeeper-lockfile-update shows the changes are not staged for commit, and a git diff shows the changes to the lockfile
https://ci.appveyor.com/project/patkub/test-greenkeeper-lockfile-appveyor/build/1.0.119

@xt0rted
Copy link

xt0rted commented Mar 14, 2018

AppVeyor supports Linux too now https://www.appveyor.com/docs/getting-started-with-appveyor-for-linux/

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

Successfully merging this pull request may close these issues.

None yet

4 participants