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

Obtain inventory version from Project.toml #2442

Closed

Conversation

goerz
Copy link
Member

@goerz goerz commented Feb 3, 2024

This always obtains the version for the inventory from the Project.toml in any parent directory of the current working directory. For virtually all normal Julia projects, this should do the right thing.

If no Project.toml can be found, show a warning and use an empty string.

This does not add a new version/inventory_version argument to HTML(): In the rare situation where the default version is inappropriate, I would recommend manually patching the objects.inv file in docs/make.jl, between makedocs and deploydocs. I'll add something to DocInventories to make this kind of patching easier.

My preferred alternative to #2433 and #2443

@goerz
Copy link
Member Author

goerz commented Feb 3, 2024

@mortenpi How do you feel about this one?

I'm still working on an alternative PR that patches in the version in deploydocs, but testing that properly is turning out to be a bit time-consuming.

Also, even in the deploydocs alternative, it has to fall back to looking at the Project.toml in most cases (for all dev deployments). It'll be far from elegant. Any situation where the folder name / selection-menu-version for a tagged release doesn't match the version in Project.toml seems like it would only be by mistake anyway.

So why not just always just go to the Project.toml directly?

I completely agree with the sentiment that "there should only be one source" for a "version", but I'd say the Project.toml should be that source.

@mortenpi
Copy link
Member

mortenpi commented Feb 5, 2024

So why not just always just go to the Project.toml directly?

I don't think we should involve Project.toml at all.. versioning should be handled by deploydocs(*)

(*) If we'd be implementing Documenter from scratch, then that's a design decision that should definitely be questioned, but I really don't want to complicate the logic here.

I completely agree with the sentiment that "there should only be one source" for a "version", but I'd say the Project.toml should be that source.

Right, but then we should also be using that in deploydocs (somehow), which would be breaking.

So I am still very much partial to #2443 without the Project.toml part. But I think I'd like to loop in @odow and/or @fredrikekre here, to see what they think (to catch you up, there's more context in #2424).

@goerz
Copy link
Member Author

goerz commented Feb 5, 2024

I don't think we should involve Project.toml at all.. versioning should be handled by deploydocs

But deploydocs does not have any access to the X.Y.Z version for a dev-deployment. So how can we set the correct version for dev without looking it up in the Project.toml?

I just don't see it: deploydocs and the "versions-selection-menu" (despite how it's labeled in the UI) does not really deal with versions, but with git tags and branches (with aliases like dev for master/main). And the git tags are assumed to be of the form vX.Y.Z, the authoritative source of which already is the Project.toml: trigger a registration, registry bot makes a release based on Project.toml, Tagbot creates a tag based on the release, CI builds the docs and deploys to a folder corresponding to the tag, and then the versions-selection-menu shows name of the folder as "Version". There is no situation in a standard setup where the "version" in deploydocs would disagree with the version in the Project.toml (apart from the versions-selection-menu not showing what version dev actually is).

So I am still very much partial to #2443 without the Project.toml part. But I think I'd like to loop in @odow and/or @fredrikekre here, to see what they think (to catch you up, there's more context in #2424).

I'll be happy to implement whatever you like best, but I have two hard requirements:

  1. There has to be some way to set the version string for the inventory in the makedocs phase, either manually with inventory_version in HTML or by reading it from the Project.toml
  2. Any project with a standard setup should end up with some appropriate (non-empty) version in its inventory for both tagged releases and dev deployments. This should happen without them having to make any changes in their configuration after updating to the version of Documenter that will include the inventory writing.

@goerz
Copy link
Member Author

goerz commented Feb 5, 2024

But deploydocs does not have any access to the X.Y.Z version for a dev-deployment. So how can we set the correct version for dev without looking it up in the Project.toml?

Unless maybe you wanted to use the version from "latest" with +dev attached. I could come around to that.

@goerz
Copy link
Member Author

goerz commented Feb 5, 2024

Never mind, that doesn't work. The deploydocs doesn't have access to any version information, except the current version if (and only if) it's a tagged release.

I'd have look at which folders exist on the gh-pages branch (which is what HTMLWriter.expand_version does), but that only becomes available in git_push, which is called at the very end of deploy_docs, and after I'd have to patch the inventory.

So all I can do is reiterate: deploydocs has no access to any information about versions at all (beyond the current git tag), so it's not a good place to try to patch version information into the inventory.

@goerz
Copy link
Member Author

goerz commented Feb 6, 2024

Ugh. One complication: This doesn't work for Julia itself, which uses a VERSION file instead of a Project.toml. Of course, the inventory for the Julia project itself is one of the most important ones, so if we can make that work out of the box without them having to adapt their documentation build (apart from updating Documenter), that would be great.

I still think this is the way to go, but also checking for a VERSION file (and maybe JuliaProject.toml) in addition to Project.toml might be necessary.

So that is the one thing where Morten's preferred #2443 could have an edge.

Edit: fixed in latest commit (now also reads VERSION file)

This is required to set the inventory version for the Julia project
itself.

Also adds (implicit) support for `JuliaProject.toml`. This is not
documented, as it's understood that `JuliaProject.toml` is just an alias
for `Project.toml`, and not something we want to confuse people with too
much.
@goerz goerz force-pushed the mg/inventory-version-from-project-toml branch from 7e35e9e to f7f960a Compare February 7, 2024 00:58
If `deploydocs` is deploying for a tagged version, verify that that
version matches the `version` in the `objects.inv` inventory file. If
not, throw an error: The inventory `version` must never contradict what
the version-selection-menu shows.
@goerz
Copy link
Member Author

goerz commented Feb 11, 2024

I've updated this to make sure there can never be a mismatch between the version detected here and the version in deploydocs that shows up in the versions-selection-menu: If the foldername in deploydocs can be parsed into a valid version (any deployment for a tagged version), then throw an error if that version does not match the version in the inventory.

Comment on lines +271 to +278
objects_inv = joinpath(realpath(target), "objects.inv")
if isfile(objects_inv)
inventory_version = _get_inventory_version(objects_inv)
deploy_version = _get_deploy_version(deploy_subfolder)
if !isempty(deploy_version) && (inventory_version != deploy_version)
error("Inventory declares version `$inventory_version`, but `deploydocs` is for version `$deploy_version`")
end
end
Copy link
Member Author

Choose a reason for hiding this comment

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

@mortenpi This is the bit of code in deploydocs that would guarantee that there is never a mismatch between the automatic version extracted from the Project.toml and what is shown in the versions-selection menu for tagged releases.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you think the error is too harsh or might be considered semver-breaking, I'd also be okay making it an @error, and potentially even overwriting the inventory_version with the deploy_version. That would be extremely close to #2448, but the "fallback" would happen in makedocs instead of deploydocs, which I'd much prefer.

Copy link
Member Author

Choose a reason for hiding this comment

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

See #2449, which might be starting to become my preferred solution (since I actually agree with you that for a tagged release, the version that comes out of deploydocs is kinda authoritative).

@goerz
Copy link
Member Author

goerz commented Feb 21, 2024

To simplify the discussion, I'm going to close this in favor of #2449.

So the choice is really only between some version of #2443 and some version of #2449.

@goerz goerz closed this Feb 21, 2024
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.

2 participants