Skip to content

Conversation

@directhex
Copy link
Contributor

With Apt, if you have X 1.0 installed, 1.1 exists in the package manifest, and say "install X", X is upgraded to 1.1. With Brew, a fatal error is thrown.

Blindly upgrading all previously installed packages is not ideal, and not a perfect mirror for the Apt behaviour, but it should mean that if X is previously installed it gets upgraded (causing a warning during install, when it's asked to be installed again with the same version), and if it's not already installed then it gets installed later.

Closes: #33471

With Apt, if you have X 1.0 installed, 1.1 exists in the package manifest, and say "install X", X is upgraded to 1.1. With Brew, a fatal error is thrown.

Blindly upgrading all previously installed packages is not ideal, and not a perfect mirror for the Apt behaviour, but it should mean that if X is previously installed it gets upgraded (causing a warning during install, when it's asked to be installed again with the same version), and if it's not already installed then it gets installed later.

Closes: dotnet#33471
Copy link
Contributor

@jashook jashook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this update our native toolsets unconditionally?

@directhex
Copy link
Contributor Author

@jashook yup. But by calling brew update, as is already the case, the cache of available versions is already "latest at time of build", so either we need to pin specific versions in install-native-dependencies.sh (i think brew supports this?), or find some other mechanism to conditionally upgrade some specific packages.

As is, updating the package manifest to latest and asking "install the latest version of these 7 packages" will always cause failures when any one of those 7 is updated, as is the case today.

Brew is not smart.

@jaredpar
Copy link
Member

Is there a way to limit the brew upgrade to just python? Looking through the log that seems to be the only source of our failures right now.

Either way we should proceed with some version of this as a stop gap measure. Builds are failing 100% right now (just verified). This will at least get us unblocked until we can find the appropriate longer term fix.

@jashook
Copy link
Contributor

jashook commented Mar 11, 2020

Also seems to me if these machines already have a python3 installed (which seems to have changed) we just should not brew install python3

@jashook
Copy link
Contributor

jashook commented Mar 11, 2020

I was suggest a small check if python3 does not exists then brew install python

@directhex
Copy link
Contributor Author

@jaredpar brew upgrade X if X is not installed is a failure, and brew install X if X is already installed but not $latest is a failure. Not installing python3 seems like it means we can't bootstrap new machines at all with install-native-dependencies.sh. Brew simply lacks a command for either install this set of packages but don't upgrade them or install this set of packages and upgrade any in this set which are out of date.

@directhex
Copy link
Contributor Author

We can put in a python specific conditional, as long as we're certain that there will never be an issue released for icu4c, openssl, autoconf, automake, libtool, or pkg-config where we're gonna end up in this exact situation again.

@jashook
Copy link
Contributor

jashook commented Mar 11, 2020

They would have to be present on the vm provided to us by azdo to have the conflict. Python3 seems understandable why they would add that, the rest may or may not be added. However, personally I find conditionally checking python3 and hoping the other packages are not added a safer solution. Seeing that this is also used by our official build (see below).

@jashook
Copy link
Contributor

jashook commented Mar 11, 2020

However, on further thought, we already always install the latest package anyways. So I think I am fine with either solution.

@jashook
Copy link
Contributor

jashook commented Mar 11, 2020

Merging to unblock the ci.

@jashook jashook merged commit a15b2a2 into dotnet:master Mar 11, 2020
@jashook
Copy link
Contributor

jashook commented Mar 11, 2020

@directhex thank you for the quick fix

directhex added a commit to directhex/runtime that referenced this pull request Mar 13, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OSX CI builds fail to install python

4 participants