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: READTHEDOCS_GIT_IDENTIFIER shouldn't be the commit SHA on versions from tags #11662

Closed
drewcassidy opened this issue Oct 9, 2024 · 14 comments · Fixed by #11875
Closed
Assignees
Labels
Accepted Accepted issue on our roadmap Bug A bug

Comments

@drewcassidy
Copy link

drewcassidy commented Oct 9, 2024

Details

I'm using READTHEDOCS_GIT_IDENTIFIER in my docs to indicate the git tag to check out for a GitHub action. Unfortunately its rendering as the raw SHA

Expected Result

READTHEDOCS_GIT_IDENTIFIER is the name of the tag that was checked out

this is what the docs say it should be

Actual Result

READTHEDOCS_GIT_IDENTIFIER is the raw commit SHA

Workaround

I guess I can do a git describe

Front logo Front conversations

@humitos
Copy link
Member

humitos commented Oct 9, 2024

What's the URL of your project? What's the URL of the build where you're noticing this?

@drewcassidy
Copy link
Author

What's the URL of your project? What's the URL of the build where you're noticing this?

Added. I pasted the URLs into the template wrong I guess

@humitos
Copy link
Member

humitos commented Oct 9, 2024

READTHEDOCS_GIT_IDENTIFIER is the raw commit SHA

Where I should check the content of this variable in your documentation? I'm not able to find the the SHA in the page you linked.

@humitos humitos added Needed: more information A reply from issue author is required Needed: replication Bug replication is required labels Oct 9, 2024
@drewcassidy
Copy link
Author

drewcassidy commented Oct 9, 2024

the example line of - uses: KSPModdingLibs/KSPBuildTools/.github/actions/assemble-release@f89bab355278f61eabb8d8848746a9ce74a256bd is auto-generated using that environment variable

It should say - uses: KSPModdingLibs/KSPBuildTools/.github/actions/[email protected] since the tag name being built is 0.0.2-alpha.7

@humitos
Copy link
Member

humitos commented Oct 10, 2024

I'm not able to reproduce this issue. I did a try at https://test-builds.readthedocs.io/en/full-feature/environment-variables.html and you can see there that READTHEDOCS_GIT_COMMIT_HASH: 185f27fa0b5d052862be2fca53c19ddb22d325e6 and READTHEDOCS_GIT_IDENTIFIER: full-feature. Those variables are pointing to the correct values.

@humitos
Copy link
Member

humitos commented Oct 10, 2024

I think that variable is never the tag name. It seems that it can only be a branch name and or a commit sha when it's a tag. Using my previous branch, I created a tag named v0.1-full-feature and built it on Read the Docs.

Taking a look at the same page, https://test-builds.readthedocs.io/en/v0.1-full-feature/environment-variables.html, we can see that both environment variables have the same value and it's a commit sha.

We use Version.identifier for READTHEDOCS_GIT_IDENTIFIER and based on what this comment says, it will always be a SHA if it's a tag:

#: The identifier is the ID for the revision this is version is for.
#: This is the commit hash (e.g. in Git).
#: If the this version is pointing to a branch,
#: then ``identifier`` will contain the branch name.
#: `None`/`null` means it will use the VCS default branch.
identifier = models.CharField(
_("Identifier"), max_length=255, null=True, blank=True
)

I found that calling git describe as you mentioned you can get the tag's name from a commit:

▶ git describe --tags 185f27fa0b5d052862be2fca53c19ddb22d325e6
v0.1-full-feature

This is a bug and we should fix it.

@humitos humitos added Bug A bug Accepted Accepted issue on our roadmap and removed Needed: more information A reply from issue author is required Needed: replication Bug replication is required labels Oct 10, 2024
@humitos humitos changed the title READTHEDOCS_GIT_IDENTIFIER does not actually have the tag name Build: READTHEDOCS_GIT_IDENTIFIER shouldn't be the commit SHA on versions from tags Oct 10, 2024
@github-project-automation github-project-automation bot moved this to Planned in 📍Roadmap Oct 10, 2024
@humitos
Copy link
Member

humitos commented Nov 28, 2024

I'm adding this issue to the roadmap. We have another user hitting this bug.

@stsewd
Copy link
Member

stsewd commented Dec 4, 2024

So, looks like we aren't saving the branch/tag name that was used in a build, we merely rely on the commit. We can infer the branch/tag name from the version, but that doesn't work for the stable version (we don't save the tag name for stable #10935), and for build from PRs (builds only know the PR number, not the branch). Are we okay with a solution that doesn't work for those two cases?

@stsewd stsewd moved this from Planned to In progress in 📍Roadmap Dec 4, 2024
@humitos
Copy link
Member

humitos commented Dec 5, 2024

I understand if we don't have the branch/tag name we will have the commit, right? We can use git describe as I mentioned in my previous comment to get the branch/tag name in those cases.

@stsewd
Copy link
Member

stsewd commented Dec 5, 2024

That will work for the case of stable I guess, but it won't work for PR builds, as we fetch it as external-{pr number}. Actually, not sure what should be the correct value for PR builds, if they are from the same repo it makes sense that the git id is the name of the branch, but for PR from forks, it doesn't make sense to use that.

@humitos
Copy link
Member

humitos commented Dec 9, 2024

For PR we will have a commit hash as well. That's what we need to return here and it will be consistent.

@stsewd
Copy link
Member

stsewd commented Dec 9, 2024

For PR we will have a commit hash as well. That's what we need to return here and it will be consistent.

Do you mean return the commit itself? That doesn't seem consistent, as we will return the branch/tag name on all other cases.

@humitos
Copy link
Member

humitos commented Dec 9, 2024

Meh, I'm sorry, I got confused.

if they are from the same repo it makes sense that the git id is the name of the branch

👍🏼

but for PR from forks, it doesn't make sense to use that.

We can return what GitHub shows for this cases: <username>:<branch> (example: PyCampES/pycampes#90). If that's too hard to get, we can return unknown for now and leave that case for later.

Screenshot_2024-12-09_17-02-58

@jeertmans
Copy link

Hi! Just came here to say that I also have this issue.

I published my documentation using tags as a way to provide permalinks, and I use sphinx.ext.linkcode to point to the corresponding code on GitHub.

However, when publishing from tags, READTHEDOCS_GIT_IDENTIFIER points to the commit SHA, not the tag name itself. While it is essentially the same, it would be better if the tag name could be provided by READTHEDOCS_GIT_IDENTIFIER, so the URL looks just better (IMO).

E.g., the following page shows documentation for tag icmlcn2025
https://differt.eertmans.be/icmlcn2025/reference/_autosummary/differt.geometry.TriangleMesh.html#differt.geometry.TriangleMesh
but clicking on the source link opens up https://github.com/jeertmans/DiffeRT/blob/5b9879a2d07625b976a84f2f7aa829259cb0d4b1/differt/src/differt/geometry/_triangle_mesh.py#L84-L698.

For your information, I build the link to source code with:

git_ref = os.environ.get("READTHEDOCS_GIT_IDENTIFIER", "main")

def linkcode_resolve(domain: str, info: dict[str, Any]) -> str | None:

    # ...

    return f"https://github.com/jeertmans/DiffeRT/blob/{git_ref}/{filename}{lines}"

Note that the issue also occurs on pull-request builds (but I can this is related to what you said above).

Not sure if that can help you :-)

@github-project-automation github-project-automation bot moved this from Needs review to Done in 📍Roadmap Jan 2, 2025
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 Bug A bug
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants