Comments (25)
The best ideas I currently have are:
- Define compilation flags in a function and only use it in library builds in a way that wouldn't alter the project's environment.
- In
docker_build_wrap.sh
, source some other user-provided file, e.g. "config_pre.sh
", before sourcinglibrary_builders.sh
.
from multibuild.
Multibuild builds fat OSX binaries by default. If you want, submit a PR moving/disabling that behavior with an environment variable.
from multibuild.
I think the user will usually want the build arteifacts on the include / library path - right? Hence the build flags being global.
It's not a bad idea adding something like env_vars.sh
or config_pre.sh
that can be executed before the source of the other stuff.
from multibuild.
@xoviat The problem is not that I'm unhappy with the default choice, or can't override it (I just did in the linked commit). The problem is that neither I, nor @skvark ever saw this choice being made in the first place.
@matthew-brett Setting up build environment and building dependencies are two different stages of the build process. The latter one is even optional. The name "library_builders
" suggests that it covers just the latter stage, not anything else.
I expected any logic that performs build environment setup to rather be together with the rest of the logic responsible for that step - i.e. somewhere like travis_<os>_steps.sh
, or docker_build_wrap.sh
(or a function in *_utils.sh
that is called from there).
from multibuild.
This is an OSS project. If you think that something should be different, submit a PR.
from multibuild.
I'm not quite sure which course of action would be in line with the big design, so prefer to discuss this first.
I need to know what to submit before investing time into it.
At this point, I have enough feedback regarding the 2nd issue and can proceed with a PR for it.
from multibuild.
Variants about relocating build environment setup code that crossed my mind. Each one has drawbacks.
For clarity, let's call each option, combined, as "Option X", and its parts that address the 1st and 2nd issues "Option X-1" and "X-2", correspondingly.
- Rename
library_builders
to something that would suggest it handles both dependencies and build environment. (Option 1-1)- no idea what
- still violates separation of concerns?
- still need two configuration files (Option 1-2)
- Split off that logic to yet another file (Option 2-1)
- redundant? there are
*_steps
,*_utils
etc already - still need two configuration files (Option 2-2 which is the same as 1-2)
- redundant? there are
- Move build environment setup code into a function (defined in
_steps
or_utils
probably as it's OS-specific) and invoke it afterconfig.sh
is sourced. The user will thus be able to override it in the latter. (Option 3)- only need one config file
- A function is not flexible enough for build environment configuration:
- Build environment almost always needs be altered for a specific project, and most often, only altered partially.
- Functions do not support partial reuse; neither is there an "inheritance" mechanism in Bash that would allow to chain a function with another with the same name.
- very complicated, error-prone
- The fact that the function is OS-specific adds yet more complication
- all other logic of
library_builders
that has inputs also needs to be moved into a function -- yet more complication
from multibuild.
We should use the third approach. Although functions don't support partial-reuse, it's what's previously been used for multibuild.
The fact that the function is OS-specific adds yet more complication
The function should be named the same for any OS, and should handle both OSX and Linux.
from multibuild.
What about the "no inheritance" part? How are you going to call the stock function from the user-defined one with the same name? (And so on ifwhen e.g. OS-independent and OS-specific parts are separated.)
from multibuild.
There's no inheritance in multibuild. See any of the other functions.
from multibuild.
No other function needs to be inherited. The very few existing functions that are designed to be overridden (e.g. build_wheel
) are trivial enough to be replaced entirely. ( "Configuration is by overriding the default build function, and defining a test function." Full stop.)
When I monkey-patched travis_linux_steps
, I copy-pasted build_multilinux
entirely and only deemed that acceptable because it's intended as a temporary solution until the fix goes into the release branch (i.e. the stock function is not expected to change in incompatible ways during that time).
from multibuild.
So, theoretically on the multibuild side:
setup_environment
if is_function "post_setup_environment" post_setup_environment # Potentially user-defined
And in config.sh
:
function post_setup_environment {
:
}
from multibuild.
Then you need an entry point before and between each step :)
To be honest, there is a way to rename a function in Bash (so you can invoke it from the overriding one). And it's not pretty. (You also need to come up with a unique identifier for the rename.)
from multibuild.
That's correct. You can add entry points where you think they should go.
from multibuild.
This makes three already: before setup_environment
, after init_library_builders
(that's where library-related envvars would go), and between them. And once we add one, removing it would break backward compatibility.
from multibuild.
Yes, of course.
from multibuild.
There's yet another option that evaded me when writing https://github.com/matthew-brett/multibuild/issues/116#issuecomment-357372450.
- If you remember, the need for
config_pre
or an entry point beforelibrary_builders
is becauselibrary_builders
has a lot of envvar inputs and there's no way to bring them into Docker from the host environment.- So, if the user can make a list of additional variables to propagate, this specific need will vanish.
- The implementation can use a Bash array to accumulate additional
docker run
args.
- The implementation can use a Bash array to accumulate additional
- So, if the user can make a list of additional variables to propagate, this specific need will vanish.
- This won't allow to define anything other than envvars. But nothing else needs to be defined at this point atm 'cuz
library_builders
doesn't have any other inputs (e.g. it doesn't call anything external upon import).
This will eliminate the need for the 1st entry point (however it's implemented).
The 2nd entry point isn't really needed atm to begin with 'cuz init_library_builders
logic doesn't use anything defined by setup_environment
on import.
And the 3rd point is already covered by config.sh
.
So, with this, we can go the 1st/2nd option minus the 2nd config file. I also came up with the proposed names -- build_env+library_builders.sh
and build_env.sh
, correspondingly.
Besides, this is arguably a more intuitive solution 'cuz the normal expectation is for the initially set envvars to persist throughout the build.
from multibuild.
@native-api - I think you're proposing something like
ENV_VAR_NAMES=(FOO BAR BAZ)
in .travis.yml
, where these variables would be copied across the the docker container with suitable -e FOO=$FOO
type lines in the build_multilinux
function?
The alternative I was proposing initially was to have an env_vars.sh
file that would have:
FOO="my_foo_contents"
BAR="my_bar_contents"
etc, and which would be sourced from within the docker container - and by the OSX builds - before sourcing the other .sh
files. The advantage of this is only that it's pretty easy to explain, in that it avoids a step of indirection. Also, you can put platform specific logic in there to deal with OSX / Linux differences. What do you think?
I'm not sure what you mean by "And the 3rd point is already covered by config.sh." - could you explain?
from multibuild.
So the env_vars.sh
seems to be the same idea as config_pre.sh
-- I guess that's okay after all?
from multibuild.
It's OK with me - yes. Is that OK with you @native-api ?
from multibuild.
from multibuild.
On issue 1, the names in https://github.com/matthew-brett/multibuild/issues/116#issuecomment-357821839 and the already listed options is all I could come up with.
Due to this being quite a memorable experience, I personally "know" about that library_builders
' little trick by now (sure as hell I do...).
So, if you have qualms about it, we can close this issue and wait for another poor sap to step on this landmine and try to come up with something better... (this can very well be me again after several months).
from multibuild.
Issue 1 is that it's not obvious that "library_builders.sh" configures the build environment for all later steps.
How about having a separate file called configure_builds.sh
that is sourced from library_builders.sh
, and suitably commented?
from multibuild.
@matthew-brett That's the option 2-1 earlier. I came up with build_env.sh
name earlier, too (not insisting on it though). Split the logic off, or rename the original file, okay either way.
from multibuild.
A dedicated configure_build.sh
also opened an opportunity to consolidate build flags set by other files in one place. Combined with the fact the name for the rename is still rather ugly, op. 2-1 looks preferrable to me now.
from multibuild.
Related Issues (20)
- Build wheels for Python 3.10 RC1? HOT 3
- Testing manylinux_2_24 HOT 6
- Move to github actions for windows, mac, linux (x86, x86_64) HOT 5
- Port CI to Github Actions HOT 8
- get_modern_cmake - version 3?
- Cent OS 5 repos gone, manylinux1 builds fail HOT 1
- Trouble building 3.10 wheels on OSX HOT 8
- Building thin arm64 wheels for MacOS 12+ HOT 5
- PLAT=x86_64 generates universal wheel for OSX & Python 3.10
- No supported wheels found for macOS arm64 HOT 1
- Recent git update has broken builds using docker images HOT 16
- Undefined symbol error with manylinux_2_28 wheel on Ubuntu 20.04 HOT 2
- LAPACK library is not found for macos builds HOT 5
- xenial_arm64v8:latest image is missing Py3.11 binary HOT 3
- Running safety prints a scary warning HOT 7
- platform = arm64 or universal2 both create x86_64 wheels HOT 1
- Use alpine 3.18 images by default for test
- python 3.11 and 3.12 builds failing on macosx and linux
- ./common_utils.sh: line 83: python: command not found HOT 4
- musllinux_1_1 is EOL, should use musllinux_1_2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from multibuild.