JuliaLang / Pkg.jl

Pkg - Package manager for the Julia programming language
https://pkgdocs.julialang.org
Other
621 stars 261 forks source link

Pkg3 ignores OS-specific requires #165

Closed kmsquire closed 5 years ago

kmsquire commented 6 years ago

Case in point is LibCURL.jl, whose REQUIRE file contains

julia 0.4
BinDeps
Compat 0.8
@windows WinRPM

On OSX, Pkg3 attempts to install WinRPM. Relevant section from Manifest.toml:

[[LibCURL]]
deps = ["BinDeps", "Compat", "WinRPM"]
git-tree-sha1 = "9b431bc66757ad5124f4d206c319996a2dfa2662"
uuid = "b27032c2-a3e7-50c8-80cd-2d36dbcbfd21"
version = "0.2.2"

Discovered when using Pkg3 to install IJulia.

(I looked through other issues, and didn't find this, but please close if I missed it.)

StefanKarpinski commented 6 years ago

With BinDeps2 we won't need these platform specific packages anymore, so I'd really like to avoid adding this feature just so that we can support something we won't need in the future. Can we just unconditionally install platform-specific packages and have the build script do nothing and succeed when run on the wrong platform and then just not load the package except on the appropriate platform? cc @Keno and @staticfloat to double check my assumption that we won't need platform specific packages like this once BinDeps2 is widely adopted.

kmsquire commented 6 years ago

That sounds fine to me. In this case, presumably the way forward would be to file an issues against WinRPM.jl to switch to BinDeps2 and only load on Windows?

StefanKarpinski commented 6 years ago

WinRPM doesn't need to switch to BinDeps2 – BinDeps2 makes it obsolete. The main action would be for it to do nothing when installed on any system besides Windows.

staticfloat commented 6 years ago

I think there will still be a need for platform-specific packages. Imagine an AudioIO package that needs PortAudio.jl installed on Linux, CoreAudio.jl on OSX, etc.... Or a plotting package that uses OpenGL.jl on Linux, DirectX.jl on Windows, etc.... We have to decide where we want the platform abstraction living, whether it's in the package manager (like we have it now, conditionally REQUIRE'ing packages) in the package (conditional imports, @static if is_windows() Pkg.add("DirectX") end skullduggery) or in the build script (everything gets installed, but gobs of code get ignored on package load/build time.

Personally, I think the cleanest way to do this is to have conditional requires. I don't think we should go gangbusters with it, and with packages that are cross-platform we shouldn't need it, but I definitely don't think it's reasonable to require every package to work equally well on every platform; some packages (even Julia-only packages) are OS-specific by design, and shouldn't be forced to deal with properly disabling themselves on non-matching platforms; instead I think that complexity should be a part of the package manager, to be able to identify packages that should be installed given certain conditions.

There's a second reason I don't think we should have this be decided by the dependent package; PortAudio may technically be installable on Windows, but it may be undesirable from a feature/performance standpoint. The PortAudio.jl author has an incentive to allow PortAudio.jl to be installed on as many systems as possible, but the AudioIO package may experience better performance choosing a WASAPI.jl package or somesuch. Given that such a situation is not that far-fetched, I think it's best to keep things such that packages can express their preference based on platform.

The fact that platform is the only thing they can use as the predicate to conditional requirements is fine, IMO. I haven't been able to come up with any reasonable situations where it would be necessary to have these conditional requirements other than platform differences.

StefanKarpinski commented 6 years ago

So that argues for allowing packages to be installed on any platform but perhaps erroring if you try to actually use them on a platform where they don't work. That means that Pkg3 can work for now without supporting platform-specific dependencies at the cost of resolving all dependencies for all platforms at the same time, which is unnecessary, but once we figure out how to support platform-specific dependencies (along with build/test/doc/dev-only dependencies), that limitation can be relaxed and we teach the resolver that it can ignore platform-specific dependencies for platforms other than the one it is actually on.

staticfloat commented 6 years ago

So that argues for allowing packages to be installed on any platform but perhaps erroring if you try to actually use them on a platform where they don't work.

I actually think that's the opposite of what I said. ;)

My example above is trying to communicate that it's better to let the depending package choose what gets installed than the depended package, so I'm arguing for this to be included within Pkg3 and not done by erroring out within packages.

StefanKarpinski commented 6 years ago

But where's the harm in allowing packages to be installed on all platforms even if they can't work? That at least decouples the problem of platform-specific dependencies from knowing that you don't care about resolving them on some platforms.

staticfloat commented 6 years ago

The majority of our platform-dependent packages do not fail gracefully when deps/build.jl throws an error. I don't think it's the right move to require that users, writing an OSX-specific package, might have to consider that Pkg3 will attempt to install their package on Windows because some higher up package is cross-platform and unconditionally depends on both OSXFoo and WinFoo. Sure, we can ask all package authors to wrap their build script in @static if is_osx() or something, but I honestly don't see how that is better than just adding in the ability for packages to have conditional requirements.

StefanKarpinski commented 6 years ago

I don't get it. It's a simple fix that allows us to solve the conditional resolution issue later. Just make the build script do nothing when run on unsupported platforms. How hard is that?

StefanKarpinski commented 6 years ago

Sure, we can ask all package authors to wrap their build script in @static if is_osx() or something, but I honestly don't see how that is better than just adding in the ability for packages to have conditional requirements.

It is better because I do not have a design for things like conditional requirements right now. You're talking about this as if I have a complete solution that I simply refuse to let you have, but I do not. If you want to think through all of the implication and alternatives of how to express and work with conditional dependencies, platform-specific dependencies, test-only dependencies, build-only dependencies, doc-only dependencies and all the rest of it, please be my guest but I do not have a solution. The options right now are:

  1. Have every package with platform-specific dependencies be broken until we have a design, or
  2. Do the simple thing that we can do right now that makes this not a pressing issue.

Seems like a pretty clear decision to me.

KristofferC commented 6 years ago

With regards to test-only and build-only dependencies, these are a bit easier because they are not used in the global resolve process and hence do not need to be in any central registry. They only need to be satisfied during test / build time and we already create a separate environment for packages upon testing / building them.

We could have

[build deps]
BinaryBuilder = "dsadsa"

[test deps]
Test = "fdsfds..."

in the Project file. These don't need to go to the registry.

staticfloat commented 6 years ago

It is better because I do not have a design for things like conditional requirements right now.

There is a point within a project's design/development stage at which it becomes counterproductive to argue over how things should work, and it should simply be decided how they will work. If we're at the stage where we just need to get this thing out the door and discussions like this aren't helping anything, fine. We can obviously live with that.

Yes, in my mind this is as easy as adding an extra field into a TOML file, something like osx_only_deps = ["Homebrew"], then doing a:

@static if is_osx()
    deps = [d for d in deps if !(d in osx_only_deps)]
end

I realize it's certainly not that easy. But the ease of something doesn't change the fact that we needed platform-only packages before, and we won't have them when we move to Pkg3, so we just need to make sure what we do have is reasonable. As an example, I don't think we have any Pkg2 packages that "disable" themselves on install when they're run on the wrong platform. Example: BinaryProvider.jl build.jl files are setup to throw an error() if that happens, which breaks any dependent package. This is by design, as build.jl files should, well, build their packages, and if they can't, they should let you know.

What would you suggest build.jl files do? Silently fail the Pkg.build("Foo"), then error out at using Foo time? Right now the "wisdom" that is embedded within the julia package ecosystem is to try and use the results of deps/build.jl, and if that fails, complain to the user that "Pkg.build("Foo") failed, so please run it again". We could say "just don't ever run using Foo on a system where Bar depends on Foo, but only on OSX"?

StefanKarpinski commented 6 years ago

Are we just going to start slapping together longer and longer names for every feature this thing might need? That seems unsustainable.

KristofferC commented 6 years ago

For some prior art, Cargo has:

[target.'cfg(windows)'.dependencies]
winhttp = "0.4.0"

which is kinda ugly but still.

Also,

[build-dependencies]
gcc = "0.3"

@StefanKarpinski Could you say what the main difficulty is? From an implementation point of view, it doesn't seem to bad so is it just figuring out how they should be listed in the TOML files?

StefanKarpinski commented 6 years ago

The main difficulty is coming up with a coherent approach that covers all of the different features we need without turning into a slapdash, ad hoc mess – which is a fairly common problem in package managers, as you may have observed. Here are features we need:

  1. Platform-specific dependencies: only needed on some platforms, not allowed on others
  2. Test-only dependencies: only needed for testing, must be compatible with main versions
  3. Build-only dependencies: only needed for build, may be incompatible with main versions
  4. Doc-only dependencies: only needed for doc generation, may be incompatible with main versions
  5. Dependency alternatives: https://github.com/JuliaLang/Juleps/issues/37
  6. Exclusive dependencies: "works with A or B but not both"
  7. Package options: https://github.com/JuliaLang/Juleps/issues/38

Note that all of this is deeply entangled in compatibility since it's about what combinations of packages and their versions are considered to be complete and satisfactory. So if anyone has a design that make all of this work in a clean, elegant way, I'm all ears. I'm not saying it's impossible – and maybe not even that hard, but it's non-trivial to satisfy all of this without creating a mess.

KristofferC commented 6 years ago

This is my suggestion:

Example Project.toml file

desc = "The next-generation Julia package manager."
keywords = ["package", "management"]
license = "MIT"
name = "Pkg3"
uuid = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"

[deps]
Dates.uuid = "ade2ca70-3891-5945-98fb-dc099432e06a"
Dates.targets = ["osx, "linux"]
Dates.compat = v"0.4"

UUIDs.uuid =  "ade2ca70-3891-5945-98fb-dc099432e06a"
UUIDs.targets = ["linux"]
UUIDs.compat = v"0.5.4"

[build-deps]
WinRPM.uuid = "ade2ca70-3891-5945-98fb-dc099432e06a"
WinRPM.targets = ["windows"]
WinRPM.compat = v"1.0"

[test-deps]
Test.uuid = "ade2ca70-3891-5945-98fb-dc099432e06a"
Test.compat = v"1.0"

Code loading:

Code loading needs to do the following:

Pkg3

martinholters commented 6 years ago

Not directly relevant to this issue, but you mention the XYZ.compat field. Do we really want/need that? If the compatibility information is only in the registry, then one can adjust wrong compat info after the fact without having contradictory information in registry and package. (Surely there is another issue for this discussion which I've missed, so feel free to point me there instead of abusing this issue any further.)

KristofferC commented 6 years ago

You need the compatibility in the package itself when you add a package that is not in the registry or you want to track a branch / commit. You also need it when you are developing a package since you might want to bump the compatibility and see that everything still works.

martinholters commented 6 years ago

I see. Thanks!

StefanKarpinski commented 6 years ago

Thanks, @KristofferC, it's helpful to have a mockup to discuss. More "typical" TOML format would be:

[deps.Dates]
uuid = "ade2ca70-3891-5945-98fb-dc099432e06a"
targets = ["osx, "linux"]
compat = "0.4"

[deps.UUIDs]
uuid =  "ade2ca70-3891-5945-98fb-dc099432e06a"
targets = ["linux"]
compat = "0.5.4"

[build-deps.WinRPM]
uuid = "ade2ca70-3891-5945-98fb-dc099432e06a"
targets = ["windows"]
compat = "1.0"

[test-deps.Test]
uuid = "ade2ca70-3891-5945-98fb-dc099432e06a"
compat = "1.0"

Problems I can see with this: the loading code, which only cares about UUIDs needs to know about and look for lots of different sections: deps.*.uuid, build-deps.*.uuid and test-deps.*.uuid. If we add a new kind of thing, e.g. doc-deps.*.uuid then the base loading code won't understand it – unless we do some kind of wildcard matching and recognize *-deps.*.uuid as a known dependency.

I had mocked up a somewhat different format:

[deps.uuids]
FFTW = “2a18a299-e228-47a1-aaa3-7cb1dce3c4ea”
JuliaFFT = “f9fe2544-2353-4bd3-aadb-a5340bdf8347”
FFTPACK = “2cefbc4b-7862-4a30-8499-7fbd1921393a”
MKL = “8a874d2c-4584-4ec5-94a3-27612bf41082”
Homebrew = “0844c15f-d7cd-49a7-8f6e-9898a4e84c8a”
WinRPM = “2bb7024a-558b-4a06-b258-244ddf9a61f1”

[deps.alternatives]
FFT = [“FFTW”, “JuliaFFT”, “FFTPACK”, “MKL”]
Installer = [“Homebrew”, “WinRPM”]

[deps.targets]
build = [“Clang”, “BinDeps”]
doc   = [“Documenter”]
test  = [“Test”, “FactCheck”]

[deps.systems]
macos   = [“Homebrew”]
windows = [“WinRPM”]

This moves the top-level section currently called deps to deps.uuids (which is more accurate since it's it gives the UUIDs of all of the dependencies). This is the only section that the Base loading code needs to know about.

The deps.alternatives section gives named sets of alternative packages. The resolver knows that only one of these needs to be resolved and installed to satisfy the project's requirements. Giving these names makes it possible for other sections to refer to them by these names (which cannot clash with other package names), so you could have something like this:

[deps.alternatives]
GCCLike = ["Clang", "GCC"]
Compiler = ["GCCLike", "MSVC"]

[deps.targets]
build = ["Compiler"]

[deps.systems]
linux   = ["GCCLike"]
macos   = ["Clang"]
windows = ["MSVC"]

This means that the build target depends on one of Clang, GCC or MSVC. This is quite different than what writing build = ["Clang", "GCC", "MSCV"] would mean – that would mean that it requires all three of these packages, whereas writing build = ["Compiler"] means that it requires only one of them. The interaction in this example with the deps.system section is interesting: the system section chooses which of the Compiler alternatives to use on each system: on Linux, build depends on a GCC-like compiler, i.e. Clang or GCC; on MacOS, build depends on Clang; on Windows, build depends on MSVC.

The deps.targets section gives different sets of action-specific dependencies (maybe it should be called deps.actions?) for things like building the project, testing, generating the documentation, etc. The semantics here are that any name that isn't mentioned in any target is required by the implicit default target. To execute the test target requires the default requirements plus the test requirements. Anything on the RHS of a definition in deps.alternatives or deps.targets is excluded from the default requirements.

I considered including deps.systems in deps.targets but it has quite different semantics. In particular, the RHS values should not be excluded from the default requirements. Rather, they should be included in the default requirements if the LHS "matches" the current system. However, another way to look at this would be to say that they are excluded from the default requirements, but the requirements on a given system are default requirements plus whatever system targets apply to the current system plus any that apply to whatever action one is trying to run.

StefanKarpinski commented 6 years ago

Note: I'm leaving compatibility constraints out of this because I don't believe it belongs in this file.

KristofferC commented 6 years ago

Problems I can see with this: the loading code, which only cares about UUIDs needs to know about and look for lots of different sections: deps..uuid, build-deps..uuid and test-deps.*.uuid

I explicitly mentioned this: "Stuff under [build-deps] and [test-deps] will be run in a new temporary environment where these entries will exist in [deps]. This environment will be created by Pkg3. So nothing should change in the current code loading except how it finds the uuid (trivial modification of the regex in Base)."

This is what we do already by the way.

KristofferC commented 6 years ago

Note: I'm leaving compatibility constraints out of this because I don't believe it belongs in this file.

Having to create a completely new separate file just to add a teensy bit of extra information seems very wasteful to me.

KristofferC commented 6 years ago

Your mockup is more a struct of arrays kind of thinking. When I look at a dependency I want to see all its information, not look through the whole file and collect all the bits and pieces spread out into the various sections. It seems easier to keep all the information about a dependency close together.

StefanKarpinski commented 6 years ago

Having to create a temporary project environment on the fly for testing and building seems like a hack that we were forced to do because test dependencies are not currently explicit, rather than something that we want to continue doing in an ideal state of affairs.

StefanKarpinski commented 6 years ago

Keep in mind that my design solves the dependency alternatives problem as well as the other problems. How would you add alternative dependencies to your scheme?

I disagree that one actually wants to see all of the information about a package all at once in this file. Having a bunch of UUIDs scattered around all over the place seems like noise – I'd rather have them all in one place and then use meaningful names everywhere else. Case in point, in your arrangement, if some package is both a build dependency and a doc dependency, it needs two separate stanzas, each of which includes the UUID and all the other information about the package. Arguably worse than that is that those UUIDs could disagree – you could use different packages with the same name for building and docing the project. Pkg3 can support that but it seems like a pretty bad situation. The syntax would also allow for the same name to appear in deps and in build-deps with different UUIDs. That situation is not supportable even in Pkg3 since you need to generate a project file for building which chooses one of those meanings for the package name. Does the build-deps package override the deps package by the same name? This seems like it leads to big issues.

It's better to structurally enforce that a project has a single global namespace of top-level dependencies across all actions – if Foo means something in the main project code then it means the same thing in the test code and vice versa. Having the name-to-UUID map in a single place where repeating a key is a syntax error has that property and avoids repeating those UUIDs all over the place in the project file. The manifest file is a very different situation since it does not have a single global namespace of package names and is designed to avoid that assumption.

KristofferC commented 6 years ago

We fundamentally have to do a new resolution when we are running tests or building because we are suddenly in a completely new situation with new dependencies. This is very naturally done by creating a new environment and resolving that one.

Also, we cannot load build-only dependencies in the package when we are not building because these might be incompatible with the project itself. The code loading must with your suggestion, therefore, parse the whole file and figure out what is build dependencies and not allow those to load. And then we build we need to tell code loading that we are building and allow it to find packages at that point etc. It is much easier to take care of that business ourselves and have the code loading in base look for [deps] and that's it.

KristofferC commented 6 years ago

I think your proposal is good except that it needs to be easier for the code loading to figure out what is build-only and test-only dependencies and ignore those when it comes to loading packages until we make a new environment where those are "normal" dependencies (like a build-only dependency is during build time).

Also, I am not sure how to fit "alternatives" into the current resolver code.

StefanKarpinski commented 6 years ago

We fundamentally have to do a new resolution when we are running tests or building because we are suddenly in a completely new situation with new dependencies. This is very naturally done by creating a new environment and resolving that one.

This is currently true because the resolver can only resolve a dependency or not resolve it. The ideal behavior, however, is for the resolver to understand that default dependencies must be compatible with the entire environment while test/build/doc dependencies only need to compatible with what this package depends on. For testing a package our current approach is pretty broken – you really want to test a package with a set of dependency versions that are an extension of the versions that you actually use it with, which is currently not necessarily the case: additional dependencies for testing can cause totally different versions of the main dependencies to be resolved. For doc and build it's arguable that you don't care, but I suspect that treating all targets the same as test is best – teach the resolver to choose versions for targets that extend the versions chosen for the main project.

The point that my design allows test/build/doc-only dependencies to be identified by the main project is a good one, although I'm not sure that's actually a bad thing. After all, the project file is only half of the loading equation – it identifies the package but in order to actually load the package, you need a manifest to tell you which version of that package to use.

StefanKarpinski commented 6 years ago

Also, I am not sure how to fit "alternatives" into the current resolver code.

This why I'm looking into more general SAT solver stuff. We probably won't implement that soon, but we still need to plan for it.

KristofferC commented 6 years ago

it identifies the package but in order to actually load the package, you need a manifest to tell you which version of that package to use.

Ok, good point, we can just choose to not put things in the manifest that we do not want loadable. So let's do this then? :D Just gotta figure out how to represent this in the registry and how to represent the compatibility. What is your opinion on using one version number with semver as in the link I posted above, instead of version-ranges?

StefanKarpinski commented 6 years ago

Having thought about the separation of deps.targets and deps.systems some more, I think that they should be together after all. My thinking is that we can have an implicit main target which includes all of the unconditional dependencies of the package that are necessary everywhere for all uses no matter what. In addition to that there would be an implicit default target which is main plus whatever system-specific targets apply. That way you can talk about main & windows even on a non-Windows system and ask the package manager to resolve that for you. So when you build on a Linux system, your default requirements would be default = main & linux and the requirements for building would be default & build = main & linux & build.

kmsquire commented 6 years ago

Bump. I'm sure people are busy, I'm just wondering about the plans/status here?
For packages which are erroring out (e.g., WinRPM on OSX/Linux, Homebrew on WIndows/Linux), is it better to submit patches which allow them to be built (but not used) on non-native platforms? Maybe someone can post a list of specific subtasks here, so that others might be able to jump in?

StefanKarpinski commented 6 years ago

I'm still really reluctant to add this unnecessary, complex feature. I also don't really understand how it somehow become my responsibility to implement a feature I don't think we should have. Perhaps someone who actually thinks this is a good idea should implement it. My concerns include:

If someone does really feel like implementing this, the design I had in mind was:

[options.installer]
macos = "Homebrew"
windows = "WinRPM"
linux = "AptGet"

Then again, I don't really think it's a good idea and we can already make this work through the relatively simple measure of having each platform-specific package implementing a no-op deps/build.jl if they're built on the wrong platform.

omus commented 6 years ago

Even with Homebrew declaring itself macOS-only wouldn't packages still need to be aware that the package is OS specific so that things like:

using Homebrew  # running not on macOS

wouldn't break?

KristofferC commented 6 years ago

wouldn't packages still need to be aware that the package is OS specific so that things like:

They already have to. Such using has to be OS-guarded. The difference here is with the automatic build step that is run when a package is installed.

StefanKarpinski commented 6 years ago

I think the right way forward here is to make this work on the package side without any package manager features—which no one feels is impossible, some people just think it's suboptimal—and then see how that plays out and what the best approaches are. Once we learn how to do this, we can build package manager features to make those approaches easier and smoother.

simonbyrne commented 6 years ago

There are occasions where things need to interact with OS-specific features (e.g. RCall and JavaCall both use WinReg to access the registry). I have intentionally made WinReg possible to install on non-Windows machines, but it is a little silly that it gets installed at all.

simonbyrne commented 6 years ago

Perhaps a bit of guidance would be useful, e.g. for OS specific packages themselves: 1) deps/build.jl should not throw an error on non-compatible OS (my understanding is that this is still called at installation time: is that correct?) 2) the package should check OS at load time and throw an appropriate error. 3) any usage of the package should first call check the OS.

simonbyrne commented 6 years ago

e.g. what I think should happen should look like https://github.com/JuliaPackaging/Homebrew.jl/pull/239

(I'm not sure what to do about WinRPM, since it technically works on Windows and RPM-based linux distros)

Nosferican commented 6 years ago

Should this issue be closed now since the approach is not to have Pkg handle these OS specific issues?

StefanKarpinski commented 6 years ago

There's still some support I can imagine. For example, declaring that a package only supports/is required on some systems. Then anything that depends on it can know that it shouldn't bother installing the package on other systems even though it will still resolve a version that could be resolved.

KristofferC commented 5 years ago

I think we can close this for now and open a new targeted issue if the need arise.