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

feedback on code loading docs #26910

Open
ssfrr opened this issue Apr 26, 2018 · 6 comments
Open

feedback on code loading docs #26910

ssfrr opened this issue Apr 26, 2018 · 6 comments
Labels
docs This change adds or pertains to documentation

Comments

@ssfrr
Copy link
Contributor

ssfrr commented Apr 26, 2018

I told @StefanKarpinski a bit ago that I'd review the new code loading docs. This isn't really an issue but I figured it was better to file it here rather than a big slack dump. Feel free to close/migrate at will.

So here's some feedback:

  • I like the App/Pub/Priv example, it's helpful to have a concrete example
  • In general this is really helpful to clarify the relationship between Environments and Depots, which were a little unclear to me before
  • "version control—i.e. a git repository" - I think you mean "e.g." here
  • why does the name symbol need to be part of the paths key? Are there cases where the same UUID would show up multiple times but with different names, each pointing to different entry points? (is this just to handle the nil UUID case?)
  • "A manifest file contains a stanza for each direct or indirect dependency of a project, including for each one, its UUID and exact version information and optionally an explicit path to its source code. Consider the following example manifest file for App" - for the private Priv package it looks like the version info is also optional, but this description makes it sound like the version is manditory.
  • In the App manifest file, how do we know which Pub and Zebra packages the private Priv package depends on? It seems like only their names but not their UUIDs are given. In this case it's unambiguous because there's only one package with each name, but if there were two different Zebra packages I don't see how we'd know which one Priv was referring to
  • in the materialized paths block the labels for which ones are in the user depot and the system depot are reversed
  • why are there different UUID mechanisms for the cases of no-project-file (nil uuid) and project-file-without-uuid (dummy hashed uuid)? Maybe this could be clarified in the Aardvark/Bobcat/Cobra example by mentioning why a given package might be in that state (e.g. why would a package ever have a Project.toml without a UUID?).
  • in the package directories section, I'm a little confused about the context in which roots/etc. are being constructed. Is this when running from the REPL with pwd() being the Package Directory, or when running a julia file in the same directory as the package directory, or something else?
@fredrikekre
Copy link
Member

FYI you can review merged PRs, would make it easier to understand the context in which your comments apply.

@ssfrr
Copy link
Contributor Author

ssfrr commented Apr 26, 2018

fair enough, I can see how this list is a bit unwieldy. I'm not expecting per-point back-and-forth though, so I figured it's reasonable to comment on the current state.

@StefanKarpinski LMK if this is a pain to walk through and I can migrate the notes to comments on the PRs some time in the next couple days (I haven't checked, but most of them probably belong to #26787)

@StefanKarpinski
Copy link
Member

Thanks for the feedback, @ssfrr. I actually like having it in an issue since it helps keep track of changes that need to be made—I can make a PR that closes this issue with all the things that need changing in response to it.

"version control—i.e. a git repository" - I think you mean "e.g." here

Well, currently it's both since git is the unique example. I went back and forth.

why does the name symbol need to be part of the paths key? is this just to handle the nil UUID case?

Yes, it's just for the nil UUID case, which is annoying but it's a pretty important case at least until we reach a point where we can rely on all packages having UUIDs—which I'm not actually sure we ever will since it's kind of nice to be able to gradually move from a package that's just a directory to one that's got a project file and a UUID and so on. Until then we need the name.

for the private Priv package it looks like the version info is also optional, but this description makes it sound like the version is manditory

By "exact version information" I actually meant the source tree hash, not the version number. The version number is redundant whereas the source tree hash is necessary. Perhaps this could be worded more clearly.

In the App manifest file, how do we know which Pub and Zebra packages the private Priv package depends on? It seems like only their names but not their UUIDs are given. In this case it's unambiguous because there's only one package with each name, but if there were two different Zebra packages I don't see how we'd know which one Priv was referring to

The rule is that the deps entry in a manifest stanza can be an array of names if all of the names occur at most once in the manifest file with the given UUID; otherwise the deps entry must be a dict mapping names to UUIDs. I didn't mention that here since this isn't really a doc for the project and manifest file formats, although we do need docs on that as well.

in the materialized paths block the labels for which ones are in the user depot and the system depot are reversed

Good catch, I used to have these reversed.

why are there different UUID mechanisms for the cases of no-project-file (nil uuid) and project-file-without-uuid (dummy hashed uuid)?

In the no project file case, you still want to be able to load dependencies and have things work the way they've worked in previous Julia versions, which means allowing the project to load top-level dependencies the same way you can in Main. The case of a project file with no UUID is not any different than having a UUID except that no one can depend on that project by its UUID since it doesn't have one. There is still a project file and you only want to allow the project to load explicitly declared dependencies in its [deps] section. The easiest way to accomplish this is to treat it like any other project file but with a "self-assigned" UUID.

in the package directories section, I'm a little confused about the context in which roots/etc. are being constructed.

Everything is relative to the package directory. Any ideas on where to clarify that?

@fredrikekre
Copy link
Member

Fixed by #27113?

@StefanKarpinski
Copy link
Member

Partly but I wasn’t able to clarify everything. It’s unclear if I should aim to address all in the text or just answer here.

@ssfrr
Copy link
Contributor Author

ssfrr commented May 26, 2018

Yeah for the most part those tweaks cleared up things for me. Thanks.

The rule is that the deps entry in a manifest stanza can be an array of names if all of the names occur at most once in the manifest file with the given UUID; otherwise the deps entry must be a dict mapping names to UUIDs. I didn't mention that here since this isn't really a doc for the project and manifest file formats, although we do need docs on that as well.

Might be worth a little note to this effect, most other features of the manifest and project files seemed pretty clearly laid out. Maybe something like:

"Notice that Pub and the private Priv both have dependencies listed, but Priv uses a more compact syntax because it doesn't need to specify the UUIDs. If only one package in the manifest file has a given name then it can be referred to by that name without the UUID."

I think for the other issues (clarifying the purpose of UUID-less project files and clarifying package directories) I don't have much more useful feedback right now. I probably just need to spend some more time playing with it and seeing how things shake out as folks start to transition.

@kshyatt kshyatt added the docs This change adds or pertains to documentation label May 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

No branches or pull requests

4 participants