fix!: avoid git submodule init hook merges#75
Conversation
|
|
||
| # Confirm the directory is empty / uninitialized before running | ||
| run git -C "$gitrepo" submodule status | ||
| assert_output --regexp '^-\b[0-9a-f]{5,40}\b child$' |
There was a problem hiding this comment.
I got this output when trying to run devbox run test
✗ uninitialized submodule gets populated
(from function `__assert_stream' in file devbox-plugins/base-config/tests/libs/bats-assert/src/assert_output.bash, line 228,
from function `assert_output' in file devbox-plugins/base-config/tests/libs/bats-assert/src/assert_output.bash, line 125,
in test file devbox-plugins/base-config/tests/ensure-submodules-populated.bats, line 134)
`assert_output --regexp '^ \b[0-9a-f]{5,40}\b child.*'' failed
Initialized empty Git repository in /private/var/folders/q0/26ph_w_s1pvfmrmqyv7rlykm0000gn/T/bats-run-RBoMkk/test/4/child-remote.git.tmp/.git/
[main (root-commit) 9b43aa2] initial
Cloning into bare repository '/var/folders/q0/26ph_w_s1pvfmrmqyv7rlykm0000gn/T/bats-run-RBoMkk/test/4/child-remote.git'...
done.
Initialized empty Git repository in /private/var/folders/q0/26ph_w_s1pvfmrmqyv7rlykm0000gn/T/bats-run-RBoMkk/test/4/parent-remote.git.tmp/.git/
Cloning into '/private/var/folders/q0/26ph_w_s1pvfmrmqyv7rlykm0000gn/T/bats-run-RBoMkk/test/4/parent-remote.git.tmp/child'...
done.
[main (root-commit) 888af00] add child submodule
2 files changed, 4 insertions(+)
create mode 100644 .gitmodules
create mode 160000 child
Cloning into bare repository '/var/folders/q0/26ph_w_s1pvfmrmqyv7rlykm0000gn/T/bats-run-RBoMkk/test/4/parent-remote.git'...
done.
-- regular expression does not match output --
regexp : ^ \b[0-9a-f]{5,40}\b child.*
output : 9b43aa2de67942e6e9f5e62fc301b995aba07ee6 child (heads/main)
--
Last output:
9b43aa2de67942e6e9f5e62fc301b995aba07ee6 child (heads/main)
I think it is something with how =~ is read on MacOS compared to Linux, atleast I have experienced it before when someone used \b in a regex.
If I change it to: assert_output --regexp '^-[0-9a-f]{5,40} child$' the test passes.
There was a problem hiding this comment.
I think I have narrowed it down, to no the word boundary itself, but rather the output_assert --regexp uses Gnu Bash feature, that somewhat isn't compatible with MAC.
I can't find the proof exactly, but their reference to Gnu Bash manual in https://github.com/bats-core/bats-assert/blob/f1e9280eaae8f86cbe278a687e6ba755bc802c1a/src/assert_regex.bash#L29 could support this thesis.
I many cases FreeBSD and Gnu isn't fully compatible for all binutils etc.
Regexp will be valid without \b so good anyway to remove them.
There was a problem hiding this comment.
I pushed (amended and force pushed) the change of removing word boundary, and then also include testing on macos in github actions, so we can detect these problems earlier and automatically in the future.
| assert_output --partial "ensure-submodules-populated: done" | ||
|
|
||
| run git -C "$gitrepo" submodule status | ||
| assert_output --regexp '^ \b[0-9a-f]{5,40}\b child.*' |
|
Works for me: |
This bugfix ensure that the devbox base-config plugin do not try to merge git submodules during the devbox base plugin init hook. An earlier feature in introduced in PR #50 that would check for uninitialized git submodules also tries to merge content. The merge was done to avoid it beeing "shadowed" by incoming changes, but it showed to be too intrusive as we don't alwasy want to merge. The fix changes behavior of the init hook in the plugin to not merge anything. However the original intention of the git submodule status check to guardrail against uninitialized git submodules persist and is improved to handle all cases now instead, one submodule at the time. Recursively as earlier also. Because of the bug, tests are introduced to show desired behavior and ensure it works as expected. Since the plugin didn't have any tests either, the tests serves as a simple start for testing and proof-of-concept. Minor changes also included: * Some markdown lint and formatting fixes in project README. * devbox used for the project itself also, to ensure BATS installed and `devbox run test` is easily available. * Github Action to run tests. * Test and release section in the project README. BREAKING CHANGE: In case you depended on automatic merge of Git Submodules in the devbox base-config plugin's init hook, this have now been disabled and only uninitialized Git submodules are initialized.
d9dd3bd to
44e2941
Compare
This bugfix ensure that the devbox base-config plugin do not try to
merge git submodules during the devbox base plugin init hook.
An earlier feature in introduced in PR #50 that would check for
uninitialized git submodules also tries to merge content. The merge was
done to avoid it beeing "shadowed" by incoming changes, but it showed to
be too intrusive as we don't alwasy want to merge.
The fix changes behavior of the init hook in the plugin to not merge
anything.
However the original intention of the git submodule status check to
guardrail against uninitialized git submodules persist and is improved
to handle all cases now instead, one submodule at the time. Recursively
as earlier also.
Because of the bug, tests are introduced to show desired behavior and
ensure it works as expected.
Since the plugin didn't have any tests either, the tests serves as a
simple start for testing and proof-of-concept.
Minor changes also included:
devbox run testis easily available.BREAKING CHANGE: In case you depended on automatic merge of Git
Submodules in the devbox base-config plugin's init hook, this have now been disabled and only uninitialized Git submodules are initialized.