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

Support [sources] section in Project.toml #3263

Closed
wants to merge 8 commits into from
Closed

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Nov 23, 2022

More details laid out in #492, but essentially:

  • Support a [sources] section in Project.toml for specifying relative path and repo locations for dependencies
  • Support a parent = "ParentName" entry in subpackage Project.toml in monorepos to allow inheriting [sources]

More details laid out in #492, but essentially:
  * Support a `[sources]` section in Project.toml for specifying relative path and repo locations for dependencies
  * Support a `parent = "ParentName"` entry in subpackage Project.toml in monorepos to allow inheriting `[sources]`
@quinnj
Copy link
Member Author

quinnj commented Dec 6, 2022

@KristofferC, @fredrikekre , or @StefanKarpinski: would one (or more) of you have time to review here? Or if you don't have any objections, I'll plan on merging in the next day or two? I expect there to be a little bit of follow up here, but I've locally moved 2 large monorepo projects over to use this and everything seems to be working smoothly (in addition to the tests added here).

@fredrikekre
Copy link
Member

I haven't looked at it in detail, but I don't like the parent thing (seems more useful for something like #1233).

Can you elaborate on what a source means? I assume it is just a backup if Pkg can't find it anywhere else?

@quinnj
Copy link
Member Author

quinnj commented Dec 6, 2022

I haven't looked at it in detail, but I don't like the parent thing (seems more useful for something like #1233).

Yes, it's essentially part of what would be required for #1233 and was coincidentally proposed by @KristofferC as he and I discussed the problems of a monorepo. Being able to specify a parent "project name" by a subproject allows the subproject to "inherit" from the parent. In the limited scope of this PR, that means inheriting [sources] of the parent, which allows the subproject to fully instantiate, test, resolve when it has other subproject dependencies.

Can you elaborate on what a source means? I assume it is just a backup if Pkg can't find it anywhere else?

Yeah, source basically corresponds to the source information we have in Manifest entries, either a local path for "devved" packages, or a repo url + rev (and optionally subdir) for "checked out" packages. This just allows us to encode this information in the Project.toml instead of only having that information available in a Manifest.toml, which is helpful for non-public packages or applications where you have a mix of public or monorepo-style subproject dependencies.

In spirit, it seems "right" to me because it allows specifying the information needed to instantiate a project without needing a checked-in Mainfest necessarily (Manifests are obviously still useful for exact reproduction of an env), but I think it's useful because from the Project.toml, we can clearly identify that a certain Project has non-traditional dependencies (checked out or otherwise).

@KristofferC
Copy link
Member

I think this PR could use more discussion about what the problem is and what it tries to solve over just describing the functionality it implements. That makes it easier to discuss if the particular feature here is the best way of solving the actual problem.

For a bit more background, @quinnj and I discussed various solutions to the issue where you have a monorepo that contains many packages and activate one of those packages and want to do some development. The annoying part is that you have to dev all the dependencies of the packages since Pkg cannot find them by name. So IIUC, that is what this PR is trying to solve.

One of my (now slightly modified) suggestions to this issue (which I personally liked) was to be able to give a set of folders (that contains locally developed packages) that could be used as a kind of implicit registry. Right now, if you do add Foo and Foo is not in a registry, Pkg will error. But the process could instead be:

  • add Foo
  • Foo is in no registry
  • Find Foo among the "implicit registries" (just folders with packages).
  • If a Foo is found, grab the path to that and use it.
  • Record the path to Foo in the manifest, similar to if you had deved it.

That would allow solving the monorepo problem by just adding the monorepo to the set of implicit registries without changing anything else.

@quinnj
Copy link
Member Author

quinnj commented Dec 12, 2022

I think this PR could use more discussion about what the problem is and what it tries to solve over just describing the functionality it implements. That makes it easier to discuss if the particular feature here is the best way of solving the actual problem.

Thanks, yes, I should have provided a bit more context to what was motivating the changes here.

Also for context, I basically provided an updated implementation of the now-stale PR here, which seemed to have broad support and consensus that it was a good idea. I.e. allowing the [sources] section in a Project.toml to specify unregistered, local, or specific url-rev dependencies for a project.

With that in mind, the real net-new functionality proposed here that hasn't really been discussed/agreed upon is the parent = "[parent project name]". An alternative to specifying parent is that each subpackage in a monorepo could just duplicate the [sources] entries that it would need to specify where the other local peer dependencies live within the monorepo, but it seems convenient, and makes moving a subpackage with lots of dependents much easier, to just specify the subpackage location once, and allow subpackages to "inherit" the knowledge of that location if needed. Specifying parent = "ParentProject" seems like a pretty easy way to allow subpackages to do that inheriting.

@KristofferC, do you feel like you have specific concerns with the [sources] functionality or the parent piece? Or concerns around the actual implementation?

@quinnj
Copy link
Member Author

quinnj commented Dec 15, 2022

@KristofferC, following some more on latter part of your response: how exactly do you imagine this "implicit registry" to work? Like, where do we specify it? In a monorepo Project.toml? Or is it just something I have to do manually via ] registry add ...? And if that's the case, then this is a new kind of registry where it doesn't have the normal Compat.toml/Deps.toml files, but is just a directory of packages?

I can see the appeal of your hypothetical walk-though, I'm just not clear on how the implementation looks.

And as another push for [sources], I like that it's self-contained in the Project.toml; i.e. all I need is the Project.toml and I can instantiate my full monorepo project, even w/ internal, unregistered package dependencies, or specific commit dependencies on public packages, etc.

I'm open to alternative solutions here; I just want to do a slight push-back since the old [sources] PR seemed to be pretty much a consensus on "yes we want this, just need to finish it" (which is why I started there), and then just added the parent idea which seems like a very easy way to allow a subpackage to also "hook in" to a parent's additional context (i.e. sources).

@StefanKarpinski
Copy link
Member

The part I'm confused about here is the project file in the monorepo root. Does the monorepo itself have to be a project/package for this scheme to work? That seems weird to me; while I can see a case where there's an umbrella package that binds a bunch of packages in a monorepo together, it seems like it would also be common for the root to just be a directory. Are you just using the root having a Project.toml file as a way to have a place to put shared project content?

@quinnj
Copy link
Member Author

quinnj commented Dec 15, 2022

The part I'm confused about here is the project file in the monorepo root. Does the monorepo itself have to be a project/package for this scheme to work? That seems weird to me; while I can see a case where there's an umbrella package that binds a bunch of packages in a monorepo together, it seems like it would also be common for the root to just be a directory. Are you just using the root having a Project.toml file as a way to have a place to put shared project content?

In the two cases I have use for this, they are application projects, so yes, the root is a full-fledged Project.toml for the application and within that application/monorepo directory, there are internal packages at a specific subdirectory. I can see why the overall PR here doesn't make as much sense if we're not considering an application monorepo.

The alternative use of a monorepo (correct me if I'm misunderstanding here), would be if we had a local (or network-accessible) directory that had a bunch of unregistered/private packages, and then N application-like projects that all wanted to reference those private packages in the shared directory? In that case, I think this solution also is in the right direction, where individual application Project.toml's could use the [sources] to provide a path to the monorepo private package directory. The parent piece doesn't quite work though, because there wouldn't be a parent Project to look for. I see two potential solutions there:

  • The private package could also use the [sources] to reference unregistered peer dependencies; of course, we kind of want to avoid this so we don't have to keep duplicating private path info all over the place
  • Allow specifying something like a Sources.toml file at the top-level of the monorepo directory that is like a Project.toml, but just contains the [sources] entries. That way, private packages could still specify parent, but we would walk up until we found the parent Project.toml name OR a Sources.toml file that it would use to find peer location information.

I feel like that wouldn't be too hard of a follow up PR to allow that use-case.

@quinnj
Copy link
Member Author

quinnj commented Jan 3, 2023

Ok, to merge? I'm eager to push forward on this.

@StefanKarpinski
Copy link
Member

I really don't feel like this design has reached a consensus. Would be good to have a call with the interested parties to hash it out. At least you, me and @KristofferC, I'd say and anyone else who wants to join.

@MilesCranmer
Copy link
Member

What is the status of this? It looks like there hasn't been activity on this PR since early January.

This seems to be the most desired feature in Pkg.jl: https://github.com/JuliaLang/Pkg.jl/issues?q=is%3Aissue+sort%3Areactions-%2B1-desc+ so would be awesome to have.

@goerz
Copy link

goerz commented Oct 11, 2023

If there is no consensus on the parent part, can we at least split this and merge the [sources] part?

The underlying issue #492 has been open for 5 years and is the biggest pain point in Pkg by far, and one of the biggest pain points of Julia overall (at least to me)

@MilesCranmer
Copy link
Member

@StefanKarpinski, regarding this:

I really don't feel like this design has reached a consensus. Would be good to have a call with the interested parties to hash it out. At least you, me and @KristofferC, I'd say and anyone else who wants to join.

What was the outcome of this call? Is this PR good as-is or was an alternative discussed?

@quinnj
Copy link
Member Author

quinnj commented Jan 1, 2024

@StefanKarpinski, regarding this:

I really don't feel like this design has reached a consensus. Would be good to have a call with the interested parties to hash it out. At least you, me and @KristofferC, I'd say and anyone else who wants to join.

What was the outcome of this call? Is this PR good as-is or was an alternative discussed?

Yeah, sorry for the slow responses here. We did a call and essentially found that there was an alternative route for monorepo support that was already mostly supported by Base loading code and only needed a minor enhancement in Pkg (the solution is that Project.toml files are allowed to specify a specific manifest file to use, with multiple subprojects being able to "share" a single root-level manifest).

I still agree that having a [sources] section would be helpful. I think the biggest requirement beyond cleaning up this PR (and removing the [parent] stuff) would be modifying JuliaRegistrator-related packages to ensure that no package with a [sources] section is allowed to register. i.e. it's meant for local applcations/development scenarios, but publicly registered packages still have the requirement that all dependencies are also required to be publicly registered.

@NHDaly
Copy link
Member

NHDaly commented Jan 3, 2024

To add more specific details, the alternative approach that Jacob describes above is to set the manifest = field in the .toml file.

For example, our codebase is split into several sub-packages under the packages/* directory, so e.g. packages/RAI_Common/Project.toml starts like this now:

name = "RAI_Common"
uuid = "<uuid>"
authors = ["<authors>"]
manifest = "../../Manifest.toml"
version = "0.1.0"

and by pointing to the root-level manifest, any pkg changes to this sub-package get reflected in the root level manifest.

This approach seems to be working for us for now. Thanks again to @quinnj, @StefanKarpinski and @fredrikekre for getting this working for us.

@goerz
Copy link

goerz commented Jan 3, 2024

That solves the problem for monorepos, but it does not solve the problem that working on multiple interdependent unregistered packages is borderline impossible. The simplest example is two unregistered packages A and B, where B depends on A, and the tests or documentation of A depend on B.

So I would emphasize that having [sources] in a Project.toml should still be a pretty high priority. I also agree that registered packages should not be using [sources], but "modifying JuliaRegistrator-related packages" is a little bit worrying, as it makes the resolution of this issue dependent on a diffuse set of stakeholders. Can we make a list of which Julia-Registrator packages exactly would need to be modified?

@KristofferC
Copy link
Member

Just a short drive by comment, I do think this is still useful and we should discuss more thoroughly the use case and pain points different people have and come up with something that hopefully works to improve the situation.

@tpgillam
Copy link

tpgillam commented Jan 13, 2024

Something like this would be very useful for us -- we have a (growing) Julia monorepo, and are feeling a lot of the same pains as already discussed here.

@quinnj; will this always install local packages as "develop" dependencies? Or will it take a copy of the files and put under ~/.julia/packages? 'develop' would certainly be the most useful behaviour as far as I'm concerned, but potentially there is an ambiguity here because ] add /path/to/Package will work if there is a git repo initialised at /path/to/Package/.

Looking outside of Julia, poetry (see --editable) stores a flag in the pyproject.toml to specify whether a package is "develop"ed rather than "add"ed .


On the 'parent' logic, does this imply that directories above the packages in the monorepo are themselves packages?

For example, we have a repository roughly like so:

packages/A/
packages/B/
packages/...
projects/research_1
projects/research_2
projects/...
...<other non-Julia stuff>...

where each subdirectory of packages/ is a package (with interdependencies), and each subdirectory of projects is a "project" - i.e. where we do want to persist the Manifest - that can depend on anything under packages/, but naturally not on each other.

In this case neither packages/ nor the repository root is a Julia environment, so there are no Project.toml files here. And I can't quite figure out what it would mean conceptually for e.g. packages/ to be a package.


To add more specific details, the alternative approach that Jacob describes above is to set the manifest = field in the .toml file.

Ah that's good to know! I've played around with it a bit, but unfortunately it doesn't seem to work nicely with our setup (especially the "projects"). Or is it requiring features that aren't available in Julia 1.10?

@nickrobinson251
Copy link
Contributor

nickrobinson251 commented Jan 13, 2024

@tpgillam, in case it's helpful, the RAI mono repo has a single top-level package that depends on the packages in packages/ (and some of those depend on other packages in packages/)

src/
test/
Project.toml
Manifest.toml
packages/
other-julia-stuff/
other-non-julia-stuff/

and then packages/SubPackage1.jl/Project.toml would look like:

name = "SubPackage1"
uuid = "..."
version = "0.1.0" # <- we never change the version, since we just always use the latest code
manifest ="../../Manifest.toml"  # <- point to top-level manifest

[deps]
SubPackage2 = "..."

@tpgillam
Copy link

Thanks @nickrobinson251 ! This is very useful.

I've played around a bit more and also discovered the following:

  1. the 'top level' Project.toml must depend on all internal packages.
  2. if using a project, and you ] dev ../../packages/SomePackage, then it will find the local dependencies via the top-level Manifest.toml (even when it has its own project-specific Manifest.toml).
  3. The top-level Project.toml only needs a [deps] section, everything else can be removed
  4. Top-level environment doesn't require src or test directories

2 above is very nice, and this was the thing I (incorrectly) had thought wouldn't work.
3 & 4 means that the top level doesn't need to be a 'package' in the strict sense (i.e. no name or UUID), which feels cleaner from my perspective.

@MilesCranmer
Copy link
Member

Does anybody know if there's a package for emulating this type of behavior? It's frustrating to work around this when editing co-dependent packages not in the same monorepo.

@KristofferC
Copy link
Member

Let's continue the [sources] discussion in #3783. This PR had two things in it, the monorepo thing + the source thing which made the discussion a bit spread out.

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.

9 participants