Re: Tags from each remote in a separate "name-space"?
On Thu, Feb 28, 2019 at 08:54:23AM +0300, Sergey Organov wrote: > Hello, > > How do I configure git to handle tags from remotes in a manner similar > to branches? > > Specifically, I want tag 'tag_name' from remote 'origin' to have local > name 'origin/tag_name', not 'tag_name', as it is by default. For a repo > with a lot of remotes[*] it would allow to keep track of what tag came from > where, as well as prevent name conflicts between tags from different > remotes (and/or local tags). On the fetch side, something like this in the config file (assuming "origin" remote here) probably would do [remote "origin"] fetch = +refs/tags/*:refs/tags/origin/* tagOpt = --no-tags I'm not so sure about the push side. PS. it would be nice if "git remote" has --no-shared-tags option or something to do this, assuming that it works perfectly well. -- Duy
RE: [BUG] All files in folder are moved when cherry-picking commit that moves fewer files
Thanks for the answers. So it seems it's not a bug, but may lead to new merge options. I worked around it anyway, so it was not a real problem. Med vänlig hälsning Linus Nilsson -Original Message- From: Elijah Newren Sent: Wednesday, 27 February 2019 18:32 To: Jeff King Cc: Phillip Wood ; Linus Nilsson ; git@vger.kernel.org Subject: Re: [BUG] All files in folder are moved when cherry-picking commit that moves fewer files On Wed, Feb 27, 2019 at 8:40 AM Jeff King wrote: > > On Wed, Feb 27, 2019 at 08:02:35AM -0800, Elijah Newren wrote: > > > > > I have found what I suspect to be a bug, or at least not > > > > desirable behavior in my case. In one branch, I have moved all > > > > files in a directory to another directory. The first directory > > > > is now empty in this branch (I haven't tested whether this is > > > > significant). > > > > > > I suspect that because you've moved all the files git thinks the > > > directory has been renamed and that's why it moves a/file2 when > > > fix is cherry-picked in the example below. I've cc'd Elijah as he > > > knows more about how the directory rename detection works. > > > > Yes, Phillip is correct. If the branch you were > > merging/cherry-picking still had any files at all in the original > > directory, then no directory rename would be detected. You can read > > up more details about how it works at > > https://git.kernel.org/pub/scm/git/git.git/tree/Documentation/techni > > cal/directory-rename-detection.txt > > Is there a way to disable it (either by config, or for a single run)? > I know there's merge.renames, but it's plausible somebody might want > file-level renames but not directory-level ones. > > -Peff Not yet. Adding such an option, similar in nature to the flags for turning off renaming detection entirely (merge.renames, diff.renames, -Xno-renames) would probably make sense (I don't see an analogy to -Xrename-threshold=, though). It might make sense as just an alternate setting of merge.renames or diff.renames, though it's possible that could get confusing with "copy" being an option. #leftoverbits for someone that wants to figure out what the option names and values should be?
Re: [PATCH] rebase docs: fix "gitlink" typo
On Thu, 28 Feb 2019 at 03:44, Kyle Meyer wrote: > Change it to "linkgit" so that the reference is properly rendered. > have `` as direct ancestor will keep their original branch point, > -i.e. commits that would be excluded by gitlink:git-log[1]'s > +i.e. commits that would be excluded by linkgit:git-log[1]'s > `--ancestry-path` option will keep their original ancestry by default. If Heh, I stumbled on this a few days ago, and have this exact patch in my w-i-p series. I found it interesting that both Asciidoctor and AsciiDoc trip on this in quite different ways. The patch is correct and tested by me, FWIW. Martin
[BUG] git-am: all colons in the beginning of a subject are lost
Hi! If there are colons in the beginning of a patch subject line `git-am' will drop them. Consider the following patch: $ cat 0001-four-colons-prepended.patch From e8213a2d10a61c9dc75521d88d656b8d5330e6bb Mon Sep 17 00:00:00 2001 From: Max Filenko Date: Tue, 12 Feb 2019 12:21:21 +0100 Subject: [PATCH] four colons prepended --- file.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/file.txt b/file.txt index 4dd1ef7..b5da95d 100644 --- a/file.txt +++ b/file.txt @@ -1 +1 @@ -This is a file. +This is a plain text file. -- 2.17.1 There will be no colons in the beginning of a commit message if I apply this patch: $ git am 0001-four-colons-prepended.patch Applying: four colons prepended The four colons already gone in the log message above. There are neither no colons in the commit subject line: $ git show commit 6341a6a2872f850ecb376c268b1b3bae54a6a74f (HEAD -> master) Author: Max Filenko Date: Tue Feb 12 12:21:21 2019 +0100 four colons prepended diff --git a/file.txt b/file.txt index 4dd1ef7..b5da95d 100644 --- a/file.txt +++ b/file.txt @@ -1 +1 @@ -This is a file. +This is a plain text file. I was able to reproduce this with git 2.17.1 on Ubuntu 18.04.2 LTS as well as with git 2.17.2 (Apple Git-113) on macOS 10.14.3. I was able to trace this down to . It seems like there are no colons already in the `state->msg' which to my understanding is being filled by `read_commit_msg()' function. I would really appreciate a hand on debugging it further. I'm re-submitting this bug report because the original one [1] wasn't really noticed. Hopefully, it's just because I've missed the proper prefix in my email's subject line :) [1]: http://public-inbox.org/git/m2lg2lxmmm.fsf@bouncer.i-did-not-set--mail-host-address--so-tickle-me/ -- Best, Max
Re: git rebase: retain original head?
Hi Nazri,
On Wed, 27 Feb 2019, Nazri Ramliy wrote:
> On Wed, Jan 9, 2019 at 10:08 PM Johannes Schindelin
> wrote:
Oh wow. Better late than never, eh?
> > Having said that, it is an unintended regression in the built-in
> > rebase. Markus, could you come up with a minimal test case, preferably
> > in the form of a patch to t/t3415-rebase-autosquash.sh?
>
> Something like this, perhaps? ("gmail converts tabs to spaces" caveat
> applies to the diff formatting):
>
> --8<--
> diff --git t/t3400-rebase.sh t/t3400-rebase.sh
> index 3e73f7584c..cb55597a8b 100755
> --- t/t3400-rebase.sh
> +++ t/t3400-rebase.sh
> @@ -59,6 +59,13 @@ test_expect_success 'rebase against master' '
> git rebase master
> '
>
> +test_expect_success 'rebase sets ORIG_HEAD' '
> + echo Add B. > expect &&
> + echo Modify A. >> expect &&
> + git log --oneline --format=%s ORIG_HEAD.. > actual &&
> + test_cmp expect actual
> +'
> +
> test_expect_success 'rebase, with and specified as
> :/quuxery' '
> test_when_finished "git branch -D torebase" &&
> git checkout -b torebase my-topic-branch^ &&
> -->8--
I made it a little more stand-alone, and can confirm that this is now
safely a regression in v2.21.0.
> Bisect shows that the first bad commit is this one:
>
> commit 21853626eac565dd42572d90724b29863f61eb3b
> Author: Johannes Schindelin
> Date: Fri Jan 18 07:09:27 2019 -0800
>
> built-in rebase: call `git am` directly
>
> I verified that by undoing the crux of that commit, and that fixes the
> failing test:
>
> -->8--
> diff --git builtin/rebase.c builtin/rebase.c
> index 7c7bc13e91..848f6740a0 100644
> --- builtin/rebase.c
> +++ builtin/rebase.c
> @@ -728,11 +728,6 @@ static int run_specific_rebase(struct rebase_options
> *opts)
> goto finished_rebase;
> }
>
> - if (opts->type == REBASE_AM) {
> - status = run_am(opts);
> - goto finished_rebase;
> - }
> -
> add_var(&script_snippet, "GIT_DIR", absolute_path(get_git_dir()));
> add_var(&script_snippet, "state_dir", opts->state_dir);
>
> --8<--
>
> But something seems off by my bisection in that the "bad" commit
> happens about 10 days after this email thread :/
Yep, I had fixed the issue before this commit, and a regression test would
have come in handy to prevent that regression from creeping in *again*.
Anyway, I'm on it.
Johannes
P.S.: If you want to follow my progress, I'll push updates to the
rebase-am-and-orig-head branch at https://github.com/dscho/git.
[PATCH v4 0/1] [GSoC][PATCH] t3600: use test_path_is_* helper functions
Replace test -(d|f|e) calls in t3600-rm.sh. Previously we were using test -(d|f|e) to verify the presence of a directory/file, but we already have helper functions, viz, test_path_is_dir, test_path_is_file and test_path_is_missing with better functionality. Rohit Ashiwal (1): t3600: use test_path_is_* functions t/t3600-rm.sh | 160 ++ 1 file changed, 84 insertions(+), 76 deletions(-) base-commit: 8104ec994ea3849a968b4667d072fedd1e688642 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-152%2Fr1walz%2Frefactor-tests-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-152/r1walz/refactor-tests-v4 Pull-Request: https://github.com/gitgitgadget/git/pull/152 Range-diff vs v3: 1: bfeba25c88 ! 1: f881f01e4f t3600: use test_path_is_* functions @@ -20,11 +20,48 @@ --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ + " test_expect_success \ - 'Post-check that bar does not exist and is not in index after "git rm -f bar"' \ +-'Pre-check that foo exists and is in index before git rm foo' \ +-'[ -f foo ] && git ls-files --error-unmatch foo' ++'Pre-check that foo exists and is in index before git rm foo' ' ++ test_path_is_file foo && ++ git ls-files --error-unmatch foo ++' + + test_expect_success \ + 'Test that git rm foo succeeds' \ +@@ + git rm --cached -f foo' + + test_expect_success \ +-'Post-check that foo exists but is not in index after git rm foo' \ +-'[ -f foo ] && test_must_fail git ls-files --error-unmatch foo' ++'Post-check that foo exists but is not in index after git rm foo' ' ++ test_path_is_file foo && ++ test_must_fail git ls-files --error-unmatch foo ++' + + test_expect_success \ +-'Pre-check that bar exists and is in index before "git rm bar"' \ +-'[ -f bar ] && git ls-files --error-unmatch bar' ++'Pre-check that bar exists and is in index before "git rm bar"' ' ++ test_path_is_file bar && ++ git ls-files --error-unmatch bar ++' + + test_expect_success \ + 'Test that "git rm bar" succeeds' \ + 'git rm bar' + + test_expect_success \ +-'Post-check that bar does not exist and is not in index after "git rm -f bar"' \ -'! [ -f bar ] && test_must_fail git ls-files --error-unmatch bar' -+'test_path_is_missing bar && test_must_fail git ls-files --error-unmatch bar' ++'Post-check that bar does not exist and is not in index after "git rm -f bar"' ' ++ test_path_is_missing bar && ++ test_must_fail git ls-files --error-unmatch bar ++' test_expect_success \ 'Test that "git rm -- -q" succeeds (remove a file that looks like an option)' \ -- gitgitgadget
[PATCH v4 1/1] t3600: use test_path_is_* functions
From: Rohit Ashiwal Replace `test -(d|f|e)` calls in t3600-rm.sh Previously we were using `test -(d|f|e)` to verify the presence of a directory/file, but we already have helper functions, viz, `test_path_is_dir`, `test_path_is_file` and `test_path_is_missing` with better functionality. These helper functions make code more readable and informative to someone new to code, also these functions have better error messages Signed-off-by: Rohit Ashiwal --- t/t3600-rm.sh | 160 ++ 1 file changed, 84 insertions(+), 76 deletions(-) diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 04e5d42bd3..3a5bd97df7 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -27,8 +27,10 @@ embedded' && " test_expect_success \ -'Pre-check that foo exists and is in index before git rm foo' \ -'[ -f foo ] && git ls-files --error-unmatch foo' +'Pre-check that foo exists and is in index before git rm foo' ' +test_path_is_file foo && +git ls-files --error-unmatch foo +' test_expect_success \ 'Test that git rm foo succeeds' \ @@ -70,20 +72,26 @@ test_expect_success \ git rm --cached -f foo' test_expect_success \ -'Post-check that foo exists but is not in index after git rm foo' \ -'[ -f foo ] && test_must_fail git ls-files --error-unmatch foo' +'Post-check that foo exists but is not in index after git rm foo' ' +test_path_is_file foo && +test_must_fail git ls-files --error-unmatch foo +' test_expect_success \ -'Pre-check that bar exists and is in index before "git rm bar"' \ -'[ -f bar ] && git ls-files --error-unmatch bar' +'Pre-check that bar exists and is in index before "git rm bar"' ' +test_path_is_file bar && +git ls-files --error-unmatch bar +' test_expect_success \ 'Test that "git rm bar" succeeds' \ 'git rm bar' test_expect_success \ -'Post-check that bar does not exist and is not in index after "git rm -f bar"' \ -'! [ -f bar ] && test_must_fail git ls-files --error-unmatch bar' +'Post-check that bar does not exist and is not in index after "git rm -f bar"' ' +test_path_is_missing bar && +test_must_fail git ls-files --error-unmatch bar +' test_expect_success \ 'Test that "git rm -- -q" succeeds (remove a file that looks like an option)' \ @@ -137,15 +145,15 @@ test_expect_success 'Re-add foo and baz' ' test_expect_success 'Modify foo -- rm should refuse' ' echo >>foo && test_must_fail git rm foo baz && - test -f foo && - test -f baz && + test_path_is_file foo && + test_path_is_file baz && git ls-files --error-unmatch foo baz ' test_expect_success 'Modified foo -- rm -f should work' ' git rm -f foo baz && - test ! -f foo && - test ! -f baz && + test_path_is_missing foo && + test_path_is_missing baz && test_must_fail git ls-files --error-unmatch foo && test_must_fail git ls-files --error-unmatch bar ' @@ -159,15 +167,15 @@ test_expect_success 'Re-add foo and baz for HEAD tests' ' test_expect_success 'foo is different in index from HEAD -- rm should refuse' ' test_must_fail git rm foo baz && - test -f foo && - test -f baz && + test_path_is_file foo && + test_path_is_file baz && git ls-files --error-unmatch foo baz ' test_expect_success 'but with -f it should work.' ' git rm -f foo baz && - test ! -f foo && - test ! -f baz && + test_path_is_missing foo && + test_path_is_missing baz && test_must_fail git ls-files --error-unmatch foo && test_must_fail git ls-files --error-unmatch baz ' @@ -194,21 +202,21 @@ test_expect_success 'Recursive test setup' ' test_expect_success 'Recursive without -r fails' ' test_must_fail git rm frotz && - test -d frotz && - test -f frotz/nitfol + test_path_is_dir frotz && + test_path_is_file frotz/nitfol ' test_expect_success 'Recursive with -r but dirty' ' echo qfwfq >>frotz/nitfol && test_must_fail git rm -r frotz && - test -d frotz && - test -f frotz/nitfol + test_path_is_dir frotz && + test_path_is_file frotz/nitfol ' test_expect_success 'Recursive with -r -f' ' git rm -f -r frotz && - ! test -f frotz/nitfol && - ! test -d frotz + test_path_is_missing frotz/nitfol && + test_path_is_missing frotz ' test_expect_success 'Remove nonexistent file returns nonzero exit status' ' @@ -232,7 +240,7 @@ test_expect_success 'refresh index before checking if it is up-to-date' ' git reset --hard && test-tool chmtime -86400 frotz/nitfol && git rm frotz/nitfol && - test ! -f frotz/nitfol + test_path_is_missing frotz/nitfol ' @@ -254,7 +262,7 @@ test_expect_success 'rm removes subdirectories recursively' ' echo content >dir/subdir/subsubdir/file &&
Re: [gitgitgadget/git] [GSoC][PATCH] t3600: `use test_path_is_*` helper functions (#152)
Hey Duy > > On the Git mailing list, Duy Nguyen wrote (reply to this): > > I was a bit worried that the "test ! something" could be incorrectly > converted because for example, "test ! -d foo" is not always the same > as "test_path_is_missing". If "foo" is intended to be a file, then the > conversion is wrong. > > But I don't think you made any wrong conversion here. All these > negative "test" are preceded by "git rm" so the expectation is always > "test ! -e". > Sorry for the late reply. Yes, I thought about it earlier and made changes thinking this only. Also when I was going through the code again, I replaced other conditionals `[ -f ]` with test_path_is_file etc. Ciao Rohit
[no subject]
Hey Duy Sorry for late reply. > > On the Git mailing list, Duy Nguyen wrote (reply to this): > > I was a bit worried that the "test ! something" could be incorrectly > converted because for example, "test ! -d foo" is not always the same > as "test_path_is_missing". If "foo" is intended to be a file, then the > conversion is wrong. > > But I don't think you made any wrong conversion here. All these > negative "test" are preceded by "git rm" so the expectation is always > "test ! -e". > Yes, I thought about it earlier and made changes thinking this only. Also when I was going through the code again, I replaced other conditionals `[ -f ]` with test_path_is_file etc. Ciao Rohit
Re: [BUG] git-am: all colons in the beginning of a subject are lost
On Thu, Feb 28, 2019 at 11:02:11AM +0100, Max Filenko wrote: > Subject: [PATCH] four colons prepended > [...] > There will be no colons in the beginning of a commit message if I apply > this patch: > > $ git am 0001-four-colons-prepended.patch > Applying: four colons prepended I suspect this has to do with the sanitization that happens as part of removing "[PATCH]". Note that if you use "-k" (to preserve the subject) it doesn't happen, though of course you also get "[PATCH]" then. If you want to pass the subject lines through verbatim, use "-k" with both format-patch and git-am. > I was able to trace this down to . It seems like there are > no colons already in the `state->msg' which to my understanding is being > filled by `read_commit_msg()' function. I would really appreciate a hand > on debugging it further. It's probably easier to debug with git-mailinfo, which has the same behavior: $ git mailinfo msg patch <0001-four-colons-prepended.patch Author: Jeff King Email: p...@peff.net Subject: four colons prepended Date: Thu, 28 Feb 2019 06:12:50 -0500 and is based on the same routines. The contents are preserved until we end up in mailinfo.c's cleanup_subject(). And there leading colons are explicitly removed: case ' ': case '\t': case ':': strbuf_remove(subject, at, 1); continue; That behavior goes all the way back to 2744b2344d (Start of early patch applicator tools for git., 2005-04-11), when Git was only 4 days old. Since it also handles cruft like "Re:", I suspect the goal there was I suspect the goal there was to remove cruft like "Re" or "Re: :" which sometimes happens. I don't know if anybody would complain if we were more careful about leaving lone colons that weren't part of a "Re" chain. -Peff
aviso final da microsoft
<<< No Message Collected >>>
[PATCH 3/4] built-in rebase: demonstrate that ORIG_HEAD is not set correctly
From: Johannes Schindelin The ORIG_HEAD pseudo ref is supposed to refer to the original, pre-rebase state after a successful rebase. Let's add a regression test to prove that this regressed: With GIT_TEST_REBASE_USE_BUILTIN=false, this test case passes, with GIT_TEST_REBASE_USE_BUILTIN=true (or unset), it fails. Reported by Nazri Ramliy. Signed-off-by: Johannes Schindelin --- t/t3400-rebase.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index 3e73f7584c..7e8d5bb200 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -59,6 +59,14 @@ test_expect_success 'rebase against master' ' git rebase master ' +test_expect_failure 'rebase sets ORIG_HEAD to pre-rebase state' ' + git checkout -b orig-head topic && + pre="$(git rev-parse --verify HEAD)" && + git rebase master && + test_cmp_rev "$pre" ORIG_HEAD && + ! test_cmp_rev "$pre" HEAD +' + test_expect_success 'rebase, with and specified as :/quuxery' ' test_when_finished "git branch -D torebase" && git checkout -b torebase my-topic-branch^ && -- gitgitgadget
[PATCH 4/4] built-in rebase: set ORIG_HEAD just once, before the rebase
From: Johannes Schindelin
Technically, the scripted version set ORIG_HEAD only in two spots (which
really could have been one, because it called `git checkout $onto^0` to
start the rebase and also if it could take a shortcut, and in both cases
it called `git update-ref $orig_head`).
Practically, it *implicitly* reset ORIG_HEAD whenever `git reset --hard`
was called.
However, what we really want is that it is set exactly once, at the
beginning of the rebase.
So let's do that.
Signed-off-by: Johannes Schindelin
---
builtin/rebase.c | 31 ++-
t/t3400-rebase.sh | 2 +-
2 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index aa469ec964..0f4e1ead49 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -369,6 +369,7 @@ static void add_var(struct strbuf *buf, const char *name,
const char *value)
#define RESET_HEAD_DETACH (1<<0)
#define RESET_HEAD_HARD (1<<1)
#define RESET_HEAD_REFS_ONLY (1<<2)
+#define RESET_ORIG_HEAD (1<<3)
static int reset_head(struct object_id *oid, const char *action,
const char *switch_to_branch, unsigned flags,
@@ -377,6 +378,7 @@ static int reset_head(struct object_id *oid, const char
*action,
unsigned detach_head = flags & RESET_HEAD_DETACH;
unsigned reset_hard = flags & RESET_HEAD_HARD;
unsigned refs_only = flags & RESET_HEAD_REFS_ONLY;
+ unsigned update_orig_head = flags & RESET_ORIG_HEAD;
struct object_id head_oid;
struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
@@ -453,18 +455,21 @@ static int reset_head(struct object_id *oid, const char
*action,
strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : "rebase");
prefix_len = msg.len;
- if (!get_oid("ORIG_HEAD", &oid_old_orig))
- old_orig = &oid_old_orig;
- if (!get_oid("HEAD", &oid_orig)) {
- orig = &oid_orig;
- if (!reflog_orig_head) {
- strbuf_addstr(&msg, "updating ORIG_HEAD");
- reflog_orig_head = msg.buf;
- }
- update_ref(reflog_orig_head, "ORIG_HEAD", orig, old_orig, 0,
- UPDATE_REFS_MSG_ON_ERR);
- } else if (old_orig)
- delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
+ if (update_orig_head) {
+ if (!get_oid("ORIG_HEAD", &oid_old_orig))
+ old_orig = &oid_old_orig;
+ if (!get_oid("HEAD", &oid_orig)) {
+ orig = &oid_orig;
+ if (!reflog_orig_head) {
+ strbuf_addstr(&msg, "updating ORIG_HEAD");
+ reflog_orig_head = msg.buf;
+ }
+ update_ref(reflog_orig_head, "ORIG_HEAD", orig,
+ old_orig, 0, UPDATE_REFS_MSG_ON_ERR);
+ } else if (old_orig)
+ delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
+ }
+
if (!reflog_head) {
strbuf_setlen(&msg, prefix_len);
strbuf_addstr(&msg, "updating HEAD");
@@ -1725,7 +1730,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
strbuf_addf(&msg, "%s: checkout %s",
getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
if (reset_head(&options.onto->object.oid, "checkout", NULL,
- RESET_HEAD_DETACH, NULL, msg.buf))
+ RESET_HEAD_DETACH | RESET_ORIG_HEAD, NULL, msg.buf))
die(_("Could not detach HEAD"));
strbuf_release(&msg);
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 7e8d5bb200..460d0523be 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -59,7 +59,7 @@ test_expect_success 'rebase against master' '
git rebase master
'
-test_expect_failure 'rebase sets ORIG_HEAD to pre-rebase state' '
+test_expect_success 'rebase sets ORIG_HEAD to pre-rebase state' '
git checkout -b orig-head topic &&
pre="$(git rev-parse --verify HEAD)" &&
git rebase master &&
--
gitgitgadget
[PATCH 0/4] Fix ORIG_HEAD behavior of the built-in rebase
It was reported by Nazri Ramliy that ORIG_HEAD was set incorrectly again, this time caused by the shortcut to call git am directly, without detouring to a Unix shell script. Patch 2/4 might look like something completely unrelated, but without it, the update to HEAD might use an incorrect reflog message. Patch 1/4 is more a "while at it" patch; while looking which code paths might need to update ORIG_HEAD, I noticed that this reset_head() call did unnecessary work. Johannes Schindelin (4): built-in rebase: no need to check out `onto` twice built-in rebase: use the correct reflog when switching branches built-in rebase: demonstrate that ORIG_HEAD is not set correctly built-in rebase: set ORIG_HEAD just once, before the rebase builtin/rebase.c | 37 + t/t3400-rebase.sh | 8 2 files changed, 29 insertions(+), 16 deletions(-) base-commit: 21853626eac565dd42572d90724b29863f61eb3b Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-153%2Fdscho%2Frebase-am-and-orig-head-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-153/dscho/rebase-am-and-orig-head-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/153 -- gitgitgadget
[PATCH 2/4] built-in rebase: use the correct reflog when switching branches
From: Johannes Schindelin
By mistake, we used the reflog intended for ORIG_HEAD.
Signed-off-by: Johannes Schindelin
---
builtin/rebase.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 813ec284ca..aa469ec964 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -475,7 +475,7 @@ static int reset_head(struct object_id *oid, const char
*action,
detach_head ? REF_NO_DEREF : 0,
UPDATE_REFS_MSG_ON_ERR);
else {
- ret = update_ref(reflog_orig_head, switch_to_branch, oid,
+ ret = update_ref(reflog_head, switch_to_branch, oid,
NULL, 0, UPDATE_REFS_MSG_ON_ERR);
if (!ret)
ret = create_symref("HEAD", switch_to_branch,
--
gitgitgadget
[PATCH 1/4] built-in rebase: no need to check out `onto` twice
From: Johannes Schindelin In the case that the rebase boils down to a fast-forward, the built-in rebase reset the working tree twice: once to start the rebase at `onto`, then realizing that the original HEAD was an ancestor, `reset_head()` was called to update the original ref and to point HEAD back to it. That second `reset_head()` call does not need to touch the working tree, though, as it does not change the actual tip commit. So let's avoid that unnecessary work. Signed-off-by: Johannes Schindelin --- builtin/rebase.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 08ec4d52c7..813ec284ca 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1740,8 +1740,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) strbuf_addf(&msg, "rebase finished: %s onto %s", options.head_name ? options.head_name : "detached HEAD", oid_to_hex(&options.onto->object.oid)); - reset_head(NULL, "Fast-forwarded", options.head_name, 0, - "HEAD", msg.buf); + reset_head(NULL, "Fast-forwarded", options.head_name, + RESET_HEAD_REFS_ONLY, "HEAD", msg.buf); strbuf_release(&msg); ret = !!finish_rebase(&options); goto cleanup; -- gitgitgadget
[GSoC] acknowledging mistakes
Hey people I had a discussion with Rafael over the #git irc channel and Thanks to him I was able to find these minute mistakes: 1. Commit message was less than 50 chars which should be around 72 chars according to coding guide lines. Should I change this to match 72? 2. My changes had some uneven use of tabs and spaces, which I made considering that pre-existing code had them too. Is there a possibility to change the whole code according to CodingGuidelines? If yes should I only change my code according to guidelines or the whole file? 3. There is no helper function for `test -s` but Rafael suggested we can make use of other helper functions to provide similar functionality, if we can. Open to suggestions and debate. These will be fixed in next revision accordingly. Thanks Rohit
[PATCH 0/1] Retire the "let Travis trigger a Windows build" hack
We have something much better now: a real Azure Pipeline. Not only is it a lot faster (due to parallelizing the test suite), it also won't time out waiting for the Windows job to start. Johannes Schindelin (1): travis: remove the hack to build the Windows job on Azure Pipelines .travis.yml | 10 ci/run-windows-build.sh | 103 2 files changed, 113 deletions(-) delete mode 100755 ci/run-windows-build.sh base-commit: 8104ec994ea3849a968b4667d072fedd1e688642 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-154%2Fdscho%2Fremove-travis-windows-hack-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-154/dscho/remove-travis-windows-hack-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/154 -- gitgitgadget
[PATCH 1/1] travis: remove the hack to build the Windows job on Azure Pipelines
From: Johannes Schindelin
Since Travis did not support Windows (and now only supports very limited
Windows jobs, too limited for our use, the test suite would time out
*all* the time), we added a hack where a Travis job would trigger an
Azure Pipeline (which back then was still called VSTS Build), wait for
it to finish (or time out), and download the log (if available).
Needless to say that it was a horrible hack, necessitated by a bad
situation.
Nowadays, however, we have Azure Pipelines support, and do not need that
hack anymore. So let's retire it.
Signed-off-by: Johannes Schindelin
---
.travis.yml | 10
ci/run-windows-build.sh | 103
2 files changed, 113 deletions(-)
delete mode 100755 ci/run-windows-build.sh
diff --git a/.travis.yml b/.travis.yml
index 36cbdea7f4..ffb1bc46f2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -21,16 +21,6 @@ matrix:
compiler:
addons:
before_install:
-- env: jobname=Windows
- os: linux
- compiler:
- addons:
- before_install:
- script:
-- >
- test "$TRAVIS_REPO_SLUG" != "git/git" ||
- ci/run-windows-build.sh $TRAVIS_BRANCH $(git rev-parse HEAD)
- after_failure:
- env: jobname=Linux32
os: linux
compiler:
diff --git a/ci/run-windows-build.sh b/ci/run-windows-build.sh
deleted file mode 100755
index a73a4eca0a..00
--- a/ci/run-windows-build.sh
+++ /dev/null
@@ -1,103 +0,0 @@
-#!/usr/bin/env bash
-#
-# Script to trigger the Git for Windows build and test run.
-# Set the $GFW_CI_TOKEN as environment variable.
-# Pass the branch (only branches on https://github.com/git/git are
-# supported) and a commit hash.
-#
-
-. ${0%/*}/lib.sh
-
-test $# -ne 2 && echo "Unexpected number of parameters" && exit 1
-test -z "$GFW_CI_TOKEN" && echo "GFW_CI_TOKEN not defined" && exit
-
-BRANCH=$1
-COMMIT=$2
-
-gfwci () {
- local CURL_ERROR_CODE HTTP_CODE
- CONTENT_FILE=$(mktemp -t "git-windows-ci-XX")
- while test -z $HTTP_CODE
- do
- HTTP_CODE=$(curl \
- -H "Authentication: Bearer $GFW_CI_TOKEN" \
- --silent --retry 5 --write-out '%{HTTP_CODE}' \
- --output >(sed "$(printf '1s/^\xef\xbb\xbf//')" >$CONTENT_FILE)
\
- "https://git-for-windows-ci.azurewebsites.net/api/TestNow?$1"; \
- )
- CURL_ERROR_CODE=$?
- # The GfW CI web app sometimes returns HTTP errors of
- # "502 bad gateway" or "503 service unavailable".
- # We also need to check the HTTP content because the GfW web
- # app seems to pass through (error) results from other Azure
- # calls with HTTP code 200.
- # Wait a little and retry if we detect this error. More info:
- #
https://docs.microsoft.com/en-in/azure/app-service-web/app-service-web-troubleshoot-http-502-http-503
- if test $HTTP_CODE -eq 502 ||
- test $HTTP_CODE -eq 503 ||
- grep "502 - Web server received an invalid response"
$CONTENT_FILE >/dev/null
- then
- sleep 10
- HTTP_CODE=
- fi
- done
- cat $CONTENT_FILE
- rm $CONTENT_FILE
- if test $CURL_ERROR_CODE -ne 0
- then
- return $CURL_ERROR_CODE
- fi
- if test "$HTTP_CODE" -ge 400 && test "$HTTP_CODE" -lt 600
- then
- return 127
- fi
-}
-
-# Trigger build job
-BUILD_ID=$(gfwci
"action=trigger&branch=$BRANCH&commit=$COMMIT&skipTests=false")
-if test $? -ne 0
-then
- echo "Unable to trigger Visual Studio Team Services Build"
- echo "$BUILD_ID"
- exit 1
-fi
-
-# Check if the $BUILD_ID contains a number
-case $BUILD_ID in
-''|*[!0-9]*) echo "Unexpected build number: $BUILD_ID" && exit 1
-esac
-
-echo "Visual Studio Team Services Build #${BUILD_ID}"
-
-# Tracing execued commands would produce too much noise in the waiting
-# loop below.
-set +x
-
-# Wait until build job finished
-STATUS=
-RESULT=
-while true
-do
- LAST_STATUS=$STATUS
- STATUS=$(gfwci "action=status&buildId=$BUILD_ID")
- test "$STATUS" = "$LAST_STATUS" || printf "\nStatus: %s " "$STATUS"
- printf "."
-
- case "$STATUS" in
- inProgress|postponed|notStarted) sleep 10 ;; # continue
-"completed: succeeded") RESULT="success"; break;; # success
- "completed: failed") break;; # failure
- *) echo "Unhandled status: $STATUS"; break;; # unknown
- esac
-done
-
-# Print log
-echo ""
-echo ""
-set -x
-gfwci "action=log&buildId=$BUILD_ID" | cut -c 30-
-
-# Set exit code for TravisCI
-test "$RESULT" = "success"
-
-save_good_tree
--
gitgitgadget
[PATCH v4 2/2] setup: fix memory leaks with `struct repository_format`
After we set up a `struct repository_format`, it owns various pieces of
allocated memory. We then either use those members, because we decide we
want to use the "candidate" repository format, or we discard the
candidate / scratch space. In the first case, we transfer ownership of
the memory to a few global variables. In the latter case, we just
silently drop the struct and end up leaking memory.
Introduce an initialization macro `REPOSITORY_FORMAT_INIT` and a
function `clear_repository_format()`, to be used on each side of
`read_repository_format()`. To have a clear and simple memory ownership,
let all users of `struct repository_format` duplicate the strings that
they take from it, rather than stealing the pointers.
Call `clear_...()` at the start of `read_...()` instead of just zeroing
the struct, since we sometimes enter the function multiple times. Thus,
it is important to initialize the struct before calling `read_...()`, so
document that. It's also important because we might not even call
`read_...()` before we call `clear_...()`, see, e.g., builtin/init-db.c.
Teach `read_...()` to clear the struct on error, so that it is reset to
a safe state, and document this. (In `setup_git_directory_gently()`, we
look at `repo_fmt.hash_algo` even if `repo_fmt.version` is -1, which we
weren't actually supposed to do per the API. After this commit, that's
ok.)
We inherit the existing code's combining "error" and "no version found".
Both are signalled through `version == -1` and now both cause us to
clear any partial configuration we have picked up. For "extensions.*",
that's fine, since they require a positive version number. For
"core.bare" and "core.worktree", we're already verifying that we have a
non-negative version number before using them.
Signed-off-by: Martin Ågren
---
cache.h | 31 ---
builtin/init-db.c | 3 ++-
repository.c | 3 ++-
setup.c | 39 +++
worktree.c| 4 +++-
5 files changed, 62 insertions(+), 18 deletions(-)
diff --git a/cache.h b/cache.h
index ca36b44ee0..8c32c904c3 100644
--- a/cache.h
+++ b/cache.h
@@ -961,6 +961,10 @@ extern char *repository_format_partial_clone;
extern const char *core_partial_clone_filter_default;
extern int repository_format_worktree_config;
+/*
+ * You _have_ to initialize a `struct repository_format` using
+ * `= REPOSITORY_FORMAT_INIT` before calling `read_repository_format()`.
+ */
struct repository_format {
int version;
int precious_objects;
@@ -972,14 +976,35 @@ struct repository_format {
struct string_list unknown_extensions;
};
+/*
+ * Always use this to initialize a `struct repository_format`
+ * to a well-defined, default state before calling
+ * `read_repository()`.
+ */
+#define REPOSITORY_FORMAT_INIT \
+{ \
+ .version = -1, \
+ .is_bare = -1, \
+ .hash_algo = GIT_HASH_SHA1, \
+ .unknown_extensions = STRING_LIST_INIT_DUP, \
+}
+
/*
* Read the repository format characteristics from the config file "path" into
- * "format" struct. Returns the numeric version. On error, -1 is returned,
- * format->version is set to -1, and all other fields in the struct are
- * undefined.
+ * "format" struct. Returns the numeric version. On error, or if no version is
+ * found in the configuration, -1 is returned, format->version is set to -1,
+ * and all other fields in the struct are set to the default configuration
+ * (REPOSITORY_FORMAT_INIT). Always initialize the struct using
+ * REPOSITORY_FORMAT_INIT before calling this function.
*/
int read_repository_format(struct repository_format *format, const char *path);
+/*
+ * Free the memory held onto by `format`, but not the struct itself.
+ * (No need to use this after `read_repository_format()` fails.)
+ */
+void clear_repository_format(struct repository_format *format);
+
/*
* Verify that the repository described by repository_format is something we
* can read. If it is, return 0. Otherwise, return -1, and "err" will describe
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 41faffd28d..04c60eaad5 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -96,7 +96,7 @@ static void copy_templates(const char *template_dir)
struct strbuf path = STRBUF_INIT;
struct strbuf template_path = STRBUF_INIT;
size_t template_len;
- struct repository_format template_format;
+ struct repository_format template_format = REPOSITORY_FORMAT_INIT;
struct strbuf err = STRBUF_INIT;
DIR *dir;
char *to_free = NULL;
@@ -148,6 +148,7 @@ static void copy_templates(const char *template_dir)
free(to_free);
strbuf_release(&path);
strbuf_release(&template_path);
+ clear_repository_format(&template_format);
}
static int git_init_db_config(const char *k, const char *v, void *cb)
diff --git a/repository.c b/repository.c
index 7b02e1dffa..df88705574 100644
--- a/repository.c
+++ b/r
[PATCH v4 0/2] setup: fix memory leaks with `struct repository_format`
This is a follow-up to v3 [1] from about a month ago. Patch 1 is
unchanged; patch 2 provides some additional documentation of the
initialization that is required, plus I've gotten rid of the compound
literal. Range-diff below.
Thanks Peff and brian for very helpful comments and discussion.
Martin
[1] https://public-inbox.org/git/cover.1548186510.git.martin.ag...@gmail.com/
Martin Ågren (2):
setup: free old value before setting `work_tree`
setup: fix memory leaks with `struct repository_format`
cache.h | 31 ---
builtin/init-db.c | 3 ++-
repository.c | 3 ++-
setup.c | 40
worktree.c| 4 +++-
5 files changed, 63 insertions(+), 18 deletions(-)
Range-diff against v3:
1: 13019979b8 = 1: 13019979b8 setup: free old value before setting `work_tree`
2: e0c4a73119 ! 2: b21704c1e4 setup: fix memory leaks with `struct
repository_format`
@@ -16,15 +16,16 @@
they take from it, rather than stealing the pointers.
Call `clear_...()` at the start of `read_...()` instead of just zeroing
-the struct, since we sometimes enter the function multiple times. This
-means that it is important to initialize the struct before calling
-`read_...()`, so document that. Teach `read_...()` to clear the struct
-upon an error, so that it is reset to a safe state, and document this.
+the struct, since we sometimes enter the function multiple times. Thus,
+it is important to initialize the struct before calling `read_...()`,
so
+document that. It's also important because we might not even call
+`read_...()` before we call `clear_...()`, see, e.g.,
builtin/init-db.c.
-About that very last point: In `setup_git_directory_gently()`, we look
-at `repo_fmt.hash_algo` even if `repo_fmt.version` is -1, which we
+Teach `read_...()` to clear the struct on error, so that it is reset to
+a safe state, and document this. (In `setup_git_directory_gently()`, we
+look at `repo_fmt.hash_algo` even if `repo_fmt.version` is -1, which we
weren't actually supposed to do per the API. After this commit, that's
-ok.
+ok.)
We inherit the existing code's combining "error" and "no version
found".
Both are signalled through `version == -1` and now both cause us to
@@ -34,11 +35,21 @@
non-negative version number before using them.
Signed-off-by: Martin Ågren
-Signed-off-by: Junio C Hamano
diff --git a/cache.h b/cache.h
--- a/cache.h
+++ b/cache.h
+@@
+ extern const char *core_partial_clone_filter_default;
+ extern int repository_format_worktree_config;
+
++/*
++ * You _have_ to initialize a `struct repository_format` using
++ * `= REPOSITORY_FORMAT_INIT` before calling `read_repository_format()`.
++ */
+ struct repository_format {
+ int version;
+ int precious_objects;
@@
struct string_list unknown_extensions;
};
@@ -146,8 +157,15 @@
}
return 0;
-@@
+ }
++static void init_repository_format(struct repository_format *format)
++{
++ const struct repository_format fresh = REPOSITORY_FORMAT_INIT;
++
++ memcpy(format, &fresh, sizeof(fresh));
++}
++
int read_repository_format(struct repository_format *format, const char
*path)
{
- memset(format, 0, sizeof(*format));
@@ -167,7 +185,7 @@
+ string_list_clear(&format->unknown_extensions, 0);
+ free(format->work_tree);
+ free(format->partial_clone);
-+ *format = (struct repository_format)REPOSITORY_FORMAT_INIT;
++ init_repository_format(format);
+}
+
int verify_repository_format(const struct repository_format *format,
--
2.21.0
[PATCH v4 1/2] setup: free old value before setting `work_tree`
Before assigning to `data->work_tree` in `read_worktree_config()`, free
any value we might already have picked up, so that we do not leak it.
Signed-off-by: Martin Ågren
Signed-off-by: Junio C Hamano
---
setup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/setup.c b/setup.c
index 1be5037f12..bb633942bb 100644
--- a/setup.c
+++ b/setup.c
@@ -411,6 +411,7 @@ static int read_worktree_config(const char *var, const char
*value, void *vdata)
} else if (strcmp(var, "core.worktree") == 0) {
if (!value)
return config_error_nonbool(var);
+ free(data->work_tree);
data->work_tree = xstrdup(value);
}
return 0;
--
2.21.0
Re: [PATCH 1/4] built-in rebase: no need to check out `onto` twice
Hi Johannes On 28/02/2019 15:27, Johannes Schindelin via GitGitGadget wrote: From: Johannes Schindelin In the case that the rebase boils down to a fast-forward, the built-in rebase reset the working tree twice: once to start the rebase at `onto`, then realizing that the original HEAD was an ancestor, `reset_head()` was called to update the original ref and to point HEAD back to it. That second `reset_head()` call does not need to touch the working tree, though, as it does not change the actual tip commit. So let's avoid that unnecessary work. I'm confused by this I think I must be missing something. If we've checked out onto then why does the working copy not need updating when we fast forward. (also why to we checkout onto before seeing if we can fast-forward but that's not related to this patch series) Best Wishes Phillip Signed-off-by: Johannes Schindelin --- builtin/rebase.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 08ec4d52c7..813ec284ca 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1740,8 +1740,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) strbuf_addf(&msg, "rebase finished: %s onto %s", options.head_name ? options.head_name : "detached HEAD", oid_to_hex(&options.onto->object.oid)); - reset_head(NULL, "Fast-forwarded", options.head_name, 0, - "HEAD", msg.buf); + reset_head(NULL, "Fast-forwarded", options.head_name, + RESET_HEAD_REFS_ONLY, "HEAD", msg.buf); strbuf_release(&msg); ret = !!finish_rebase(&options); goto cleanup;
Re: [PATCH] commit-tree: utilize parse-options api
On Wed, Feb 27, 2019 at 10:46:49PM -0400, Brandon Richardson wrote: > > If we are going to go this route, I think you might actually want macros > > that take both "unset" and "args" and make sure that we're not in a > > situation the callback doesn't expect (e.g., "!unset && !arg"). That > > lets us continue to declare those at the top of the callback. > > In doing a quick search, I found a fair number instances of this: > ... > BUG_ON_OPT_NEG(unset); > > if (!arg) > return -1; > ... Those are probably my fault. The originals guarded against an unexpected "unset" by checking "!arg" and returning an error. But it made the compiler's -Wunused-parameter complain, so I added the BUG_ON_OPT_NEG() calls as an assertion. At that point the "if (!arg)" could never trigger, and could have been removed. > So a macro like this could be useful. I've also found a few instances of this: > > BUG_ON_OPT_NEG(unset); > BUG_ON_OPT_ARG(arg); These ones are different. The second one is checking that "arg" _is_ NULL (i.e., we expect that the options struct provided the right flag to disallow an argument). And that's orthogonal to the unset flag, so it would not be right to conflate the two in a single macro. -Peff
Re: [PATCH RFC 01/20] cat-file: reuse struct ref_format
On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote:
> Start using ref_format struct instead of simple char*.
> Need that for further reusing of formatting logic from ref-filter.
Makes sense.
> struct batch_options {
> + struct ref_format format;
> int enabled;
> int follow_symlinks;
> int print_contents;
> @@ -24,7 +26,6 @@ struct batch_options {
> int all_objects;
> int unordered;
> int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
> - const char *format;
> };
Not a huge deal, but unless there's a compelling reason to move the
field around in the struct, the diff is easier to read if the deleted
and added lines stay in the same place.
> @@ -491,9 +492,6 @@ static int batch_objects(struct batch_options *opt)
> int save_warning;
> int retval = 0;
>
> - if (!opt->format)
> - opt->format = "%(objectname) %(objecttype) %(objectsize)";
> -
This assignment moves down to cmd_cat_file(). I don't see any reason
that shouldn't work, but it makes reviewing easier if there aren't
unexpected changes (so if it doesn't need moved in the grand scheme of
things, leave it as it was; if it does, it should either come in its own
patch, or get a note in the commit message as to why it needed to move).
-Peff
Re: [PATCH RFC 02/20] ref-filter: rename field in ref_array_item stuct
On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote: > Rename objectname field to oid in struct ref_array_item. > We usually use objectname word for string representation > of object id, so oid explains the content better. OK. I suspect the original was selected to match the %(objectname) placeholder. But I agree that "oid" is the more common variable name, and I think the connection between the placeholder and the variable should be pretty clear. -Peff
Re: [PATCH RFC 03/20] ref-filter: add rest formatting option
On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote: > Add rest option that allows to add string into ref_array_item > and then put it into specific place of the output. > We are using it now in cat-file command: user could put anything > in the input after objectname, and it will appear in the output > in place of %(rest). This would make: git for-each-ref --format='%(rest)' do something. But what (and could it ever be useful or meaningful)? Should we add an option to ref-filter to enable/disable this placeholder? -Peff
Re: [PATCH RFC 04/20] for-each-ref: tests for new atom %(rest) added
On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote: > Add tests for new formatting atom %(rest). > We need this atom for cat-file command. While I do normally encourage splitting up commits, in this case I think it would make sense to squash this together with patch 3. There's nothing to say here about what %(rest) is that isn't already said in that commit message. That said, I'm still not sure that for-each-ref should be supporting %(rest) at all. We should hopefully already have coverage of cat-file using "%(rest)" (and if not, we should add some to make sure it's not regressed by the conversion). -Peff
Re: [RFC PATCH v3 1/5] clone: test for our behavior on odd objects/* content
Hi, Ævar I'm finishing the required changes in this series to send a v4, but when submitting to travis ci, I got some errors on the t5604-clone-reference test: https://travis-ci.org/MatheusBernardino/git/builds/57587 Do you have any idea why? Best, Matheus Tavares On Tue, Feb 26, 2019 at 9:28 AM Ævar Arnfjörð Bjarmason wrote: > > Add tests for what happens when we locally clone .git/objects > directories where some of the loose objects or packs are symlinked, or > when when there's unknown files there. > > I'm bending over backwards here to avoid a SHA1 dependency. See [1] > for an earlier and simpler version that hardcoded a SHA-1s. > > This behavior has been the same for a *long* time, but hasn't been > tested for. > > There's a good post-hoc argument to be made for copying over unknown > things, e.g. I'd like a git version that doesn't know about the > commit-graph to copy it under "clone --local" so a newer git version > can make use of it. > > But the behavior showed where with symlinks seems pretty > random. E.g. if "pack" is a symlink we end up with two copies of the > contents, and only transfer some symlinks as-is. > > In follow-up commits we'll look at changing some of this behavior, but > for now let's just assert it as-is so we'll notice what we'll change > later. > > 1. https://public-inbox.org/git/20190226002625.13022-5-ava...@gmail.com/ > > Signed-off-by: Ævar Arnfjörð Bjarmason > --- > t/t5604-clone-reference.sh | 142 + > 1 file changed, 142 insertions(+) > > diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh > index 4320082b1b..cb0dc22d14 100755 > --- a/t/t5604-clone-reference.sh > +++ b/t/t5604-clone-reference.sh > @@ -221,4 +221,146 @@ test_expect_success 'clone, dissociate from alternates' > ' > ( cd C && git fsck ) > ' > > +test_expect_success 'setup repo with garbage in objects/*' ' > + git init S && > + ( > + cd S && > + test_commit A && > + > + cd .git/objects && > + >.some-hidden-file && > + >some-file && > + mkdir .some-hidden-dir && > + >.some-hidden-dir/some-file && > + >.some-hidden-dir/.some-dot-file && > + mkdir some-dir && > + >some-dir/some-file && > + >some-dir/.some-dot-file > + ) > +' > + > +test_expect_success 'clone a repo with garbage in objects/*' ' > + for option in --local --no-hardlinks --shared --dissociate > + do > + git clone $option S S$option || return 1 && > + git -C S$option fsck || return 1 > + done && > + find S-* -name "*some*" | sort >actual && > + cat >expected <<-EOF && > + S--dissociate/.git/objects/.some-hidden-file > + S--dissociate/.git/objects/some-dir > + S--dissociate/.git/objects/some-dir/.some-dot-file > + S--dissociate/.git/objects/some-dir/some-file > + S--dissociate/.git/objects/some-file > + S--local/.git/objects/.some-hidden-file > + S--local/.git/objects/some-dir > + S--local/.git/objects/some-dir/.some-dot-file > + S--local/.git/objects/some-dir/some-file > + S--local/.git/objects/some-file > + S--no-hardlinks/.git/objects/.some-hidden-file > + S--no-hardlinks/.git/objects/some-dir > + S--no-hardlinks/.git/objects/some-dir/.some-dot-file > + S--no-hardlinks/.git/objects/some-dir/some-file > + S--no-hardlinks/.git/objects/some-file > + EOF > + test_cmp expected actual > +' > + > +test_expect_success SYMLINKS 'setup repo with manually symlinked objects/*' ' > + git init T && > + ( > + cd T && > + test_commit A && > + git gc && > + ( > + cd .git/objects && > + mv pack packs && > + ln -s packs pack > + ) && > + test_commit B && > + ( > + cd .git/objects && > + find ?? -type d >loose-dirs && > + last_loose=$(tail -n 1 loose-dirs) && > + mv $last_loose a-loose-dir && > + ln -s a-loose-dir $last_loose && > + first_loose=$(head -n 1 loose-dirs) && > + ( > + cd $first_loose && > + obj=$(ls *) && > + mv $obj ../an-object && > + ln -s ../an-object $obj > + ) && > + find . -type f | sort >../../../T.objects-files.raw && > + find . -type l | sort >../../../T.objects-links.raw > + ) > + ) && > + git -C T fsck && > + git -C T rev-list --all --objects >T.objects > +' > + > + > +test_expect_success SYMLINKS 'clone
Re: [PATCH RFC 05/20] cat-file: remove split_on_whitespace
On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote:
> Get rid of split_on_whitespace field in struct expand_data.
> expand_data may be global further as we use it in ref-filter also,
> so we need to remove cat-file specific fields from it.
OK, that makes some sense.
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index e5de596611800..60f3839b06f8c 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -203,13 +203,6 @@ struct expand_data {
>*/
> int mark_query;
>
> - /*
> - * Whether to split the input on whitespace before feeding it to
> - * get_sha1; this is decided during the mark_query phase based on
> - * whether we have a %(rest) token in our format.
> - */
> - int split_on_whitespace;
It looks like we lose this name and comment in the movement, though
(it's now "is_rest"). If it's just a local variable inside
batch_objects(), I don't know that we need the comment. But I think it's
more than is_rest, isn't it?
It looks like we auto-enable it when --textconv or --filters is given.
Can we stick with the split_on_whitespace name (which I think is also
more descriptive about how we intend it to be used)?
> @@ -491,6 +482,7 @@ static int batch_objects(struct batch_options *opt)
> struct expand_data data;
> int save_warning;
> int retval = 0;
> + int is_rest = strstr(opt->format.format, "%(rest)") != NULL ||
> opt->cmdmode;
I'm not excited by this loose parsing. It would do the wrong thing in
some funny corner cases (e.g., "%%(rest)").
We should be able to ask the format parser whether the "rest"
placeholder was used. That's what the initial strbuf_expand() call is
doing. I see that it's hard for us to pass something to its callback
outside of expand_data (since after all, expand_data takes up the
void-pointer data slot).
But doesn't that point to this being the wrong change (or perhaps the
wrong time to make it)? I think we'd want to keep using our own local
expand_data as long as we are not using ref-filter. And then ref-filter
would grow its own struct to hold the data that _it_ needs. Some of that
would be duplicates of what we have here, but that's OK. When we cut
over to ref-filter, that's when we'd drop the fields here.
And eventually we'd drop that strbuf_expand(), too, as ref-filter would
do the parsing. But at that point we wouldn't want this strstr() either:
we'd have ref-filter parse the format, and then check the parsed atoms
to see if one of them is "rest".
-Peff
Re: [PATCH RFC 06/20] cat-file: remove mark_query from expand_data
On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote: > Get rid of mark_query field in struct expand_data. > expand_data may be global further as we use it in ref-filter also, > so we need to remove cat-file specific fields from it. > > All globals that I add through this patch will be deleted in the end, > so treat it just as the middle step. So this is a similar situation to the split_on_whitespace thing we have in the previous patch. I think many of my comments there could apply here. I.e., do we need to be removing them from expand_data now, instead of just moving the bits from expand_data over to ref-filter? But if we assume for a moment that doing it that way isn't feasible (or at least isn't as easy as this way), then I think what this patch does is preferable to the previous one. By making it a global variable, we can still interact with it from the expand callback, even if it's not part of expand_data(). So the previous patch could make "split_on_whitespace" a global, and then continue to set it from expand_atom() as the current code does. -Peff
Re: [PATCH RFC 07/20] cat-file: remove skip_object_info
On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote: > Get rid of skip_object_info field in struct expand_data. > expand_data may be global further as we use it in ref-filter also, > so we need to remove cat-file specific fields from it. > > All globals that I add through this patch will be deleted in the end, > so treat it just as the middle step. OK, makes sense in the same way that the previous patch does. I actually wonder if it would make sense to just do them all in a single patch; the justification is identical for all cases. But I'm also OK leaving them separate. -Peff
Re: [PATCH RFC 08/20] cat-file: remove rest from expand_data
On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote: > Get rid of rest field in struct expand_data. > expand_data may be global further as we use it in ref-filter also, > so we need to remove cat-file specific fields from it. > > All globals that I add through this patch will be deleted in the end, > so treat it just as the middle step. Same comments apply as patch 7 (and 6 and 5). :) -Peff
Re: [PATCH RFC 09/20] ref-filter: make expand_data global
On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote: > Put struct expand_data into global scope to reuse it > in cat-file. So this is the payoff for moving all those things out of expand_data. Instead of just replicating the bits it needs in ref-filter, we're making it globally available. At this point in the series, I'm still unconvinced that this is the right direction, but I haven't read all the way to the end yet. This probably needs a better name. In the context of cat-file, expand_data is the data struct we feed to strbuf_expand(). But in the global namespace of all of Git, it needs a more descriptive name. This likely goes away (or becomes private to ref-filter.c) in the end, but it probably needs a different name there, too. We're not calling strbuf_expand() from there. -Peff
Re: [PATCH RFC 10/20] cat-file: inline stream_blob
On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote: > Inline function stream_blob, it simplifies further > migrating process. I'd have to see what exactly gets simplified later on, but I'm mildly negative on this by itself. The reason this function was added in 98f425b453 (cat-file: handle streaming failures consistently, 2018-10-30) was to keep the outcomes consistent. The function right now isn't _too_ long, so we're really just duplicating the message text. But I wonder if it might eventually get more complicated, if we ever do the "future work" discussed in 98f425b453. So this seems like a step in the wrong direction. -Peff
Re: [PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter
On Fri, Feb 22, 2019 at 06:50:06PM +0300, Olga Telezhnaya wrote: > It was a long way for me, I got older (by 1 year) and smarter > (hopefully), and maybe I will finish my Outreachy Internship task for > now. (I am doing it just for one year and a half, that's OK) Welcome back! Sorry to be a bit slow on the review. I've read through and commented on patch 10. Some of my comments were "I'll have to see how this plays out later in the series", so you may want to hold off on responding until I read the rest. :) > If serious: > In this patch we remove cat-file formatting logic and reuse ref-filter > logic there. As a positive side effect, cat-file now has many new > formatting tokens (all from ref-filter formatting), including deref > (like %(*objectsize:disk)). I have already tried to do this task one > year ago, and it was bad attempt. I feel that today's patch is much > better. I'm still concerned that this is going to regress the performance of cat-file noticeably without some big cleanups in ref-filter. Here are timings on linux.git before and after your patches: [before] $ time git cat-file --unordered --batch-all-objects --batch-check >/dev/null real 0m16.602s user 0m15.545s sys 0m0.495s [after] $ time git cat-file --unordered --batch-all-objects --batch-check >/dev/null real 0m27.301s user 0m24.549s sys 0m2.752s I don't think that's anything particularly wrong with your patches. It's the existing strategy of ref-filter (in particular how it is very eager to allocate lots of separate strings). And it may be too early to switch cat-file over to it. > I also have a question about site https://git-scm.com/docs/ > I thought it is updated automatically based on Documentation folder in > the project, but it is not true. I edited docs for for-each-ref in > December, I still see my patch in master, but for-each-ref docs in > git-csm is outdated. Is it OK? Yeah, as Eric noted, we only build docs for the tagged releases. In theory it would be easy to just build the tip of master nightly, but the data model for the site would need quite a bit of adjustment. -Peff
Re: [PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter
On Fri, Feb 22, 2019 at 06:50:06PM +0300, Olga Telezhnaya wrote: > In my opinion, it still has some issues. I mentioned all of them in > TODOs in comments. All of them considered to be separate tasks for > other patches. Some of them sound easy and could be great tasks for > newbies. One other thing I forgot to mention: your patches ended up on the list in jumbled order. How do you send them? Usually `send-email` would add 1 second to the timestamp of each, so that threading mail readers sort them as you'd expect (even if they arrive out of order due to the vagaries of SMTP servers). -Peff
Prevent reset --hard from deleting everything if one doesn't have any commits yet
I accidentally executed git reset —hard in a project that doesn’t have any commits yet. git erased everything, everything I’ve worked the past week, I believe this is not a desired behavior, considering I’m not able to undo that action, because git doesn’t have any history whatsoever.
Questions on GSoC 2019 Ideas
Hi, everyone I've been in the mailing list for a couple weeks now, mainly working on my gsoc micro-project[1] and in other patches that derived from it. I also have been contributing to the Linux Kernel for half an year, but am now mainly just supporting other students here at USP. I have read the ideas page for the GSoC 2019 and many of them interest me. Also, looking around git-dev materials on the web, I got to the GSoC 2012 ideas page. And this one got my attention: https://github.com/peff/git/wiki/SoC-2012-Ideas#improving-parallelism-in-various-commands I'm interested in parallel computing and that has been my research topic for about an year now. So I would like to ask what's the status of this GSoC idea. I've read git-grep and saw that it is already parallel, but I was wondering if there is any other section in git in which it was already considered to bring parallelism, seeking to achieve greater performance. And also, if this could, perhaps, be a GSoC project. I'm also interested in the 'convert script to builtin' task, and I was wondering if 'git-bisect' would be a good candidate for that. Thanks, Matheus Tavares [1]: https://public-inbox.org/git/20190226051804.10631-1-matheus.bernard...@usp.br/
Gooday To You,
Gooday To You, Please i need your kind Assistance. I will be very glad if you can assist me to receive this sum of ( $22. Million US dollars.) into your bank account for the benefit of our both families, reply me if you are ready to receive this fund.
Re: Questions on GSoC 2019 Ideas
Hi Matheus, On Thu, Feb 28, 2019 at 10:46 PM Matheus Tavares Bernardino wrote: > > I've been in the mailing list for a couple weeks now, mainly working > on my gsoc micro-project[1] and in other patches that derived from it. > I also have been contributing to the Linux Kernel for half an year, > but am now mainly just supporting other students here at USP. > > I have read the ideas page for the GSoC 2019 and many of them interest > me. Also, looking around git-dev materials on the web, I got to the > GSoC 2012 ideas page. And this one got my attention: > https://github.com/peff/git/wiki/SoC-2012-Ideas#improving-parallelism-in-various-commands > > I'm interested in parallel computing and that has been my research > topic for about an year now. So I would like to ask what's the status > of this GSoC idea. I've read git-grep and saw that it is already > parallel, but I was wondering if there is any other section in git in > which it was already considered to bring parallelism, seeking to > achieve greater performance. And also, if this could, perhaps, be a > GSoC project. I vaguely remember that we thought at one point that all the low hanging fruits had already been taken in this area but I might be wrong. > I'm also interested in the 'convert script to builtin' task, and I was > wondering if 'git-bisect' would be a good candidate for that. There is an ongoing work on that by Tanushree Tumane, an Outreachy intern. The end of the internship is March 5, but it seems to me that there is not a lot of work left on the project, and hopefully Tanushree or her mentors will take care of that. Best, Christian. > Thanks, > Matheus Tavares > > [1]: > https://public-inbox.org/git/20190226051804.10631-1-matheus.bernard...@usp.br/
[BUG] completion.commands does not remove multiple commands
Hi,
I was playing with the completion.commands config added in
6532f3740b ("completion: allow to customize the completable
command list", 2018-05-20) and noticed an issue removing
multiple commands.
I wanted to remove completion for cherry and mergetool, so I
added them both to the config:
git config completion.commands "-cherry -mergetool"
But git still completes cherry in this case, only removing
mergetool. Swapping the items in the config yields the
opposite result (cherry is removed and mergetool is not).
With luck this will be a clear and easily resolved issue in
list_cmds_by_config() (in help.c).
Here's an example in test form:
-- 8< --
diff --git c/t/t9902-completion.sh w/t/t9902-completion.sh
index 3a2c6326d8..06cee36ae5 100755
--- c/t/t9902-completion.sh
+++ w/t/t9902-completion.sh
@@ -1483,6 +1483,14 @@ test_expect_success 'git --help completion' '
test_completion "git --help core" "core-tutorial "
'
+test_expect_failure 'completion.commands removes multiple commands' '
+ test_config completion.commands "-cherry -mergetool" &&
+ git --list-cmds=list-mainporcelain,list-complete,config |
+ grep ^cherry >actual &&
+ echo cherry-pick >expected &&
+ test_cmp expected actual
+'
+
test_expect_success 'setup for integration tests' '
echo content >file1 &&
echo more >file2 &&
-- 8< --
That's not quite normal form for t9902, but I was undable to
create a working test using the test_completion functions.
The tests set GIT_TESTING_PORCELAIN_COMMAND_LIST and
GIT_TESTING_ALL_COMMAND_LIST which override the settings in
the completion script. I played a bit with adjusting those
to add cherry{,-pick} to the command lists, unsetting those
vars for the test, and such, to no avail.
In the end, I'm not really sure that calling --list-cmds
directly is such a bad way to go for testing this issue.
--
Todd
Dear Friend,
Dear Friend, I am Mrs Clara David. am sending you this brief letter to solicit your partnership to transfer $18.5 million US Dollars.I shall send you more information and procedures when I receive positive response from you. please send me a message in my Email box (mrsclarada...@gmail.com) as i wait to hear from you. Best regard Mrs Clara David.
Re: [BUG] completion.commands does not remove multiple commands
On Thu, Feb 28, 2019 at 05:31:23PM -0500, Todd Zullinger wrote:
> Hi,
>
> I was playing with the completion.commands config added in
> 6532f3740b ("completion: allow to customize the completable
> command list", 2018-05-20) and noticed an issue removing
> multiple commands.
>
> I wanted to remove completion for cherry and mergetool, so I
> added them both to the config:
>
> git config completion.commands "-cherry -mergetool"
>
> But git still completes cherry in this case, only removing
> mergetool. Swapping the items in the config yields the
> opposite result (cherry is removed and mergetool is not).
I can reproduce your problem, though the test you included passes for me
even with the current tip of master. I suspect this is the problem:
diff --git a/help.c b/help.c
index 520c9080e8..026f881715 100644
--- a/help.c
+++ b/help.c
@@ -393,8 +393,8 @@ void list_cmds_by_config(struct string_list *list)
const char *p = strchrnul(cmd_list, ' ');
strbuf_add(&sb, cmd_list, p - cmd_list);
- if (*cmd_list == '-')
- string_list_remove(list, cmd_list + 1, 0);
+ if (sb.buf[0] == '-')
+ string_list_remove(list, sb.buf + 1, 0);
else
string_list_insert(list, sb.buf);
strbuf_release(&sb);
though I can't help but wonder if the whole thing would be simpler using
string_list_split().
-Peff
Re: [BUG] completion.commands does not remove multiple commands
On Thu, Feb 28, 2019 at 05:31:23PM -0500, Todd Zullinger wrote:
> I was playing with the completion.commands config added in
> 6532f3740b ("completion: allow to customize the completable
> command list", 2018-05-20) and noticed an issue removing
> multiple commands.
>
> I wanted to remove completion for cherry and mergetool, so I
> added them both to the config:
>
> git config completion.commands "-cherry -mergetool"
>
> But git still completes cherry in this case, only removing
> mergetool. Swapping the items in the config yields the
> opposite result (cherry is removed and mergetool is not).
>
> With luck this will be a clear and easily resolved issue in
> list_cmds_by_config() (in help.c).
Indeed, this seems to fix it for me:
diff --git a/help.c b/help.c
index 520c9080e8..f2c6f0c9f7 100644
--- a/help.c
+++ b/help.c
@@ -393,8 +393,8 @@ void list_cmds_by_config(struct string_list *list)
const char *p = strchrnul(cmd_list, ' ');
strbuf_add(&sb, cmd_list, p - cmd_list);
- if (*cmd_list == '-')
- string_list_remove(list, cmd_list + 1, 0);
+ if (*sb.buf == '-')
+ string_list_remove(list, sb.buf + 1, 0);
else
string_list_insert(list, sb.buf);
strbuf_release(&sb);
Re: [WIP 0/7] CDN offloading of fetch response
Hi, Sorry for the slow followup. Thanks for probing into the design --- this should be useful for getting the docs to be clear. Christian Couder wrote: > So it's likely that users will want a way to host on such sites > incomplete repos using CDN offloading to a CDN on another site. And > then if the CDN is not accessible for some reason, things will > completely break when users will clone. I think this would be a broken setup --- we can make it clear in the protocol and server docs that you should only point to a CDN for which you control the contents, to avoid breaking clients. That doesn't prevent adding additional features in the future e.g. for "server suggested alternates" --- it's just that I consider that a separate feature. Using CDN offloading requires cooperation of the hosting provider. It's a way to optimize how fetches work, not a way to have a partial repository on the server side. [...] > On Tue, Feb 26, 2019 at 12:45 AM Jonathan Nieder wrote: >> This doesn't stop a hosting provider from using e.g. server options to >> allow the client more control over how their response is served, just >> like can be done for other features of how the transfer works (how >> often to send progress updates, whether to prioritize latency or >> throughput, etc). > > Could you give a more concrete example of what could be done? What I mean is passing server options using "git fetch --server-option". For example: git fetch -o priority=BATCH origin master or git fetch -o avoid-cdn=badcdn.example.com origin master The interpretation of server options is up to the server. >> What the client *can* do is turn off support for packfile URLs in a >> request completely. This is required for backward compatibility and >> allows working around a host that has configured the feature >> incorrectly. > > If the full content of a repo is really large, the size of a full pack > file sent by an initial clone could be really big and many client > machines could not have enough memory to deal with that. And this > suppose that repo hosting providers would be ok to host very large > repos in the first place. Do we require the packfile to fit in memory? If so, we should fix that (to use streaming instead). Thanks, Jonathan
Re: [PATCH v2 0/3] asciidoctor-extensions: fix spurious space after linkgit
On Wed, Feb 27, 2019 at 07:17:51PM +0100, Martin Ågren wrote: > Just like v1 [1], this v2 removes a spurious space which shows up in a > large number of places in our manpages when Asciidoctor expands the > linkgit:foo[bar] macro. The only difference is a new paragraph in the > commit message of the first patch to explain why we need to explicitly > list a file we depend on. > > Thanks Eric and brian for your comments on v1. > > [1] https://public-inbox.org/git/cover.1551123979.git.martin.ag...@gmail.com/ > > Martin Ågren (3): > Documentation/Makefile: add missing xsl dependencies for manpages > Documentation/Makefile: add missing dependency on > asciidoctor-extensions > asciidoctor-extensions: fix spurious space after linkgit > > Documentation/Makefile | 4 ++-- > Documentation/asciidoctor-extensions.rb | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) This version looks good to me. Thanks again for getting this cleaned up. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [WIP 7/7] upload-pack: send part of packfile response as uri
On 2019.02.23 15:39, Jonathan Tan wrote:
> Teach upload-pack to send part of its packfile response as URIs.
>
> An administrator may configure a repository with one or more
> "uploadpack.blobpackfileuri" lines, each line containing an OID and a
> URI. A client may configure fetch.uriprotocols to be a comma-separated
> list of protocols that it is willing to use to fetch additional
> packfiles - this list will be sent to the server. Whenever an object
> with one of those OIDs would appear in the packfile transmitted by
> upload-pack, the server may exclude that object, and instead send the
> URI. The client will then download the packs referred to by those URIs
> before performing the connectivity check.
>
> Signed-off-by: Jonathan Tan
> ---
> builtin/pack-objects.c | 63 ++
> fetch-pack.c | 58 +++
> t/t5702-protocol-v2.sh | 54 +
> upload-pack.c | 78 ++
> 4 files changed, 247 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index a9fac7c128..73309821e2 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -110,6 +110,8 @@ static unsigned long window_memory_limit = 0;
>
> static struct list_objects_filter_options filter_options;
>
> +static struct string_list uri_protocols = STRING_LIST_INIT_NODUP;
> +
> enum missing_action {
> MA_ERROR = 0, /* fail if any missing objects are encountered */
> MA_ALLOW_ANY, /* silently allow ALL missing objects */
> @@ -118,6 +120,14 @@ enum missing_action {
> static enum missing_action arg_missing_action;
> static show_object_fn fn_show_object;
>
> +struct configured_exclusion {
> + struct oidmap_entry e;
> + char *uri;
> +};
> +static struct oidmap configured_exclusions;
> +
> +static struct oidset excluded_by_config;
> +
> /*
> * stats
> */
> @@ -832,6 +842,23 @@ static off_t write_reused_pack(struct hashfile *f)
> return reuse_packfile_offset - sizeof(struct pack_header);
> }
>
> +static void write_excluded_by_configs(void)
> +{
> + struct oidset_iter iter;
> + const struct object_id *oid;
> +
> + oidset_iter_init(&excluded_by_config, &iter);
> + while ((oid = oidset_iter_next(&iter))) {
> + struct configured_exclusion *ex =
> + oidmap_get(&configured_exclusions, oid);
> +
> + if (!ex)
> + BUG("configured exclusion wasn't configured");
> + write_in_full(1, ex->uri, strlen(ex->uri));
> + write_in_full(1, "\n", 1);
> + }
> +}
> +
> static const char no_split_warning[] = N_(
> "disabling bitmap writing, packs are split due to pack.packSizeLimit"
> );
> @@ -1125,6 +1152,25 @@ static int want_object_in_pack(const struct object_id
> *oid,
> }
> }
>
> + if (uri_protocols.nr) {
> + struct configured_exclusion *ex =
> + oidmap_get(&configured_exclusions, oid);
> + int i;
> + const char *p;
> +
> + if (ex) {
> + for (i = 0; i < uri_protocols.nr; i++) {
> + if (skip_prefix(ex->uri,
> + uri_protocols.items[i].string,
> + &p) &&
> + *p == ':') {
> + oidset_insert(&excluded_by_config, oid);
> + return 0;
> + }
> + }
> + }
> + }
> +
> return 1;
> }
>
> @@ -2726,6 +2772,19 @@ static int git_pack_config(const char *k, const char
> *v, void *cb)
> pack_idx_opts.version);
> return 0;
> }
> + if (!strcmp(k, "uploadpack.blobpackfileuri")) {
> + struct configured_exclusion *ex = xmalloc(sizeof(*ex));
> + const char *end;
> +
> + if (parse_oid_hex(v, &ex->e.oid, &end) || *end != ' ')
> + die(_("value of uploadpack.blobpackfileuri must be "
> + "of the form ' ' (got '%s')"), v);
> + if (oidmap_get(&configured_exclusions, &ex->e.oid))
> + die(_("object already configured in another "
> + "uploadpack.blobpackfileuri (got '%s')"), v);
> + ex->uri = xstrdup(end + 1);
> + oidmap_put(&configured_exclusions, ex);
> + }
> return git_default_config(k, v, cb);
> }
>
> @@ -3318,6 +3377,9 @@ int cmd_pack_objects(int argc, const char **argv, const
> char *prefix)
>N_("do not pack objects in promisor packfiles")),
> OPT_BOOL(0, "delta-islands", &use_delta_islands,
>N_("respect islands during delta compression")),
> + OPT_STRING_LIS
Re: [WIP 7/7] upload-pack: send part of packfile response as uri
> So we process the packfile URIs one by one as we receive them from the
> server? If we expect these packfiles to be large (otherwise why are we
> bothering to offload them to the CDN), is there a risk that the
> connection to the server might time out while we're downloading from the
> CDN?
You're right that this is undesirable - this is one of the things that I
will fix, as I mention in the cover letter ("starting CDN downloads...")
[1].
> Please take a look. Feel free to comment on anything, but I prefer
> comments on the major things first (e.g. my usage of a separate process
> (http-fetch) to fetch packfiles, since as far as I know, Git doesn't
> link to libcurl; any of the design decisions I described above). I know
> that there are some implementation details that could be improved (e.g.
> parallelization of the CDN downloads, starting CDN downloads *after*
> closing the first HTTP request, holding on to the .keep locks until
> after the refs are set), but will work on those once the overall design
> is more or less finalized.
[1] https://public-inbox.org/git/20190301000954.ga47...@google.com/
Re: [PATCH] doc/fsck: discuss mix of --connectivity-only and --dangling
Jeff King writes: > I'm actually a little torn on this. We could consider this a bug, and > the "option" to disable it when you want things to go fast is to say > "--no-dangling". That leaves no way to say "show me the list of > unreachable objects, but don't bother spending extra time on dangling > analysis". But I don't think I've ever really wanted that list of > unreachable objects anyway (and besides, you could do it pretty easily > with cat-file, rev-list, and comm). > > So I sketched up what it might look like to just fix the bug (but kick > in only when needed), which is below. [jch: I am still mostly offline til the next week, but I had a chance to sit in front of my mailbox long enough, so...] As the primariy purose of the --conn-only option being such, perhaps we should have made --no-dangling the default when --conn-only is in effect. But if --conn-only is made to do the right thing while showing dangling and unreachable properly sifted into their own bins, like this patch does, what's the difference between that and the normal --no-conn-only, other than performance and corrupt blobs left unreported? Perhaps if we are going that route, it might even make sense to rename --conn-only to --skip-parsing-blobs or something. Thanks.
Re: [GSoC] acknowledging mistakes
Rohit Ashiwal writes: > 1. Commit message was less than 50 chars which should be around 72 chars >according to coding guide lines. Should I change this to match 72? Simple things do not need that many letters to tell ;-) The suggestion of 72 is about the maximum. If you are doing something in a single patch that needs a longer title, it generally is a sign that you are trying to do too much in a single patch and should be splitting the patch into more digestable smaller steps. And the purpose of having a maximum is to nudge patch authors to realize that. > 2. My changes had some uneven use of tabs and spaces, which I made >considering that pre-existing code had them too. Is there a >possibility to change the whole code according to CodingGuidelines? >If yes should I only change my code according to guidelines or the >whole file? I think you are talking about t3600, which uses an ancient style. If this were a real project, then the preferred order would be - A preliminary patch (or a series of patches) that modernizes existing tests in t3600. Just style updates and adding or removing nothing else. - Update test that use "test -f" and friends to use the helpers in t3600. > 3. There is no helper function for `test -s` but Rafael suggested we can >make use of other helper functions to provide similar functionality, >if we can. If we often see if a path is an non-empty file in our tests (not limited to t3600), then it may make sense to add a new helper test_path_is_non_empty_file in t/test-lib-functions.sh next to where test_path_is_file and friends are defined. Thanks. [jch: I am still mostly offline til the next week, but I had a chance to sit in front of my mailbox long enough, so...]
Re: [BUG] All files in folder are moved when cherry-picking commit that moves fewer files
As you know that I've always been skeptical to this rename-directory business, this probably won't come as a surprise, but I seriously think directory renames should be made opt-in, and as a separate option, making the option three-way. I.e. - do we do any renames (yes/no)? - if we do do renames, do we base the decision only on the contents of the thing along, or do we allow neighbouring files affect the decision? That is, in addition to the traditional --renames or -M, we'd have a separate bool --allow-directory-renames that is by default off and is a no-op if the former is off. We had to fix a breakage recently for 3-way fallback case, and we explained the fix as needed because the case lacks the full information, but I think even with the full information at hand, the rename-directory heurisitcs is inherently riskier than the content based rename detection. Suppose you had F1, F2, ... in directory D1, and moved them all to D1/D2. In the meantime, somebody else adds Fn to directory D1. It may be likely that some variant of Fn would want to go to D1/D2, but it also is very likely that there should be a difference between D1/Fn somebody else had, and the contents of D1/D2/Fn in the merge result. Perhaps D1/F1 in your preimage used to refer to another path in the top-level directory as "../top", but the reference would have been rewritten to "../../top" when you moved D1/F1 to D1/D2/F1, and the person doing the merge should at least check if D1/Fn that comes from the other party needs a similar adjustment while merged. In the above scenario, if there were D1/Fn in _your_ preimage and all the other party did was to make in-place modification, the story is vastly different. Most likely you would have made most of, if not all, the adjustment necessary for D1/Fn to sit in its new location, while the other party kept the relative reference to other places intact, so we can say that both parties have say in the contents of the auto-merged result. The "since neighgours moved, this must also want to move the same way" heuristics does not give a chance to the party that is not aware of the move to prepare the contents appropriate for the new location, by definition, so the onus is on the person who merges to adjust the contents. Thanks. [jch: I am still mostly offline til the next week, but I had a chance to sit in front of my mailbox long enough, so...]
Re: Do test-path_is_{file,dir,exists} make sense anymore with -x?
Ævar Arnfjörð Bjarmason writes:
> I swear I'm not just on a mission to ruin everyone's GSOC projects. This
> patch definitely looks good, and given that we have this / document it
> makes sense.
>
> However. I wonder in general if we've re-visited the utility of these
> wrappers and maybe other similar wrappers after -x was added.
>
> Back when this was added in 2caf20c52b ("test-lib: user-friendly
> alternatives to test [-d|-f|-e]", 2010-08-10) we didn't have -x.
> ...
> But 4 years after this was added in a136f6d8ff ("test-lib.sh: support -x
> option for shell-tracing", 2014-10-10) we got -x, and then with "-i -v -x":
I think two things need to be considered separately.
- Do the path-is-file and friends make the test source easier to
read and undrstand? Special bonus if it helps us by making it
harder to write a wrong test.
- Do these helpers make the output from the test execution easier
to diagnose or harder?
If your primary compalint is the latter (which I think it is, and I
share the same feeling to a certain degree), I think it is to throw
the baby with bathwater to get rid of path-is-* family.
And as to the former question, I think we even are getting special
bonus. Often when people write tests to ensure a fix that left an
unwanted file behind would say "! test -f unwanted", but if we say
"path-is-missing unwanted" that would catch not just a regular file
but also catch other kinds of filesystem entities.
As to readablity, I do not think "test -f/-d" etc are unnecessary
hard to read, but using path-is-* does not make it harder to read,
so I'd say it would not give us much to revert to the bare "test -f"
and friends.
Unless you are after squeezing the last cycle spent executing a
shell builtin in the test scripts by using bare-bones "test -f",
that is. But that is not among the two I said we need to consider
separately, so I won't go there.
Thanks.
[jch: I am still mostly offline til the next week, but I had a
chance to sit in front of my mailbox long enough, so...]
Re: ag/sequencer-reduce-rewriting-todo, was Re: What's cooking in git.git (Feb 2019, #04; Sun, 24)
Alban Gruin writes: >> Was still being worked on, but seems to have stalled. >> cf. >> cf. <97f77aca-bd19-f763-349a-de40c4b94...@talktalk.net> > > I’m still working on this. I sent a v7 shortly after the release of v2.21.0- > rc0, and I almost finished the v8. Thanks for an update. Perhaps then I should wait for v8 and instead spend our cycles on other topics until then. Thanks. [jch: I am still mostly offline til the next week, but I had a chance to sit in front of my mailbox long enough, so...]
Re: [PATCH RFC 04/20] for-each-ref: tests for new atom %(rest) added
пт, 1 мар. 2019 г. в 00:11, Jeff King : > > On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote: > > > Add tests for new formatting atom %(rest). > > We need this atom for cat-file command. > > While I do normally encourage splitting up commits, in this case I think > it would make sense to squash this together with patch 3. There's > nothing to say here about what %(rest) is that isn't already said in > that commit message. Agree, will squash. > > That said, I'm still not sure that for-each-ref should be supporting > %(rest) at all. We should hopefully already have coverage of cat-file > using "%(rest)" (and if not, we should add some to make sure it's not > regressed by the conversion). If we want to use ref-filter formatting logic in cat-file, we have to add this atom in ref-filter. I agree that we do not need it in ref-filter, and that's why I left %(rest) in cat-file docs (it's in the end of the patch). But in the code, I am not sure we want to make one more array with specific cat-file atoms (or atoms for other command). > > -Peff
Re: [PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter
пт, 1 мар. 2019 г. в 00:41, Jeff King : > > On Fri, Feb 22, 2019 at 06:50:06PM +0300, Olga Telezhnaya wrote: > > > It was a long way for me, I got older (by 1 year) and smarter > > (hopefully), and maybe I will finish my Outreachy Internship task for > > now. (I am doing it just for one year and a half, that's OK) > > Welcome back! > > Sorry to be a bit slow on the review. I've read through and commented on > patch 10. Some of my comments were "I'll have to see how this plays out > later in the series", so you may want to hold off on responding until I > read the rest. :) > > > If serious: > > In this patch we remove cat-file formatting logic and reuse ref-filter > > logic there. As a positive side effect, cat-file now has many new > > formatting tokens (all from ref-filter formatting), including deref > > (like %(*objectsize:disk)). I have already tried to do this task one > > year ago, and it was bad attempt. I feel that today's patch is much > > better. > > I'm still concerned that this is going to regress the performance of > cat-file noticeably without some big cleanups in ref-filter. Here are > timings on linux.git before and after your patches: > > [before] > $ time git cat-file --unordered --batch-all-objects --batch-check >/dev/null > real 0m16.602s > user 0m15.545s > sys 0m0.495s > > [after] > $ time git cat-file --unordered --batch-all-objects --batch-check >/dev/null > real 0m27.301s > user 0m24.549s > sys 0m2.752s > > I don't think that's anything particularly wrong with your patches. It's > the existing strategy of ref-filter (in particular how it is very eager > to allocate lots of separate strings). And it may be too early to switch > cat-file over to it. I have a guess that we need to add batch printing argument to our general printing functions, that could make my version faster. > > > I also have a question about site https://git-scm.com/docs/ > > I thought it is updated automatically based on Documentation folder in > > the project, but it is not true. I edited docs for for-each-ref in > > December, I still see my patch in master, but for-each-ref docs in > > git-csm is outdated. Is it OK? > > Yeah, as Eric noted, we only build docs for the tagged releases. In > theory it would be easy to just build the tip of master nightly, but the > data model for the site would need quite a bit of adjustment. > > -Peff
Re: [PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter
пт, 1 мар. 2019 г. в 00:43, Jeff King : > > On Fri, Feb 22, 2019 at 06:50:06PM +0300, Olga Telezhnaya wrote: > > > In my opinion, it still has some issues. I mentioned all of them in > > TODOs in comments. All of them considered to be separate tasks for > > other patches. Some of them sound easy and could be great tasks for > > newbies. > > One other thing I forgot to mention: your patches ended up on the list > in jumbled order. How do you send them? Usually `send-email` would add 1 > second to the timestamp of each, so that threading mail readers sort > them as you'd expect (even if they arrive out of order due to the > vagaries of SMTP servers). Oh, that's one more bug in submitgit, I guess. I will not use it anymore, OK, it's time to change the habits. > > -Peff
fast-import fails with case sensitive tags due to case insensitive lock files
Hi,
I have a problem with fast-import on an NTFS drive: If I try to import tags
which are identical apart from their casing a failure due to identical lock
file names occurs.
I am running git for windows 2.15.1.2 x64 on a Windows 10 machine (10.0.15063):
$ git --version --build-options
git version 2.15.1.windows.2
built from commit: 5d5baf91824ec7750b103c8b7c4827ffac202feb
sizeof-long: 4
machine: x86_64
MCVE:
(echo "commit refs/heads/master" &&
echo "mark :1" &&
echo "committer me <> 0 +" &&
echo "data 0" &&
echo "" &&
echo "tag tag_A" &&
echo "from :1" &&
echo "tagger me <> 0 +" &&
echo "data 0" &&
echo "" &&
echo "tag tag_a" &&
echo "from :1" &&
echo "tagger me <> 0 +" &&
echo "data 0" &&
echo "") | git fast-import
Instead of having 1 commit with two tags ("tag_A" and "tag_a") I get his error
message:
Unpacking objects: 100% (4/4), done.
error: cannot lock ref 'refs/tags/tag_a': Unable to create
'C:/tmp/.git/refs/tags/tag_a.lock': File exists.
Another git process seems to be running in this repository, e.g.
an editor opened by 'git commit'. Please make sure all processes
are terminated then try again. If it still fails, a git process
may have crashed in this repository earlier:
remove the file manually to continue.
Best regards,
Jonathan
Re: Prevent reset --hard from deleting everything if one doesn't have any commits yet
Am 28.02.19 um 22:43 schrieb Manuel Guilamo: > I accidentally executed git reset —hard in a project that doesn’t > have any commits yet. git erased everything, everything I’ve worked > the past week, I believe this is not a desired behavior, considering > I’m not able to undo that action, because git doesn’t have any > history whatsoever. I tested this, and it does not happen for me as long as I do not `git add` anything. So, I assume you did `git add` your content and then you did a `git reset --hard`. In that case, I'm afraid Git behaved as designed and "doesn't have any commits" is a red herring. -- Hannes
