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

release.sh -q builds single-platform pexes locally #5563

Merged
merged 2 commits into from Mar 8, 2018

Conversation

Projects
None yet
3 participants
@illicitonion
Copy link
Contributor

illicitonion commented Mar 6, 2018

This makes it much easier to do things like build a linux pex in a docker container and run it on a remote machine, without having to go via travis.

@illicitonion illicitonion requested a review from stuhood Mar 6, 2018

@stuhood

stuhood approved these changes Mar 6, 2018

Copy link
Member

stuhood left a comment

We reaaaally need to break this up further, and/or port bits to python.

@@ -773,7 +815,8 @@ while getopts "hdntcloepw" opt; do
l) list_packages ; exit $? ;;
o) list_owners ; exit $? ;;
e) fetch_and_check_prebuilt_wheels ; exit $? ;;
p) build_pex ; exit $? ;;
p) build_pex fetch ; exit $? ;;
q) build_pex build ; exit $? ;;

This comment has been minimized.

@stuhood

stuhood Mar 6, 2018

Member

github won't let me comment directly on it, but: should update the help above re: "mutually exclusive".


local linux_platform="linux_x86_64"
local osx_platform="macosx_10.10_x86_64"

case "${mode}" in

This comment has been minimized.

@stuhood

stuhood Mar 6, 2018

Member

This mode/switch approach feels like it results in more local variables to keep in your head than explicit function calls would.

This comment has been minimized.

@illicitonion

illicitonion Mar 7, 2018

Contributor

By explicit function call, do you mean effectively copy-pasting build_pex into a build_local_pex function? That's what I originally did, but the parameters needed for code-sharing felt pretty small (just the platforms array and dest string)... But maybe I'm missing the thing you're proposing?

This comment has been minimized.

@stuhood

stuhood Mar 7, 2018

Member

By explicit function call, do you mean effectively copy-pasting build_pex into a build_local_pex function?

I meant more like decomposing build_pex into more functions: maybe "functions to fetch/build whls" and then "function to actually build the pex". But ignore me: feel free to land as is. More important is just that we eventually invest to port this.

@wisechengyi

This comment has been minimized.

Copy link
Contributor

wisechengyi commented Mar 7, 2018

Is it intended only a single platform pex is to be built here?

If it's not too much additional effort to build multiple platform pex, can we include that as well? so iteration on both platform would be much faster.

@illicitonion

This comment has been minimized.

Copy link
Contributor

illicitonion commented Mar 8, 2018

@wisechengyi multi-platform pex is hard because rust doesn't support cross-compiling out of the box.

We could add an "If you're on OSX, fetch and build the native wheel in a docker container" mode, but I would definitely want that done as a separate change.

@illicitonion illicitonion force-pushed the twitter:dwagnerhall/singleplatformpexes branch from 054cdad to 0aa990b Mar 8, 2018

@illicitonion illicitonion merged commit be5ec00 into pantsbuild:master Mar 8, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@illicitonion illicitonion deleted the twitter:dwagnerhall/singleplatformpexes branch Apr 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment