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

SubmoduleAwareBuilder incorrectly works with "deep" submodules and when len(submodules) > 2 #414

Open
belonesox opened this issue Dec 18, 2021 · 1 comment
Labels
bug Something is clearly a bug effort/medium Can be done in 1-2 days gain/medium Affects multiple users

Comments

@belonesox
Copy link

SubmoduleAwareBuilder incorrectly works with "deep" submodules and when len(submodules) > 2

Lets consider https://github.com/microsoft/onnxruntime.

Look, at submodule lists: modules can include backslashes in their names…

$ git submodule--helper list
160000 a104e0cf23be4fe848f7ef1f3e8996fe429b06bb 0       cmake/external/SafeInt/safeint
160000 523d5e03d86c26267ee6bdf17dd20f6ce6bdadd7 0       cmake/external/coremltools
160000 c3cceac115c072fb63df1836ff46d8c60d9eb304 0       cmake/external/cub
160000 3c73d91c0b04e2b59462f0a741be8c07024c1bc0 0       cmake/external/cxxopts
160000 e7e1482087f58913b80a20b04d5c58d9d6d90155 0       cmake/external/date
160000 277508879878e0a5b5b43599b1bea11f66eb3c6c 0       cmake/external/dlpack
160000 d10b27fe37736d2944630ecd7557cefa95cf87c9 0       cmake/external/eigen
160000 a3d65c80d32c3e584b7aab41d516a0043b2a5e84 0       cmake/external/emsdk
160000 6df40a2471737b27271bdd9b900ab5f3aec746c7 0       cmake/external/flatbuffers
160000 7d0d9061d83b663ce05d9de5da3d5865a3845b79 0       cmake/external/googlebenchmark
160000 53495a2a7d6ba7e0691a7f3602e9a5324bba6e45 0       cmake/external/googletest
160000 db78ac1d7716f56fc9f1b030b715f872f93964e4 0       cmake/external/json
160000 7a2ed51a6b682a83e345ff49fc4cfd7ca47550db 0       cmake/external/libprotobuf-mutator
160000 f412df7a2b64421e1f1d61fde6055a6ea288e8f5 0       cmake/external/mimalloc
160000 21cace4e574180ba64d9307a5e4ea9e5e94d3e8d 0       cmake/external/mp11
160000 436617053d0f39a1019a371c3a9aa599b3cb2cea 0       cmake/external/nsync
160000 be76ca7148396176784ba8733133b9fb1186ea0d 0       cmake/external/onnx
160000 e9456d57605c883cdf985e634ab483e2c1500bb1 0       cmake/external/onnx-tensorrt
160000 d4b2aff0c890ae38bad87c20f5731333db2a2cc1 0       cmake/external/onnxruntime-extensions
160000 2dc747c574b68a808ea4699d26942c8132fe2b09 0       cmake/external/protobuf
160000 5916273f79a21551890fd3d56fc5375a78d1598d 0       cmake/external/pytorch_cpuinfo
160000 4244cd1cb492fa1d10986ec67f862964c073f844 0       cmake/external/re2
160000 373eb09e4c5d2b3cc2493f0949dc4be6b6a45e81 0       cmake/external/tensorboard
160000 9ec2b92d180dff8877e402018b97baa574031b8b 0       cmake/external/tvm
160000 094a73d4e43a25651555bab9b1fd6373262208c3 0       cmake/external/tvm_update
160000 e8c599bca6c56c44b6730ad93f6abbc9ecd60fc1 0       cmake/external/wil
160000 352281313fe1c4313bc222cb9de222afd50c822f 0       server/external/spdlog

and definitely crashed

run_command(git_archive_cmd)

because parent subdirectory for output tar does not exists.

Also, here exists a try to concatenate tar archives for submodules

run_command("tar -Af %s" % tarfiles)

But actually, it does not work, if len(submodules)>2.
See «Only the first archive (the one after the -f option) gets its EOF marker removed. If you try to concatenate more than 2 archives the last archive(s) will be "hidden" behind file-end-markers.»

So resulting tar need to be «retared/fixed» to not loose additional archives/submodules.

And if we "retar" and "fix tar", it will be crashing on

chunk_props[member] = int(chunk_props[member], 8)

because "tar that maded by tar" allows "devmajor" props like empty string.

belonesox added a commit to belonesox/tito that referenced this issue Dec 18, 2021
t0fik added a commit to t0fik/tito that referenced this issue Sep 29, 2022
FrostyX pushed a commit that referenced this issue Oct 4, 2022
@FrostyX
Copy link
Member

FrostyX commented Jan 1, 2024

This seems to be fixed by PR #427. Can we close @belonesox?

@FrostyX FrostyX added bug Something is clearly a bug effort/medium Can be done in 1-2 days gain/medium Affects multiple users labels Jan 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is clearly a bug effort/medium Can be done in 1-2 days gain/medium Affects multiple users
Projects
None yet
Development

No branches or pull requests

2 participants