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

docs: Lucene-Net-Documentation.yml - Automation is broken for API docs #1004

Open
1 task done
NightOwl888 opened this issue Oct 30, 2024 · 4 comments
Open
1 task done
Labels
docs Applies to the API docs or website is:bug pri:high Project Infrastructure up-for-grabs This issue is open to be worked on by anyone

Comments

@NightOwl888
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

The automation doesn't work because the docs.ps1 script makes assumptions that aren't true in CI:

  1. The docfx tool is already installed, so a dotnet tool restore will function
  2. The docfx command is on the path

Expected Behavior

  1. docs.ps1 will install docfx if it doesn't already exist
  2. docs.ps1 won't rely on the system path, which isn't stable across environments. Instead, it should use --tool-path to set the install location, and use the same location (absolute path) to execute the tool.
  3. docs.ps1 should install docfx in a versioned folder so future versions of the script can execute on the same machine as the current version

Steps To Reproduce

Run the workflow at: https://github.com/apache/lucenenet/actions/workflows/Lucene-Net-Documentation.yml on the master branch.

Exceptions (if any)

https://github.com/apache/lucenenet/actions/runs/11490755057

Workload updates are available. Run dotnet workload list for more information.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.codecs.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.core.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.analysis-common.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.analysis-kuromoji.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.analysis-morfologik.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.analysis-opennlp.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.analysis-phonetic.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.analysis-smartcn.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.analysis-stempel.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.benchmark.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.classification.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.expressions.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.facet.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.grouping.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.icu.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.highlighter.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.join.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.memory.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.misc.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.queries.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.queryparser.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.replicator.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.sandbox.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.spatial.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.suggest.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.test-framework.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.demo.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.codecs.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.
Building api metadata for D:\a\lucenenet\lucenenet\main-repo\websites\apidocs\docfx.core.json...
Cannot find a tool in the manifest file that has a command named 'docfx'.

Lucene.NET Version

N/A

.NET Version

N/A

Operating System

windows-latest

Anything else?

No response

@NightOwl888 NightOwl888 added Project Infrastructure up-for-grabs This issue is open to be worked on by anyone is:bug docs Applies to the API docs or website pri:high labels Oct 30, 2024
@paulirwin
Copy link
Contributor

paulirwin commented Oct 30, 2024

A few subtle corrections:

  1. The docfx tool is already installed, so a dotnet tool restore will function
  2. The docfx command is on the path

The current version of the script makes neither of these assumptions (apart from "so a dotnet tool restore will function"). dotnet tool restore restores any tools in .config/dotnet-tools.json as local tools. If dotnet tool restore is failing, that is not the same thing as assuming that "the docfx tool is already installed."

As for "The docfx command is on the path" - that is definitely not an assumption, because of the above. The script was updated to use dotnet tool run docfx instead of assuming it is on the path. The only thing it assumes is on the path is dotnet. Did you mean dotnet instead of docfx in those two points?

I agree that we can change the tool path location to be more reliable.

@NightOwl888
Copy link
Contributor Author

What I meant was dotnet tool restore does not install the tool if it does not already exist on the system. So, it took a while to work out that I needed to use dotnet tool install instead.

There are a few problems with this approach:

  1. It isn't clear where the tool is being physically installed on the machine and whether it is being done in a custom way in a virtual environment.
  2. GitHub Actions does not use the system path, it uses a text file. If dotnet tool updates the system path, it has no effect in the virtual environment.
  3. Since it isn't clear where the tool is, we have to search for it by outputting the files in the log.
  4. When I finally did figure out where the tool was, adding it to the path didn't even work. If it is not supposed to be, then that probably explains why this didn't work. But that wasn't clear.

It is simpler to understand if the path is inside of our repo directory than to have it install it in some magic directory on the machine. We don't have to worry about whether the current CI environment knows where the tool is, because we know. It is very clear by looking at the script where the tool is. And we can be 100% sure we don't have to touch any settings on the environment to get it to function, so it will be portable across all environments.

I am not sure why we would ever need to use dotnet tool restore. If we use a versioned local folder to store the tool locally, we could just check for the existence of the version folder. If it is there, run it. If not, create the folder and install it using dotnet tool install with --tool-path. The tool would never be out of date. Then at some future date when we upgrade the version, it will be able to accommodate both versions on the same machine by switching commits/branches because the version folders would be side by side.

@paulirwin
Copy link
Contributor

I'm confused, I just tested locally removing docfx from my system and dotnet tool restore does indeed restore it. That's what the command is supposed to do, per the docs:

The dotnet tool restore command finds the tool manifest file that is in scope for the current directory and installs the tools that are listed in it.

https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-tool-restore

Now, whether or not this needs customization of the tool path to support running in a CI environment is a different story. But the command is certainly intended to install the tools, and at least locally it achieves this purpose.

I am not sure why we would ever need to use dotnet tool restore.

The goal of this was so that we can specify the version in the dotnet-tools.json file instead of having to have them in sync across the docs and site ps1 files.

@NightOwl888
Copy link
Contributor Author

Gotcha. Then there is something different about the GitHub Actions environment. I listed all of the files on the disk and it definitely wasn't installed. I also checked the dotnet tool list command and it wasn't there either locally or globally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Applies to the API docs or website is:bug pri:high Project Infrastructure up-for-grabs This issue is open to be worked on by anyone
Projects
None yet
Development

No branches or pull requests

2 participants