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

problem with building the Oscar documentation #547

Closed
ThomasBreuer opened this issue Jul 9, 2021 · 10 comments
Closed

problem with building the Oscar documentation #547

ThomasBreuer opened this issue Jul 9, 2021 · 10 comments
Assignees

Comments

@ThomasBreuer
Copy link
Member

Currently building the Oscar documentation causes problems.
After the call Pkg.activate(joinpath(Oscar.oscardir, "docs")) in Oscar.build_doc(), pkgdir(Oscar) gets a wrong value.
Namely, although the loaded Oscar is the develop version, pkgdir(Oscar) points to the released version in ~/.julia/packages.
(Is this a bug or a feature?)

pkgdir is used in docs/make_work.jl to specify source and destination when copying files from AA, Nemo, and Hecke.
For Oscar (the destination), we can use Oscar.oscardir instead, and in this case I can build the documentation.

But what about the source directories: If one has develop versions of the packages in question installed, and wants their documentation to be used instead of the files in ~/.julia/packages, what shall be used instead of pkgdir?

@fingolfin
Copy link
Member

I think this has been resolved?!? More or less... I hope sigh.

In the meantime, also doc/README.md should be improved. Building the docs this way "from scratch" doesn't work for me.

@benlorenz
Copy link
Member

benlorenz commented Aug 9, 2021

Running the new build_doc() still fails for me, it tries to add a released Oscar version in combination with AbstractAlgebra 0.21 which fails to resolve.
I don't see where the current Oscar dev version would be added to the environment which I would expect in one of those places:

Oscar.jl/src/Oscar.jl

Lines 127 to 132 in 99d6dbd

function doc_init()
Pkg.activate(joinpath(oscardir, "docs")) do
Pkg.instantiate()
Base.include(Main, joinpath(oscardir, "docs", "make_work.jl"))
end
end

Oscar.jl/src/Oscar.jl

Lines 153 to 161 in 99d6dbd

function build_doc()
if !isdefined(Main, :BuildDoc)
doc_init()
end
Pkg.activate(joinpath(oscardir, "docs")) do
Base.invokelatest(Main.BuildDoc.doit, Oscar, false, true)
end
open_doc()
end

It probably works for everyone who already has a Manifest.toml in docs/ referring to the correct Oscar directory.

I think it needs a Pkg.develop (or Pkg.add) as in the CI:

run: julia --project=docs/ -e 'using Pkg; Pkg.develop(PackageSpec(path=pwd())); Pkg.instantiate()'

@benlorenz
Copy link
Member

On second thought: can we just remove everything except Documenter* from that docs/Project.toml since after #550, make_work.jl uses the path from the loaded Oscar and doesn't load Oscar again?

@fingolfin
Copy link
Member

Yeah, perhaps that would work. I am on the road right now, but perhaps you can give it a try,

@fingolfin
Copy link
Member

ah, doctest will fail (see also issue #604 which we already discussed some time ago). Also, I am not sure about our CI job creating the docs, they may need to be adjusted, too. And it may need to keep at least the Oscar dependency in docs/Project.toml... and the Pkg.develop resp. Pkg.add you mentioned.

@benlorenz
Copy link
Member

It does load without the deps but fails the doctests as you expected. I will try to add the Pkg command.

@fingolfin
Copy link
Member

... Or we get away from the whole Oscar.build_doc() business to a more "standard" way of building the docs -- as long as it retains the important feature of being able to build the docs multiple times in a single session, using Revise, to reduce the doc edit-build cycle. I can look into that tonight, but not now, sorry

@benlorenz
Copy link
Member

I have done a rework to make build_doc more robust in #606

@micjoswig
Copy link
Member

I kind of stress-tested the build_doc() over the last few days. It works for me now. @ThomasBreuer : please check and close this ticket.

@ThomasBreuer
Copy link
Member Author

The problem I had originally reported had vanished in my local installation after #550 got merged. The current version works for me as well. Thus this issue can be closed.

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

No branches or pull requests

4 participants