Refine build tooling #327
Refine build tooling #327
Conversation
| compilejobUbuntu: | ||
| strategy: | ||
| fail-fast: false | ||
| runs-on: ubuntu-latest | ||
| name: Binder_on_Ubuntu | ||
| steps: | ||
| - name: Install | ||
| run: | | ||
| set -x | ||
| uname -a | ||
| cat /etc/issue | ||
| sudo apt install python3 python3-dev clang make cmake | ||
| - name: Checkout | ||
| uses: actions/[email protected] | ||
| - name: Compile and Test | ||
| run: | | ||
| python3 build-and-run-tests.py -j 2 --llvm-version 14.0.5 | ||
|
|
There was a problem hiding this comment.
Could you please use something like ubuntu-24.04 instead of latest? We do not want this to suddenly start failing if/when GitHub update latest-runner image.
| print('LLVM:{} + Binder install is detected at {}, skipping LLVM installation and Binder building procedures...\n'.format(llvm_version, build_dir)) | ||
|
|
||
| else: | ||
| elif not update_binder: |
There was a problem hiding this comment.
This seems a bit odd, - if signature does not match then it is clear that Binder upgrade is needed. Could you please elaborate on the logic here? My initial impression for a new --update-binder option would be that it will trigger script to check if such upgrade is needed and rebuild Binder if so. Do you see this differently? If so please elaborate.
There was a problem hiding this comment.
The whole point of this option is to not re-compile LLVM every time, which somehow happened for me each time I called the script after changing Binder code. So with this, LLVM will only be build if --update-binder is not specified.
There was a problem hiding this comment.
Hm… it sounds like there is a disconnect here, please let me know if below is not what you are observing on your system:
-- there should not be automatic rebuild unless binder repository have a new commits, - ie changing code should not trigger the rebuilds.
-- to avoid re-build checks (and potential rebuilds) the simplest way is to pass --binder <path-to-binary> option when using build-and-run-tests.py.
Note that I do like the idea of having --update-binder option, but I though it will trigger be something like "rebuild binder binaries even if no new commits are found" so its implementation should probably just call ninja in the right dir (this is what usually do by hands)
| parser.add_argument('--pybind11', default='', help='Path to pybind11 source tree') | ||
|
|
||
| parser.add_argument("--accept", action="store_true", help="Run tests and accept new tests results as reference") | ||
| parser.add_argument("--accept", action="store_true", help="Run tests and ask to accept new tests results as reference") |
There was a problem hiding this comment.
Since we on this, probably "optional" since it is not directly related to new code: name of this option now clearly misleading, especially with introduction of --accept-all. How about something like --interactive-accept (I am open to suggestions if you have better ideas).
There was a problem hiding this comment.
Yep, --interactive-accept sounds more descriptive, good idea! Didn't want to change compatibility there, but if you are happy with that, I can change this.
There was a problem hiding this comment.
I am fine with changing compatibility in this case (I do not think this option is used in automation), so please feel to rename it. Thanks,
2 participants
This PR adds a bit of helpful functionality for building Binder during dev:
build.pyandbuild-and-run-tests.pyhave a new flag--update-binderwhich re-compiles Binder without re-compiling LLVM. This is faster for development when using these scripts to build Binder.test/self-test.pyhas a new flag--accept-allthat accepts all changes, instead of asking about each change.git diffcan then be used to show all the changes.Some further small fixes:
test/T42.stl.names.multi.hppwas missing a header include forstring._pybind11_version_inbuild.pyto the latest commit of the RosettaCommons Pybind11 fork.build-and-run-tests.pyscript.