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

CI with prebuilds for Windows #309

Merged
merged 1 commit into from
Nov 8, 2016
Merged

Conversation

vweevers
Copy link
Member

@vweevers vweevers commented Sep 11, 2016

This AppVeyor configuration runs unit tests and produces prebuilds for each commit. Tagged commits trigger an upload to GitHub.

Which means the AppVeyor job will be the first to create a GH release, whenever someone pushes a tag. Does anyone foresee problems with that? Alternatively, we can manually download artifacts from AppVeyor and attach them to a GH release. WDYT?

See this dummy project and its artifacts as an example.

Tackles #305 and #288 (comment).

Tasks:

  • Create organizational account owner (which email address?)
  • Add collaborators (who is interested?) as administrators
  • Add GH token secret to appveyor.yml
  • Write a tool to trigger builds for previous commits. Then we can make Windows prebuilds retroactively: apv build 3e9d9468 --account level --project leveldown

@vweevers
Copy link
Member Author

Oh, looks like there's already an AppVeyor account active. @ralphtheninja?

@ralphtheninja
Copy link
Member

@vweevers Aye, I played around with that a while ago. Should I remove it?

@vweevers
Copy link
Member Author

@ralphtheninja yea, let's start with a clean slate.

Can I perhaps use something like [email protected] for the AppVeyor account? To clarify, with what AppVeyor calls a "organizational" account, which requires a unique email address, we can have multiple admins - identified by their GitHub email. And use it for other level projects too if we want to.

@ralphtheninja
Copy link
Member

@ralphtheninja yea, let's start with a clean slate.

Deleted it!

Can I perhaps use something like [email protected] for the AppVeyor account?

Sounds like a good idea. I believe @0x00A has access to the domain

@vweevers
Copy link
Member Author

Is it OK to create a machine user in the Level org? Looks to be the only way to limit the scope of AppVeyor to the Level repositories. As for the email address, I'll use a temporary one. It can be changed at a later time in both AppVeyor and GitHub.

@vweevers vweevers mentioned this pull request Sep 27, 2016
@vweevers
Copy link
Member Author

vweevers commented Sep 27, 2016

CI is in progress, with prebuilds 🎉

@juliangruber we should merge this PR before the next release, so that the master branch has a appveyor.yml. I've disabled automatic uploading for now.

@heapwolf
Copy link

@vweevers I do maintain that domain, any ideas for setting up email that we could all access? There might be something better than me adding it to my Google Apps account.

@vweevers
Copy link
Member Author

vweevers commented Oct 20, 2016

@0x00A I don't know a better solution than email forwarding with the registrar (some like Namecheap offer free forwarding to multiple recipients) or through Google Apps. Maybe we don't even need access for all, because I can also add people to AppVeyor, with added benefit that each can have their own notification settings. At least I think so; AppVeyor's team features are a bit wonky in my experience so far.

@ralphtheninja
Copy link
Member

ralphtheninja commented Oct 29, 2016

@vweevers Can we merge this? :) Maybe also update to node 7 before that (linux and darwin was built for node 7)

@vweevers
Copy link
Member Author

vweevers commented Nov 2, 2016

@ralphtheninja I'll add node 7 to the matrix, drop node < 4, and upload a prebuild for 7. In the weekend probably.

@vweevers
Copy link
Member Author

vweevers commented Nov 2, 2016

Or do we want to keep 0.12?

@juliangruber
Copy link
Member

Or do we want to keep 0.12?

yeah we should, it'll be supported until the end of the year

@vweevers
Copy link
Member Author

vweevers commented Nov 7, 2016

Dang it, all prebuilds are failing with the same error.

prebuild info begin Prebuild version 4.4.0
prebuild info build Preparing to prebuild [email protected] for v4 on win32-x64 using node-gyp
prebuild http GET https://nodejs.org/download/release/v4.6.1/node-v4.6.1-headers.tar.gz
prebuild http 200 https://nodejs.org/download/release/v4.6.1/node-v4.6.1-headers.tar.gz
prebuild http GET https://nodejs.org/download/release/v4.6.1/SHASUMS256.txt
prebuild http GET https://nodejs.org/download/release/v4.6.1/win-x64/node.lib
prebuild http GET https://nodejs.org/download/release/v4.6.1/win-x86/node.lib
prebuild http 200 https://nodejs.org/download/release/v4.6.1/SHASUMS256.txt
prebuild http 200 https://nodejs.org/download/release/v4.6.1/win-x64/node.lib
prebuild http 200 https://nodejs.org/download/release/v4.6.1/win-x86/node.lib
prebuild ERR! build Failed to locate `node.h` and `node_version.h`.

@ralphtheninja
Copy link
Member

Dang it, all prebuilds are failing with the same error.

Aye, just noticed that as well.

@vweevers
Copy link
Member Author

vweevers commented Nov 8, 2016

Fixed by a good night's sleep. I used the command prebuild -b %nodejs_version%, but the nodejs_version variable is not an exact version anymore (ed213aa). So I'll have to do:

for /f %%i in ('node -v') do set exact_nodejs_version=%%i
prebuild -b %exact_nodejs_version% --strip

I wish prebuild had a flag to only build the current node version, but this 'll do.

@vweevers
Copy link
Member Author

vweevers commented Nov 8, 2016

Squashed, green, ready to merge.

@ralphtheninja @juliangruber would you like access to appveyor? for redundancy

@ralphtheninja
Copy link
Member

@ralphtheninja @juliangruber would you like access to appveyor? for redundancy

Sure!

for /f %%i in ('node -v') do set exact_nodejs_version=%%i
prebuild -b %exact_nodejs_version% --strip

You can probably remove --strip since it doesn't work on windows

@vweevers
Copy link
Member Author

vweevers commented Nov 8, 2016

@ralphtheninja added you!

You can probably remove --strip since it doesn't work on windows

True, I just kept it to express intent. Can't hurt, anyway.

@ralphtheninja ralphtheninja merged commit 81eec42 into Level:master Nov 8, 2016
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.

4 participants