Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

AppVeyor: fully automated build of 32- and 64-bit Windows binaries #1135

Closed
wants to merge 2 commits into from

Conversation

saper
Copy link
Member

@saper saper commented Sep 9, 2015

For pull requests and pushes to master just build 32-bit binaries
and do not store artifacts.

Whenever a tag is pushed to the "release" branch - produce
full set of 32-bit and 64-bit binaries, create a GitHub
release (if does not exist already) and attach the binaries
to the release.

Issues open:

http://bit.ly/twoBuilds

AppVeyor starts two builds in response to the "new commit"
and "new tag" events.

http://bit.ly/bad_build_success

Parse error in the appveyor.yml causes the system to
silently ignore the commit.

http://bit.ly/appvLessDup

Is there a way to remove duplicate stanzas from
both configurations?

@saper
Copy link
Member Author

saper commented Sep 9, 2015

I'd love to have your feedback @am11 @xzyfer. Right now it looks pretty cool to me! See the demo release https://github.com/saper/node-sass/releases/tag/v3.3.auto - one binary didn't make it due to timeout.

@xzyfer
Copy link
Contributor

xzyfer commented Sep 9, 2015

@saper NEAT!

Is there a way to remove duplicate stanzas from both configurations?

You could try YAML style inheritance eg.

development: &defaults
  adapter: mysql
  encoding: utf8
  database: acme_development
  username: root

test:
  <<: *defaults
  database: acme_test

cucumber:
  <<: *defaults
  database: acme_cucumber

@am11
Copy link
Contributor

am11 commented Sep 9, 2015

This is brilliant! 👍
Probably we will not encounter timeout issues on our Sass org pro account, but probably a good idea to have some suitable action (like rebuild the commit) triggered by on_success and on_failure callbacks etc. Currently, AppVeyor does not support rebuilding a single job: appveyor/ci#244.

@xzyfer, I didn't knew about the YAML inheritance! Thanks for enlightening us. 😄

@saper
Copy link
Member Author

saper commented Sep 9, 2015

https://ci.appveyor.com/tools/validate-yaml says: Invalid setting or section: << (Line: 13, Column: 4)

I think I have enough AppVeyor torture for today :)

@xzyfer
Copy link
Contributor

xzyfer commented Sep 9, 2015

IIRC merging is a common, but non-standard extension. I guess AppVeyors YAML parser doesn't support it :(

@saper
Copy link
Member Author

saper commented Sep 9, 2015

I have disabled artifact storage for the casual testing configuration (normal builds). Do we need them? Are there any limits on how much we can store?

@am11
Copy link
Contributor

am11 commented Sep 9, 2015

AFAICT, there is no limit on storage for Free or Pro accounts on AppVeyor cloud. At least I couldn't find it documented anywhere. Over at LibSass.net, I packaged the entire project folder (with sources) in artifacts and it has not complained so far: https://github.com/darrenkopp/libsass-net/blob/5c0ccec56/appveyor.yml#L20 (which is probably not a good idea to store ~60MBs x 4 jobs per commit and will probably violate the general "fair usage" clause of any cloud storage at some point)

@saper
Copy link
Member Author

saper commented Sep 9, 2015

So the current setup (build for the release only) seems to be okay.

We still need to push the artifacts using your PowerShell snippet since otherwise AppVeyor refuses to deploy the files. (You cannot deploy out of the build files).

Maybe they delete artifacts once the build logs are removed, but I doubt it (extra effort + general cloudy "no delete" policy).

@xzyfer
Copy link
Contributor

xzyfer commented Sep 10, 2015

Could we plug this into node-pre-gyp? I like automating this but I'd rather not have a bespoke solution.

@saper
Copy link
Member Author

saper commented Sep 10, 2015

... er which one you do not like? Not sure what I was talking about:)

@saper
Copy link
Member Author

saper commented Sep 10, 2015

Here is a completed build:

https://github.com/saper/node-sass/releases/tag/v3.3.build

