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

Use prebuild for binaries #12

Merged
merged 6 commits into from
Apr 2, 2019
Merged

Conversation

mischnic
Copy link
Member

@mischnic mischnic commented Apr 1, 2019

The CI job should call the prebuild script to build+package and then the prebuild-upload script to upload attach the binary to a GitHub release (https://github.com/prebuild/prebuild#uploading). (I have no idea how to tell Azure that)

#10

  • I guess yarn install on CI should always --build-from-source ?

@mischnic
Copy link
Member Author

mischnic commented Apr 1, 2019

I didn't change the azure config, but the Windows test abruptly stops without any error message

@devongovett
Copy link
Member

hmm, weird. re-ran and it passed.

@devongovett
Copy link
Member

Interesting. I kind of assumed it would just publish the pre-built binaries as part of the npm package but that seems not to be the case. Instead, it downloads it dynamically at install time. Wonder why...

@mischnic
Copy link
Member Author

mischnic commented Apr 2, 2019

Interesting. I kind of assumed it would just publish the pre-built binaries as part of the npm package but that seems not to be the case. Instead, it downloads it dynamically at install time. Wonder why...

Publishing to S3 or GItHub is the common way to do this, though there are libraries to publish to npm as well: https://github.com/prebuild/prebuildify

Advantage of publishing every prebuilt binary in the npm tarball:

  • install speed
  • would also work if user disabled npm scripts

Major Disadvantage:

  • Size

@devongovett
Copy link
Member

Yeah makes sense. I'm close to getting things to upload to GitHub via azure pipelines.

@devongovett devongovett merged commit 390aeac into parcel-bundler:master Apr 2, 2019
@@ -41,6 +43,16 @@ jobs:
displayName: Install Watchman

- script: npm install
Copy link
Member Author

Choose a reason for hiding this comment

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

Should be npm install --build-from-source to ensure the CI doesn't download a prebuilt binary

Copy link
Member

Choose a reason for hiding this comment

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

ah good catch, thanks. added on master.

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.

2 participants