Skip to content
This repository has been archived by the owner on Jun 25, 2024. It is now read-only.

Move lib into a separate repository #7

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

tcmal
Copy link

@tcmal tcmal commented May 13, 2024

This PR moves lib out of this repository, and into a separate one here. This is meant to make maintenance easier by allowing us to develop the two separately.

It also removes the dependency on nixpkgs, and some usages of maintainers/teams that were stopping evaluation. Probably these should come from the community repo instead.

@tcmal
Copy link
Author

tcmal commented May 13, 2024

I think at this point it would also be helpful to run nixfmt on the whole tree and start enforcing that, but I guess there's not really a good time to do that.

@vlinkz
Copy link
Member

vlinkz commented May 13, 2024

I noticed that now https://github.com/auxolotl/lib has nixpkgs as an input, and as such nixpkgs is present within core's flake.lock file. Once we switch over to using core as the source for nix, this would create a circular dependency. Would it be possible to have a separate flake for tests? Or perhaps resolve the dependence via this: flake.md.

@tcmal
Copy link
Author

tcmal commented May 13, 2024

I think we could also have the lib flake export a function that takes a *pkgs instance and returns a derivation (but this makes running the tests a little weirder). Not sure which is best.

Either way, will need to hold off on this until we have another way of getting nixfmt and nixdoc, which are both used in the lib flake.

@isabelroses
Copy link
Member

and some usages of maintainers/teams that were stopping evaluation. Probably these should come from the community repo instead.

As I mentioned here #2 (comment), that is the plan. But lets save that for a later PR.

@isabelroses
Copy link
Member

isabelroses commented May 13, 2024

Hopefully you can remove the upstream decency when #8 is merged. Then we can do inputs.auxlib.inputs.core.follows = self; or something like that?

@Sorixelle
Copy link
Member

I'm not entirely convinced this is a good idea. What's the use case for taking lib out of core? The two are going to be very closely intertwined with each other (especially if lib is going to do anything relating to stringing together derivations a la callPackage), and putting it in another repo means it's one more place we have to manage issues and PRs - ideally, we'd like to keep that as low as possible while still giving SIGs the autonomy they need. Considering Core would likely be the ones maintaining lib, I'm not sure I see a pressing need to split them out at the moment. One might argue that it allows lib to be consumed independently of everything else, but if it's going to depend on Core anyway, that's kind of a moot point.

@tcmal
Copy link
Author

tcmal commented May 14, 2024

putting it in another repo means it's one more place we have to manage issues and PRs

I generally find the same amount of issues across multiple repos easier to handle than all in one repo. Synchronising changes is obviously harder, but this would be an issue even if lib was here, so long as lib is consumed by sig repos.

especially if lib is going to do anything relating to stringing together derivations a la callPackage

The set of functions in there right now is currently just the same as nixpkgs - there might be things we don't want to provide or need to rethink. If there are library functions that are actually dependent on the package set (ie trivial builders) then I agree they probably should be in here.

My understanding is that callPackage deals with dependency injection and overrides, which we probably need to rethink for a multi-repo structure regardless. However we deal with it, this is a slightly different concern than packaging imo so it makes sense for it to not be here.

One might argue that it allows lib to be consumed independently of everything else, but if it's going to depend on Core anyway, that's kind of a moot point.

Right now this is basically a limitation of flakes, since core is only really a dev dependency. Having thought about it more, I think lib should probably have a different flake inside it for docs and tests, unless there's some better way to handle this I'm not aware of (possibly just doing tests outside of flakes?).

tcmal added 3 commits May 14, 2024 14:17
this is pending figuring out the system we want to use for
maintainers/teams, and keeps everything evaluating in the meantime.
@vlinkz
Copy link
Member

vlinkz commented May 14, 2024

Right now this is basically a limitation of flakes, since core is only really a dev dependency. Having thought about it more, I think lib should probably have a different flake inside it for docs and tests, unless there's some better way to handle this I'm not aware of (possibly just doing tests outside of flakes?)

What about storing the tests in a separate branch with its own flake that depends on lib and core?

@tcmal
Copy link
Author

tcmal commented May 14, 2024

Sure, or tests and docs` could just be in a subdirectory. Probably best to open an issue in auxolotl/lib

@Sorixelle
Copy link
Member

Sorixelle commented May 15, 2024

I generally find the same amount of issues across multiple repos easier to handle than all in one repo.

Keeping repo count down isn't just for maintainers - having an additional repo to track issues in adds additional confusion to users when they go to eg. report an issue ("is this a bug in Core or lib?"). There's also the extra overhead of being another repo to checkout, with the possibility of grabbing multiple different copies of it depending on the state of lockfiles. I'm not sure I'm seeing much benefit to outweigh the downsides here.

What about storing the tests in a separate branch with its own flake that depends on lib and core?

I'm weary of doing this - I'm not a fan of branches in one repo having completely divergent histories, it doesn't lend well to code discovery (since users have to be aware the branch exists to know that code is even there). Plus, it becomes impossible to update the tests and the actual code at the same time in one PR. It's also an issue that just wouldn't exist if lib was a part of the Core repo, which IMO furthers the case for not splitting.

@tcmal
Copy link
Author

tcmal commented May 15, 2024

having an additional repo to track issues in adds additional confusion to users when they go to eg. report an issue ("is this a bug in Core or lib?")

To me they seem pretty easily separated, but I guess I would think that lol.

I'm weary of doing this - I'm not a fan of branches in one repo having completely divergent histories, it doesn't lend well to code discovery

Agreed.

I think right now we don't have consensus on what exactly core is meant to be, so I think we should come back to this during/after the upcoming meeting.

@jeff-hykin
Copy link

jeff-hykin commented May 15, 2024

I'm not entirely convinced this is a good idea. What's the use case for taking lib out of core?

I think it will be helpful on the maintaince side.

  • Tests for lib will be trivial to run because they're system independent, people can contribute/iterate faster when code isnt system dependent. Some people who contribute to lib might never touch core.
  • Even just being able to look at the git history of lib independent of core would likely be helpful. When debugging complicated bugs it would be nice to "walk back" core changes without also walking back lib changes. (Or vice versa)
  • In terms of accepting contributions from random strangers, I often jumped into package definitions but didn't know where any of the arguments were coming from, including lib. Trying to figure out where that argument was coming from and if there had been any modifications to it was hard. But if a package definition simply had import <auxlib> and I could go to the repo it was defined in, that would me much more approachable in terms of figuring out what code is actually doing.
  • I think it will be easier to combine lib and core later, but the reverse isn't necessarily true. So if we change our mind later its no big deal so long as they're split now.
  • IMO Lib should have different PR standards, like documentation for each helper with examples, while helpers inside of core don't necessarily need the same degree of documentation/examples and formatting.

Even outside of maintaince though:

  • IMO lib should be pure, no system, no derivations. Keeping it in a separate repo helps for flakes that are trying to stay simple, which is why there already is an isolated nixpkgs.lib flake. I've got two standard JS libraries, one for Javascript code that runs everywhere (browser, node, deno, bun) and another that only runs on deno. Having that split has been great in terms of a mental model and keeping changes organized. The aux lib could be the same way.
  • Lib is the standard lib (I at least I think we agree on that), so its going to contain helpers that core doesn't even use for bootstrapping. Core is almost certainly going to be complicated, and I think we should keep it as small and focused on bootstrapping as possible. Lib being external is one way to minimize the complexity inside of core.
  • I have often wanted to learn part of nixpkgs, step by step, but its always just one big tangled ball. Plenty of people might not care to learn nixpkgs core since its going to have a lot of complexity and system-specific caveats, but lib would be a great place to start, especially since its filled with tools they could probably use for many nix tasks.
  • For minimal applications and bundling I think people should be able to import a pinned version of lib without importing core. Some flakes are just a bunch of nix helpers, not a derivation. I think one day we might see nix bundling (like js bundling) as well, and it will be a lot easier to tree-shake lib than it will be to tree shake a bootstrapping core codebase.
  • Even though I'm not a huge fan of submodules, as a compromise I would be okay with core having lib as a submodule if thats more acceptable. I don't think debugging of core would need changes in lib through, which I think is the normal usecase of submodules

@jeff-hykin
Copy link

I'm not a fan of branches in one repo having completely divergent histories

I have tried this a number of times for myself thinking it was a good idea. I've had a pretty bad time every time.

The only time it was a mediocre experience was when it was an "inherited" model, with one base branch and the others just always auto-merging that base branch into themselves. But even then it was a difficult mental model needing to not make a change on a branch, go to base, make the change, then back to the feature branch, and merge base. I very frequently forgot and had to redo commits on base.

Very much not an experience I would want to try with a team of people.

@jeff-hykin
Copy link

jeff-hykin commented May 15, 2024

one more place we have to manage issues and PRs

For issues, I agree @Sorixelle one place is nice. I had problems reporting "nix" issues and them telling me "no that's a nixpkgs issue" and I didnt understand the difference at the time. I think we should consider disabling issues on both lib and core, or locking them to contributors only. Then all issues can be reported through either an aux or auxpkgs repo. For lib and core, if its only contributors making issues, I think the separation of lib and core would be helpful.

For PR's I think having it as an additional place for PR's is very much a good thing. A core PR and a lib PR should have different standards and CI tests.

@jeff-hykin
Copy link

jeff-hykin commented May 15, 2024

I think right now we don't have consensus on what exactly core is meant to be,

I'll throw my 2 cents on this here since I probably can't meet.

I think core is a useful separation for keeping messy bootstrapping isolated, for keeping a small mental model for onboarding new contributors trying to understand aux, for staying small to prevent security issues, and for ensuring we don't accidentally bloat the dependencies of the nix/aux cli (e.g. we shouldn't rope in nodejs just to get semver parsing). To keep it as small as possible, and to have an objective definition to avoid hurt feelings, I'm in favor of it being defined as the minimum packages necessary to build nix CLI. With the one additional job of making those packages (for building nix CLI) look as similar as possible across platforms (distinguishing platform-specific core from platform-agnostic core).

I say this specifically in contrast to the subjective idea that core is "commonly used build tools for all packages". Which I think, in the long run, will be the cause of many debates and hurt feelings if someone's favorite library isn't considered "core". It also isn't necessarily a helpful "soft" defintion for keeping the messy bootstrapping code isolated, since some commonly used stuff doesn't have messy bootstrapping (e.g. it would be just as easy to maintain a commonly-used package outside of core). If you guys end up wanting this kind of "broad" core definition despite this, I would at least advocate for an objective definition, like "is a dependency of at least 40% of packages", to avoid hurt-feelings and subjective debates later.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants