Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI fails for Nightly build of Julia due to compat requirement for OpenBLAS #47158

Closed
Azzaare opened this issue Oct 21, 2021 · 24 comments · Fixed by #47659
Closed

CI fails for Nightly build of Julia due to compat requirement for OpenBLAS #47158

Azzaare opened this issue Oct 21, 2021 · 24 comments · Fixed by #47659
Labels
CI continuous integration

Comments

@Azzaare
Copy link
Contributor

Azzaare commented Oct 21, 2021

It is not blocking, but CI fails for Nightly build of Julia when I registered packages today. It comes from compat requirements of OpenBLAS

ERROR: Unsatisfiable requirements detected for package OpenBLAS_jll [4536629a]:
 OpenBLAS_jll [4536629a] log:
 ├─possible versions are: 0.3.17 or uninstalled
 └─restricted to versions 0.3.13 by an explicit requirement — no versions left

Can be found in #47157 and #47155 (at least)

@fredrikekre
Copy link
Member

Strange, instantiate works but it fails in precompile:

 pkg> precompile
ERROR: Unsatisfiable requirements detected for package OpenBLAS_jll [4536629a]:
 OpenBLAS_jll [4536629a] log:
 ├─possible versions are: 0.3.17 or uninstalled
 └─restricted to versions 0.3.13 by an explicit requirement — no versions left

@fredrikekre
Copy link
Member

cc @IanButterworth maybe you know why this happens?

@IanButterworth
Copy link
Member

Does a resolve fail too?

That PR added a resolve when explicitly running Pkg.precompile

@IanButterworth
Copy link
Member

Could it be that the manifest was generated by an older julia version?

@fredrikekre
Copy link
Member

A resolve fail too, yes. The manifest is this one: https://github.com/JuliaRegistries/General/blob/master/.ci/Manifest.1.8.toml

@IanButterworth
Copy link
Member

restricted to versions 0.3.13 by an explicit requirement — no versions left

Where's the explicit requirement? The Project doesn't have compat for it, and the manifest isn't pinned?

@fredrikekre
Copy link
Member

Maybe enforced by it beeing a stdlib jll package?

@IanButterworth
Copy link
Member

Master is on 0.3.17+1

But the error message is confusing. It's not explicitly required to be 0.3.13 anywhere.
Perhaps the resolver cannot change that version because it's a stdlib, which it assumes is because it's been explicitly required? So some check logic is wrong. Also we should probably allow stdlibs to be updated during a resolve?

A quick fix in this case would be to delete and re-resolve the manifest, but it would be good to fix the resolver in cases like this

@fredrikekre
Copy link
Member

Why is precompile resolving at all?

@IanButterworth
Copy link
Member

IanButterworth commented Oct 21, 2021

I added a resolve during Pkg.precompile because there's no guarantee then that the environment is resolved. When it's automatically called, we know the environment is resolved already, so it doesn't resolve then.

If the environment isn't properly resolved during Pkg.precompile it can cause precompilation of the wrong package versions, and miss detection of circular dependencies.

I do think there seems to be a resolver bug here that's worth fixing. IMO a manifest should be re-resolvable if stdlibs have changed version

Otherwise, you're going to precompile the wrong OpenBLAS_jll version then hit load-time precompilation

@KristofferC
Copy link
Member

I think https://github.com/JuliaLang/Pkg.jl/blob/master/src/HistoricalStdlibs.jl needs to be kept in sync with master. Not sure.

@IanButterworth
Copy link
Member

That sounds reasonable. I thought Pkg tests were added to check that?
cc. @staticfloat

@IanButterworth
Copy link
Member

IanButterworth commented Oct 21, 2021

I added a @show at
https://github.com/JuliaLang/Pkg.jl/blob/959f8b7dac1ac922667eda1b4e35cfbc857145b7/src/Operations.jl#L1286

               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.8.0-DEV.784 (2021-10-21)
 _/ |\__'_|_|_|\__'_|  |  Commit 1838881489 (0 days old master)
|__/                   |

julia> import Pkg
[ Info: Precompiling Pkg [44cfe95a-1eb2-52ea-b672-e2afdf69b79f]

(Pkg) pkg> activate /Users/ian/Downloads/General
  Activating new project at `~/Downloads/General`

(General) pkg> resolve
ctx.julia_version = v"1.8.0-DEV.784"
  No Changes to `~/Downloads/General/Project.toml`
  No Changes to `~/Downloads/General/Manifest.toml`

@staticfloat AFAIU this should be nothing unless we're requesting a specific julia version (the BB tooling)

Perhaps that's the issue?

But.. I don't know why the above didn't error.. it's https://github.com/JuliaRegistries/General/blob/master/.ci/Project.toml and https://github.com/JuliaRegistries/General/blob/master/.ci/Manifest.1.8.toml

@IanButterworth
Copy link
Member

@fredrikekre
Copy link
Member

I added a resolve during Pkg.precompile because there's no guarantee then that the environment is resolved.

Ok, that is a bit surprising to me. It feels like a do-not-touch-toml operation.

@IanButterworth
Copy link
Member

If we had a is_resolved(env) function, then we could error if not resolved? Maybe that would be better than force-resolving?

@fredrikekre
Copy link
Member

Yea. As it is now reproducibility doesn't hold since "just download the toml files and instantiate" might modify things, right?

@IanButterworth
Copy link
Member

So if the manifest has an old version of a stdlib in it, it's better that Pkg.instantiate errors rather than updates the stdlibs and succeeds?

I guess this is mostly an issue for stdlib changes that happen on the same julia version, so it's mostly a bleeding edge problem, so erroring seems reasonable?

@IanButterworth
Copy link
Member

And that erroring strategy would've still caused this CI to fail, so perhaps equally disruptive

@IanButterworth
Copy link
Member

IanButterworth commented Oct 21, 2021

"just download the toml files and instantiate" might modify things, right?

Actually, it wont. The Pkg.precompile that's called by Pkg.instantiate currently doesn't re-resolve. Only when Pkg.precompile is explicitly called

@KristofferC
Copy link
Member

i.e. I think this should be set to nothing?

I agree with this.

@IanButterworth
Copy link
Member

IanButterworth commented Oct 21, 2021

Ways forward I see:

  1. Keep as-is
  2. Keep as-is but allow the resolver to update stdlibs (Always allow stdlibs to update during a resolve JuliaLang/Pkg.jl#2787)
  3. Don't resolve during explicit Pkg.precompile and instead error if the environment is not resolved
  4. 3 but warn only

Seems like JuliaLang/Pkg.jl#2787 should be done either way

@giordano
Copy link
Member

My understanding is that JuliaLang/Pkg.jl#2787 is not going to be merged any time soon (at least not as is), can we please temporarily skip the test on nightly? Seeing all pull requests with failing tests is of no use.

@DilumAluthge
Copy link
Member

can we please temporarily skip the test on nightly? Seeing all pull requests with failing tests is of no use.

Agreed. Can you do the PR? It should be as simple as commenting out the "nightly" line in the CI workflow file.

giordano added a commit that referenced this issue Oct 22, 2021
To be restored after #47158 is fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI continuous integration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants