Closed pagran closed 1 year ago
Please document in the code why we need to set the flag on top of the Dir field.
Also, are we actually sure that this will fix the bug? We already tried once and didn't get it :)
I first thought that writing a regression test would be good, to ensure the code works, but then again we'll stop using git apply
hopefully sooner than later. So I don't think it's needed.
I can extend modinfo test with linker rebuild and additionally add a panic in case of a "silent" patch skip
I don't think it's worth it to be honest. The test will be slow, it has nothing to do with module version info, and we'll stop relying on executing git
soon. But we should manually verify that the bug is fixed.
But it makes sense to add an extra check in case of "silent" skipping patches
Sure, why not. Perhaps you can do something like:
[!short] env OUR_LINKER_CACHE=/some/empty/dir
...
# ensure no panic
[!short] exec built-binary
You should verify this with master, but I think this would always trigger the failure, and it would keep go test -short
as fast as before.
Or, to keep the tests more on topic, we could instead make the linker.txtar test set up a git repo with one commit, and do the similar [!short] env OUR_LINKER_CACHE=/some/empty/dir
line to always patch and build a new linker with go test
, but not go test -short
. I think I prefer it that way, to keep all the linker stuff under one test.
Without --git-dir
flag:
--- FAIL: TestScript (0.17s)
--- FAIL: TestScript/linker (3.53s)
testscript.go:429: WORK=/tmp/go-test-script1447969318/script-linker
> [!short] [exec:git] exec git init -q
> [!short] [exec:git] env GARBLE_CACHE_DIR=$WORK/linker-cache
> garble build
[stderr]
# test/main
cannot get modified linker: expected 2 applied patches, actually 0:
Skipped patch 'cmd/link/internal/ld/pcln.go'.
Skipped patch 'cmd/link/internal/ld/pcln.go'.
exit status 2
[exit status 1]
FAIL: testdata/script/linker.txtar:4: unexpected command failure
FAIL
exit status 1
FAIL mvdan.cc/garble 3.720s
git
is very smart and tries to find repository in parent directory. This is a very strange situation, but in ourmodinfo
test, this is exactly what happens:Fix prevents this behavior