I am a bit unhappy about http://bit.ly/twoBuilds since it's really tricky to get the build running on the release branch and the tag at the same time. git push -f mostly doesn't work for me, I had to push a brand new commit and delete the other build.

For pull requests and pushes to master just build 32-bit binaries
and do not store artifacts.

Whenever a tag is pushed to the "release" branch - produce
full set of 32-bit and 64-bit binaries, create a GitHub
release (if does not exist already) and attach the binaries
to the release.

Issue open:

http://bit.ly/twoBuilds

 AppVeyor starts two builds in response to the "new commit"
 and "new tag" events.
matrix:
- nodejs_version: 0.10
- nodejs_version: 0.12
- nodejs_version: 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xzyfer
Copy link
Contributor

xzyfer commented Sep 14, 2015

I've been thinking about this and I don't like AppVeyor creating the GH releases for us. I'd personally rather keep this as a manual step for the interim, until we move to node-pre-gyp.

I also personally really dislike having release notes a commit message. Release notes tend to be

  • heavily edited
  • collaborative

@saper
Copy link
Member Author

saper commented Sep 14, 2015

On Mon, 14 Sep 2015, Michael Mifsud wrote:

I've been thinking about this and I don't like AppVeyor creating the
GH releases for us. I'd personally rather keep this as a manual step
for the interim, until we move to node-pre-gyp.

There is no magic that node-pre-gyp provides. It cannot create GH release
as well (see #1137 (comment))

I also personally really dislike having release notes a commit message. Release notes tend to be

  • heavily edited
  • collaborative

You can always edit it on GitHub. I only needed something
to put there in case AppVeyor is the first one to create
the release. It can be a static boilerplate, I just picked
up the commit contents since it sounded like a cool idea to me,
but those are by no means "final" release notes.

I haven't tested this, but if you create the release
manually before your trigger the CI build - your
description there will stay (AppVeyor does not touch
GH releases if it finds them, it only creates one
if it does not exist - with the script-supplied description).

@xzyfer
Copy link
Contributor

xzyfer commented Sep 14, 2015

Thinking about the double build problem. I think the problem is that you're
pushing the tag. Have you tried creating a released via the GH UI? Releases
create tags. That sounds like it'll work better with the AppVeyor approach.

I almost exclusively rely on GH releases for creating tags.
On 15 Sep 2015 00:12, "Marcin Cieślak" notifications@github.com wrote:

On Mon, 14 Sep 2015, Michael Mifsud wrote:

I've been thinking about this and I don't like AppVeyor creating the
GH releases for us. I'd personally rather keep this as a manual step
for the interim, until we move to node-pre-gyp.

There is no magic that node-pre-gyp provides. It cannot create GH release
as well (see
#1137 (comment))

I also personally really dislike having release notes a commit message.
Release notes tend to be

  • heavily edited
  • collaborative

You can always edit it on GitHub. I only needed something
to put there in case AppVeyor is the first one to create
the release. It can be a static boilerplate, I just picked
up the commit contents since it sounded like a cool idea to me,
but those are by no means "final" release notes.

I haven't tested this, but if you create the release
manually before your trigger the CI build - your
description there will stay (AppVeyor does not touch
GH releases if it finds them, it only creates one
if it does not exist - with the script-supplied description).


Reply to this email directly or view it on GitHub
#1135 (comment).

@xzyfer
Copy link
Contributor

xzyfer commented Sep 14, 2015

For reference by GH release workflow is

  • npm version patch --no-git
  • git add package.json
  • git commit -m "v3.3.3"
  • git push

When CI is green

  • create the GH release
    • which creates the tag
  • git pull --rebase
  • npm publish

@saper saper closed this in 30b2b14 Sep 18, 2015
@xzyfer xzyfer added this to the next.patch milestone Sep 18, 2015
@xzyfer xzyfer removed this from the next.patch milestone Oct 27, 2015
@xzyfer xzyfer modified the milestones: v3.4.0, next.patch Oct 27, 2015
jiongle1 pushed a commit to scantist-ossops-m2/node-sass that referenced this pull request Apr 7, 2024
Fix call function incorrectly expanding map arguments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants