Bootstrap the native engine from live sources.

Review Request #4345 — Created Oct. 31, 2016 and submitted

jsirois
pants
jsirois/issues/3999
3999, 4014
2b6e063...
pants-reviews
nhoward_tw, stuhood
This change to the `pants` script requires that the engine is
bootstrapped from live local sources and as a result requires pants
devs to have a rust distribution installed.

 build-support/bootstrap_native | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
 pants                          |  2 ++
 src/rust/engine/.gitignore     |  1 +
 src/rust/engine/Cargo.lock     |  4 +++
 src/rust/engine/Cargo.toml     |  7 ++++++
 src/rust/engine/src/lib.rs     |  2 ++
 6 files changed, 82 insertions(+)

Manually tested caching as follows:

$ rm -rf ~/.cache/pants/bin/native-engine ~/.cargo ~/.multirust && git clean -fdx src/rust

$ time ./pants --version
A rust installation could not be found, installing via the instructions at https://www.rustup.rs ...
info: downloading installer
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: downloading component 'rustc'
 45.4 MiB /  45.4 MiB (100 %)   2.2 MiB/s ETA:   0 s                
info: downloading component 'rust-std'
 58.3 MiB /  58.3 MiB (100 %)   2.4 MiB/s ETA:   0 s                
info: downloading component 'rust-docs'
  7.4 MiB /   7.4 MiB (100 %)   2.2 MiB/s ETA:   0 s                
info: downloading component 'cargo'
  4.2 MiB /   4.2 MiB (100 %)   1.5 MiB/s ETA:   0 s                
info: installing component 'rustc'
info: installing component 'rust-std'
info: installing component 'rust-docs'
info: installing component 'cargo'
info: default toolchain set to 'stable'

  stable installed - rustc 1.12.1 (d4f39402a 2016-10-19)

