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.

Sign up for GitHub

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 #10168

Merged
merged 16 commits into from
Mar 28, 2023
Merged

Build: expose VCS-related environment variables #10168

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
73d573d
Build: export `READTHEDOCS_CANONICAL_URL` variable
humitos Mar 21, 2023
d3f1bff
Docs: document `READTHEDOCS_CANONICAL_URL`
humitos Mar 21, 2023
39c1fd3
Docs: add another example
humitos Mar 21, 2023
58c49c3
Test: check the `READTHEDOCS_CANONICAL_URL` is present
humitos Mar 21, 2023
04088d5
Apply suggestions from code review
humitos Mar 21, 2023
5fde482
Update docs/user/reference/environment-variables.rst
humitos Mar 21, 2023
5bb3d74
Docs: fix references
humitos Mar 21, 2023
4687f09
Test: check for `canonical_url` on Version endpoint
humitos Mar 21, 2023
3114f95
Build: expose VCS-related environment variables
humitos Mar 21, 2023
0ffc99a
Apply suggestions from code review
humitos Mar 21, 2023
8432ef5
Update docs/user/reference/environment-variables.rst
humitos Mar 21, 2023
420ba97
Merge branch 'main' into humitos/repository-env-variables
benjaoming Mar 22, 2023
4b2eb75
Build: rename the env variable to use CLONE_URL
humitos Mar 22, 2023
6d4dd3b
Merge branch 'main' of github.com:readthedocs/readthedocs.org into hu…
humitos Mar 23, 2023
77af82e
Merge branch 'main' of github.com:readthedocs/readthedocs.org into hu…
humitos Mar 27, 2023
735d21b
Build: rename env variables to use GIT on their names
humitos Mar 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions docs/user/reference/environment-variables.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,31 @@ All :doc:`build processes </builds>` have the following environment variables au
:Example: ``https://docs.readthedocs.io/ja/stable/``
:Example: ``https://example--17.org.readthedocs.build/fr/17/``

.. envvar:: READTHEDOCS_REPOSITORY_CLONE_URL
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason we're calling it CLONE_URL, instead of REPOSITORY_URL to match the help_text?

This is the same URL defined in your Project's :term:dashboard in :menuselection:Admin --> Settings --> Repository URL.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's because the repository will also have a "web URL", i.e. the base URL for where the file can be viewed and edited, so ssh://[email protected]/repo is different from https://frontend.com/repo/

Copy link
Member

@ericholscher ericholscher Mar 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Seems the doc is not correct then when it says it's the same as your Repository URL in the dashboard?

Users are definitely going to be confused if it's different, but only in some cases 🙃

Copy link
Member Author

Choose a reason for hiding this comment

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

"Repository URL in the dashboard" is what we use as Project.repo, which is the same of the content of this variable. Do you have a suggestion how to make this clearer? I think having all these URLs is just confusing, but necessary, tho.

I want to make the distinction between HTML_URL and CLONE_URL mainly because they are not going to be always the same. In community, they will mostly be the same, tho.


URL for the remote source repository, from which the documentation is cloned.
It could be HTTPS, SSH or any other URL scheme supported by Git.
This is the same URL defined in your Project's :term:`dashboard` in :menuselection:`Admin --> Settings --> Repository URL`.

:Example: ``https://github.com/readthedocs/readthedocs.org``
:Example: ``[email protected]:readthedocs/readthedocs.org.git``

.. envvar:: READTHEDOCS_REPOSITORY_IDENTIFIER
Copy link
Member

@ericholscher ericholscher Mar 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is this a repository id? Or a version ID?

This is probably the name I'm the most unclear on whether it's correct.

Again though, perfect naming is hard, and the examples tell most of the story.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like READTHEDOCS_GIT_IDENTIFIER better. In fact I like everything better with git :)

READTHEDOCS_GIT_CLONE_URL
READTHEDOCS_GIT_COMMIT_HASH
READTHEDOCS_GIT_IDENTIFIER

it's easier to understand. I think the old VCS semi-supported backends can just have to deal with that. If we want to really support something, we might as well re-introduce "VCS" or have stuff like READTHEDOCS_HG_CLONE_URL... but that's not relevant now.

Copy link
Member

Choose a reason for hiding this comment

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

Yea, if we wanted it to be generic, we could use VCS, but I agree GIT is probably better, and forward looking 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I'm going to change the names to use GIT 👍🏼


Contains the Git identifier that was *checked out* from the remote repository URL.
Possible values are either a branch or tag name.

:Example: ``v1.x``
:Example: ``bugfix/docs-typo``
:Example: ``feature/signup``
:Example: ``update-readme``

.. envvar:: READTHEDOCS_REPOSITORY_IDENTIFIER_HASH
Copy link
Member

Choose a reason for hiding this comment

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

I don't love how long this name is, but it seems reasonably explicit.

I think IDENTIFIER and HASH are kind of the same thing, but I'm not sure if git has a canonical name for this. I couldn't quickly find it in the docs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added HASH here to be explicit about a commit in particular. We are using IDENTIFIER for the branch/tag name as well. I didn't find a better name for this 😞


Git commit hash identifier checked out from the repository URL.
humitos marked this conversation as resolved.
Show resolved Hide resolved

:Example: ``1f94e04b7f596c309b7efab4e7630ed78e85a1f1``

.. seealso::

:doc:`/environment-variables`
Expand Down
6 changes: 6 additions & 0 deletions readthedocs/doc_builder/director.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,12 @@ def get_rtd_env_vars(self):
"READTHEDOCS_OUTPUT": os.path.join(
self.data.project.checkout_path(self.data.version.slug), "_readthedocs/"
),
"READTHEDOCS_REPOSITORY_CLONE_URL": self.data.project.repo,
# TODO: we don't have access to the database from the builder.
# We need to find a way to expose HTML_URL here as well.
# "READTHEDOCS_REPOSITORY_HTML_URL": self.data.project.remote_repository.html_url,
"READTHEDOCS_REPOSITORY_IDENTIFIER": self.data.version.identifier,
"READTHEDOCS_REPOSITORY_IDENTIFIER_HASH": self.data.build["commit"],
}
return env

Expand Down
3 changes: 3 additions & 0 deletions readthedocs/projects/tests/test_build_tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ def test_get_env_vars(self, load_yaml_config, build_environment, config, externa
"READTHEDOCS_OUTPUT": os.path.join(
self.project.checkout_path(self.version.slug), "_readthedocs/"
),
"READTHEDOCS_REPOSITORY_CLONE_URL": self.project.repo,
"READTHEDOCS_REPOSITORY_IDENTIFIER": self.version.identifier,
"READTHEDOCS_REPOSITORY_IDENTIFIER_HASH": self.build.commit,
}

self._trigger_update_docs_task()
Expand Down