Readthedocs.org: "edit on github" / "view on github" links for "stable" -> 404

[ThomasWaldmann]

See there:

http://borgbackup.readthedocs.org/en/stable/

Link at top right points to:
https://github.com/borgbackup/borg/blob/55fef2457e483701254e5b2a7d4a1a60963971a8/docs/index.rst
(which is 404)

Same for the view/edit links in the bottom left box, they also point to same 404 location.

OTOH, the docs shown for "stable" are (currently) correctly showing 0.28.2 release, which is the latest release.

I tried a "stable" rebuild, did not help.

[anarcat]

it would seem more reliable to show a link to a branch there, it seems to me...

[anarcat]

55fef2457e483701254e5b2a7d4a1a60963971a8 is the hash of the 0.28.2 _tag_ on github:

$ git show 55fef2457e483701254e5b2a7d4a1a60963971a8 | head
tag 0.28.2
Tagger: Thomas Waldmann <[email protected]>
Date:   Sun Nov 15 22:01:29 2015 +0100

tagged/signed release 0.28.2
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAABCgAGBQJWSPKpAAoJECQ6z6lR944BBt4P/2sRtiN7prKpK7dO8QAPuEiM
Tb4gzaLJRf1ZjSGwFGQ+Ma3YXf3OBMG+ZAZIUtn7q4b3BoetuekCIWcGc9FPxOT5

it seems that GH doesn't render that properly transparently, so it could be a github bug as well...

[ThomasWaldmann]

Ah, interesting. I only looked in git log output for the hash, but the tags do not show up there.

[ThomasWaldmann]

could somebody please have a look?

[agjohnson]

This is ticket has not been triaged yet, as we have not had time to focus on this. Feel free to supply test cases or a patch that address the issue you are seeing.

[ThomasWaldmann]

could this be fixed please?

we get new tickets on our tracker all the time because "edit on github" is broken...

could it be related to that I gpg-sign my release tags in git?

[timabbott]

I'm seeing the same problem with the files written in markdown in project using a mix of markdown and rST (the "edit on GitHub" links go to e.g. foo.rst when the page was generated from e.g. foo.md):
See e.g. http://zulip.readthedocs.org/en/latest/integration-guide.html

[ThomasWaldmann]

@timabbott are you signing your release tags with gpg, too?

[anarcat]

well, i don't know about others but this _is_ fixed here. the original post here says that the stable release page leads to a 404 page (which is still 404). but the stable page now points to a different page which works fine. also, tagged releases work fine too: they leads to a working link.

i wonder if github's support for signed commits fixed this or something.

anyways, it looks to me like the original issue is fixed. @timabbott's issue seems related not to git tags but to the markdown/rst mix, an issue that should be fixed on RTFD's side, not github.

[ThomasWaldmann]

@anarcat right, it works now for our stable tag. \o/

thus, I am closing this one - please file different bugs in a different issue.

[timabbott]

Sounds reasonable, opened https://github.com/rtfd/readthedocs.org/issues/2130 for the markdown issue -- I hadn't realized it was a different bug. Thanks!

[ThomasWaldmann]

Hmm, the issue seems to have come back:

http://borgbackup.readthedocs.io/en/stable/index.html "edit on github" points to:

https://github.com/borgbackup/borg/blob/89707fc6084c196fa3f973a91937218288f59e03/docs/index.rst

... which is 404.

[glasserc]

I've also seen this. http://kinto.readthedocs.io/en/stable/ "edit on github" points to https://github.com/Kinto/kinto/blob/9c331e13643a53ca76208f4efa6c9bb651b165b4/docs/index.rst , which 404s. Like @anarcat said, the hash shown is the hash of the tag, rather than that of the commit. Looking at http://readthedocs.org/api/v1/version/1957996/?format=json, I see the problematic hash listed as the "identifier".

[ThomasWaldmann]

BTW, how about a setting whether one wants to have "edit on github" links or not?

I'ld just switch it off so we would never have to bother with it again. We have a link to github in our docs, so we don't really need that extra link. ;-)

[magopian]

@ThomasWaldmann your last command should be in another issue I believe ;)

