Precompiled binary support #407

rchipka · 2016-05-11T04:00:44Z

This pull request addresses issue #395 by uploading Windows binaries to GitHub Releases. If a user installs libxmljs on Windows, node-pre-gyp will use one of the precompiled binaries.

This also fixes issue #392 and adds AppVeyor CI testing for Windows builds.

rchipka · 2016-05-11T23:35:48Z

Updated to libxmljs 0.17.1 and everything compiles fine on Windows.

The issue now is that the baseurl_xml test fails on Node 0.12+ and the test process completely exits on Node 0.10.

The doctype base URL issue should be easy to debug. For some reason libxml is having trouble resolving the relative path to the doctype declaration file.

I'm not sure why Node 0.10 abruptly exits at some point during the test. Something must be causing a segfault. This issue might be harder to solve.

rchipka · 2016-05-11T23:38:35Z

Note: Prior to updating my branch to 0.17.1, libxmljs compiled fine on all versions of Node and all tests ran successfully.

defunctzombie · 2016-05-12T02:05:15Z

squash commits

defunctzombie · 2016-05-12T02:05:46Z

@polotek can you setup appveyor? or add us as administrators on this repo so we can do it? or move the repo to one of our accounts?

rchipka · 2016-05-12T02:18:20Z

@defunctzombie @polotek I put all the configuration info that AppVeyor needs to build libxmljs into the config file, so it should be as simple as creating an AppVeyor account and pointing it to this GitHub Repo.

Before we merge this PR in, I'd like to see all the AppVeyor builds/tests passing first. Maybe I should create a separate pull request that fixes the original build problem on windows (#ifdef _WIN32) and just merge that for now.

I thought that this PR was good to go before I updated to 0.17.1 and ran into these issues. Other Windows users may have the same issues I mentioned earlier about the tests not passing and crashing in early versions of Node. That being said, libxmljs would at least build on Windows with the correct #ifdef's merged in a separate PR.

rchipka · 2016-05-12T02:24:56Z

Also, if the libxml version bump is causing the instability issues on Windows, then it will probably take more time solve these issues. Alternatively, we could disable the unit testing on AppVeyor and just use the Windows build status/binary uploading as-is.

gagern · 2016-05-12T07:58:35Z

Is this discrepancy intentional?

image: Visual Studio 2015

build_script:
  - npm install --msvs_version=2013

Should this be 2015 in the install call as well?

rchipka · 2016-05-13T13:49:12Z

@gagern It's there to ensure that it will compile on older versions as well. I've changed the config to do this via an environment variable instead.

The issue where the test suite seemed to be crashing was actually this. All the tests are working fine, it's just that there's an issue with nodeunit in Windows where the output can be cut off.

Now I just need to resolve the failing DTD baseUrl test.

rchipka · 2016-05-13T20:03:02Z

test/xml_parser.js

@@ -56,6 +56,12 @@ module.exports.recoverable_parse = function(assert) {
 };

 module.exports.baseurl_xml = function(assert) {
+    if (/^win/.test(process.platform)) {


NOTE: Ignoring the baseurl_xml test on Windows until we find out why libxml isn't resolving the relative path.

Is there a bug report to track this? If so it would be nice to see its number cross referenced, perhaps both here in the discussion and in the source code comment.

rchipka · 2016-05-13T20:08:53Z

The baseurl_xml test is now skipped on Windows until we find out why libxml isn't resolving relative paths for XML doctype declarations.

@defunctzombie @gagern I just need to squash the commits and make sure the builds all succeed. Then this PR should be ready for review and possible release into 0.17.2.

defunctzombie · 2016-05-14T01:45:16Z

README.md

@@ -1,6 +1,8 @@
 # Libxmljs
 [![Build Status](https://secure.travis-ci.org/polotek/libxmljs.svg?branch=master)](http://travis-ci.org/polotek/libxmljs)

+[![AppVeyor Build Status](https://ci.appveyor.com/api/projects/status/cf862a98w7qsajpl?svg=true)](https://ci.appveyor.com/project/rchipka/libxmljs)


will need to reference this repo url right?

This is the link to the polotek/libxmljs builds on my account. You, or anyone else for that matter, can create an AppVeyor account, point it to this repo, and it'll build and test exactly the same.

So it doesn't really matter which or how many different AppVeyor accounts point to this repo, as long as we link to one of them we'll have a badge and a link to the build/test logs.

You or I could create an AppVeyor account named "libxmljs" or "polotek" and then email the credentials to @polotek. That way he owns the AppVeyor account in the Readme.

Awesome. As long as it works, good with me. Will it automatically build when we push new code?

rchipka · 2016-05-14T20:49:51Z

As soon as @polotek adds an AppVeyor webhook, it will build for all branches and only upload binaries when there's tagged commit on the master branch. The badge shows the current build status of the master branch.

The webhook URL for "polotek/libxmljs" on my account is https://ci.appveyor.com/api/github/webhook?id=cf862a98w7qsajpl

defunctzombie · 2016-05-14T22:19:00Z

@rchipka we've moved the repo to a new libxmljs organization. This will let us all manage it better. Let me know what I need to setup on the repo webhook wise to get the builds going.

rchipka · 2016-05-15T01:33:24Z

@defunctzombie Should be the same process. Project settings -> Webhooks -> Add Webhook. From there just put in the previously mentioned webhook URL and then you can choose which messages it will receive (commits, etc.)

defunctzombie · 2016-05-15T01:49:13Z

@rchipka which messages should it receive?

rchipka · 2016-05-15T01:59:52Z

@defunctzombie I believe it works fine if you check "Just the push event"

defunctzombie · 2016-05-15T02:01:42Z

@rchipka added

defunctzombie · 2016-05-15T02:02:10Z

package.json

+        "module_name" : "xmljs",
+        "module_path" : "./build/Release/",
+        "host"        : "https://github.com",
+        "remote_path" : "./polotek/libxmljs/releases/download/v{version}/",


probably should be updated or org path now

update the repo and bugs url while you're in here since we moved the repo

defunctzombie · 2016-05-15T02:03:09Z

LGTM

defunctzombie · 2016-05-15T02:03:44Z

appveyor.yml

@@ -0,0 +1,87 @@
+environment:
+  github_release_token:
+    secure: en/UnoFHA3RsSAHM5c6zb3RAMa4V01lC3pYG1YrHSeOPFu78SUA/0lOjNe3pp8RG


is this something that should be secret?

Nope, this string is encrypted using AppVeyor's secure variables feature.

rchipka · 2016-05-15T12:35:51Z

@polotek If you'd like AppVeyor to also build/test each pull request you have to enable the "pull request" event in the webhook settings. So "push" and "pull request" should be checked.

rchipka · 2016-05-24T14:26:40Z

This is good to merge for now. The discussed Windows issues/improvements can be made once we have a working Windows release.

fafnirical · 2016-06-06T19:50:41Z

Any updates on this, by chance?

rchipka · 2016-06-06T23:45:51Z

@fafnirical Still good to merge as far as I'm concerned.

hildjj · 2016-08-31T08:04:53Z

Bump. Not only do I need this, but I'm waiting to see if it works so that I can do the moral equivalent in a couple of other packages.

rchipka · 2017-01-11T21:30:15Z

Merging because:

It's been almost a year
The changes won't hurt anything
No further exceptions have been raised
Tired of hearing about Windows compilation issues

bchr02 · 2017-01-11T22:51:23Z

@rchipka why not use node-pre-gyp-github within appveyor.yml ps section so that the command to upload the binary will be cross platform? This will make it easier in the future to add precompiled binary support to Linux one day which in my opinion is important too?

rchipka mentioned this pull request May 11, 2016

Publish master as 0.17.2 to support Node 6.1.0 #406

Closed

rchipka mentioned this pull request May 11, 2016

[WIP] add appveyor.yml config for windows build testing #382

Closed

gagern mentioned this pull request May 12, 2016

Use AppVeyor configuration from libxmljs-mt #387

Closed

rchipka reviewed May 13, 2016
View reviewed changes

rchipka force-pushed the issue-395 branch from 0a81588 to a247e7b Compare May 13, 2016 20:20

defunctzombie reviewed May 14, 2016
View reviewed changes

rchipka force-pushed the issue-395 branch from 5838d9c to 021fc33 Compare May 14, 2016 21:03

defunctzombie reviewed May 15, 2016
View reviewed changes

rchipka force-pushed the issue-395 branch from 5478b0c to 0e1691f Compare May 15, 2016 02:13

Precompiled binary support

0e1691f

rchipka force-pushed the issue-395 branch from 8afadd3 to 0e1691f Compare May 18, 2016 16:11

rchipka merged commit 96e257f into libxmljs:master Jan 11, 2017

rchipka mentioned this pull request Jan 11, 2017

Error: Python executable "C:\Python\python.EXE" is v3.5.1, which is not supported by gyp while installing rchipka/node-osmosis#143

Closed

This was referenced Jan 17, 2017

Fix travis-ci links in README #429

Closed

Improve Windows support #361

Closed

Fix docs for move from polotek/libxmljs to libxmljs/libxmljs #412

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Precompiled binary support #407

Precompiled binary support #407

rchipka commented May 11, 2016

rchipka commented May 11, 2016 •

edited

rchipka commented May 11, 2016 •

edited

defunctzombie commented May 12, 2016

defunctzombie commented May 12, 2016

rchipka commented May 12, 2016 •

edited

rchipka commented May 12, 2016 •

edited

gagern commented May 12, 2016

rchipka commented May 13, 2016 •

edited

rchipka May 13, 2016

gagern May 17, 2016

rchipka commented May 13, 2016 •

edited

defunctzombie May 14, 2016

rchipka May 14, 2016

defunctzombie May 14, 2016

rchipka commented May 14, 2016 •

edited

defunctzombie commented May 14, 2016

rchipka commented May 15, 2016

defunctzombie commented May 15, 2016

rchipka commented May 15, 2016

defunctzombie commented May 15, 2016

defunctzombie May 15, 2016

defunctzombie May 15, 2016

defunctzombie commented May 15, 2016

defunctzombie May 15, 2016

rchipka May 15, 2016

rchipka commented May 15, 2016

rchipka commented May 24, 2016

fafnirical commented Jun 6, 2016

rchipka commented Jun 6, 2016

hildjj commented Aug 31, 2016

rchipka commented Jan 11, 2017

bchr02 commented Jan 11, 2017

Precompiled binary support #407

Precompiled binary support #407

Conversation

rchipka commented May 11, 2016

rchipka commented May 11, 2016 • edited

rchipka commented May 11, 2016 • edited

defunctzombie commented May 12, 2016

defunctzombie commented May 12, 2016

rchipka commented May 12, 2016 • edited

rchipka commented May 12, 2016 • edited

gagern commented May 12, 2016

rchipka commented May 13, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rchipka commented May 13, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rchipka commented May 14, 2016 • edited

defunctzombie commented May 14, 2016

rchipka commented May 15, 2016

defunctzombie commented May 15, 2016

rchipka commented May 15, 2016

defunctzombie commented May 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

defunctzombie commented May 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rchipka commented May 15, 2016

rchipka commented May 24, 2016

fafnirical commented Jun 6, 2016

rchipka commented Jun 6, 2016

hildjj commented Aug 31, 2016

rchipka commented Jan 11, 2017

bchr02 commented Jan 11, 2017

rchipka commented May 11, 2016 •

edited

rchipka commented May 11, 2016 •

edited

rchipka commented May 12, 2016 •

edited

rchipka commented May 12, 2016 •

edited

rchipka commented May 13, 2016 •

edited

rchipka commented May 13, 2016 •

edited

rchipka commented May 14, 2016 •

edited