rework yum-requirements #1019
rework yum-requirements #1019
Conversation
…ainer prior to running the primary build
|
This also allows us to remove sudo from our docker images. |
I'm -1 to that change. This makes using the docker image for debugging harder for no useful benefit. What's the reason to remove sudo? |
|
In our current builds we cannot run sudo at all for aarch64 or ppc64le. This means we cannot yum install anything whatsoever. There are two viable solution mechanisms to this problem, either check for the existence of the yum_requirements at the docker entrypoint prior to switching to the conda user, or building a new image as in this PR . The approach here is more explicit and has less docker entrypoint magic than the alternative |
|
We don't really need to remove sudo. but we also don't need to use it anymore either |
Why can we do this for the x86_64 build? |
|
The core issue is mostly around native vs emulated execution. On native systems it seems that sudo works as expected, but running sudo under qemu when you are not uid 0 just fails with messages about not being uid 0. In the interest of keeping the build approaches mostly the same we change both. |
|
Example PR where this was applied. conda-forge/tk-feedstock#32 |
|
Does multiarch/qemu-user-static#17 (comment) work? |
|
how on earth did you find that....., dunno, it might |
|
looks like that does work. See https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=13649 do we want to keep some of the bits around how we execute yum so that adding yum requirements does not require a rerender? |
|
I'm okay with keeping this if it was moved inside the docker container |
|
@isuruf made that change |
|
whoa, magical!!! |
| {{ yum_build_setup }}{% endif -%} | ||
|
|
||
| # If we have yum requirements build a new specialized image that we will use to run our build in that includes | ||
| # those dependencies. |
There was a problem hiding this comment.
Can you update this comment or remove it?
There was a problem hiding this comment.
Actually are we able to just revert all of the changes to this file now?
There was a problem hiding this comment.
we could, though this does add the advantage that we dont need a rerender if we've changed yum_requirements, and it simplifies configure_feedstock.py
There was a problem hiding this comment.
Yeah, this is roughly how I proposed yum_requirements.txt originally ( #135 ), but Phil made a compelling argument that they should be listed explicitly ( #134 ). Ultimately I don't feel strongly either way, but it seems the crux of the needed change is quite simple; so, it would be nice if this PR reflected that. Orthogonal changes are better left for other PRs IMHO.
| -e UPLOAD_PACKAGES \ | ||
| $DOCKER_IMAGE \ | ||
| {{ docker.command }} \ | ||
| /home/conda/feedstock_root/${PROVIDER_DIR}/build_steps.sh |
There was a problem hiding this comment.
Also are we able to revert the changes here too?
There was a problem hiding this comment.
we could, but this is just whitespace
|
Since @isuruf's suggestion worked, would encourage slimming this down to the minimal required change. If I'm not mistaken, that amounts to one line. Though please feel free to correct me if I'm wrong. |
|
Wow, that sudo change is insane! Great find @isuruf! |
This removes the need to specially deal with yum_requirements in conda-smithy itself. This allows us to get rid of a fair bit of code and remove the need to rerender when just changing yum_requirements