JuliaDocs / LiveServer.jl

Simple development server with live-reload capability for Julia.
MIT License
143 stars 26 forks source link

Infinity loop with Literate #165

Closed kaipartmann closed 1 year ago

kaipartmann commented 1 year ago

When trying to modify the simple example created with LiveServer.servedocs_literate_example("littest"), I get an infinity-loop with LiveServer.

I want to use the simple folder structure suggested here in the documentation (https://docs.juliahub.com/LiveServer/CGL3l/1.2.0/man/ls+lit/) where the *.jl literate files and the *.md files are in the same directory.

docs
├── src
│  ├── pg1.jl
│  └── index.md
└── make.jl

Contents of pg1.jl:

# # Test literate
# We can include some code like so:
f(x) = x^5
f(1)

Contents of index.md:

# Test
A test

Contents of make.jl:

using Documenter, Literate
src = joinpath(@__DIR__, "src")
lit = src
for (root, _, files) in walkdir(lit), file in files
    splitext(file)[2] == ".jl" || continue
    ipath = joinpath(root, file)
    opath = splitdir(replace(ipath, lit => src))[1]
    Literate.markdown(ipath, opath)
end
makedocs(
    sitename = "littest",
    pages = [
        "Home" => "index.md",
        "Other page" => "pg1.md"
    ]
)
julia> versioninfo()
Julia Version 1.9.1
Commit 147bdf428cd (2023-06-07 08:27 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin22.4.0)
  CPU: 16 × Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.6 (ORCJIT, skylake)
  Threads: 1 on 16 virtual cores

(@v1.9) pkg> status
  [16fef848] LiveServer v1.2.0

I am grateful for any help!

tlienart commented 1 year ago

Hello!

tldr: use servedocs(literate="") (which is the convention as per the docstring when the literate files are in docs/src)

Explanation

(I'd appreciate a PR to the readme making some of this more explicit btw)

The docstring of servedocs indicates the literate KW:

https://github.com/tlienart/LiveServer.jl/blob/96c1e426a1a841298a76a3aed155d2dcec21c9db/src/utils.jl#L180-L185

but the reason why it should be indicated to LiveServer where the literate scripts are is explained in the code here:

https://github.com/tlienart/LiveServer.jl/blob/96c1e426a1a841298a76a3aed155d2dcec21c9db/src/utils.jl#L122-L127

the code below it does this (in short it assumes that *.jl files in the indicated directory lead to a generated *.md file in that same directory that can be ignored by LiveServer (otherwise an infinite loop as you indicated).

closing for now, I hope this helps!

kaipartmann commented 1 year ago

Thank you @tlienart for your detailed explanation!

I made some changes to README.md that explain this: https://github.com/kaipartmann/LiveServer.jl/blob/db95db74795e06332f79c86d0a2f397b32b53192/README.md#serve-docs-with-literatejl

However, while writing and checking this, I discovered that if the generated *.md files are in a subdirectory of src with a different directory name than in literate, an update on the make.jl file causes an $\infty$ loop.

Using servedocs with the literate keyword:

servedocs(literate=joinpath("docs","literate"))

Everything is fine:

docs
├── literate
│   └── generated
│       └── example.jl <-- Literate file
└── src
    ├── generated
    │   └── example.md <-- generated by Literate
    └── index.md

No loop if example.jl changes, but updates on make.jl (even just saving without changes) cause an $\infty$ loop:

docs
├── literate
│   └── example.jl     <-- Literate file
└── src
    ├── generated
    │   └── example.md <-- generated by Literate
    └── index.md

This folder structure could be helpful if someone wants to specify docs/src/generated within a .gitignore file, for example. The only way I get this running without a loop is by manually specifying the location of the generated *.md files:

servedocs(skip_dirs=[joinpath("docs","src","generated")])

How can this behavior be explained?

Could it be helpful to add tips on manually specifying the location of the *.md files to README.md?

tlienart commented 1 year ago

Yeah so LiveServer's servedocs needs two things:

  1. to know where the literate files are so it can watch them for changes a. if the literate files are somewhere under docs/src then this is the case by default (everything under docs/src is watched for changes) b. if the literate files are somewhere else, then there needs to be literate="path/to/literate/files" so these files are added to the watcher and modifying them triggers
  2. to know what files to skip because they've been generated and so should not re-trigger a make call

Several cases that you've encountered:

This could be cleaned up by providing an additional keyword literate_output_dir so that, in general with literate you'd do:

servedocs(
  literate_input_dir="parent/directory/of/literate/files",
  literate_output_dir="parent/directory/of/generated/md/files"
) 

where literate_input_dir would effectively be syntactic sugar for literate and literate_output_dir would extend skip_dirs; but it might still be preferable in that it's clearer what a user must do.

If you have an opinion on the above lmk, otherwise I'll just add this

kaipartmann commented 1 year ago

Your proposed syntax changes are a great idea and would make it easier to understand what must be done to use Literate properly together with servedocs. However, the new kwargs would somehow be duplicates of skip_dirs, skip_files, include_dirs, andinclude_files. If I understand correctly, you can also replicate the behavior of the current literate keyword because one can specify the two things servedocs needs to know with these four kwargs.

It could be worth considering just a slight change of the current literate keyword, e.g.:

`literate_dir=nothing`: Path to a directory containing literate scripts. If nothing,
                        it's assumed there are no literate scripts. Any `*.jl` file
                        in the folder (or subfolders) is checked for changes and is
                        assumed to generate a `*.md` file with the same name. To avoid
                        an ∞ loop, all `*.md` files in the `src` folder (and all
                        subfolders) with the same basename as one of the literate
                        `*.jl` files are ignored and not checked for changes.

Then, the literate_dir keyword would cover all cases except if the generated *.md files had a different filename than the *.jl files. But for this case, one could add an appropriate comment to the README to manually specify the generated files with the skip_* arguments.

The additional dir in the keyword argument name would clarify what should be specified. And only one keyword is required because the user does not have to indicate the location of the generated files if they have the same basename as the .jl files.

What do you think?

tlienart commented 1 year ago

Thanks for your suggestion, I'll open a PR with:

tlienart commented 1 year ago

closed by #167 + patch release + update to docs (link fixed in readme); @kaipartmann kindly have a look and lmk if that works for you, thanks!

GiggleLiu commented 1 year ago

Wield, no matter what I tried, the serve_docs function does not respect my skip_dir or skip_dirs argument and goes to infinite loops.

        skip_dirs=[
            joinpath("docs", "src", "generated"),
            joinpath("docs", "build"),
        ],

This is my make.jl literate build file

const EXAMPLE_DIR = pkgdir(TensorInference, "examples")
const LITERATE_GENERATED_DIR = pkgdir(TensorInference, "docs", "src", "generated")
for each in readdir(EXAMPLE_DIR)
    workdir = joinpath(LITERATE_GENERATED_DIR, each)
    cp(joinpath(EXAMPLE_DIR, each), workdir; force=true)
    input_file = joinpath(workdir, "main.jl")
    @info "building" input_file
    Literate.markdown(input_file, workdir; execute=true)
end
GiggleLiu commented 1 year ago

I have no idea why the following script still runs into infinite loops

    using LiveServer;
    LiveServer.servedocs(;
        doc_env=true,
        skip_dirs=[
            joinpath("docs"),    # skip everything in docs
        ]
    )
GiggleLiu commented 1 year ago

Maybe we should consider switch to #124 The current file watcher contains too many stateful operations, which is very difficult to help improve.

GiggleLiu commented 1 year ago

@tlienart You forgot to add abspath in one place. Please check this PR #168

tlienart commented 1 year ago

@GiggleLiu I don't think abspath has anything to do here but I'll investigate the scenario you discuss above.

In the meantime, it would be helpful if you could reproduce exactly the scenario below and let me know what happens please:

  1. add LiveServer#master
  2. using LiveServer; LiveServer.servedocs_literate_example(); cd("servedocs_literate_example/")
  3. servedocs(literate_dir=joinpath("docs", "literate")), navigate to localhost:8000/man/pg1/ in a browser
  4. modify the fourth line of docs/literate/pg1.jl to e.g. f(10)

the changes should be reflected in your browser without any looping; I just re-did this and it should be fine. If not, please open a separate issue quoting the Julia version, OS and confirming you did exactly the above.

Note that 1.2.1 hasn't been released yet (even though it's been tagged) so assuming you had added LiveServer the standard way, the PR #167 above did not affect you and the problem you have encountered is elsewhere.

tlienart commented 1 year ago

Ok I've worked a bit with your repo, here are steps which will make things work, I'll think a bit about how to potentially make what you initially had work:

make.jl basically remove generated

const EXAMPLE_DIR = pkgdir(TensorInference, "examples")
const LITERATE_GENERATED_DIR = pkgdir(TensorInference, "docs", "src")

for (root, _, files) in walkdir(EXAMPLE_DIR)
    for file in files
        endswith(file, ".jl") || continue
        ipath = joinpath(root, file)
        opath = splitdir(replace(ipath, EXAMPLE_DIR => LITERATE_GENERATED_DIR))[1]
        Literate.markdown(ipath, opath)
    end
end

# ...
    pages=[
        "Home" => "index.md",
        "Examples" => [
            "Asia network" => "asia/main.md",

then call

servedocs(literate_dir=pkgdir(TensorInference, "examples"))

that should work as expected, if you edit examples/asia/main.jl the changes will be reflected without triggering an infinite loop.

allowing a different hierarchy in src for the generated file

for the moment it's assumed that if your literate file is in [root]/some/path/foo.jl that the generated content will go to src/some/path/foo.md.

We can relax this by adding (yet another) keyword indicating what folder under src should be looked at so that the generated content can be assumed to go to [folder-under-src]/some/path/foo.md (in your case with that being generated).

GiggleLiu commented 1 year ago

@GiggleLiu I don't think abspath has anything to do here but I'll investigate the scenario you discuss above.

In the meantime, it would be helpful if you could reproduce exactly the scenario below and let me know what happens please:

  1. add LiveServer#master
  2. using LiveServer; LiveServer.servedocs_literate_example(); cd("servedocs_literate_example/")
  3. servedocs(literate_dir=joinpath("docs", "literate")), navigate to localhost:8000/man/pg1/ in a browser
  4. modify the fourth line of docs/literate/pg1.jl to e.g. f(10)

the changes should be reflected in your browser without any looping; I just re-did this and it should be fine. If not, please open a separate issue quoting the Julia version, OS and confirming you did exactly the above.

Note that 1.2.1 hasn't been released yet (even though it's been tagged) so assuming you had added LiveServer the standard way, the PR #167 above did not affect you and the problem you have encountered is elsewhere.

I tested on both master and latest release, the infinite loop is consistent. I am using Julia 1.9.2, Ubuntu 23.04. I printed the path, the fp is not an abspath, fixing it resolved my issue. It is related to this if statement: https://github.com/tlienart/LiveServer.jl/blob/3900afe59b51e4eac8a98ebb11bf949f1e7f617a/src/utils.jl#L36

GiggleLiu commented 1 year ago

Thank for your suggestions, but I believe the behavior of skip_dirs still needs to be fixed.

tlienart commented 1 year ago

Thanks @GiggleLiu , the PR #169 (basically your PR + one addition after testing with your repo) should fix the problem you reported. I also re-wrote the relevant doc page to clarify the different cases.