[PATCH v4] test_dir_is_empty: fix edge cases with newlines and hyphens

of `ls -a` has at most two lines (for `.` and `..`), which should be better behaved, and adds the `--` delimiter before the directory name when invoking `ls`. The newly added unit test fails before this change and passes after it. Signed-off-by: William Chargin --- This patch depends on "t: f

[PATCH v4] test_dir_is_empty: fix edge cases with newlines and hyphens

of `ls -a` has at most two lines (for `.` and `..`), which should be better behaved, and adds the `--` delimiter before the directory name when invoking `ls`. The newly added unit test fails before this change and passes after it. Signed-off-by: William Chargin --- This patch depends on "t: f

Re: [PATCH v3] test_dir_is_empty: properly detect files with newline in name

Thanks for the review. > We usually avoid "touch" unless the timestamp of the file is > significant. Makes sense. Will change as you suggest. > This is an abuse of test_must_fail() which is intended strictly for > testing 'git' invocations which might fail for reasons other than the > expected

[PATCH v3] test_dir_is_empty: properly detect files with newline in name

test fails before this change and passes after it. Signed-off-by: William Chargin --- This patch depends on "t: factor out FUNNYNAMES as shared lazy prereq" (2018-08-06), available from `wc/make-funnynames-shared-lazy-prereq`. The code will work on master, but the test will not run due to

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

> That will recurse any subdirectories, possibly wasting time, but since > the point is that we expect it to be empty, that's probably OK. One caveat involves invocations of `test_must_fail test_dir_is_empty`, wherein we _don't_ actually expect the directory to be empty. It looks like there might

[PATCH] t: factor out FUNNYNAMES as shared lazy prereq

atch, `git grep 'test_\(set\|lazy\)_prereq.*FUNNYNAMES'` has only one result. Signed-off-by: William Chargin --- Note: I've tested this only on an Ubuntu-like system, where FUNNYNAMES and FUNNIERNAMES are both naturally satisfied. I've verified that the tests correctly skip when the prereqs are s

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

Hi, > This subject line will appear out of context in "git shortlog" output, > so it's useful to pack in enough information to briefly summarize what > the change does. I'm happy to do so. I think that "simplify" is misleading, because this is a bug fix, not a refactoring. I like your first

[PATCH v2] t/test-lib: make `test_dir_is_empty` more robust

fails before this change and passes after it. Signed-off-by: William Chargin --- I originally wrote this patch for the standalone Sharness library, but that library advises that such patches be sent to the Git mailing list first. t/t-basic.sh| 29 + t

Re: [PATCH 0/1] t/test-lib: make `test_dir_is_empty` more robust

Hi Jonathan, > This information belongs in the commit message Agreed: I included it at the start of the commit message, though I suppose that the wording in the cover letter is clearer, so I've amended the commit to use that wording instead. > Continuing the note about

[PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

and passes after it. Signed-off-by: William Chargin --- t/t-basic.sh| 29 + t/test-lib-functions.sh | 2 +- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/t/t-basic.sh b/t/t-basic.sh index 34859fe4a..3885b26f9 100755 --- a/t/t

[PATCH 0/1] t/test-lib: make `test_dir_is_empty` more robust

While the `test_dir_is_empty` function appears correct in most normal use cases, it can fail when filenames contain newlines. I originally wrote this patch for the standalone Sharness library, but that library advises that such patches be sent to the Git mailing list first. William Chargin (1

Re: [PATCH v2] sha1-name.c: for ":/", find detached HEAD commits

> Junio typically applies bugfixes as close to the bug-source as possible, > which allows them to be merged-up into various releases (rather than > cherry-picked, which would be required if built on top of 'master'). > > Ideally this is directly on top of the commit that introduced the bug, >

Re: [PATCH v2] sha1-name.c: for ":/", find detached HEAD commits

Contents look good to me. I don't understand why the file name in your patch is sha1_name.c as opposed to sha1-name.c (I see e5e5e0883 from 2018-04-10, but that sounds pretty old), but I trust that whatever you're doing there is correct. > Thanks for working on this. You're quite welcome. Thanks

Re: [PATCH v2] sha1-name.c: for ":/", find detached HEAD commits

> As we discussed during the review on v1, ":/" > is *NOT* pathspec (that is why having these tests in t4208 is wrong > but we are following existing mistakes). Ah, I understand the terminology better now. Thanks. I'll change the commit message wording to use "extended SHA-1s" instead of

[PATCH v2] sha1-name.c: for ":/", find detached HEAD commits

ched state. If multiple worktrees exist, only the current worktree's HEAD is considered reachable. This is consistent with the existing behavior for other per-worktree refs: e.g., bisect refs are considered reachable, but only within the relevant worktree. Signed-off-by: Jeff King Signed-off-by: Willi

Re: [PATCH] sha1-name.c: for ":/", find detached HEAD commits

> > Does for_each_ref() iterate over per-worktree refs (like "bisect", > > perhaps)? > > No. To be clear: it iterates only over the per-worktree refs for the current worktree. So, if you mark an unreachable commit as "bad" with bisect, then that commit is reachable (and found by ":/") in the

Re: [PATCH] sha1-name.c: for ":/", find detached HEAD commits

> we care about reachability only from the detached HEAD here, so this > must _not_ use test_commit, which creates an extra tag. Right. I can add a comment to that effect. > Please avoid unnecessary reflowing of earlier lines of the paragrpah > when the only change is to insert "or from HEAD" in

[PATCH] sha1-name.c: for ":/", find detached HEAD commits

ched state. Signed-off-by: William Chargin Based-on-patch-by: Jeff King --- Documentation/revisions.txt | 10 +- sha1-name.c | 1 + t/t4208-log-magic-pathspec.sh | 14 ++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Documentation/revis

Re: Unexpected behavior with :/ references

Yep, I agree with your analysis. I'd be happy to test and commit this, probably later today. Thanks for your input, and for the patch! Best, WC

Re: Unexpected behavior with :/ references

After further investigation, it appears that ":/foo" indeed resolves to the commit with message "foobar" (in the above example) if the commits are not all created at the same time: e.g., by adding `sleep 1` between the commit commands, or exporting `GIT_AUTHOR_DATE`. This leaves only the question

Unexpected behavior with :/ references

Hello, I'm experiencing strange behavior with :/ references, which seems to be inconsistent with the explanation in the docs on two counts. First, sometimes the matched commit is not the youngest. Second, some commits cannot be found at all, even if they are reachable from HEAD. Here is a script

Re: In some rebases, `exec git -C ...` has wrong working directory

Hi Johannes, Thanks for your reply. Part of my confusion was regarding the interaction between `-C` and `--git-dir`. For instance, we have $ git --git-dir target -C /tmp/tmp.Cl4aXMSVis init Initialized empty Git repository in /tmp/tmp.Cl4aXMSVis/target/ which makes sense and is what I

In some rebases, `exec git -C ...` has wrong working directory

Here is a repro script: #!/bin/sh set -eux git --version tmpdir="$(mktemp -d)" cd "${tmpdir}" mkdir target repo cd repo git init touch file; git add file git commit -m "Initial commit" git rebase HEAD --exec "git -C ${tmpdir}/target init" The end of