Simplify drone.star #200
Simplify drone.star #200
Conversation
Add a `job` function which only requires compiler, cxxstd and OS for the simple cases and allow additional arguments for the more complex ones. The result is a matrix-like, easy to read list of jobs.
Allows to use an empty string to set environment variables to an empty string
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #200 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 20 20
Branches 10 10
=========================================
Hits 20 20 Continue to review full report at Codecov.
|
|
My personal preference is to just have a plain matrix like before, because it's easy to understand and debug. If the plan is to have There was an issue that came up in boostorg/url recently which was that .drone.star initially had cxx versions cxxstd='03,11,14,17,20' for each job. It is computationally expensive, because the number of jobs is already long and then for each one it's running 5 versions '03,11,14,17,20' which causes every job to take a lot of time. The solution in boostorg/url, and what Alan did, is to scale that back and only test '17,20', for example. Of course there are pros and cons since it's somewhat reduced coverage. But it's faster, which users might like, and its cheaper in terms of CPU cycles. Not sure if I have an opinion on that either way. what do you think? |
That is also my goal, but from a different aspect. I find
easier to understand than
"easy to understand" to me means to easily see what configurations are exactly tested. So I'm using a syntax similar to GHA and remove duplication where possible. In my testing this was also helpful already as adding a new compiler or a different OS image was much easier. E.g. the compiler version changes in 1 place, not 4.
I wanted to put that up for discussion, i.e. if this should be the way to go or if that (combined into a single one) function should go into each repos drone.star. That would have the advantage of being more transparent making it easier to "understand and debug" as I agree putting more stuff into a "black box" may make it harder to set up the stuff initially. On the other hand having "high-level input" and the
In #184 (comment) you wrote
So that's why I made the default to be exhaustive so that the template shows all possible combinations and users (library authors) can choose what to keep as they see fit. But I do see that as an issue as well and wouldn't use the whole thing as-is either. Not sure how to decide which standards to test either. Maybe the whole range for the oldest and newest compiler per OS/stdlib and only oldest and newest supported standard for those inbetween except for e.g. GCC5 where I'd use |
|
The title of the PR is "Simplify drone.star". But in fact drone.star has 150 new lines of code in it now, which is overall more complex than a plain matrix. Of course, that's what Alan is doing also. There are now two competing solutions for automating a drone file. I'm not enthusiastic. but I will go along. You are proposing to leave the function in the .drone.star file to avoid it being a blackbox. I was sort of expecting functions to be moved to functions.star. I don't know. It could be either way. Either way. If all the code will remain in the .drone.star file, then you have my permission to merge it. If the code will be moved to functions.star, the PR should be updated. And then I will copy functions.star to the drone server. |
Add a
jobfunction which only requires compiler, cxxstd and OS for the simple cases and allow additional arguments for the more complex ones.The result is a matrix-like, easy to read list of jobs. An entry looks like:
This will create a nice name, start the right docker image, install the required compiler package(s) and set the
B2_*env variables, especiallyB2_TOOLSET/B2_COMPILERandB2_CXXSTD.There is also support for sanitizers (ASAN, UBSAN, TSAN, Valgrind) which set up the required Drone- and B2-options and default the variant to "debug" as well as defining
BOOST_NO_STRESS_TEST.See the commented function signature for the full options.
Split into 2 functions where the
job_implfunction is suitable for inclusion intofunctions.starandjob(which automatically addsglobalenv) is to be defined in.drone.star. Alternatively the whole function (both combined) could be in.drone.starto aid developers using it by being more transparent what it does.If I didn't miss any there should be all jobs included from the current drone.star with additions to compilers, OSes and C++-standards.
I was thinking to simplify e.g.
job(compiler='clang-7', cxxstd='03,11,14,17,2a', os='ubuntu-20.04', stdlib='libc++', install='libc++-7-dev libc++abi-7-dev'),such that thejob_implfunction auto-installs the libc++ packages together with the compiler. A pro would be to handle the broken libc++12 package there (need libunwind-12) and shorter commands but we would need to handle outliers such asinstall='libc++-dev libc++abi-dev'for clang-6 on Ubuntu 16.We could also auto-deduce when we need to set
add_llvm=Truebased on compiler and OS. But not sure if we really should.