rustc-guide submodule issues

[ehuss]

Recent PR (#62733) failed due to some issues with updating the rustc-guide submodule. It's not clear exactly the extent of what's wrong, but here are some issues I see:

• I can't find anything that actually runs the rustc-guide tests.
• Running them locally, they fail with bad links.

Error: there are broken links
	Caused By: "https://github.com/rust-lang/compiler-team/blob/master/experts/MAP.md" returned 404 Not Found

• Note, this is especially awkward for me to deal with, because it only runs on linux, which is not my native platform.
• The rustc-guide entry was never added to toolstate.

I'll also note that I am concerned that running external link checks on CI will likely be unreliable, and I do not want PRs to be blocked because of it.

cc @mark-i-m

[mark-i-m]

Sorry, CI has been have rate-limiting issues recently. There is supposed to be a cron job that checks the links every day, but issues are masked by the rate-limiting.

I will try to get on this today.

I can't find anything that actually runs the rustc-guide tests.

Our CI on the rust-lang/rustc-guide repo runs them for each PR and a CI cron job is supposed to run them nightly. Unfortunately, I don't think we can do much better. Links just break sometimes.

Running them locally, they fail with bad links.

This link broke very recently. Unfortunately, since I haven't had time to look into CI failures, I missed this one. It should be fixed now.

In general, most breakage comes from compiler code changes, so I expect the CI to find them quickly.

Note, this is especially awkward for me to deal with, because it only runs on linux, which is not my native platform.

Sorry about this. @Michael-F-Bryan would it be possible to get linkcheck to default to reqwest? Or expose such a feature? That would help tremendously.

The rustc-guide entry was never added to toolstate.

Huh? @pietroalbini any ideas? (Sorry, I have pinged you a lot recently)

I'll also note that I am concerned that running external link checks on CI will likely be unreliable, and I do not want PRs to be blocked because of it.

This is the first time such a thing has happened. Also, to my knowledge, it ahould not block a PR but trigger link toolstate. Clearly, I have done something wrong.

[pietroalbini]

cc @kennytm for toolstate

[ehuss]

Our CI on the rust-lang/rustc-guide repo runs them for each PR and a CI cron job is supposed to run them nightly. Unfortunately, I don't think we can do much better. Links just break sometimes.

My point is, there is no reason for rustc-guide to be in toolstate if it never runs. Without being tested, there's no "state" to add to the toolstate. Tools usually run here to generate that state.

Huh? pietroalbini any ideas? (Sorry, I have pinged you a lot recently)

When a tool is added, it has to be manually added to the toolstate (like I did here).

But to be clear, I do not want to add rustc-guide to the toolstate as-is. If it is testing external links, it will likely fail too often. I think that will be too disrupting to update PRs.

[mark-i-m]

@ehuss Ah, I guess that is why toolstate is not working properly.

I very much want the guided added to toolstate. It's incredibly hard to discover these sorts of breakage with stumbling over it in other PRs to the guide. This makes it harder to keep the guide up to date and it also makes contribution to the guide more painful.

I'll also add that I don't think it will be a regular problem. It was somewhat unlucky that the compiler-team repo broke at the same time as your PR.

I would ask that we do an experiment: enable toolstate for the guide, and if it breaks another PR, we can disable it.

[mark-i-m]

@ehuss If it is any comfort, I enabled github restrictions on rustc-guide that prevent merging PRs/pushing to master without green CI.

[ehuss]

That's fair, I'm fine with trying for a while and see how it goes. Also, I'm not advocating removing it from the toolstate (sorry I'm not being clear). I meant that changing it so that it is allowed to fail when the submodule is updated at all times (even week before release). That is, changing checktools.sh to allow failures of rustc-guide, even when the submodule is updated. But still report status changes on the website and the issue tracker.

Can you take care of the two updates I mentioned above (adding it to the json file, and updating checktools.sh to actually test it)?

[mark-i-m]

That is, changing checktools.sh to allow failures of rustc-guide, even when the submodule is updated. But still report status changes on the website and the issue tracker.

Yes, that was the original goal, but clearly, I'm not doing that right. Is this the necessary change? Or do I need to explicitly allow it to fail somewhere? https://github.com/rust-lang/rust/compare/master...mark-i-m:rustc-guide-toolstate-check?expand=1

[mark-i-m]

Also, I opened: rust-lang-nursery/rust-toolstate#12

[ehuss]

Yes, that was the original goal, but clearly, I'm not doing that right. Is this the necessary change? Or do I need to explicitly allow it to fail somewhere?

Oh, if that's the goal, there are a few changes I think:

• Remove rustc-guide from status_check. It's only purpose there is to fail the build if the submodule is updated and the status is not test-pass.
• Somehow exclude rustc-guide from the SIX_WEEK_CYCLE check. That triggers during the last week, and if any PR causes a tool to regress, it is rejected. We probably don't want that for rustc-guide.

I would verify with @kennytm exactly what to do there, but that's how I see it.

[mark-i-m]

@ehuss Thanks! I opened #62759

[Michael-F-Bryan]

@Michael-F-Bryan would it be possible to get linkcheck to default to reqwest?

@mark-i-m I'm not quite sure what you mean here. Are you looking for the follow-web-links option under Configuration?

[mark-i-m]

@Michael-F-Bryan No, we are looking for a way to make the linkchecker platform-agnostic. Currently, it seems to depend on libssl, which is only available on x86_64 linux. If there was an option to build it with reqwest, this could be alleviated.

[mati865]

OpenSSL supports all the main targets so it should be easy to make linkchecker work everywhere.

[mark-i-m]

@mati865 That was not our experience in #59772

[mati865]

I don't use OS X or MSVC but it looks relatively easy to make it work on 32 bit Linux and 32/64 bit windows-gnu. I think I should be able to come up with solution on platforms I mentioned somewhere around next week.

[mark-i-m]

@mati865 That would be awesome! Thanks 🎉

[Michael-F-Bryan]

@Michael-F-Bryan No, we are looking for a way to make the linkchecker platform-agnostic. Currently, it seems to depend on libssl, which is only available on x86_64 linux.

mdbook-linkcheck doesn't directly depend on libssl. I'm guessing what's happened is reqwest has the default-tls feature flag enabled by default, which means it uses whatever TLS library is provided by the platform (via native-tls).

That should imply mdbook-linkcheck is as platform-agnostic as native-tls is, shouldn't it? I've also run it on a Windows 10 PC several times in the past without any issues (my normal dev environment is Linux).

[mark-i-m]

@Michael-F-Bryan I guess, but if that is the case, then why is it not working on some platforms? See the errors in #59772

[mati865]

@mark-i-m it didn't work on i686 Linux because it's cross compiled from x86_64 Linux and there was no 32 bit openssl installed. I think I've fixed it but I'm waiting on the build to finish.
After that I'll check other platforms and open PR if everything is ok.

[ehuss]

I think all the issues have been sufficiently fix, so I'm going to close this.

[mark-i-m]

@mati865 Thanks for your help!

@ehuss Thanks for your cooperation! I think I underestimated how often links break, though I think we have made spurious failures much more manageable.