-
-
Notifications
You must be signed in to change notification settings - Fork 269
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
Feature request: scan subdirs for updated versions/projects #1874
Comments
I'm trying to work around this issue by modifying my
Sadly, it's not working. Any thoughts? I could go back to adding a Here's the stacktrace: ERROR: MethodError: no method matching getindex(::Nothing, ::String)
Stacktrace:
[1] #build_versions#47(::Bool, ::Function, ::Pkg.Types.Context, ::Array{Base.UUID,1}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Pkg/src/Operations.jl:1044
[2] build_versions at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Pkg/src/Operations.jl:1033 [inlined]
[3] #add_or_develop#62(::Array{Base.UUID,1}, ::Symbol, ::Function, ::Pkg.Types.Context, ::Array{Pkg.Types.PackageSpec,1}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Pkg/src/Operations.jl:1204
[4] #add_or_develop at ./none:0 [inlined]
[5] #add_or_develop#15(::Symbol, ::Bool, ::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}, ::Function, ::Pkg.Types.Context, ::Array{Pkg.Types.PackageSpec,1}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Pkg/src/API.jl:69
[6] (::getfield(Pkg.API, Symbol("#kw##add_or_develop")))(::NamedTuple{(:mode,),Tuple{Symbol}}, ::typeof(Pkg.API.add_or_develop), ::Pkg.Types.Context, ::Array{Pkg.Types.PackageSpec,1}) at ./none:0
[7] do_develop!(::Dict{Symbol,Any}, ::Array{Pkg.Types.PackageSpec,1}, ::Dict{Symbol,Any}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Pkg/src/REPLMode.jl:714
[8] #invokelatest#1(::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}, ::Function, ::Any, ::Any, ::Vararg{Any,N} where N) at ./essentials.jl:697
[9] invokelatest(::Any, ::Any, ::Vararg{Any,N} where N) at ./essentials.jl:696
[10] do_cmd!(::Pkg.REPLMode.PkgCommand, ::Pkg.REPLMode.MiniREPL) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Pkg/src/REPLMode.jl:603
[11] #do_cmd#33(::Bool, ::Function, ::Pkg.REPLMode.MiniREPL, ::String) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Pkg/src/REPLMode.jl:577
[12] (::getfield(Pkg.REPLMode, Symbol("#kw##do_cmd")))(::NamedTuple{(:do_rethrow,),Tuple{Bool}}, ::typeof(Pkg.REPLMode.do_cmd), ::Pkg.REPLMode.MiniREPL, ::String) at ./none:0
[13] top-level scope at none:0 |
The code to repro this seems to not work 100%? It gave me a Project.toml that looked like
I guess it should be something like
Couldn't you check in a manifest in |
Yes, that looks borked. Strange, it worked perfectly for me. I just tested again, and again it was fine. The Manifest solution seems to work, many thanks for suggesting what should have been obvious! I have actively avoided them for various reasons (worries about testing stale rather than latest versions of pkgs, concerns about how well they generalize across Julia versions, etc), but this does sound like an excellent workaround. I think we still might consider not making an Manifest.toml obligate, but for the time being this is a simple solution. |
Is it possible to add multiple |
There's no way to do that currently, and I'm not sure we want to go down that path. I could turn out to be wrong but it feels like one of those annoyances that may sort itself out over time if we suffer through the discomfort for a while and figure out other ways to deal with it. If you're running code against multiple different Julia versions, isn't it better to not check the manifest in and let the resolver pick versions that are compatible with your Julia version? |
Currently that's the only way I know of, other than adding a |
You could |
I am confused a bit. This is similar to timholy/SnoopCompile.jl@bb56316 and my whole issue on it #1828. If running this script is a bad thing we should not do it at all (including in CI). If it is a good thing why we don't do it by default. |
No, the |
Both suggestions that I made were are also meant for the installation phase of a package. |
No they would be executed after installation. The build script runs after all packages have been resolved. You should not run Pkg operations in the build script and timholy/SnoopCompile.jl@bb56316 is invalid. |
Worth mentioning that trying to do this from the |
That is probably the Powershell issue I gave a solution for here. |
I have no objection to running it before doing any Pkg operation.
Now, I need to dev these packages everywhere that I want to test a developing SnoopCompile branch. |
No, this is only because those packages are not registered yet. When they are you only have to |
I must say that even with this dedicated issue I have a bit of a hard time wrapping my head about what needs to be done and what
actually means. This is literally what happens when a package is developed and you run |
(@v1.6) pkg> st SnoopCompile SnoopCompileCore SnoopCompileAnalysis SnoopCompileBot
Status `~/.julia/environments/v1.6/Project.toml`
[aa65fe97] SnoopCompile v1.6.0
[9ea4277c] SnoopCompileAnalysis v1.6.0
[1d5e0e55] SnoopCompileBot v1.6.0
[e2b509da] SnoopCompileCore v1.6.0
(@v1.6) pkg> dev SnoopCompile
Path `/home/tim/.julia/dev/SnoopCompile` exists and looks like the correct package. Using existing path.
Resolving package versions...
ERROR: Unsatisfiable requirements detected for package SnoopCompileBot [1d5e0e55]:
SnoopCompileBot [1d5e0e55] log:
├─possible versions are: 1.6.0 or uninstalled
└─restricted to versions 1.6.1-1.6 by SnoopCompile [aa65fe97] — no versions left
└─SnoopCompile [aa65fe97] log:
├─possible versions are: 1.6.1 or uninstalled
└─SnoopCompile [aa65fe97] is fixed to version 1.6.1
(@v1.6) pkg> resolve
No Changes to `~/.julia/environments/v1.6/Project.toml`
No Changes to `~/.julia/environments/v1.6/Manifest.toml`
(@v1.6) pkg> dev SnoopCompile
Path `/home/tim/.julia/dev/SnoopCompile` exists and looks like the correct package. Using existing path.
Resolving package versions...
ERROR: Unsatisfiable requirements detected for package SnoopCompileBot [1d5e0e55]:
SnoopCompileBot [1d5e0e55] log:
├─possible versions are: 1.6.0 or uninstalled
└─restricted to versions 1.6.1-1.6 by SnoopCompile [aa65fe97] — no versions left
└─SnoopCompile [aa65fe97] log:
├─possible versions are: 1.6.1 or uninstalled
└─SnoopCompile [aa65fe97] is fixed to version 1.6.1
(@v1.6) pkg> dev SnoopCompile SnoopCompileCore SnoopCompileAnalysis SnoopCompileBot
[ Info: Resolving package identifier `SnoopCompileCore` as a directory at `~/.julia/dev/SnoopCompile/SnoopCompileCore`.
[ Info: Resolving package identifier `SnoopCompileAnalysis` as a directory at `~/.julia/dev/SnoopCompile/SnoopCompileAnalysis`.
[ Info: Resolving package identifier `SnoopCompileBot` as a directory at `~/.julia/dev/SnoopCompile/SnoopCompileBot`.
Path `/home/tim/.julia/dev/SnoopCompile` exists and looks like the correct package. Using existing path.
Path `SnoopCompileCore` exists and looks like the correct package. Using existing path.
Path `SnoopCompileAnalysis` exists and looks like the correct package. Using existing path.
Path `SnoopCompileBot` exists and looks like the correct package. Using existing path.
Resolving package versions...
Updating `~/.julia/environments/v1.6/Project.toml`
[aa65fe97] ~ SnoopCompile v1.6.0 ⇒ v1.6.1 `~/.julia/dev/SnoopCompile`
[9ea4277c] ~ SnoopCompileAnalysis v1.6.0 ⇒ v1.6.1 `../../dev/SnoopCompile/SnoopCompileAnalysis`
[1d5e0e55] ~ SnoopCompileBot v1.6.0 ⇒ v1.6.1 `../../dev/SnoopCompile/SnoopCompileBot`
[e2b509da] ~ SnoopCompileCore v1.6.0 ⇒ v1.6.1 `../../dev/SnoopCompile/SnoopCompileCore`
Updating `~/.julia/environments/v1.6/Manifest.toml`
[aa65fe97] ~ SnoopCompile v1.6.0 ⇒ v1.6.1 `~/.julia/dev/SnoopCompile`
[9ea4277c] ~ SnoopCompileAnalysis v1.6.0 ⇒ v1.6.1 `../../dev/SnoopCompile/SnoopCompileAnalysis`
[1d5e0e55] ~ SnoopCompileBot v1.6.0 ⇒ v1.6.1 `../../dev/SnoopCompile/SnoopCompileBot`
[e2b509da] ~ SnoopCompileCore v1.6.0 ⇒ v1.6.1 `../../dev/SnoopCompile/SnoopCompileCore` I'm not sure how it would be implemented, but the above is not ideal. |
Am I understand it wrongly, or a - JULIA_DEPOT_PATH=/tmp/pkgs julia-latest --startup-file=no --project -e 'using Pkg; Pkg.test("BigPkg", coverage=false)'
+ JULIA_DEPOT_PATH=/tmp/pkgs julia-latest --startup-file=no --project -e 'using Pkg; Pkg.update(); Pkg.test("BigPkg", coverage=false)'
|
What's the not ideal part? That you have to dev them in one command? |
My understanding is that when you Take
|
Yes. I made a dev.jl for my personal use to do that. |
Just to point out, there is no concept (right now at least) of a sub-package. There are just packages and they can now be stored in the same git-repo. It is not clear at all to me that automatically devving all packages that are in the same repo just because you dev one of them is something you would always want.
Would be better to do all the devs in one command ( |
Deciding specifically what is wrong is definitely a gray zone. Let me try in several steps:
It seems to me that we need some way of saying, "this collection of packages belongs together." Maybe something like a new
section to the "umbrella" But I'm open to other solutions. I definitely don't want to create src-chaos if there's an easier way to smooth the workflow. |
I strongly agree with you there. This should be something you opt into. I'm not certain how flexible this needs to be? Should it be "all one or none"? Or would people want to group some packages but not others? E.g.,
|
It should be stated that the more that |
Warning noted. But again, in the case I really care about, I've already done the cutting up (all the way back in the Julia 0.x for small x days). I want to glue them back together. |
Does this mean How about package tests? What would happen when |
This was a too long read to go into all details, so please excuse me if I've missed or misunderstood something important. I think the layout
where
where the top level (I have no personal need of this. My interest in subdir packages has always been on the case where the Julia package is only a smaller part of a repo.) |
I think I disagree with you here, @KristofferC & @timholy. If you create a multi-package repo in a single .git repository, I think this sort of makes sense.
|
I like the simplicity of this file layout, but this might not be a practical way to declare the dependencies for Subpackages with MORE dependenciesIf I were to add another requirement: it would be nice if certain subpackages could also have more dependencies than the root package. For example:
In the future, it might make sense to bundle a GUI to aid in using SnoopCompile. If we include it as a sub-project with additional dependencies, it means we don't have to add Gtk.jl to all Julia environments that want to use SnoopCompile. Well, maybe a GUI for SnoopCompile wouldn't really be appropriate, but I hope you get the idea. |
Before answering, I would like to rename What I like about this topology is that
I really like this idea. The only problem is that it is slightly less intuitive than @timholy 's original proposal where |
I intentionally chose a non-package name for the root of the repo because
|
I'm not sure I understand. Yes, I think it is a tooling problem: the package/registration system lacks the ability to deal with multiple packages in the same .git repo in a way that is practical for development, testing, and keeping dependencies to a minimum. The subpackage concept is simply an abstraction we use to describe our problem/solution. To me, this feels alot like the one file per function paradigm in Matlab. Yes, you can just bite your tongue and break everything into singe files, but you tend to get lost in your files, and development time suffers. Actually, it is worse than the one file per function issue, because it causes long delays in creating releases. Even the part of "push" ing each individual .git repo/julia package to Github is a pain when your solution is broken down into so many repositories.
Yes. A package was originally defined at the .git repo level. So yes, when you add that package, Julia pulls the entire repo. I assume most people know this. The decision to tie packages to a .git repo seems to be the core reason why there is a need for the sub-package concept. And I agree, this might require a relatively major redesign of Pkg. If it can be done in an effective way by adding support for subdirectories, as well as a few registration scripts, then that would be excellent! Sadly, I'm not convinced that's all that we need to make this aspect of package development user-friendly. |
Well, that's where we disagree. I don't think Pkg needs (more) changes to effectively support multiple packages in one repository. I just think you shouldn't try to place a package at the root of the repository if it includes multiple packages. |
Yeah, when I first suggested this feature, I did not imagine that one package would be in a subdirectory of another. |
If "effectively" is the operative word here, I think the difficulty @timholy is having to get SnoopCompile working in a multi-package context says otherwise. Maybe Pkg doesn't need more changes, but something probably does. I just don't know what that thing is at this time.
I have no strong opinion on this. In this context, I'm completely fine with there not being a package at the root of my repositories. This restriction doesn't really pose an obstacle for my particular use cases. In fact, it might end up being less confusing to add this restriction. |
The simplest package development workflow is to have one big package for all related functions. As @ma-laforge said, developing one big repo makes version control less a headache, especially when the change involves multiple packages. A gripe is that this giant package makes precompilation time a big issue for both developers and users. I wanted to echo @timholy that the concept of sub-packages, IMO, gives us the best of two worlds that 1) it's easier to make a change to multiple sub-packages without propagating incompleteness to downstream packages 2) it doesn't explode the precompilation time. |
FWIW, I am very much losing the overview of this issue, what is proposed, what is considered bug reports, what is considered feature requests, what is just random discussion from the SnoopCompile split etc. I'm going to close this, so please open a new issue with a concrete descriptive proposal. |
Let's go back to the beginning and apply the layout from #1874 (comment) to the reproducer. Then the
Performing the same steps as in the issue description, I get
@timholy, is this enough to solve this part of the puzzle? |
With apologies to @timholy, when this got to "relatively major redesign of Pkg" I think we lost the thread here. Let's try again with some more focused issues. If folks want to continue the larger brainstorming here, feel free. |
I apologize as well. When I said "this might require a relatively major redesign of Pkg", I actually meant I expect the changes won't be a couple of non-trivial one liners. Dealing with multi-package repositories in a way that is easy for development, unit testing, and publication/registration requires a bit of thought. This is especially true if we want to maintain minimum sub-package dependencies to avoid pulling in more packages than a project requires. However, for the most part, I think the package system does an excellent job dealing with very complex issues. |
This issue has come up again on Slack and the fact that I "hated it" was mentioned as the reason for abandoning the idea. To be clear: I would be totally fine with a feature that allowed
The proposals being discussed here violate both of those and introduces a subtly different new concept—so subtle that from the user's perspective it's identical to a submodule, it only different from the developer's perspective, which seems not great, Bob. It also introduces a new syntax which, so that the user has to guess which of |
The reason for the new syntax is only to be backward compatible with the current semantics of |
If the package opts into it, then changing the behavior of |
Yeah, that's probably a better version of opting into it than the caller. |
There are several counter-arguments to the need for this feature in the first place:
Again: this does not mean that I reject the problem, just some shapes of solutions. Introducing a package namespace concept is slightly more palatable since it's a fairly clear concept, as opposed to subpackages, which seems a bit contradictory: "a package is a unit of reusable code"; "a subpackage is an even smaller unit of reusable code". From the namespace perspective, if we see |
I wouldn't get too stuck on the particular names I picked for everything. To me, it doesn't matter if it is called a "subpackage", a package I don't see how it overlaps with the package namespace proposal at all. That is only about how one can disambiguate the |
The overlap is that they both introduce |
Sorry. I didn't notice you replied. Package namespaces. Yes. That sounds great to me. I don't really need the subpackage concept. I understand that might end up causing subtle issues. That was not the intent. Need:
|
Summary
For repositories that have more than one package on them, and some of the packages depend on others, CI runs into trouble because
julia --project
picks the "umbrella" Project.toml files which sets a new version just for that one package. If the umbrella package depends on new versions of the subdirectory packages, the resolver doesn't know that it can get them from the current PR.Current workaround
If you're running CI with GitHub Actions, add something like
Travis users should add something similar.
Reproducer
Following on the lengthy discussion in #1251, I thought since all the people who actually know Pkg internals were confused about what I was asking, I would create something that is close to an actual test case (and therefore may assist in developing the requested feature). The executive summary here is that Pkg does not yet support updating its awareness of sub-package versions based on a new pull request.
Setup: create a blank directory to do this test in, I chose
/tmp/pkgs
. Save the script below as/tmp/pkgs/generate_test.jl
, make sure you'vecd /tmp/pkgs
, launch Julia asJULIA_DEPOT_PATH=/tmp/pkgs julia --startup-file=no
(I was using Julia 1.6 just to get the latest) and theninclude
the script. Here's the script:Then I navigate to
dev
and do this:tim@diva:/tmp/pkgs/dev$ JULIA_DEPOT_PATH=/tmp/pkgs JL_PKG=BigPkg julia-master --startup-file=no -e 'using Pkg; Pkg.build(verbose=true)'
For some reason, executing the tests as in the default travis script doesn't seem to work for me, but I can do this:
So, the obvious problem is that
SubPkg
is a dependency ofBigPkg
, and this new version ofBigPkg
requires[email protected]
, but[email protected]
has not been registered yet.In #1251, @fredrikekre correctly points out that this is no different from how it works normally. But when you have long dependency chains, how it works normally is really painful. When you have a single git repo, you would like to be able to leverage that organization to develop all the packages in concert. To the developer, the most sensible unit is the repo, not the package.
What do we need? I think the only thing missing is some command that will update the session's notion of what package versions are available by scanning the Project.toml files of all packages listed in the
[deps]
section ofBigPkg
. If it does that, and then discovers that in the checked-out copy, lo and behold there is a[email protected]
, then it can proceed and try everything together.The text was updated successfully, but these errors were encountered: