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

Add workspace feature #3841

Merged
merged 41 commits into from
Mar 25, 2024
Merged

Add workspace feature #3841

merged 41 commits into from
Mar 25, 2024

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Mar 12, 2024

Edit: The description below is not 100% accurate to the final implementation.

This PR requires JuliaLang/julia#53653.

This allows one to define a subproject which is an incremental addition to the dependency graph of the "base project". When the sub project is active it can load any dependency from the base project and the sub project itself. When Pkg resolves, it resolves among the base project and all sub projects and the full dependency graph is stored in a single manifest next to the base project's manifest.

Subprojects are defined by specifying a list of folders in the project file as:

subprojects = ["test", "benchmarks"]

Right now, the base project has to be one level above any sub project.

TODO:

  • More thorough tests especially regarding merging of compats and sources among projects.
  • Docs
  • Hook up Pkg.status output
  • Test nested subprojects
  • Show Pkg prompt as Pkg/test> if test is a subproject to Pkg.

As a follow-up to this I want to make it so that the test subproject is special and that running Pkg.test if this is active should be equivalent to:

Pkg.activate("test")
Pkg.resolve()
include("test/runtests.jl")

Other follow up is to perhaps enhance Pkg.status() to show more clearly what packages comes from the base project and what comes from the sub project.

@KristofferC
Copy link
Member Author

I should test how instantiate works with sub projects.

src/Types.jl Outdated Show resolved Hide resolved
test/subprojects.jl Outdated Show resolved Hide resolved
@KristofferC
Copy link
Member Author

It would be nice if dev Foo in a subproject would just work in case it is already devved in the base project's manifest.

@jakobnissen
Copy link
Contributor

jakobnissen commented Mar 13, 2024

Some feedback:

  • In a project where the main project has Automa added with compat "1", and the sub-project has Automa added with compat "=1.0.2" (not the newest version), when showing (mypkg) pkg> status, it falsely displays that Automa can be upgraded:
⌃ [67c07d97] Automa v1.0.2

This also occurs the other way around: If the parent project has a restrictive dependency but the subproject does not, the subproject's version is held back, but the green ^ still appears in the subproject's status, suggesting the package can be updated.

  • When the subproject depends on BenchmarkTools but the parent does not, BenchmarkTools shows up in pkg> status of the parent project. However, it's not part of the project, nor can it be loaded. This might be a bug

@KristofferC
Copy link
Member Author

KristofferC commented Mar 13, 2024

it falsely displays that Automa can be upgraded:

Thanks, we need to collect compat for all projects in

compat_spec = get_compat(env.project, pkg.name)
.

When the subproject depends on BenchmarkTools but the parent does not, BenchmarkTools shows up in pkg> status of the parent project.

Yeah, the status output hasn't really been hooked up to this yet. Right now it shows everything merged among all projects.

@KristofferC
Copy link
Member Author

@jakobnissen, both your issues should be solved now. There is also an update on the julia side for the corresponding PR.

@KristofferC
Copy link
Member Author

Should check that manifest = works when specified in the root base project.

@Roger-luo
Copy link

First, I'd like to thank the huge effort behind this!

I believe at some point on slack, someone proposed subprojects to be a way resolving the local module loading. More specifically, I like this to make mono-packages with multiple subpackages easier. So, for example, in our package Yao, we have YaoBase, YaoBlocks, etc. as submodules for Yao, and

  • We have to separate these submodules into packages due to concerns about loading, precompilation, etc. So, a parallel precompile can happen.
  • These small packages are closely related (you can tell from the name). Thus, the versions are often bumped together and tested; thus, they live in a mono-repo.

So, what I'd like to have for a subproject is not only allowing loading dependencies from the base project but also allowing the base project to depend on several subprojects. This way, I can have a single package Yao with subprojects named Yao.Base and Yao.Blocks, which are loaded locally within the project by adding extra dependencies from each subproject. Users do not need to search for YaoBase, but they can directly depend on submodules without loading the entire package. I believe this would also clean up the registry a bit and make the offline experience better. This "lazy" loaded subprojects should also improve the loading times for some big packages without registering new packages in the registery.

But again I don't think this feature is in conflict with this PR. Just want to bring this up, maybe we can chat a bit more during JuliaCon.

@oxinabox
Copy link
Contributor

To confirm I am understanding correctly.
the core feature that the resolver would always be restricted to using packages from parent/siblings/nephew/etc packages in the mono-repo (so you couldn't accidentally get an independent version of Foo.jl within Foo/Bar.jl).
Which is advantage over putting stuff like relative paths in the Manifest, or fiddling with LOAD_PATH.

@KristofferC
Copy link
Member Author

Which is advantage over putting stuff like relative paths in the Manifest, or fiddling with LOAD_PATH.

I'll say some things that would work with this:

No need for TestEnv.jl

Instead, make test/Project.toml a subproject of the package, and add the test-specific deps to the test-project. Pkg.test is now just julia --project=test runtests.jl, no need for a temporary manifest etc.
Also, the [targets] is now not special for test, you can do the same thing for docs or benchmarks.

Relative paths in Manifest.toml.

I'm not sure what this means. Sure, you can dev things with your "root" project active but as soon as you activate one of the packages in the monorepo and do a resolve you are going to have to dev everything unregistered there again.
These might have different versions etc then what you have in your root project.

LOAD_PATH pushing

Packages are not resolved together in the different environments in the load path so you might load incompatible versions here (if your root project has a required dependency version that is different from what that same dependency resolved to in the other environment). You also need to mutate this global state at the top of the script file if you want to mutate LOAD_PATH which is a bit inelegant.

@KristofferC KristofferC requested a review from a team as a code owner March 15, 2024 20:59
@oxinabox
Copy link
Contributor

To be clear my comment was in agreement that those were good things, for the reasons you listed.
My question was: am i correct in understanding that the way subprojects work is that it makes sure parent/siblings/nephew/etc packages in the mono-repo are always what is resolved to

@KristofferC
Copy link
Member Author

am i correct in understanding that the way subprojects work is that it makes sure parent/siblings/nephew/etc packages in the mono-repo are always what is resolved to

Every project in the "workspace" will resolve together and generate a single manifest, yes.

@KristofferC
Copy link
Member Author

  • Should write packages in workspace to manifest
  • Also write extensions for package to manifest

@KristofferC
Copy link
Member Author

KristofferC commented Mar 18, 2024

(test) pkg> dev ..
   Resolving package versions...
  No Changes to `~/JuliaPkgs/Pkg.jl/test/Project.toml`
    Updating `~/JuliaPkgs/Pkg.jl/Manifest.toml`
  [0dad84c5] + ArgTools v1.1.2
  [56f22d72] + Artifacts v1.11.0
  [f43a241f] + Downloads v1.6.0
  [7b1f6079] + FileWatching v1.11.0
  [b27032c2] + LibCURL v0.6.4
  [76f85450] + LibGit2 v1.11.0

Missing showing that Pkg got added to the project file for some reason.

@KristofferC
Copy link
Member Author

(MonoRepo/PrivatePackage/test) pkg> st
Status `~/MonoRepo/PrivatePackage/test/Project.toml`
  [a23bb17a] PrivatePackage v0.1.0 `PrivatePackage` # this is now relative to manifest
  [8dfed614] Test v1.11.0

should print paths relative to project when doing a st and it is showing direct deps

@KristofferC
Copy link
Member Author

I'm seeing some (I think) spurious warnings like:

Warning The project dependencies or compat requirements have changed since the manifest was last resolved. It is recommended to `Pkg.resolve()` or consider `Pkg.update()` if necessary.

@KristofferC KristofferC changed the title Add "subproject" feature Add workspace feature Mar 20, 2024
@lassepe
Copy link
Contributor

lassepe commented Mar 25, 2024

I'm a bit confused about the workflow here, in particular in the context of test/ being a subproject:

From the discussion above, I understand that one has to specify test/ as a subproject in the parent Project.toml. Would that mean that, when instantiating the parent, it gets resolved together with all the test dependencies (even when not testing). If that is true, couldn't it lead to confusing downgrades of packages due to test dependencies?

@KristofferC
Copy link
Member Author

If that is true, couldn't it lead to confusing downgrades of packages due to test dependencies?

I don't know if they are confusing or not. But being able to run the tests with the same versions as you actually run your code seems like a feature to me. Of course, you are not forced to put test into the workspace.

@lassepe
Copy link
Contributor

lassepe commented Mar 25, 2024

I fully agree that having julia --project=test/ runtests.jl "just work" would be amazing. I am just a little bit surprised that the "parent project" would have to know about the subprojects at all. I would have thought that the test/Project.toml may include a pointer to the parent (maybe via the new [source] feature) to make julia --project=test/ runtests.jl work without affecting julia --project .

That said, the fact that the proposed design has worked well for Rust is probably a good indicator that the idea is sound and I don't want to add noise to this discussion. So please feel free to ignore my feedback if you don't share these concerns.

@KristofferC
Copy link
Member Author

I fully agree that having julia --project=test/ runtests.jl "just work" would be amazing. I am just a little bit surprised that the "parent project" would have to know about the subprojects at all.

There is another possible (somewhat orthogonal) feature that could be explored (which could probably use the name "subproject") which is a project whose dependencies are the union of those listed in its own project and that of its "parent" (which is specified in the project file of the subproject).

I am not fully clear exactly what the rules should be for resolving etc for that.

@KristofferC KristofferC merged commit 162634c into master Mar 25, 2024
9 checks passed
@KristofferC KristofferC deleted the kc/subprojects branch March 25, 2024 20:15

This structure is particularly beneficial for developers using a monorepo approach, where a large number of unregistered packages may be involved. It's also useful for adding documentation or benchmarks to a package by including additional dependencies beyond those of the package itself.

Workspace can be nested, that ism a project that itself defines a workspace can also be part of another workspace.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this wonderful feature, just pointing out the typo ism above!

KristofferC added a commit that referenced this pull request May 9, 2024
This feature allows one to define a "workspace" in a project file by adding

```toml
[workspace]
projects = ["packageA", "docs", "test"]
```

to it. All projects in a workspace are resolved together into a single manifest file in the "root" project (the project that is not included by any other project in a workspace). The compat of all dependencies are intersected among the projects.

Currently, every project in a workspace must be in a folder at the same level as the project that includes it into the workspace.

If the test folder is included in the workspace, running `Pkg.test` is similar to just activating the test project and including the `runtest.jl` file.

Many functions now take the `workspace` argument to decide if the function should operate on the active project or on the full workspace:
- `Pkg.precompile`
- `Pkg.status`
- `Pkg.why`
- `Pkg.instantiate`
KristofferC added a commit that referenced this pull request Jul 5, 2024
This feature allows one to define a "workspace" in a project file by adding

```toml
[workspace]
projects = ["packageA", "docs", "test"]
```

to it. All projects in a workspace are resolved together into a single manifest file in the "root" project (the project that is not included by any other project in a workspace). The compat of all dependencies are intersected among the projects.

Currently, every project in a workspace must be in a folder at the same level as the project that includes it into the workspace.

If the test folder is included in the workspace, running `Pkg.test` is similar to just activating the test project and including the `runtest.jl` file.

Many functions now take the `workspace` argument to decide if the function should operate on the active project or on the full workspace:
- `Pkg.precompile`
- `Pkg.status`
- `Pkg.why`
- `Pkg.instantiate`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants