search

mapbox / node-cpp-skel

Skeleton for bindings to C++ libraries for Node.js using node-addon-api
Creative Commons Zero v1.0 Universal
72 stars 10 forks source link

Duplicate publishing builds in matrix #137

Closed mapsam closed 6 years ago

mapsam commented 6 years ago

It appears https://github.com/mapbox/node-cpp-skel/commit/0ba3b2cf860eeddc36bf84b4e05c2e806e2ed26f#diff-354f30a63fb0907d4ad57269548329e3 may have introduced some duplicate jobs in the travis matrix that are publishable. This results in a race condition when publishing binaries, such that a commit with [publish binary] in it will never 100% succeed since node-pre-gyp specifically will not overwrite binaries that already exist on s3.

This was discovered in https://github.com/mapbox/vtcomposite/pull/44

Here is a node-cpp-skel build that has some failing jobs since others have already published. The duplicates are across macos and linux. Specifically, here are the two builds that are both producing the following binary:

node-cpp-skel/v0.1.0-mapsamtmp/Release/node-v48-linux-x64.tar.gz

🍏 linux job that passed

🍎 linux job that failed since it produced the same binary but the above job finished first.

Digging a bit further, each of the jobs that result in duplicates are supposed to be "toolset" jobs, which means we pass in something like TOOLSET=asan to build a downloadable address sanitizer job. node-pre-gyp is configured to place these in a specific s3 path (defined here with {toolset} and defaults to an empty string). We pass the TOOLSET matrix variable as an argument down to node-pre-gyp. It expects the argument to be --toolset but we are inadvertently passing it as --TOOLSET which means it is not picked up. I think if we fix this argument the builds should be working again.

Confirming the toolset builds do not publish as expected, listing out available objects on s3, we would expect two binaries and a "toolset" build to exist. node-cpp-skel does not have the toolset.

~$ aws s3 ls s3://mapbox-node-binary/@mapbox/node-cpp-skel/v0.1.0-mapsamtmp/Debug/
2018-06-06 11:47:06      43704 node-v48-darwin-x64.tar.gz
2018-06-06 11:46:59     230701 node-v48-linux-x64.tar.gz

cc @springmeyer @millzpaugh @GretaCB @allieoop

mapsam commented 6 years ago

Updating TOOLSET to toolset did the trick! Fix over at https://github.com/mapbox/node-cpp-skel/pull/138

mapsam commented 6 years ago

Confirmed asan/ binaries exist:

~$ aws s3 ls s3://mapbox-node-binary/@mapbox/node-cpp-skel/v0.1.0-mapsamtmp2/Debug/
                           PRE asan/
2018-06-06 13:50:54      43704 node-v48-darwin-x64.tar.gz
2018-06-06 13:50:51     230701 node-v48-linux-x64.tar.gz