Test that including stdsimd as a submodule in libstd succeeds

[gnzlbg]

So I'm not sure how thorough this test is - cc @alexcrichton is ./x.py check enough? it doesn't appear to catch the issue that @jethrogb ran into, but AFAICT the stdsimd submodule is being appropriately replaced and ./x.py succeeds =/

I haven't enabled this for mipsel and the thumb targets, but for all other targets this appears to work.

[jethrogb]

I think you should use git to do the syncing. Also I'm pretty sure x.py will revert any non-committed changes you make to submodules

[gnzlbg]

To let git do the sync'ing, I have to update the .gitmodules to the repo of the code, then I have to go into src/stdsimd and fetch the repo, pull in the right commit, and then commit the upgrade of stdsimd anyways in the rust-lang/rust repo.

I thought it would just be easier to overwrite the stdsimd code in rust-lang/rust with the one that travis already checked out. I might need to commit the changes though.

[jethrogb]

Why not just do something like this:

cd src/stdsimd
git remote add CI ../../path/to/stdsimd
git fetch CI
git checkout (commit)

[gnzlbg]

I'll give that a try. Doesn't one still need to update the .gitmodules file ?

[gnzlbg]

fatal: reference is not a tree: 129db7f

[jethrogb]

I think that means you don't have the remote configured/fetched correctly. The code you have now looks nothing like I had suggested, I'm not sure if/how this would work.

[gnzlbg]

The code you suggested produced the exact same error. As I mentioned before, the .gitmodules of rust-lang/rust points to rust-lang-nursery/stdsimd and not to the CI branch. I think it has to be updated for any of this to work.

[alexcrichton]

I personally feel like this is a bit overboard on the CI budget/complexity, I'm fine discovering these issues after the fact, they don't really come up all that often I think

[jethrogb]

I personally feel like this is a bit overboard on the CI budget/complexity, I'm fine discovering these issues after the fact, they don't really come up all that often I think

I don't agree with this. In fact, I think that any flow that gates landing things in rust master that could possibly be influenced by stdsimd should also be tested in stdsimd CI.

stdsimd's main purpose is to be a submodule of core/std. However, because stdsimd PRs are not tested in this combination, the chances are high that something will land that will break building core/std. This puts a large burden on the person who requires updates of stdsimd in core/std (because they need some bug fix or new feature to be included upstream): their upstream changes are potentially now blocked by changes made in stdsimd that have nothing to do with the stdsimd changes they want to include upstream. Instead, this responsibility should lie with the author of the broken code.

As for this not happening "all that often", two different core/std-breaking bugs (#599 and #603) were introduced in just the 3 weeks since stdsimd was last updated upstream.

[alexcrichton]

@jethrogb I'm basically personally the architect of CI for crates like libc and rust-lang/rust. It's great to test things, and believe me when I say I don't undervalue it. I have had lots of experience with things like this, and this sort of integration is practically guaranteed to be far more work to keep running than it would be to fix regressions and not acutally effective at preventing all regressions. Preventing regressions like this sort of integration is just being vigilant and not refactoring every few weeks.

Updating any submodule in rust-lang/rust is a PITA because it turns out there's a lot of vectors to introduce subtle bugs. We just do the best we can with the resources we have.

[gnzlbg]

I have the same opinion as Alex, if this turns out to take too much CI time or be a pain to maintain then it’s probably not worth it. In my tests running x.py check against libst adds 3 minutes to each build bot, most of which run now for ~10 minutes. It doesn’t really make much sense to enable this on all of them, but depending on how painful this ends up being, it might be worth it to add it to one or two of them.

…

On Fri 23. Nov 2018 at 20:38, Alex Crichton ***@***.***> wrote: @jethrogb <https://github.com/jethrogb> I'm basically personally the architect of CI for crates like libc and rust-lang/rust. It's great to test things, and believe me when I say I don't undervalue it. I have had lots of experience with things like this, and this sort of integration is practically guaranteed to be far more work to keep running than it would be to fix regressions and not acutally effective at preventing all regressions. Preventing regressions like this sort of integration is just being vigilant and not refactoring every few weeks. Updating *any* submodule in rust-lang/rust is a PITA because it turns out there's a lot of vectors to introduce subtle bugs. We just do the best we can with the resources we have. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#602 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA3Npkp1LQszh33-jGMmaBsdOR6o7fEGks5uyE7DgaJpZM4Yvo9g> .

[alexcrichton]

As another example of why we should not do this, I believe the motivational factor of #603 wouldn't have been caught by this. The bug there is in documentation, not in the standard library itself.

I'm gonna go ahead and close this as I feel relatively strongly this won't end up pulling its weight in the meantime, but we can always reconsider later!