info: using existing install for 'stable-x86_64-unknown-linux-gnu'
info: override toolchain for '/home/jsirois/dev/pantsbuild/jsirois-pants' set to 'stable-x86_64-unknown-linux-gnu'

  stable-x86_64-unknown-linux-gnu unchanged - rustc 1.12.1 (d4f39402a 2016-10-19)

   Compiling engine v0.0.1 (file:///home/jsirois/dev/pantsbuild/jsirois-pants/src/rust/engine)
    Finished debug [unoptimized + debuginfo] target(s) in 0.25 secs
1.3.0dev0

real    1m21.751s
user    0m4.877s
sys 0m2.087s

$ time ./pants --version
1.3.0dev0

real    0m0.604s
user    0m0.403s
sys 0m0.053s

CI went green here:
https://travis-ci.org/pantsbuild/pants/builds/173325950

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
  1. This will likely fail CI (not sure if Travis images include (modern) rust), but it should be a good jump-start on the bootstrap code.   could imagine just skipping bootstrap if the pre-requisite check for cargo fails for now.  Those deving on the native engine know who they are and can be expected to know what to do in the short term it would seem.
  2. 
      
  1. Nick and Stu - if you can patch this in and test on OSX I'll be grateful. There is a tiny bit of OSX-specific code using sw_vers that I think is right, but you know how that goes.

    1. Thanks John! First thing tomorrow.

  2. 
      
  1. Looks great! Thanks John.

    1. Does it also work great? Before submitting, I'd really like to get a confirmed OSX working use with a rm -rf ~/.cache/pants/bin/native-engine ~/.cargo ~/.multirust && git clean -fdx src/rust to preface.

    2. One bit of data here: apparently I had been using the rust installer from rust-lang.org, which needed to be uninstalled first in order for rustup to proceed. As long as the output from rustup is displayed, this should be fine, as it errors out indicating that you need to uninstall the manually installed copy first.

    3. ...let me confirm that first though.

    4. If the rust installer from rust-lang.org is in use, cargo is detected, and rustup doesn't run... so that should be fine.

  2. build-support/bootstrap_native (Diff revision 6)
     
     

    "if the user"

  3. 
      
  1. 
      
  2. build-support/bootstrap_native (Diff revision 7)
     
     
     

    Looks like it might not work on OSX:

    $ ./pants goals
    /Users/stuhood/src/pants/build-support/bootstrap_native: line 14: ${KERNEL,,}: bad substitution
       Compiling engine v0.0.1 (file:///Users/stuhood/src/pants/src/rust/engine)
        Finished debug [unoptimized + debuginfo] target(s) in 0.36 secs
    ln: /Users/stuhood/src/pants/src/rust/engine/target/debug/libengine.: No such file or directory
    
    1. -readonly KERNEL=$(uname -s)
      +readonly KERNEL=$(uname -s | tr '[:upper:]' '[:lower:]')
      # NB: ${varname,,} is a very cryptic way to lowercase a variable's contents in bash.
      -case "${KERNEL,,}" in
      +case "${KERNEL}" in

      works though.

    2. Friggen OSX. OK, thanks, I'll lcase differently.
    3. With this edit, confirmed that the bootstrap works on OSX without rust/rustup already installed.

      Will check the behaviour when it is installed via the official installer.

    4. Thanks for checking this all out Stu - lcasing fixed.
  3. 
      
  1. OK - wonky 8 (and 9) diffs corrected by 10, so the last review feedback can be seen in the 7-10 interdiff.

  2. 
      
  1. Ship It!
  2. 
      
  1. 
      
  2. src/rust/engine/Cargo.lock (Diff revision 10)
     
     

    Should Cargo.lock be in .gitignore?

    According to http://doc.crates.io/guide.html#cargotoml-vs-cargolock, Cargo.lock is for binaries, whereas the intention here seems to build from src?

    FWIW I tried

    git clean -fdx
    rm src/rust/engine/Cargo.lock 
    ./pants goals
    

    and it still works.

    1. No: Cargo.lock is a lockfile for freezing dependency versions. It should be committed.

  3. src/rust/engine/Cargo.toml (Diff revision 10)
     
     

    Does version mean to correspond to readonly VERSION=0.0.1 above? If so, it would be good to have comments in both places.

    Or better to have build-support/bootstrap_native read this?

    1. Submitted before I noticed your comments Yi - but yes, VERSION should == the version here and a comment is appropriate.  If Stu doesn't add one in his RB to land the seed native engine code, I'll circle back and comment and/or add a brief doc.
    2. Stu got rid of the version/VERSION correspondence requirement by making the native engine code dependency content hash based so this has been addressed.
  4. 
      
  1. Ship It!
  2. 
      
Review request changed

Status: Closed (submitted)

Change Summary:

Now on master:

git log -1 origin/master
commit a89e9d7c4cc8c0d402c86e4c6bea12e32f9ae0ee
Author: John Sirois <john.sirois@gmail.com>
Date:   Fri Nov 4 13:16:09 2016 -0600

    Bootstrap the native engine from live sources.
    
    This change to the `pants` script requires that the engine is
    bootstrapped from live local sources and as a result requires pants
    devs to have a rust distribution installed.
    
    Testing Done:
    Manually tested caching as follows:
    ```
    $ rm -rf ~/.cache/pants/bin/native-engine ~/.cargo ~/.multirust && git clean -fdx src/rust
    
    $ time ./pants --version
    A rust installation could not be found, installing via the instructions at https://www.rustup.rs ...
    info: downloading installer
    info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
    info: downloading component 'rustc'
     45.4 MiB /  45.4 MiB (100 %)   2.2 MiB/s ETA:   0 s
    info: downloading component 'rust-std'
     58.3 MiB /  58.3 MiB (100 %)   2.4 MiB/s ETA:   0 s
    info: downloading component 'rust-docs'
      7.4 MiB /   7.4 MiB (100 %)   2.2 MiB/s ETA:   0 s
    info: downloading component 'cargo'
      4.2 MiB /   4.2 MiB (100 %)   1.5 MiB/s ETA:   0 s
    info: installing component 'rustc'
    info: installing component 'rust-std'
    info: installing component 'rust-docs'
    info: installing component 'cargo'
    info: default toolchain set to 'stable'
    
      stable installed - rustc 1.12.1 (d4f39402a 2016-10-19)
    
    info: using existing install for 'stable-x86_64-unknown-linux-gnu'
    info: override toolchain for '/home/jsirois/dev/pantsbuild/jsirois-pants' set to 'stable-x86_64-unknown-linux-gnu'
    
      stable-x86_64-unknown-linux-gnu unchanged - rustc 1.12.1 (d4f39402a 2016-10-19)
    
       Compiling engine v0.0.1 (file:///home/jsirois/dev/pantsbuild/jsirois-pants/src/rust/engine)
        Finished debug [unoptimized + debuginfo] target(s) in 0.25 secs
    1.3.0dev0
    
    real    1m21.751s
    user    0m4.877s
    sys     0m2.087s
    
    $ time ./pants --version
    1.3.0dev0
    
    real    0m0.604s
    user    0m0.403s
    sys     0m0.053s
    ```
    
    CI went green here:
      https://travis-ci.org/pantsbuild/pants/builds/173325950
    
    Bugs closed: 3999, 4014
    
    Reviewed at https://rbcommons.com/s/twitter/r/4345/
Loading...