-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[DO NOT MERGE] Testing subrepo by branch #13378
Conversation
@mxnet-label-bot add [pr-work-in-progress] |
.gitmodules
Outdated
[submodule "3rdparty/tvm"] | ||
path = 3rdparty/tvm | ||
url = https://github.com/dmlc/tvm | ||
[submodule "3rdparty/onnx-tensorrt"] | ||
path = 3rdparty/onnx-tensorrt | ||
url = https://github.com/onnx/onnx-tensorrt.git | ||
[submodule "nvidia_cub"] | ||
path = nvidia_cub |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be 3rdparty/nvidia_cub (or just leave it as cub). THat's probably why Ci is failing :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doh.
923028f
to
553f11c
Compare
I know you don't intend to merge this, but do you mind explaining why we rename the folders instead of just updating the repository link? Is there any technical reason or is the idea to just separate the old and the new version? |
553f11c
to
135734e
Compare
@marcoabreu Yeah it's a good question. It's actually explained really well in the main PR that prompted me to open this pr: #13322 Basically what I'm testing in this PR is to see if there's any way we can avoid taking down everyone's build and CI whenever there's a force push in a submodule. So far it seems like even if we track a branch it hard codes some sha1 somewhere, so I'm not sure if it's possible. BTW, real test of this PR will be that once it builds I'll add a third dummy submodule, and then I'll force push to it in a branch to see what happens. |
Anton submitted a patch to make CI resilient against submodule problems. That should be fine now. But yeah it's a good point about developers. In the end, they have to update the submodules anyway. Hmmm, hard choice |
Gotcha, thanks for the explanation. I'm really curious :) |
135734e
to
777da61
Compare
@mxnet-label-bot add [Build] |
@KellenSunderland ping for update. Is this PR still needed? Thanks |
PR not needed. |
[DO NOT MERGE] Testing subrepo by branch