Regarding the issue at hand, i've investigated a bit, and here are my findings:
When checking for the "identifier" to use, there's a special case when the slug is STABLE:
https://github.com/rtfd/readthedocs.org/blob/b5d06e856ec0eea6f340bc078ab747254b516cd0/readthedocs/builds/models.py#L117

This will return the identifier which is the commit hash. This is what's causing problem, because it's the hash of the tag, not of a commit, and thus GitHub doesn't know how to display it. As you can see from the comment there, the issue is that we don't have the original branch name.

So maybe we could store the branch name instead of the tag hash (which seems useless in our case?) in the identifier field.

This should be done in the update_stable_version I guess (specifically line 716 and 726).

I'll submit a PR with this naive fix, to open up the discussion.

[ericholscher]

This has been fixed (see http://mockingjay.readthedocs.io/en/stable/ -- it properly links to the hash of the commit on github)

[ThomasWaldmann]

@ericholscher click on first link in top post, then on "edit on github".

[anarcat]

this is not fixed. i still see this problem all the time. it could be a bug with github that doesn't properly recognize certain hashes, but it's still a problem regardless.

[ericholscher]

Where did the commit 55fef2457e483701254e5b2a7d4a1a60963971a8 come from for borg backup? GitHub doesn't seem to recognize it, but I imagine it came from the project at some point. Did you delete and retag a release or something?

Either that, or we have a bug that is pulling tags in from random places.

[anarcat]

it's not a commit, it's a tag, and it's the whole bug here - github doesn't display tag hashes correctly when you ask them as commits:

$ git show 55fef2457e483701254e5b2a7d4a1a60963971a8
tag 0.28.2
Tagger: Thomas Waldmann <[email protected]>
Date:   Sun Nov 15 22:01:29 2015 +0100

tagged/signed release 0.28.2
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAABCgAGBQJWSPKpAAoJECQ6z6lR944BBt4P/2sRtiN7prKpK7dO8QAPuEiM
Tb4gzaLJRf1ZjSGwFGQ+Ma3YXf3OBMG+ZAZIUtn7q4b3BoetuekCIWcGc9FPxOT5
gL4ebFkAPXZ24KrU+c+QCHgPf80E2DfqNZYQsv2yaEPfr0W0c40vOP9bdhmOgZF4
z1GLUrE/36PbVkcneIar7fu/YRV5Rc0Ohl+5g4se+EOyhXQBCcAQkGFZefTpe4eq
s9uVkBTx6eD15BSlEebv34Fxc0SPAj2+UIcrIPtDPrUVLv62wfNu5btwS+41FxXs
2m7QJKXWQAmsM24NGLePGsFP5RJLg+E3IF1ytAdYh3ecHk5xsvoiD3c/z0MAO7Cm
3ukiWQVVX6xpLjpM0iEzBrtaxTv0zPqh1r9n9ecIv2PNVxwmcJ27C8IgMqZ0SpLS
w6whl2mimv7C4Ww7vG1qTzjRjBzLqAOOdZ9SIwVrzAjCYcCSS8wDSpJeIArjDroc
mwbeZoIrWWz40XBFNOsoJQ2a2s98SvJoyO/y8wSnPrgSAlc+bKqGlKnmRLDEASml
dDhGJMywJhS0U5H9fDkJcPCR0zlRk24RqUVISN63e0C6zjsWfwoOpZYFmLF4tIT2
OBbd64Ca7+OkIktQgXC5ujBioX3QBUF7Oris6yVJINYP/eC9fGNX0oYNasoLMiu0
h2eYx+ToARnXU8QFcWLU
=cjuM
-----END PGP SIGNATURE-----

commit 3a72fbe418e075ec7af8695a50dee50603d09b0d
Author: Thomas Waldmann <[email protected]>
Date:   Sun Nov 15 20:30:58 2015 +0100

    update CHANGES

diff --git a/docs/changes.rst b/docs/changes.rst
index 9d215e63..f60edd59 100644
--- a/docs/changes.rst
+++ b/docs/changes.rst
@@ -16,8 +16,13 @@ Other changes:
 - do not create docs sources at build time (just have them in the repo),
   completely remove have_cython() hack, do not use the "mock" library at build
   time, #384
-- docs: explain how to regenerate usage and API files (build_api or
-  build_usage) and when to commit usage files directly into git, #384
+- avoid hidden import, make it easier for PyInstaller, easier fix for #218
+- docs:
+
+  - add description of item flags / status output, fixes #402
+  - explain how to regenerate usage and API files (build_api or
+    build_usage) and when to commit usage files directly into git, #384
+  - minor install docs improvements


 Version 0.28.1

so either you:

1. patch github to make it DTRT here (hahaha, just kidding, you're SOL there)
2. ask them nicely to fix this (couldn't resist)
3. handle those cases by hand and inspect the ref given to extract a valid commit... (looks like this is the only option)

[techtonik]

OMG. commit hash and tag hash are two different things? tag has them both? But why?

[goerz]

A related problem is if the documentation is generated via apidoc, such that no ReST file exists on Github. This will always lead to a 404 error, see #2719. Please add an option to disable these links.

[polyzen]

@goerz, in that case, perhaps rtfd/sphinx_rtd_theme/issues/324? :p

[techtonik]

So can anybody explain what exactly happens?

1. rtfd clones project sources
2. rtfd tries to find branch? tag? for /stable/ URL <-- how does it do this, is there a log for this operation
3. rtfd tries to find commit id for branch/tag name found in step 2
4. rtfd updates to specified commit id
5. rtfd builds the doc
6. rtfd inserts commit id into GitHub edit link URL

The problem is that GitHub skips some commits? Or don't show them by commit id?

It would be nice if build logs were a part of docs footer or something with stats etc.

[anarcat]

OMG. commit hash and tag hash are two different things? tag has them both? But why?

a tag is built on top of a commit, and has extra information, just like branches are not commits, tags are not just commits either. see this document for a good overview of references and tags in git.

1. rtfd clones project sources
2. rtfd tries to find branch? tag? for /stable/ URL <-- how does it do this, is there a log for this operation
3. rtfd tries to find commit id for branch/tag name found in step 2
4. rtfd updates to specified commit id
5. rtfd builds the doc
6. rtfd inserts commit id into GitHub edit link URL

The problem is that GitHub skips some commits? Or don't show them by commit id?

The problem is that RTFD skips step 3 and 4, AFAIK - it just uses the tag hash instead of the commit hash.

[ericholscher]

Sounds like ya'll have done some good work debugging this issue. The next step is to submit a patch that hopefully fixes the issue. I saw a previous attempt at this that I closed thinking it was fixed in my testing (#2428) -- that might be a good place to start.

[techtonik]

I don't get a feeling that we've debugged the issue. These are all assumptions. Can we have a command that we can checkout and run to repeat rtfd behaviour?

[ericholscher]

https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/projects/models.py#L698 is probably a good place to start.

[techtonik]

https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/projects/models.py#L698 is probably a good place to start.

Lame Django question - where is self.versions defined in this file? I don't see any properties in Project object with such name.

[rawtaz]

We're having the exact same problem over at the restic documentation, with e.g. http://restic.readthedocs.io/en/stable/installation.html - the below discussion is with this example page in mind.

For the latest and v0.7.1 versions, the "Edit on GitHub" link works just fine. It's just the stable version that has the problem this issue is about.

Now, the RTD documentation states that "We also create a stable version, if your project has any tagged releases. stable will be automatically kept up to date to point at your highest version".

To me this sounds like RTD is aware of what the highest version is. In restic's case, currently v0.7.1 for which there is a tag with the same name. Considering that RTD also lists latest, stable and then every release tag in the sidebar where you select the version to display, it sure seems to understand what versions we have.

To solve this issue, can't you simply put the highest tag's _name_ in the URLs of the links, instead of as now the highest tag's commit hash?

To be clear, in our case the "Edit on GitHub" link in the example page above is currently https://github.com/restic/restic/blob/402b6f5f8d0171d7061cbeab67725fce794fc376/doc/installation.rst - the hash referencing the commit of the v0.7.1 tag. If instead of this hash you would just use the highest tag name, the URL would instead be https://github.com/restic/restic/blob/v0.7.1/doc/installation.rst, and this works just fine.

This would be in line with what the documentation states about the stable version referencing the highest version, so I think it seems rather straight forward to solve it this way.

[stsewd]

@ThomasWaldmann (and other people having this issue) can you try re-building your project and see if the problem is solved?

According to https://github.com/rtfd/readthedocs.org/pull/3615#pullrequestreview-106594994 it should.

[anarcat]

i rebuilt the "stable" branch of the borg repo (not sure why i still have access to that, @ThomasWaldmann, btw ;), and we still see the issue.

[anarcat]

the tag links are a little better, but still ackward. e.,g. it goes to https://github.com/borgbackup/borg/edit/1.1.4/docs/usage/general.rst which says "Sorry, it looks like your fork is outdated!"

[agjohnson]

Testing a version stable, the Edit link gives me a 404 still. The View link still works for me, but if I try to edit this file on GitHub, the edit button is disabled and tells me that "i must be on a branch" -- so perhaps this is a clue.

Reopening.

[ThomasWaldmann]

can we just remove (or disable by default) the annoying / buggy features like this or that "outdated library version" note?

the problem is not just that readers encounter these bugs and report them HERE, they also frequently report them at the documented software project, causing work for the maintainers of these. and once you have explained it to one user who found it, the next one will find it and open another issue...

[rawtaz]

@agjohnson I have the exact same symptoms in e.g. https://restic.readthedocs.io/en/stable/040_backup.html. I have asked the project owner of restic to trigger a rebuild of the docs, but haven't heard from him yet on whether or not he did that.

Would be great if stable just pointed to the last release tag instead, that would have solved it by now.

[stsewd]

That's because commits nor tags can't be edited. So I think this is kind of another issue (hide the Edit button when a version isn't a branch).

[rawtaz]

@stsewd has a good point, one cannot edit tags either. So at the GitHub end we're left with the branches that exist in the repo. Usually this is master and perhaps some other dev branches.

In order for the edit link to have a purpose, perhaps it simply needs to point to the main branch at all times, regardless of which version you are looking at in RTD?

How to deduct/define the "main" branch is another question though. Maybe the answer to it lies in just looking at what branch the Latest version corresponds to.

EDIT: Then again; What happens on GitHub is a separate concern. Perhaps RTD did what it's supposed to do by linking to the proper file in the proper commit/tag/branch, and that's the end of it, in the sense that it's then up to the user to create a branch from that commit/tag if they want to edit. Just like the "Fork me on GitHub" links on project pages just goes to the original repository, instead of actually starting to fork the original repo into the user's GitHub account.

[rawtaz]

The restic doc project was rebuilt in RTD, but the same state still applies, for obvious reasons. Not much else we can do about it suppose :) Thanks for your hard work!

[techtonik]

I am +1 on reticketing the issue. There were some changes since 2015 and the info needs to be researched again.

[stsewd]

Yeah, I don't think this is a bug, is more like an improvement, and it needs a design decision about how to deal with links to no editable versions (tags).

[xeroc]

On my deployment, it wouldn't even work on master branch?
See http://how.bitshares.works/en/master/

[stsewd]

@xeroc I think you are facing another issue https://github.com/rtfd/readthedocs.org/issues/1917

[techtonik]

@stsewd are you sure looking at the BitShares docs:

© Copyright 2018, BitShares Blockchain Foundation. Revision 3c056912.
Built with Sphinx using a theme provided by Read the Docs.

[stsewd]

@techtonik @xeroc that's another bug, please see https://github.com/rtfd/readthedocs.org/issues/4671

[xeroc]

thanks for the link.

[stsewd]

So, I just discovered that we have this, is_editable thing (sphinx only)

https://github.com/rtfd/readthedocs.org/blob/0f5d979c221da7140138359fc06660ebec6fff9b/readthedocs/doc_builder/backends/sphinx.py#L127-L132

It was added here https://github.com/rtfd/readthedocs.org/commit/4619c8297780c1c0d3e557a9712045e279fd6e95#diff-4ade07b2b17f38e60bfcb27301c99934

The commit msg describes what we are seeing now. But it was never used on the conf.py context.

Anyway, I think we can use that now in the flyout menu with github and friends

https://github.com/rtfd/readthedocs.org/blob/0f5d979c221da7140138359fc06660ebec6fff9b/readthedocs/restapi/templates/restapi/footer.html#L79-L88