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

Build: expose VCS-related environment variables #9423

Closed
pradyunsg opened this issue Jul 10, 2022 · 7 comments · Fixed by #10168
Closed

Build: expose VCS-related environment variables #9423

pradyunsg opened this issue Jul 10, 2022 · 7 comments · Fixed by #10168
Labels
Accepted Accepted issue on our roadmap Feature New feature

Comments

@pradyunsg
Copy link

So... with sphinx-basic-ng, a bunch of the theme developers in the Sphinx ecosystem are trying to get a shared base theme that we all build upon.

Notably, this has reusable implementations of various design "components" (https://sphinx-basic-ng.readthedocs.io/en/latest/usage/components/). In a few of them1,

html_theme_options = {
    "source_repository": "https://github.com/pradyunsg/sphinx-basic-ng/",  # you can skip the trailing /
    "source_branch": "main",
    "source_directory": "docs",  # or "docs/"
}

It would be great if builds on Read the Docs could auto-populate these theme variables, when they're not provided by the user.

That would make the community-maintained themes that are using these components "just work" on Read the Docs, similar to how the Read the Docs theme works! Since this is designed to be a shared implementation across themes (Furo and Lutra already use it, there's efforts underway to get this for pydata-sphinx-theme and sphinx-book-theme as well), it's somewhat theme-agnostic and would be a nice user experience improvement!

Notably, it would allow themes to remove custom code for supporting VCS information conditionally on Read the Docs (Furo does this!).

Footnotes

  1. edit-this-page and view-this-page

@pradyunsg
Copy link
Author

FWIW, this is (in effect) a replacement for https://github.com/readthedocs/readthedocs.org/blob/main/docs/user/guides/edit-source-links-sphinx.rst .

If you'd prefer to provide a fully-formatted link instead, that's coming in pradyunsg/sphinx-basic-ng#34 -- which has the relevant documentation updates too. :)

@humitos
Copy link
Member

humitos commented Jul 11, 2022

It would be great if builds on Read the Docs could auto-populate these theme variables, when they're not provided by the user.

Would it be useful if Read the Docs passes these as environment variables (e.g. READTHEDOCS_SOURCE_REPOSITORY, etc) and it's the theme itself that defines them to the Sphinx context? We are moving away of defining things automagically behind the scenes for the users because it creates confusing scenarios.

@stsewd stsewd added the Needed: more information A reply from issue author is required label Aug 18, 2022
@pradyunsg
Copy link
Author

Would it be useful if Read the Docs passes these as environment variables (e.g. READTHEDOCS_SOURCE_REPOSITORY, etc) and it's the theme itself that defines them to the Sphinx context?

That works!

@humitos humitos added Feature New feature Accepted Accepted issue on our roadmap and removed Needed: more information A reply from issue author is required labels Oct 4, 2022
@humitos humitos moved this to Planned in 📍Roadmap Oct 4, 2022
@humitos humitos changed the title Autopopulate VCS variables used by sphinx-basic-ng Build: expose VCS-related environment variables Jan 31, 2023
@benjaoming
Copy link
Contributor

READTHEDOCS_SOURCE_REPOSITORY works for me, too, but I think it should be READTHEDOCS_SOURCE_REPOSITORY_WEB_URL since the source repository has other properties that we can also expose over time if requested? I think that READTHEDOCS_SOURCE_REPOSITORY_URL is ambiguous :) We can also trim _SOURCE.

  • READTHEDOCS_GIT_WEB_URL: Indicates the root path of the repo website, i.e. https://github.com/org/repo
  • READTHEDOCS_GIT_PROVIDER: "github"/"bitbucket"/"gitlab" - is that useful for you, @pradyunsg ?
  • READTHEDOCS_GIT_IDENTIFIER: Populated with name of tag, branch or commit hash (only one of them and selected in that priority - if tag exists, uses tag, if branch uses branch otherwise commit hash)
  • READTHEDOCS_SOURCE_DIR: If the Sphinx builder is active, we can put the directory here (note @humitos maybe good to call the new READTHEDOCS_OUTPUT READTHEDOCS_OUTPUT_DIR instead?)

@mgeier
Copy link
Contributor

mgeier commented Feb 18, 2023

I'm not against additional environment variables, those could be very handy in some situations, but I wanted to mention that the RTD build process already provides some (all?) of the mentioned information in html_context.

The source repository can be found by looking at display_gitlab, combined with gitlab_user and gitlab_repo (same for bitbucket and github).

I don't know if the actual branch name is available, and I'm not sure how reliable it would be anyway, but the commit is available in the field commit.

The source directory is available in conf_py_path (strictly speaking, conf.py doesn't have to be in the source directory, but normally it is).

I'm using all this information to create a source link at the bottom of each page in my insipid theme, see https://insipid-sphinx-theme.readthedocs.io/en/0.4.1/configuration.html#confval-html_show_sourcelink

@humitos
Copy link
Member

humitos commented Feb 22, 2023

@mgeier

I'm not against additional environment variables, those could be very handy in some situations, but I wanted to mention that the RTD build process already provides some (all?) of the mentioned information in html_context.

We are moving away from built-in Sphinx-only features and trying to support other tools in a cleaner and generic way. So, the additional environment variables would be useful for these other documentation tools as well (e.g. MkDocs, Docusaurus, etc).

I think the right way to keep supporting Sphinx-only features is by creating more Sphinx extensions than adding them into the Read the Docs' build process itself. We have tested this already with some extensions and we are happy with the results, so we will probably keep moving in that direction.

BTW, thanks for pointing out where to get this information from with the current build process! 👍🏼

@humitos
Copy link
Member

humitos commented Feb 22, 2023

@benjaoming

I wouldn't use the word GIT in the variables, I think it just add noise and we currently support other as well. Yes, we want to deprecate them too 😄

These are my suggestions following your notes:

  • READTHEDOCS_REPOSITORY_URL (GitHub URL, like https://github.com/readthedocs/readthedocs.org/)
  • READTHEDOCS_REPOSITORY_IDENTIFIER (e.g. main or v1.0.0 or a1b2c3 --matching what's used in the Version object)

Thinking a little more about READTHEDOCS_SOURCE_DIR, what should be its value for different doctools? How we will get it correctly? What's the value for Docusaurus, or Gatsby, for example? I'm not sure we can commit ourselves to populate it with the right value. However, this is related to #9088

I'd move forward with the first two that are the ones that we already know, and leave the third one to continue discussing the deciding how to implement it.

humitos added a commit that referenced this issue Mar 21, 2023
Introduce 3 new variables:

* `READTHEDOCS_REPOSITORY_URL`
* `READTHEDOCS_REPOSITORY_IDENTIFIER`
* `READTHEDOCS_REPOSITORY_IDENTIFIER_HASH`

Closes #9423
@github-project-automation github-project-automation bot moved this from Planned to Done in 📍Roadmap Mar 28, 2023
humitos added a commit that referenced this issue Mar 28, 2023
* Build: export `READTHEDOCS_CANONICAL_URL` variable

This variable communicates what's the canonical URL for the version it's running
the build. It will allow doctool/theme authors to implement the correct HTML tag
required:

```
<link rel="canonical" href="" />
```

Note the final `href` on each page will be `READTHEDOCS_CANONICAL_URL` + `page`.

Closes readthedocs/readthedocs-ops#1320

* Docs: document `READTHEDOCS_CANONICAL_URL`

Add it to the reference page.

* Docs: add another example

* Test: check the `READTHEDOCS_CANONICAL_URL` is present

* Apply suggestions from code review

Co-authored-by: Benjamin Balder Bach <benjamin@readthedocs.org>

* Update docs/user/reference/environment-variables.rst

Co-authored-by: Benjamin Balder Bach <benjamin@readthedocs.org>

* Docs: fix references

* Test: check for `canonical_url` on Version endpoint

* Build: expose VCS-related environment variables

Introduce 3 new variables:

* `READTHEDOCS_REPOSITORY_URL`
* `READTHEDOCS_REPOSITORY_IDENTIFIER`
* `READTHEDOCS_REPOSITORY_IDENTIFIER_HASH`

Closes #9423

* Apply suggestions from code review

Co-authored-by: Benjamin Balder Bach <benjamin@readthedocs.org>

* Update docs/user/reference/environment-variables.rst

* Build: rename the env variable to use CLONE_URL

We will want to expose `HTML_URL` as well, but that will require some extra work
since we don't have access to `RemoteRepository` from the builder.

I'm keeping it commented for now, but reserving the names.

* Build: rename env variables to use GIT on their names

---------

Co-authored-by: Benjamin Balder Bach <benjamin@readthedocs.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Feature New feature
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants