[PATCH v2 3/4] git-remote-mediawiki: use no-private-update capability on dumb push
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 3 +++
1 file changed, 3 insertions(+)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index f8d7d2c..22300a1 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -590,6 +590,9 @@ sub mw_capabilities {
print {*STDOUT} "import\n";
print {*STDOUT} "list\n";
print {*STDOUT} "push\n";
+ if ($dumb_push) {
+ print {*STDOUT} "no-private-update\n";
+ }
print {*STDOUT} "\n";
return;
}
--
1.8.4.12.g98a4f55.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] git-remote-mediawiki: no need to update private ref in non-dumb push
We used to update the private ref ourselves, but this update is now done
by default (since 664059fb62).
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 1 -
1 file changed, 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 22300a1..c9a4805 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1214,7 +1214,6 @@ sub mw_push_revision {
}
if (!$dumb_push) {
run_git(qq(notes --ref=${remotename}/mediawiki add -f
-m "mediawiki_revision: ${mw_revision}" ${sha1_commit}));
- run_git(qq(update-ref -m "Git-MediaWiki push"
refs/mediawiki/${remotename}/master ${sha1_commit} ${sha1_child}));
}
}
--
1.8.4.12.g98a4f55.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] git-remote-mediawiki: add test and check Makefile targets
There are a few level 4 and 2 perlcritic issues in the current code. We make level 5 fatal, and keep level 2 as warnings. Signed-off-by: Matthieu Moy --- contrib/mw-to-git/Makefile | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile index 76fcd4d..f206f96 100644 --- a/contrib/mw-to-git/Makefile +++ b/contrib/mw-to-git/Makefile @@ -24,6 +24,11 @@ INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \ all: build +test: all + $(MAKE) -C t + +check: perlcritic test + install_pm: install $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM) @@ -41,4 +46,7 @@ clean: rm $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM) perlcritic: - perlcritic -2 *.perl + perlcritic -5 $(SCRIPT_PERL) + -perlcritic -2 $(SCRIPT_PERL) + +.PHONY: all test check install_pm install clean perlcritic -- 1.8.4.12.g98a4f55.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] transport-helper: add no-private-update capability
Since 664059fb62 (Felipe Contreras, Apr 17 2013, transport-helper: update
remote helper namespace), a 'push' operation on a remote helper updates
the private ref by default. This is often a good thing, but it can also
be desirable to disable this update to force the next 'pull' to re-import
the pushed revisions.
Allow remote-helpers to disable the automatic update by introducing a new
capability.
Signed-off-by: Matthieu Moy
---
Change since v1: just changed the capability name.
Documentation/gitremote-helpers.txt | 6 ++
git-remote-testgit.sh | 1 +
t/t5801-remote-helpers.sh | 11 +++
transport-helper.c | 7 +--
4 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/Documentation/gitremote-helpers.txt
b/Documentation/gitremote-helpers.txt
index 0827f69..1eacf1e 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -120,6 +120,12 @@ connecting (see the 'connect' command under COMMANDS).
When choosing between 'push' and 'export', Git prefers 'push'.
Other frontends may have some other order of preference.
+'no-private-update'::
+ When using the 'refspec' capability, git normally updates the
+ private ref on successful push. This update is disabled when
+ the remote-helper declares the capability
+ 'no-private-update'.
+
Capabilities for Fetching
^
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 2109070..6d2f282 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -38,6 +38,7 @@ do
echo "*export-marks $gitmarks"
fi
test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
+ test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo
"no-private-update"
echo
;;
list)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 8c4c539..613f69a 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -182,6 +182,17 @@ test_expect_success 'push update refs' '
)
'
+test_expect_success 'push update refs disabled by no-private-update' '
+ (cd local &&
+ echo more-update >>file &&
+ git commit -a -m more-update &&
+ git rev-parse --verify testgit/origin/heads/update >expect &&
+ GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE=t git push origin update &&
+ git rev-parse --verify testgit/origin/heads/update >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'push update refs failure' '
(cd local &&
git checkout update &&
diff --git a/transport-helper.c b/transport-helper.c
index 63cabc3..3328394 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -27,7 +27,8 @@ struct helper_data {
push : 1,
connect : 1,
signed_tags : 1,
- no_disconnect_req : 1;
+ no_disconnect_req : 1,
+ no_private_update : 1;
char *export_marks;
char *import_marks;
/* These go from remote name (as in "list") to private name */
@@ -205,6 +206,8 @@ static struct child_process *get_helper(struct transport
*transport)
strbuf_addstr(&arg, "--import-marks=");
strbuf_addstr(&arg, capname + strlen("import-marks "));
data->import_marks = strbuf_detach(&arg, NULL);
+ } else if (!prefixcmp(capname, "no-private-update")) {
+ data->no_private_update = 1;
} else if (mandatory) {
die("Unknown mandatory capability %s. This remote "
"helper probably needs newer version of Git.",
@@ -723,7 +726,7 @@ static void push_update_refs_status(struct helper_data
*data,
if (push_update_ref_status(&buf, &ref, remote_refs))
continue;
- if (!data->refspecs)
+ if (!data->refspecs || data->no_private_update)
continue;
/* propagate back the update to the remote namespace */
--
1.8.4.12.g98a4f55.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Introduce publish tracking branch
Felipe Contreras writes: > Hi, > > As it has been discussed before, our support for triangular workflows is > lacking, and the following patch series aims to improve that situation. I may be stating the obvious, but isn't your series a duplicate of remote.pushDefault, introduced in 1.8.3? If not, how does it complement it? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] transport-helper: add no-private-update capability
On Mon, Sep 2, 2013 at 2:19 AM, Matthieu Moy wrote: > Since 664059fb62 (Felipe Contreras, Apr 17 2013, transport-helper: update > remote helper namespace), a 'push' operation on a remote helper updates > the private ref by default. This is often a good thing, but it can also > be desirable to disable this update to force the next 'pull' to re-import > the pushed revisions. > > Allow remote-helpers to disable the automatic update by introducing a new > capability. > > Signed-off-by: Matthieu Moy Acked-by: Felipe Contreras Or Reviewed-by, or whatever. > --- > Change since v1: just changed the capability name. > > Documentation/gitremote-helpers.txt | 6 ++ > git-remote-testgit.sh | 1 + > t/t5801-remote-helpers.sh | 11 +++ > transport-helper.c | 7 +-- > 4 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/Documentation/gitremote-helpers.txt > b/Documentation/gitremote-helpers.txt > index 0827f69..1eacf1e 100644 > --- a/Documentation/gitremote-helpers.txt > +++ b/Documentation/gitremote-helpers.txt > @@ -120,6 +120,12 @@ connecting (see the 'connect' command under COMMANDS). > When choosing between 'push' and 'export', Git prefers 'push'. > Other frontends may have some other order of preference. > > +'no-private-update':: > + When using the 'refspec' capability, git normally updates the > + private ref on successful push. This update is disabled when > + the remote-helper declares the capability > + 'no-private-update'. > + > There's an extra unnecessary space here, no? -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Introduce publish tracking branch
On Mon, Sep 2, 2013 at 2:25 AM, Matthieu Moy wrote: > Felipe Contreras writes: >> As it has been discussed before, our support for triangular workflows is >> lacking, and the following patch series aims to improve that situation. > > I may be stating the obvious, but isn't your series a duplicate of > remote.pushDefault, introduced in 1.8.3? How can you currently do this in v1.8.3? % git push --set-publish github master % git push --set-publish backup test feature-a % git branch -vv master x [origin/master, github/master] foo test x [master, backup/test: ahead 2] foo feature-a x [master, backup/feature-a] foo % git checkout master % git rebase # onto origin/master % git push # to github/master % git checkout feature-a % git rebase # onto master % git push --force # to backup/feature-a % git checkout test % git branch --set-publish github/my-test % git push # to github/my-test remote.pushDefault doesn't let you do any of that stuff. Maybe if you only push to one remote, and all the branches have the same names, and you don't need to keep track of them (with git branch -vv). In other words, there's just no comparison. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git svn fetch segfault on exit
I'm facing the same issue. I'm using git-1.8.4 and subversion 1.8.3 on Ubuntu. I'm proposing a different modification to Ra.pm: Jonathan's modifications basically disable the use of $RA. The modifications above ensure that the destructor of $RA is called before doing an apr_terminate(). Best regards, Uli. -- View this message in context: http://git.661346.n2.nabble.com/git-svn-fetch-segfault-on-exit-tp7592205p7595155.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/4] transport-helper: add dont-update-private capability
Since 664059fb62 (Felipe Contreras, Apr 17 2013, transport-helper: update
remote helper namespace), a 'push' operation on a remote helper updates
the private ref by default. This is often a good thing, but it can also
be desirable to disable this update to force the next 'pull' to re-import
the pushed revisions.
Allow remote-helpers to disable the automatic update by introducing a new
capability.
Signed-off-by: Matthieu Moy
---
> There's an extra unnecessary space here, no?
Indeed (the new option is shorter, and I didn't rewrap). It doesn't
really matter, but in case, here's an updated version.
Documentation/gitremote-helpers.txt | 5 +
git-remote-testgit.sh | 1 +
t/t5801-remote-helpers.sh | 11 +++
transport-helper.c | 7 +--
4 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/Documentation/gitremote-helpers.txt
b/Documentation/gitremote-helpers.txt
index 0827f69..ee9e134 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -120,6 +120,11 @@ connecting (see the 'connect' command under COMMANDS).
When choosing between 'push' and 'export', Git prefers 'push'.
Other frontends may have some other order of preference.
+'no-private-update'::
+ When using the 'refspec' capability, git normally updates the
+ private ref on successful push. This update is disabled when
+ the remote-helper declares the capability 'no-private-update'.
+
Capabilities for Fetching
^
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 2109070..6d2f282 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -38,6 +38,7 @@ do
echo "*export-marks $gitmarks"
fi
test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
+ test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo
"no-private-update"
echo
;;
list)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 8c4c539..613f69a 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -182,6 +182,17 @@ test_expect_success 'push update refs' '
)
'
+test_expect_success 'push update refs disabled by no-private-update' '
+ (cd local &&
+ echo more-update >>file &&
+ git commit -a -m more-update &&
+ git rev-parse --verify testgit/origin/heads/update >expect &&
+ GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE=t git push origin update &&
+ git rev-parse --verify testgit/origin/heads/update >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'push update refs failure' '
(cd local &&
git checkout update &&
diff --git a/transport-helper.c b/transport-helper.c
index 63cabc3..3328394 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -27,7 +27,8 @@ struct helper_data {
push : 1,
connect : 1,
signed_tags : 1,
- no_disconnect_req : 1;
+ no_disconnect_req : 1,
+ no_private_update : 1;
char *export_marks;
char *import_marks;
/* These go from remote name (as in "list") to private name */
@@ -205,6 +206,8 @@ static struct child_process *get_helper(struct transport
*transport)
strbuf_addstr(&arg, "--import-marks=");
strbuf_addstr(&arg, capname + strlen("import-marks "));
data->import_marks = strbuf_detach(&arg, NULL);
+ } else if (!prefixcmp(capname, "no-private-update")) {
+ data->no_private_update = 1;
} else if (mandatory) {
die("Unknown mandatory capability %s. This remote "
"helper probably needs newer version of Git.",
@@ -723,7 +726,7 @@ static void push_update_refs_status(struct helper_data
*data,
if (push_update_ref_status(&buf, &ref, remote_refs))
continue;
- if (!data->refspecs)
+ if (!data->refspecs || data->no_private_update)
continue;
/* propagate back the update to the remote namespace */
--
1.8.4.12.g98a4f55.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Turn off pathspec magic on "{checkout,reset,add} -p" on native Windows builds
On Mon, Sep 02, 2013 at 08:42:18AM +0200, Johannes Sixt wrote: > Am 9/1/2013 4:08, schrieb Nguyễn Thái Ngọc Duy: > > git-add--interactive.perl rejects arguments with colons in 21e9757 > > (Hack git-add--interactive to make it work with ActiveState Perl - > > 2007-08-01). Pathspec magic starts with a colon, so it won't work if > > these pathspecs are passed to git-add--interactive.perl running with > > ActiveState Perl. Make sure we only pass plain paths in this case. > > > > Signed-off-by: Nguyễn Thái Ngọc Duy > > --- > > Johannes, can you check the test suite passes for you with this > > patch? I assume that Cygwin Perl behaves differently and does not hit > > this limit. So I keep the special case to GIT_WINDOWS_NATIVE only. > > I'll resend the patch with a few others on the same topic if it works > > for you. > > It does not help. The error in git-add--interactive is avoided, but the > failure in t2016-checkout-patch.sh is now: > > expecting success: > set_state dir/foo work head && > # the third n is to get out in case it mistakenly does not apply > (echo y; echo n; echo n) | (cd dir && git checkout -p foo) && > verify_saved_state bar && > verify_state dir/foo head head > > No changes. > not ok 13 - path limiting works: foo inside dir > > and the same "No changes." happens in t7105-reset-patch.sh Right. Because I got rid of ':(prefix)foo' form but I passed 'foo' instead of 'dir/foo'. How about this on top? -- 8< -- diff --git a/builtin/add.c b/builtin/add.c index 3402239..a138360 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -257,9 +257,15 @@ int run_add_interactive(const char *revision, const char *patch_mode, if (revision) args[ac++] = revision; args[ac++] = "--"; +#ifdef GIT_WINDOWS_NATIVE + GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP); + for (i = 0; i < pathspec->nr; i++) + args[ac++] = pathspec->items[i].match; +#else for (i = 0; i < pathspec->nr; i++) /* pass original pathspec, to be re-parsed */ args[ac++] = pathspec->items[i].original; +#endif status = run_command_v_opt(args, RUN_GIT_CMD); free(args); -- 8< -- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Turn off pathspec magic on "{checkout,reset,add} -p" on native Windows builds
Am 9/2/2013 11:30, schrieb Duy Nguyen: > On Mon, Sep 02, 2013 at 08:42:18AM +0200, Johannes Sixt wrote: >> Am 9/1/2013 4:08, schrieb Nguyễn Thái Ngọc Duy: >>> git-add--interactive.perl rejects arguments with colons in 21e9757 >>> (Hack git-add--interactive to make it work with ActiveState Perl - >>> 2007-08-01). Pathspec magic starts with a colon, so it won't work if >>> these pathspecs are passed to git-add--interactive.perl running with >>> ActiveState Perl. Make sure we only pass plain paths in this case. >>> >>> Signed-off-by: Nguyễn Thái Ngọc Duy >>> --- >>> Johannes, can you check the test suite passes for you with this >>> patch? I assume that Cygwin Perl behaves differently and does not hit >>> this limit. So I keep the special case to GIT_WINDOWS_NATIVE only. >>> I'll resend the patch with a few others on the same topic if it works >>> for you. >> >> It does not help. The error in git-add--interactive is avoided, but the >> failure in t2016-checkout-patch.sh is now: >> >> expecting success: >> set_state dir/foo work head && >> # the third n is to get out in case it mistakenly does not apply >> (echo y; echo n; echo n) | (cd dir && git checkout -p foo) && >> verify_saved_state bar && >> verify_state dir/foo head head >> >> No changes. >> not ok 13 - path limiting works: foo inside dir >> >> and the same "No changes." happens in t7105-reset-patch.sh > > Right. Because I got rid of ':(prefix)foo' form but I passed 'foo' > instead of 'dir/foo'. How about this on top? > > -- 8< -- > diff --git a/builtin/add.c b/builtin/add.c > index 3402239..a138360 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -257,9 +257,15 @@ int run_add_interactive(const char *revision, const char > *patch_mode, > if (revision) > args[ac++] = revision; > args[ac++] = "--"; > +#ifdef GIT_WINDOWS_NATIVE > + GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP); > + for (i = 0; i < pathspec->nr; i++) > + args[ac++] = pathspec->items[i].match; > +#else > for (i = 0; i < pathspec->nr; i++) > /* pass original pathspec, to be re-parsed */ > args[ac++] = pathspec->items[i].original; > +#endif > > status = run_command_v_opt(args, RUN_GIT_CMD); > free(args); > -- 8< -- With this patch, the two tests pass. Which features do we lose on Windows with the previous patch and this fixup? -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Turn off pathspec magic on "{checkout,reset,add} -p" on native Windows builds
On Mon, Sep 2, 2013 at 5:41 PM, Johannes Sixt wrote: > Which features do we lose on Windows with the previous patch and this fixup? New pathspec magic :(glob), :(literal) and :(icase). You can still use them via --*-pathspecs or equivalent env variables. You just can't enable them per individual pathspec. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git svn fetch segfault on exit
> Every git svn call that involves a fetch produces a segmentation fault on > exit (but the operation succeeds). >From what I see, this looks quite similiar to the 'serf' issue I've recently reported to the serf-dev mailing list [1]. It should be fixed by now, so, the latest serf@trunk build should work fine. [1]: https://groups.google.com/d/msg/serf-dev/gOn9HTUN98U/pz0_AqdrmJYJ Regards, Evgeny Kotkov -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git svn fetch segfault on exit
Am 02.09.2013 13:57, schrieb Evgeny Kotkov: Every git svn call that involves a fetch produces a segmentation fault on exit (but the operation succeeds). From what I see, this looks quite similiar to the 'serf' issue I've recently reported to the serf-dev mailing list [1]. It should be fixed by now, so, the latest serf@trunk build should work fine. [1]: https://groups.google.com/d/msg/serf-dev/gOn9HTUN98U/pz0_AqdrmJYJ I've tried the patch for 'serf' included in the thread mentioned above and I can confirm that it fixes the issue for me. Thanks Evgeny! Best regards, Uli. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Teach git to change to a given directory using -C option
On Sun, Sep 01, 2013 at 12:48:23AM -0400, Eric Sunshine wrote: > On Fri, Aug 30, 2013 at 9:35 AM, Nazri Ramliy wrote: >> With this new option, the above can be done with less keystrokes: > > Grammar: s/less/fewer/ > > More below... Thanks for taking the time to review this patch! The fix for the above, and the following issues are in the re-roll below. > The synopsis at the top of git.txt mentions --git-dir and --work-tree. > For consistency, -C probably ought to be mentioned there, as well. Fixed. > Other options which accept a directory, such as --git-dir and > --work-tree, are documented as accepting , but -C is > inconsistently documented as accepting . Fixed. >> + Run as if git were started in instead of the current >> + working directory. If multiple -C options are given, subsequent >> + directory arguments are interpreted relative to the previous one: -C >> + /usr -C src is equivalent to -C /usr/src. This option affects options > > The fragment "interpreted relative" seems ambiguous when absolute > paths are involved. In this re-roll the above text is now: -C :: Run as if git were started in instead of the current working directory. If multiple -C options are given, subsequent relative arguments is interpreted relative to the previous effective directory: "-C /usr -C src" is equivalent to "-C /usr/src", while "-C src -C /usr" is equivalent to "C /usr". This option ... > For existing options accepting an argument, the argument is formatted > as . The -C option does not follow suit. > > As mentioned above, all other options accepting a directory are > documented as taking , but -C is inconsistent and is documented > as taking 'directory' instead. Fixed as "[-C ]" >> +test_description='"-C " option and it effects on other >> path-related options' > > s/it/its/ > s/// Fixed. >> +test_expect_success '"git -C " runs git from the directory ' ' > > s///g Fixed. > Modern git tests tend to place the expected and actual outputs in > files and then use test_cmp to verify that they are identical. For > instance: > > echo "initial in dir1" >expected && > git -C dir1 log --format="%s" >actual && > test_cmp expected actual Fixed. > It would make sense also to test multiple -C options with combinations > of absolute and and relative paths. Fixed - I've added one more test for testing that "-C ./here -C /there" is equivalent to "-C /there" at the end of t0056-git-C.sh. nazri -- >8 -- Subject: [PATCH] Teach git to change to a given directory using -C option This is similar in spirit to to "make -C dir ..." and "tar -C dir ...". Currently it takes more effort (keypresses) to invoke git command in a different directory than the current one without leaving the current directory: 1. (cd ~/foo && git status) git --git-dir=~/foo/.git --work-dir=~/foo status GIT_DIR=~/foo/.git GIT_WORK_TREE=~/foo git status 2. (cd ../..; git grep foo) 3. for d in d1 d2 d3; do (cd $d && git svn rebase); done While doable the methods shown above are arguably more suitable for scripting than quick command line invocations. With this new option, the above can be done with fewer keystrokes: 1. git -C ~/foo status 2. git -C ../.. grep foo 3. for d in d1 d2 d3; do git -C $d svn rebase; done A new test script is added to verify the behavior of this option with other path-related options like --git-dir and --work-tree. Signed-off-by: Nazri Ramliy --- Documentation/git.txt | 16 +- git.c | 15 -- t/t0056-git-C.sh | 83 +++ 3 files changed, 111 insertions(+), 3 deletions(-) create mode 100755 t/t0056-git-C.sh diff --git a/Documentation/git.txt b/Documentation/git.txt index 83edf30..7a1369a 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -9,7 +9,7 @@ git - the stupid content tracker SYNOPSIS [verse] -'git' [--version] [--help] [-c =] +'git' [--version] [--help] [-C ] [-c =] [--exec-path[=]] [--html-path] [--man-path] [--info-path] [-p|--paginate|--no-pager] [--no-replace-objects] [--bare] [--git-dir=] [--work-tree=] [--namespace=] @@ -395,6 +395,20 @@ displayed. See linkgit:git-help[1] for more information, because `git --help ...` is converted internally into `git help ...`. +-C :: + Run as if git were started in instead of the current working + directory. If multiple -C options are given, subsequent relative + arguments are interpreted relative to the previous effective directory: + "-C /usr -C src" is equivalent to "-C /usr/src", while "-C src -C /usr" + is equivalent to "C /usr". This option affects options that expect path + name like --git-dir and --work-tree in that their interpretations of + the path names would be made relative to the effective working + directory caused by the -C option. For example the foll
Re: [PATCH v2 3/8] refs: factor update_ref steps into helpers
On 09/01/2013 02:08 AM, Junio C Hamano wrote: > Brad King writes: >> static struct ref_lock *lock; > > Not the fault of this patch, as the original update_ref() had it > this way, but it is not necessary to keep the value of this variable > across invocations. Let's drop "static" from here, and also the > corresponding variable in the new update_ref(). Fixed in next revision. Thanks, -Brad -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/8] refs: factor delete_ref loose ref step into a helper
On 08/31/2013 12:30 PM, Michael Haggerty wrote: > Given that ret is only returned, you could restore the filename before > the if statement and replace the ret variable with an immediate return > statement: Good idea. Fixed in next revision. Thanks, -Brad -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/8] refs: add update_refs for multiple simultaneous updates
On 08/31/2013 02:19 PM, Michael Haggerty wrote:
> s/themeselves/themselves/
Fixed.
>> +struct ref_update *u1 = (struct ref_update *)(r1);
>> +struct ref_update *u2 = (struct ref_update *)(r2);
>
> If you declare u1 and u2 to be "const struct ref_update *" (i.e., add
> "const"), then you have const correctness and don't need the explicit
> casts. (And the parentheses around r1 and r2 are superfluous in any case.)
Fixed.
>> +ret = strcmp(u1->ref_name, u2->ref_name);
>
> Is there a need to compare more than ref_name? If two entries are found
> with the same name, then ref_update_reject_duplicates() will error out
Junio mentioned possibility of auto-combining identical entries which would
need full ordering. I think that can be added later so for now we can sort
only by ref name. Thanks.
>> +if (!strcmp(updates[i - 1].ref_name, updates[i].ref_name))
>> +break;
>
> The error handling code could be right here instead of the "break"
> statement, removing the need for the "if" conditional.
Fixed.
>> +/* Allocate work space: */
>> +updates = xmalloc(sizeof(struct ref_update) * n);
>
> It seems preferred here to write
>
> updates = xmalloc(sizeof(*updates) * n);
>
> as this will continue to work if the type of updates is ever changed.
Yes, thanks.
> Similarly for the next lines.
>
>> +types = xmalloc(sizeof(int) * n);
>> +locks = xmalloc(sizeof(struct ref_lock *) * n);
>> +delnames = xmalloc(sizeof(const char *) * n);
>
> An alternative to managing separate arrays to hold types and locks would
> be to include the scratch space in struct ref_update and document it
> "for internal use only; need not be initialized by caller". On the one
> hand it's ugly to cruft up the "interface" with internal implementation
> details; on the other hand there is precedent for this sort of thing
> (e.g., ref_lock::force_write or lock_file::on_list) and it would
> simplify the code.
I think the "goto cleanup" reorganization simplifies the code enough
to not need this. After changing "updates" to an array of pointers
it needs to be separate so we can sort. Also "delnames" needs to be
a separate array to pass to repack_without_refs.
>> +/* Copy, sort, and reject duplicate refs: */
>> +memcpy(updates, updates_orig, sizeof(struct ref_update) * n);
>> +qsort(updates, n, sizeof(struct ref_update), ref_update_compare);
>
> You could save some space and memory shuffling (during memcpy() and
> qsort()) if you would declare "updates" to be an array of pointers to
> "struct ref_update" rather than an array of structs. Sorting could then
> be done by moving pointers around instead of moving the structs. This
> would also make it easier for update_refs() to pass information about
> the references back to its caller, should that ever be needed.
Good idea. Changed in next revision.
>> +ret |= update_ref_write(action,
>> +updates[i].ref_name,
>> +updates[i].new_sha1,
>> +locks[i], onerr);
>> +locks[i] = 0; /* freed by update_ref_write */
>> +}
>> +
>
> Hmmm, if one of the calls to update_ref_write() fails, would it be safer
> to abort the rest of the work (especially the reference deletions)?
Yes. Since we already have the lock at this point something must be
going pretty wrong if this fails so it is best to abort altogether.
>> +free(updates);
>> +free(types);
>> +free(locks);
>> +free(delnames);
>> +return ret;
>> +}
>
> There's a lot of duplicated cleanup code in the function. If you put a
> label before the final for loop, and if you initialize the locks array
> to zeros (e.g., by using xcalloc()), then the three exits could all
> share the same code "ret = 1; goto cleanup;".
Done, thanks.
>> +struct ref_update {
>
> Please document this structure, especially the relationship between
> have_old and old_sha1.
Done. I also moved it to the top of the header just under ref_lock
so it can be used by other APIs later.
Thanks,
-Brad
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/8] refs: add update_refs for multiple simultaneous updates
On 09/01/2013 02:08 AM, Junio C Hamano wrote: >> Though the refs themeselves cannot be modified together in a single > > "themselves". Fixed. > I notice that we are using an array of structures and letting qsort > swap 50~64 bytes of data Michael suggested this too, so fixed. > Optionally we could silently dedup multiple identical updates and > not fail it in ref-update-reject-duplicates. But that does not have > to be done until we find people's script would benefit from such a > nicety. We can always be less strict about input later, so I'd like to keep the implementation simpler for now. > By the way, unless there is a strong reason not to do so, > post-increment "i++" (and pre-decrement "--i", if you use it) is the > norm around here. Okay, fixed in entire series. >> +locks[i] = 0; /* freed by update_ref_write */ > > I think what is assigned here is a NULL pointer. Yes, fixed. Thanks, -Brad -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] update-ref: support multiple simultaneous updates
On 08/31/2013 02:42 PM, Michael Haggerty wrote: > On 08/30/2013 08:12 PM, Brad King wrote: >> +If all s can be locked with matching s >> +simultaneously all modifications are performed. Otherwise, no > > Comma after "simultaneously". Fixed. > I agree with Junio that your quoting rules are peculiar. I won't disagree. That's why I asked for suggestions in the original PATCH/RFC cover letter ;) >> +/* Allocate and zero-init a struct ref_update: */ > > Here you can use ARRAY_GROW(). See > > Documentation/technical/api-allocation-growing.txt Fixed. Thanks, -Brad -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] update-ref: support multiple simultaneous updates
On 08/30/2013 06:51 PM, Junio C Hamano wrote: > Brad King writes: >> +With `--stdin`, update-ref reads instructions from standard input and >> +performs all modifications together. Empty lines are ignored. >> +Each non-empty line is parsed as whitespace-separated arguments. >> +Use single-quotes to enclose whitespace and backslashes and an >> +unquoted backslash to escape a single quote. > > That is somewhat unusual. > > When we need to deal with arbitrary strings (like pathnames), other > parts of the system usually give the user two interfaces, --stdin > with and without -z, and the strings are C-quoted when run without > the -z option, and terminated with NUL when run with the -z option. Great, this was the kind of suggestion I was looking for in the original PATCH/RFC cover letter. Thanks. I'll start with the C-quoted version and think about adding -z once we've agreed on that format. >> +Specify updates with >> +lines of the form: >> + >> +[--no-deref] [--] [] > > What is -- doing here? refs given to update-ref begin with refs/ > (otherwise it is HEAD), no? The existing update-ref command line can be used to create all kinds of refs, even starting in "-". I didn't want to add any restriction in the stdin format. I'm not opposed to it though. Thanks, -Brad -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 8/8] update-ref: add test cases covering --stdin signature
On 08/31/2013 11:41 PM, Eric Sunshine wrote: >> + rm -f stdin && >> + touch stdin && > > Unless the timestamp of 'stdin' has particular significance, modern > git tests avoid 'touch' in favor of creating the empty file like this > > >stdin && Fixed. >> + git update-ref --stdin < stdin && > > Style: Git test scripts omit whitespace following <, >, <<, and >>. Fixed. >> +test_expect_success 'stdin fails with bad line lines' ' > > Despite the semantic relationship between all these cases, if there is > a regression in one case, the person reading the verbose output has to > study it carefully to determine the offending case. If you decompose > this monolith so that each case is in its own test_expect_success, > then the regressed case becomes immediately obvious. Yes, of course. Fixed. > multi-line preparations of 'stdin' might be more readable with a heredoc: > > cat >stdin <<-EOF && > $a $m > $b $m > $a $m > EOF Fixed. Thanks, -Brad -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/8] reset: rename update_refs to reset_refs
The function resets refs rather than doing arbitrary updates.
Rename it to allow a future general-purpose update_refs function
to be added.
Signed-off-by: Brad King
---
builtin/reset.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index afa6e02..789ee48 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -219,7 +219,7 @@ static const char **parse_args(const char **argv, const
char *prefix, const char
return argv[0] ? get_pathspec(prefix, argv) : NULL;
}
-static int update_refs(const char *rev, const unsigned char *sha1)
+static int reset_refs(const char *rev, const unsigned char *sha1)
{
int update_ref_status;
struct strbuf msg = STRBUF_INIT;
@@ -350,7 +350,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (!pathspec && !unborn) {
/* Any resets without paths update HEAD to the head being
* switched to, saving the previous head in ORIG_HEAD before. */
- update_ref_status = update_refs(rev, sha1);
+ update_ref_status = reset_refs(rev, sha1);
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(lookup_commit_reference(sha1));
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 8/8] update-ref: add test cases covering --stdin signature
Extend t/t1400-update-ref.sh to cover cases using the --stdin option.
Signed-off-by: Brad King
---
t/t1400-update-ref.sh | 256 +
1 file changed, 256 insertions(+)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index e415ee0..b6d7dfa 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -302,4 +302,260 @@ test_expect_success \
'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER)' \
'test OTHER = $(git cat-file blob "master@{2005-05-26 23:42}:F")'
+a=refs/heads/a
+b=refs/heads/b
+c=refs/heads/c
+z=
+e='""'
+pws='path with space'
+
+test_expect_success 'stdin test setup' '
+ echo "$pws" >"$pws" &&
+ git add -- "$pws" &&
+ git commit -m "$pws"
+'
+
+test_expect_success 'stdin works with no input' '
+ >stdin &&
+ git update-ref --stdin stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: no ref on line: " err
+'
+
+test_expect_success 'stdin fails on bad input line with only --' '
+ echo "--" >stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: no ref on line: --" err
+'
+
+test_expect_success 'stdin fails on bad input line with only --bad-option' '
+ echo "--bad-option" >stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: unknown option --bad-option" err
+'
+
+test_expect_success 'stdin fails on bad ref name' '
+ echo "~a $m" >stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: invalid ref format on line: ~a $m" err
+'
+
+test_expect_success 'stdin fails on badly quoted input' '
+ echo "$a \"master" >stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: badly quoted argument: \\\"master" err
+'
+
+test_expect_success 'stdin fails on bad input line with too many arguments' '
+ echo "$a $m $m $m" >stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: too many arguments on line: $a $m $m $m" err
+'
+
+test_expect_success 'stdin fails on bad input line with too few arguments' '
+ echo "$a" >stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: missing new value on line: $a" err
+'
+
+test_expect_success 'stdin fails with duplicate refs' '
+ cat >stdin <<-EOF &&
+$a $m
+$b $m
+$a $m
+EOF
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: Multiple updates for ref '"'"'$a'"'"' not allowed." err
+'
+
+test_expect_success 'stdin create ref works with no old value' '
+ echo "$a $m" >stdin &&
+ git update-ref --stdin expect &&
+ git rev-parse $a >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'stdin create ref works with zero old value' '
+ echo "$b $m $z" >stdin &&
+ git update-ref --stdin expect &&
+ git rev-parse $b >actual &&
+ test_cmp expect actual &&
+ git update-ref -d $b &&
+ echo "$b $m $e" >stdin &&
+ git update-ref --stdin expect &&
+ git rev-parse $b >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'stdin create ref works with path with space to blob' '
+ echo "refs/blobs/pws \"$m:$pws\"" >stdin &&
+ git update-ref --stdin expect &&
+ git rev-parse refs/blobs/pws >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'stdin create ref fails with wrong old value' '
+ echo "$c $m $m~1" >stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: Cannot lock the ref '"'"'$c'"'"'" err &&
+ test_must_fail git rev-parse --verify -q $c
+'
+
+test_expect_success 'stdin create ref fails with bad old value' '
+ echo "$c $m does-not-exist" >stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: invalid old value on line: $c $m does-not-exist" err &&
+ test_must_fail git rev-parse --verify -q $c
+'
+
+test_expect_success 'stdin create ref fails with bad new value' '
+ echo "$c does-not-exist" >stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: invalid new value on line: $c does-not-exist" err &&
+ test_must_fail git rev-parse --verify -q $c
+'
+
+test_expect_success 'stdin update ref works with right old value' '
+ echo "$b $m~1 $m" >stdin &&
+ git update-ref --stdin expect &&
+ git rev-parse $b >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'stdin update ref fails with wrong old value' '
+ echo "$b $m~1 $m" >stdin &&
+ test_must_fail git update-ref --stdin err &&
+ grep "fatal: Cannot lock the ref '"'"'$b'"'"'" err &&
+ git rev-parse $m~1 >expect &&
+ git rev-parse $b >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'stdin delete ref fails with wrong old value' '
+ echo "$a $e $m~1" >stdin &&
+ test_must_fail git update-ref --st
[PATCH v3 4/8] refs: factor delete_ref loose ref step into a helper
Factor loose ref deletion into helper function delete_ref_loose to allow
later use elsewhere.
Signed-off-by: Brad King
---
refs.c | 27 +--
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index 4347826..ab9d22e 100644
--- a/refs.c
+++ b/refs.c
@@ -2450,24 +2450,31 @@ static int repack_without_ref(const char *refname)
return commit_packed_refs();
}
+static int delete_ref_loose(struct ref_lock *lock, int flag)
+{
+ if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
+ /* loose */
+ int err, i = strlen(lock->lk->filename) - 5; /* .lock */
+
+ lock->lk->filename[i] = 0;
+ err = unlink_or_warn(lock->lk->filename);
+ lock->lk->filename[i] = '.';
+ if (err && errno != ENOENT)
+ return 1;
+ }
+ return 0;
+}
+
int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
{
struct ref_lock *lock;
- int err, i = 0, ret = 0, flag = 0;
+ int ret = 0, flag = 0;
lock = lock_ref_sha1_basic(refname, sha1, delopt, &flag);
if (!lock)
return 1;
- if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
- /* loose */
- i = strlen(lock->lk->filename) - 5; /* .lock */
- lock->lk->filename[i] = 0;
- err = unlink_or_warn(lock->lk->filename);
- if (err && errno != ENOENT)
- ret = 1;
+ ret |= delete_ref_loose(lock, flag);
- lock->lk->filename[i] = '.';
- }
/* removing the loose one could have resurrected an earlier
* packed one. Also, if it was not loose we need to repack
* without it.
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/8] refs: add function to repack without multiple refs
Generalize repack_without_ref as repack_without_refs to support a list
of refs and implement the former in terms of the latter.
Signed-off-by: Brad King
---
refs.c | 29 ++---
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index ab9d22e..599504b 100644
--- a/refs.c
+++ b/refs.c
@@ -2414,25 +2414,35 @@ static int curate_packed_ref_fn(struct ref_entry
*entry, void *cb_data)
return 0;
}
-static int repack_without_ref(const char *refname)
+static int repack_without_refs(const char **refnames, int n)
{
struct ref_dir *packed;
struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
struct string_list_item *ref_to_delete;
+ int i, removed = 0;
+
+ /* Look for a packed ref: */
+ for (i = 0; i < n; i++)
+ if (get_packed_ref(refnames[i]))
+ break;
- if (!get_packed_ref(refname))
- return 0; /* refname does not exist in packed refs */
+ /* Avoid locking if we have nothing to do: */
+ if (i == n)
+ return 0; /* no refname exists in packed refs */
if (lock_packed_refs(0)) {
unable_to_lock_error(git_path("packed-refs"), errno);
- return error("cannot delete '%s' from packed refs", refname);
+ return error("cannot delete '%s' from packed refs",
refnames[i]);
}
packed = get_packed_refs(&ref_cache);
- /* Remove refname from the cache: */
- if (remove_entry(packed, refname) == -1) {
+ /* Remove refnames from the cache: */
+ for (i = 0; i < n; i++)
+ if (remove_entry(packed, refnames[i]) != -1)
+ removed = 1;
+ if (!removed) {
/*
-* The packed entry disappeared while we were
+* All packed entries disappeared while we were
* acquiring the lock.
*/
rollback_packed_refs();
@@ -2450,6 +2460,11 @@ static int repack_without_ref(const char *refname)
return commit_packed_refs();
}
+static int repack_without_ref(const char *refname)
+{
+ return repack_without_refs(&refname, 1);
+}
+
static int delete_ref_loose(struct ref_lock *lock, int flag)
{
if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 6/8] refs: add update_refs for multiple simultaneous updates
Add 'struct ref_update' to encode the information needed to update or
delete a ref (name, new sha1, optional old sha1, no-deref flag). Add
function 'update_refs' accepting an array of updates to perform. First
sort the input array to order locks consistently everywhere and reject
multiple updates to the same ref. Then acquire locks on all refs with
verified old values. Then update or delete all refs accordingly. Fail
if any one lock cannot be obtained or any one old value does not match.
Though the refs themselves cannot be modified together in a single
atomic transaction, this function does enable some useful semantics.
For example, a caller may create a new branch starting from the head of
another branch and rewind the original branch at the same time. This
transfers ownership of commits between branches without risk of losing
commits added to the original branch by a concurrent process, or risk of
a concurrent process creating the new branch first.
Signed-off-by: Brad King
---
refs.c | 100
refs.h | 20 +
2 files changed, 120 insertions(+)
diff --git a/refs.c b/refs.c
index 599504b..53e8774 100644
--- a/refs.c
+++ b/refs.c
@@ -3237,6 +3237,106 @@ int update_ref(const char *action, const char *refname,
return update_ref_write(action, refname, sha1, lock, onerr);
}
+static int ref_update_compare(const void *r1, const void *r2)
+{
+ const struct ref_update * const *u1 = r1;
+ const struct ref_update * const *u2 = r2;
+ return strcmp((*u1)->ref_name, (*u2)->ref_name);
+}
+
+static int ref_update_reject_duplicates(struct ref_update **updates, int n,
+ enum action_on_err onerr)
+{
+ int i;
+ for (i = 1; i < n; i++)
+ if (!strcmp(updates[i - 1]->ref_name, updates[i]->ref_name)) {
+ const char *str =
+ "Multiple updates for ref '%s' not allowed.";
+ switch (onerr) {
+ case MSG_ON_ERR:
+ error(str, updates[i]->ref_name); break;
+ case DIE_ON_ERR:
+ die(str, updates[i]->ref_name); break;
+ case QUIET_ON_ERR:
+ break;
+ }
+ return 1;
+ }
+ return 0;
+}
+
+int update_refs(const char *action, const struct ref_update **updates_orig,
+ int n, enum action_on_err onerr)
+{
+ int ret = 0, delnum = 0, i;
+ struct ref_update **updates;
+ int *types;
+ struct ref_lock **locks;
+ const char **delnames;
+
+ if (!updates_orig || !n)
+ return 0;
+
+ /* Allocate work space: */
+ updates = xmalloc(sizeof(*updates) * n);
+ types = xmalloc(sizeof(*types) * n);
+ locks = xcalloc(n, sizeof(*locks));
+ delnames = xmalloc(sizeof(*delnames) * n);
+
+ /* Copy, sort, and reject duplicate refs: */
+ memcpy(updates, updates_orig, sizeof(*updates) * n);
+ qsort(updates, n, sizeof(*updates), ref_update_compare);
+ ret = ref_update_reject_duplicates(updates, n, onerr);
+ if (ret)
+ goto cleanup;
+
+ /* Acquire all locks while verifying old values: */
+ for (i = 0; i < n; i++) {
+ locks[i] = update_ref_lock(updates[i]->ref_name,
+ (updates[i]->have_old ?
+ updates[i]->old_sha1 : NULL),
+ updates[i]->flags,
+ &types[i], onerr);
+ if (!locks[i]) {
+ ret = 1;
+ goto cleanup;
+ }
+ }
+
+ /* Perform updates first so live commits remain referenced: */
+ for (i = 0; i < n; i++)
+ if (!is_null_sha1(updates[i]->new_sha1)) {
+ ret = update_ref_write(action,
+ updates[i]->ref_name,
+ updates[i]->new_sha1,
+ locks[i], onerr);
+ locks[i] = NULL; /* freed by update_ref_write */
+ if (ret)
+ goto cleanup;
+ }
+
+ /* Perform deletes now that updates are safely completed: */
+ for (i = 0; i < n; i++)
+ if (locks[i]) {
+ delnames[delnum++] = locks[i]->ref_name;
+ ret |= delete_ref_loose(locks[i], types[i]);
+ }
+ ret |= repack_without_refs(delnames, delnum);
+ for (i = 0; i < delnum; i++)
+ unlink_or_warn(git_path("logs/%s", delnames[i]));
+ clear_loose_ref_cache(&ref_cache);
+
+cleanup:
+ for (i = 0; i < n;
[PATCH v3 0/8] Multiple simultaneously locked ref updates
Hi Folks, Here is the third revision of a series to support locking multiple refs at the same time to update all of them consistently. The previous revisions of the series can be found at $gmane/233260 and $gmane/233458. Updates since the previous revision of the series: * Incorporated style fixes suggested in patches 6-8. * In patch 3, the local "lock" variables in update_ref_lock and update_ref now drop the existing "static" declaration. * In patch 4, delete_ref_loose internals have been cleaned up as Michael suggested. * In patch 6: - struct ref_update has been documented - update_refs now takes an array of pointers to struct ref_update as Michael and Junio both suggested - update_refs return cases were simplified with a label and goto - update_refs now stops immediately if any ref write fails - ref_update_compare now compares only the ref name * In patch 7, another new input format is proposed. It now uses quoting based on unquote_c_style. * In patch 8, more new test cases have been added. Failure cases are now covered in separate steps to simplify diagnosis. -Brad Brad King (8): reset: rename update_refs to reset_refs refs: report ref type from lock_any_ref_for_update refs: factor update_ref steps into helpers refs: factor delete_ref loose ref step into a helper refs: add function to repack without multiple refs refs: add update_refs for multiple simultaneous updates update-ref: support multiple simultaneous updates update-ref: add test cases covering --stdin signature Documentation/git-update-ref.txt | 20 ++- branch.c |2 +- builtin/commit.c |2 +- builtin/fetch.c |3 +- builtin/receive-pack.c |3 +- builtin/reflog.c |2 +- builtin/replace.c|2 +- builtin/reset.c |4 +- builtin/tag.c|2 +- builtin/update-ref.c | 103 ++- fast-import.c|2 +- refs.c | 191 refs.h | 22 +++- sequencer.c |3 +- t/t1400-update-ref.sh| 256 ++ 15 files changed, 578 insertions(+), 39 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 7/8] update-ref: support multiple simultaneous updates
Add a --stdin signature to read update instructions from standard input
and apply multiple ref updates together. Use an input format that
supports any update that could be specified via the command-line,
including object names like "branch:path with space".
Signed-off-by: Brad King
---
Documentation/git-update-ref.txt | 20 +++-
builtin/update-ref.c | 103 +-
2 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index 0df13ff..01019f1 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely
SYNOPSIS
[verse]
-'git update-ref' [-m ] (-d [] | [--no-deref]
[])
+'git update-ref' [-m ] (-d [] | [--no-deref]
[] | --stdin)
DESCRIPTION
---
@@ -58,6 +58,24 @@ archive by creating a symlink tree).
With `-d` flag, it deletes the named after verifying it
still contains .
+With `--stdin`, update-ref reads instructions from standard input and
+performs all modifications together. Empty lines are ignored.
+Each non-empty line is parsed as whitespace-separated arguments.
+Quote arguments containing whitespace as if in C source code.
+Specify updates with lines of the form:
+
+ [--no-deref] [--] []
+
+Lines of any other format or a repeated produce an error.
+Specify a zero to delete a ref and/or a zero
+to make sure that a ref not exist. Use either 40 "0" or the
+empty string (written as "") to specify a zero value.
+
+If all s can be locked with matching s
+simultaneously, all modifications are performed. Otherwise, no
+modifications are performed. Note that while each individual
+ is updated or deleted atomically, a concurrent reader may
+still see a subset of the modifications.
Logging Updates
---
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 51d2684..12a3c76 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -2,23 +2,115 @@
#include "refs.h"
#include "builtin.h"
#include "parse-options.h"
+#include "quote.h"
static const char * const git_update_ref_usage[] = {
N_("git update-ref [options] -d []"),
N_("git update-ref [options] []"),
+ N_("git update-ref [options] --stdin"),
NULL
};
+static int updates_alloc;
+static int updates_count;
+static const struct ref_update **updates;
+
+static const char* update_refs_stdin_next_arg(const char* next,
+ struct strbuf *arg)
+{
+ /* Skip leading whitespace: */
+ while (isspace(*next))
+ ++next;
+
+ /* Return NULL when no argument is found: */
+ if (!*next)
+ return NULL;
+
+ /* Parse the argument: */
+ strbuf_reset(arg);
+ if (*next == '"') {
+ if (unquote_c_style(arg, next, &next))
+ die("badly quoted argument: %s", next);
+ return next;
+ }
+ while (*next && !isspace(*next))
+ strbuf_addch(arg, *next++);
+ return next;
+}
+
+static void update_refs_stdin(const char *line)
+{
+ int options = 1, flags = 0, argc = 0;
+ char *argv[3] = {0, 0, 0};
+ struct strbuf arg = STRBUF_INIT;
+ struct ref_update *update;
+ const char *next = line;
+
+ /* Skip blank lines: */
+ if (!line[0])
+ return;
+
+ /* Parse arguments on this line: */
+ while ((next = update_refs_stdin_next_arg(next, &arg)) != NULL) {
+ if (options && arg.buf[0] == '-')
+ if (!strcmp(arg.buf, "--no-deref"))
+ flags |= REF_NODEREF;
+ else if (!strcmp(arg.buf, "--"))
+ options = 0;
+ else
+ die("unknown option %s", arg.buf);
+ else if (argc >= 3)
+ die("too many arguments on line: %s", line);
+ else {
+ argv[argc++] = xstrdup(arg.buf);
+ options = 0;
+ }
+ }
+ strbuf_release(&arg);
+
+ /* Allocate and zero-init a struct ref_update: */
+ update = xcalloc(1, sizeof(*update));
+ ALLOC_GROW(updates, updates_count+1, updates_alloc);
+ updates[updates_count++] = update;
+
+ /* Set the update ref_name: */
+ if (!argv[0])
+ die("no ref on line: %s", line);
+ if (check_refname_format(argv[0], REFNAME_ALLOW_ONELEVEL))
+ die("invalid ref format on line: %s", line);
+ update->ref_name = argv[0];
+ argv[0] = 0;
+
+ /* Set the update new_sha1 and, if specified, old_sha1: */
+ if (!argv[1])
+ die("missing new value on line: %s", line);
+ if (*argv[1] && get_sha1(argv[1], update->new_sha1))
+
[PATCH v3 2/8] refs: report ref type from lock_any_ref_for_update
Expose lock_ref_sha1_basic's type_p argument to callers of
lock_any_ref_for_update. Update all call sites to ignore it by passing
NULL for now.
Signed-off-by: Brad King
---
branch.c |2 +-
builtin/commit.c |2 +-
builtin/fetch.c|3 ++-
builtin/receive-pack.c |3 ++-
builtin/reflog.c |2 +-
builtin/replace.c |2 +-
builtin/tag.c |2 +-
fast-import.c |2 +-
refs.c |7 ---
refs.h |2 +-
sequencer.c|3 ++-
11 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/branch.c b/branch.c
index c5c6984..f2d383f 100644
--- a/branch.c
+++ b/branch.c
@@ -291,7 +291,7 @@ void create_branch(const char *head,
hashcpy(sha1, commit->object.sha1);
if (!dont_change_ref) {
- lock = lock_any_ref_for_update(ref.buf, NULL, 0);
+ lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);
if (!lock)
die_errno(_("Failed to lock ref for update"));
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 10acc53..be08f41 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1618,7 +1618,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
!current_head
? NULL
: current_head->object.sha1,
- 0);
+ 0, NULL);
nl = strchr(sb.buf, '\n');
if (nl)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index d784b2e..28e4025 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -246,7 +246,8 @@ static int s_update_ref(const char *action,
rla = default_rla.buf;
snprintf(msg, sizeof(msg), "%s: %s", rla, action);
lock = lock_any_ref_for_update(ref->name,
- check_old ? ref->old_sha1 : NULL, 0);
+ check_old ? ref->old_sha1 : NULL,
+ 0, NULL);
if (!lock)
return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
STORE_REF_ERROR_OTHER;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e3eb5fc..a323070 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -524,7 +524,8 @@ static const char *update(struct command *cmd)
return NULL; /* good */
}
else {
- lock = lock_any_ref_for_update(namespaced_name, old_sha1, 0);
+ lock = lock_any_ref_for_update(namespaced_name, old_sha1,
+ 0, NULL);
if (!lock) {
rp_error("failed to lock %s", name);
return "failed to lock";
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 54184b3..28d756a 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -366,7 +366,7 @@ static int expire_reflog(const char *ref, const unsigned
char *sha1, int unused,
* we take the lock for the ref itself to prevent it from
* getting updated.
*/
- lock = lock_any_ref_for_update(ref, sha1, 0);
+ lock = lock_any_ref_for_update(ref, sha1, 0, NULL);
if (!lock)
return error("cannot lock ref '%s'", ref);
log_file = git_pathdup("logs/%s", ref);
diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..1ecae8d 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -105,7 +105,7 @@ static int replace_object(const char *object_ref, const
char *replace_ref,
else if (!force)
die("replace ref '%s' already exists", ref);
- lock = lock_any_ref_for_update(ref, prev, 0);
+ lock = lock_any_ref_for_update(ref, prev, 0, NULL);
if (!lock)
die("%s: cannot lock the ref", ref);
if (write_ref_sha1(lock, repl, NULL) < 0)
diff --git a/builtin/tag.c b/builtin/tag.c
index af3af3f..2c867d2 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -577,7 +577,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (annotate)
create_tag(object, tag, &buf, &opt, prev, object);
- lock = lock_any_ref_for_update(ref.buf, prev, 0);
+ lock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);
if (!lock)
die(_("%s: cannot lock the ref"), ref.buf);
if (write_ref_sha1(lock, object, NULL) < 0)
diff --git a/fast-import.c b/fast-import.c
index 23f625f..5c329f6 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1678,7 +1678,7 @@ static int update_branch(struct branch *b)
return 0;
if (read_ref(b->name, old_sha1))
hashclr(old_sha1);
- lock = lock_any_ref_for_update(b->name, old_sha1, 0);
+ lock = lock_any_ref_for_update(b->name, ol
[PATCH v3 3/8] refs: factor update_ref steps into helpers
Factor the lock and write steps and error handling into helper functions
update_ref_lock and update_ref_write to allow later use elsewhere.
Expose lock_any_ref_for_update's type_p to update_ref_lock callers.
While at it, drop "static" from the local "lock" variable as it is not
necessary to keep across invocations.
Signed-off-by: Brad King
---
refs.c | 30 --
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index c69fd68..4347826 100644
--- a/refs.c
+++ b/refs.c
@@ -3170,12 +3170,13 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
}
-int update_ref(const char *action, const char *refname,
- const unsigned char *sha1, const unsigned char *oldval,
- int flags, enum action_on_err onerr)
+static struct ref_lock *update_ref_lock(const char *refname,
+ const unsigned char *oldval,
+ int flags, int *type_p,
+ enum action_on_err onerr)
{
- static struct ref_lock *lock;
- lock = lock_any_ref_for_update(refname, oldval, flags, NULL);
+ struct ref_lock *lock;
+ lock = lock_any_ref_for_update(refname, oldval, flags, type_p);
if (!lock) {
const char *str = "Cannot lock the ref '%s'.";
switch (onerr) {
@@ -3183,8 +3184,14 @@ int update_ref(const char *action, const char *refname,
case DIE_ON_ERR: die(str, refname); break;
case QUIET_ON_ERR: break;
}
- return 1;
}
+ return lock;
+}
+
+static int update_ref_write(const char *action, const char *refname,
+ const unsigned char *sha1, struct ref_lock *lock,
+ enum action_on_err onerr)
+{
if (write_ref_sha1(lock, sha1, action) < 0) {
const char *str = "Cannot update the ref '%s'.";
switch (onerr) {
@@ -3197,6 +3204,17 @@ int update_ref(const char *action, const char *refname,
return 0;
}
+int update_ref(const char *action, const char *refname,
+ const unsigned char *sha1, const unsigned char *oldval,
+ int flags, enum action_on_err onerr)
+{
+ struct ref_lock *lock;
+ lock = update_ref_lock(refname, oldval, flags, 0, onerr);
+ if (!lock)
+ return 1;
+ return update_ref_write(action, refname, sha1, lock, onerr);
+}
+
struct ref *find_ref_by_name(const struct ref *list, const char *name)
{
for ( ; list; list = list->next)
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 7/8] update-ref: support multiple simultaneous updates
On 09/02/2013 01:48 PM, Brad King wrote:
> + /* Parse the argument: */
> + strbuf_reset(arg);
> + if (*next == '"') {
> + if (unquote_c_style(arg, next, &next))
> + die("badly quoted argument: %s", next);
> + return next;
> + }
> + while (*next && !isspace(*next))
> + strbuf_addch(arg, *next++);
> + return next;
This quoting proposal was written in response to $gmane/233479:
On 08/30/2013 06:51 PM, Junio C Hamano wrote:
> When we need to deal with arbitrary strings (like pathnames), other
> parts of the system usually give the user two interfaces, --stdin
> with and without -z, and the strings are C-quoted when run without
> the -z option, and terminated with NUL when run with the -z option.
1. Do we want to allow arbitrary non-space characters in unquoted
arguments (while loop above) or reserve some syntax for future use?
2. Thinking about how the -z variation might work, I ran:
$ git grep '\[0\] == '"'"'"' -- '*.c'
builtin/check-attr.c: if (line_termination && buf.buf[0] == '"') {
builtin/check-ignore.c: if (line_termination && buf.buf[0] == '"') {
builtin/checkout-index.c: if (line_termination &&
buf.buf[0] == '"') {
builtin/hash-object.c: if (buf.buf[0] == '"') {
builtin/mktree.c: if (line_termination && path[0] == '"') {
builtin/update-index.c: if (line_termination && path_name[0] == '"') {
builtin/update-index.c: if (line_termination && buf.buf[0] ==
'"') {
All of these support quoting only in the non-z mode (the hash-object.c
line follows a getline using hard-coded '\n'). However, they are
all in cases looking for one value on a line or at the end of a line
so their -z option allows NUL-terminated lines containing LF.
What distinguishes the "update-ref --stdin" case is that we want to
represent multiple arguments on one line, each allowing arbitrary
characters or an empty string. From a brief search a couple places
I found that do something related are:
* apply: Read multiple paths from a diff header, using unquote_c_style
for quoted paths and separated by spaces. There is no -z input mode.
* config: Output keyword=value\n becomes keyword\nvalue\0 in -z mode.
This works because the first piece (keyword) cannot have a LF
and there is at most one value so all LFs belong to it.
* quote.c: sq_dequote_to_argv handles single quotes like a shell
would but allows only one space between arguments. No -z mode.
This is similar to my v2 proposal.
If we use unquote_c_style and spaces to divide LF-terminated lines,
how shall we divide arguments on NUL-terminated lines?
Thanks,
-Brad
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Call For Papers
Dear Colleagues, You are invited to upload your papers to our upcoming conferences Paris, France, October 29-31, 2013. More details: http://www.wseas.org Scientific Sponsors: a) University of Zagreb, Croatia, b) Music Academy "Studio Musica", Italy, c) Constanta Maritime University, Romania, d) European Institute of Informatics and Educational Technology in Belgrade, Serbia, e) Ain Shams University, Egypt Nanjing, China, November 17-19, 2013. More details: http://www.wseas.org Scientific Sponsors: a) Nanjing Forestry University, China, b) College of Computer Science & Department of Biomedical Informatics, Asia University, Taiwan c) Music Academy "Studio Musica", Italy, Publication: Accepted Papers will be published in our: (a) Hard-Copy of the Proceedings (Indexed in ISI, SCOPUS, AMS, ACS, CiteSeerX, Zentralblatt, British Library, EBSCO, SWETS, EMBASE, CAS etc) and (b) CD-ROM Proceedings (Indexed in ISI, SCOPUS, AMS, ACS, CiteSeerX, Zentralblatt, British Library, EBSCO, SWETS, EMBASE, CAS etc) (c) After new peer, thorough review of their extended versions in a well-known Journal (SCOPUS, AMS, Elsevier, Zentrablat, ACM etc... indexed) (d) E-Library with free access REVIEWE PROCESS: Papers in WSEAS Conferences and Journals are subject to thorough peer review. The names of the Reviewers, see: http://www.wseas.org/wseas/cms.action?id=5628 appear in the Proceedings and are, consequently, sent to all collaborating indexes. Qualified colleagues are always invited to participate in the review process. Visit this page for more details: http://www.wseas.org/wseas/cms.action?id=88 Nobody can qualify to become a WSEAS Reviewer without proper academic qualifications (i.e. recent publications in SCOPUS and ISI). Reviewers whose review work is not thorough or who are not longer active are immediately removed from our reviewers' list: http://www.wseas.org/wseas/cms.action?id=5321 Therefore, our list only includes active reviewers. The names of all conference reviewers are also published in the WSEAS post-conference reports. See: http://www.wseas.org/wseas/cms.action?id=87 Additionally, WSEAS is the only academic publisher with open access journals (public PDF on the web plus hard copy), where the authors do not pay any article processing fees. See: http://www.wseas.org/wseas/cms.action?id=43 INVITED PLENARY SPEECHES: See them by conference in http://www.wseas.org Feel free to visit the web site of the conference at http://www.wseas.org for all the information that you need. Each page has a submission link for you to upload your papers. Kindly note that we do not accept papers by email and that you should only upload your full paper as we do not accept abstracts. We are at your disposal if you have any questions. The WSEAS Team Ag. I. Theologou 17-23 Zografou, 15773, Athens, Greece Phone +30 210 747 3313 FAX +30 211 800 1964 URL: http://www.wseas.org This message satisfies the requirements of the European legislation on advertising (Directiva 2002/58/CE of the European Parliament). If you do not wish to receive e-mails from us (i.e.if you want to un~subscribe), send an email to wseas-t...@wseas.org with the following command as Subject: un~subscribe email1, email2, email3, where Email: email1, email2, email3 are all the possible emails that you have. For example un~subscribe johnsm...@gmail.com, jsm...@server.mbu.uk etc.. Please accept our apologies for any inconvenience caused. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-p4 out of memory for very large repository
I guess you could try changing the OOM score for git-fast-import. change /proc//oomadj. I think a value of -31 would make it very unlikely to be killed. On 29/08/13 23:46, Pete Wyckoff wrote: cmt...@gmail.com wrote on Wed, 28 Aug 2013 11:41 -0400: On Mon, Aug 26, 2013 at 09:47:56AM -0400, Corey Thompson wrote: You are correct that git-fast-import is killed by the OOM killer, but I was unclear about which process was malloc()ing so much memory that the OOM killer got invoked (as other completely unrelated processes usually also get killed when this happens). Unless there's one gigantic file in one change that gets removed by another change, I don't think that's the problem; as I mentioned in another email, the machine has 32GB physical memory and the largest single file in the current head is only 118MB. Even if there is a very large transient file somewhere in the history, I seriously doubt it's tens of gigabytes in size. I have tried watching it with top before, but it takes several hours before it dies. I haven't been able to see any explosion of memory usage, even within the final hour, but I've never caught it just before it dies, either. I suspect that whatever the issue is here, it happens very quickly. If I'm unable to get through this today using the incremental p4 sync method you described, I'll try running a full-blown clone overnight with top in batch mode writing to a log file to see whether it catches anything. Thanks again, Corey Unforunately I have not made much progress. The incremental sync method fails with the output pasted below. The change I specified is only one change number above where that repo was cloned... I usually just do "git p4 sync @505859". The error message below crops up when things get confused. Usually after a previous error. I tend to destroy the repo and try again. Sorry I don't can't explain better what's happening here. It's not a memory issue; it reports only 24 MB used. So I tried a 'git p4 rebase' overnight with top running, and as I feared I did not see anything out of the ordinary. git, git-fast-import, and git-p4 all hovered under 1.5% MEM the entire time, right up until death. The last entry in my log shows git-fast-import at 0.8%, with git and git-p4 at 0.0% and 0.1%, respectively. I could try again with a more granular period, but I feel like this method is ultimately a goose chase. Bizarre. There is no good explanation why memory usage would go up to 32 GB (?) within one top interval (3 sec ?). My theory about one gigantic object is debunked: you have only the 118 MB one. Perhaps there's some container or process memory limit, as Luke guessed, but it's not obvious here. The other big hammer is "strace". If you're still interested in playing with this, you could do: strace -vf -tt -s 200 -o /tmp/strace.out git p4 clone and hours later, see if something suggests itself toward the end of that output file. -- Pete -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] revision: introduce --exclude= to tame wildcards
Am 31.08.2013 01:55, schrieb Junio C Hamano:
> People often find "git log --branches" etc. that includes _all_
> branches is cumbersome to use when they want to grab most but except
> some. The same applies to --tags, --all and --glob.
>
> Teach the revision machinery to remember patterns, and then upon the
> next such a globbing option, exclude those that match the pattern.
>
> With this, I can view only my integration branches (e.g. maint,
> master, etc.) without topic branches, which are named after two
> letters from primary authors' names, slash and topic name.
>
> git rev-list --no-walk --exclude=??/* --branches |
> git name-rev --refs refs/heads/* --stdin
>
> This one shows things reachable from local and remote branches that
> have not been merged to the integration branches.
>
> git log --remotes --branches --not --exclude=??/* --branches
>
> It may be a bit rough around the edges, in that the pattern to give
> the exclude option depends on what globbing option follows. In
> these examples, the pattern "??/*" is used, not "refs/heads/??/*",
> because the globbing option that follows the -"-exclude="
> is "--branches". As each use of globbing option resets previously
> set "--exclude", this may not be such a bad thing, though.
I argued "--except should trump everything" earlier, but the case
involving --not
--branches --except maint --not master
to mean the same as
--branches --except maint master
just does not make sense.
An alternative would be that --not would divide the command line
arguments into ranges within which one --except would subtract
subsequent refs from earlier globbing arguments in the same range.
That's not simpler to explain than your current proposal.
So I like the relative simplicity of this approach. Here is a bit of
documentation.
--- 8< ---
Subject: [PATCH] document --exclude option
Signed-off-by: Johannes Sixt
---
Documentation/rev-list-options.txt | 15 +++
1 file changed, 15 insertions(+)
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index 5bdfb42..650c252 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -174,6 +174,21 @@ parents) and `--max-parents=-1` (negative numbers denote
no upper limit).
is automatically prepended if missing. If pattern lacks '?',
'{asterisk}',
or '[', '/{asterisk}' at the end is implied.
+--exclude=::
+
+ Do not include refs matching '' that the next `--all`,
+ `--branches`, `--tags`, `--remotes`, or `--glob` would otherwise
+ consider. Repetitions of this option accumulate exclusion patterns
+ up to the next `--all`, `--branches`, `--tags`, `--remotes`, or
+ `--glob` option (other options or arguments do not clear
+ accumlated patterns).
++
+The patterns given should not begin with `refs/heads`, `refs/tags`, or
+`refs/remotes` when applied to `--branches`, `--tags`, or `--remotes`,
+restrictively, and they must begin with `refs/` when applied to `--glob`
+or `--all`. If a trailing '/{asterisk}' is intended, it must be given
+explicitly.
+
--ignore-missing::
Upon seeing an invalid object name in the input, pretend as if
--
1.8.4.33.gd68f7e8
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git installation on Mac OS X - problem
Sorry... I misread the README... it does indeed say /usr/local/git, which is where the new binary was installed correctly. Eyes have been crossed by all of these software re-installations and terminal commands. :P Thanks. ~Ted On 2013-09-02, at 1:16 PM, Ted Wood wrote: > > I've recently re-installed all of my MacPorts installations due to a > third-party application corrupting my installation. Upon attempting to > install git-core via MacPorts, it hangs on the "Building" stage, with no disk > activity (clang process is active in Terminal). So, I attempted to download > the .pkg install from git-scm.com. This Installer says the installation was > complete, but Apple's outdated version of git (1.7.3) is still the active > binary. > > Two observations: > 1) The README.txt file says that Git will be installed into /usr/local/bin > 2) The installation actually seems to be installed into /usr/bin, according > to the installation log, which I have attached. This is the same location as > Apple's version of Git. > > Of particular interest is the PATH variable setting at the very top of the > log file, which makes no reference to /usr/local/bin... only /usr/bin. > >Installer[18576]: Env: PATH=/usr/bin:/bin:/usr/sbin:/sbin > > I've rarely experienced problems with installing software in the past. Is > this an oversight on the part of the package assembler, or am I missing > something? > > ~Ted > > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/23] pack v4: commit object encoding
On Tue, Aug 27, 2013 at 11:25 AM, Nicolas Pitre wrote: > This goes as follows: > > - Tree reference: either variable length encoding of the index > into the SHA1 table or the literal SHA1 prefixed by 0 (see > add_sha1_ref()). > > - Parent count: variable length encoding of the number of parents. > This is normally going to occupy a single byte but doesn't have to. > > - List of parent references: a list of add_sha1_ref() encoded references, > or nothing if the parent count was zero. With .pack v3 it's impossible to create delta cycles (3b910d0 add tests for indexing packs with delta cycles - 2013-08-23) but it is possible with .pack v4 (and therefore at least index-pack needs to detect and reject them), correct? Some malicious user can create commit A with parent ref 1, then make the SHA-1 table so that ref 1 is A. The same with the new tree representation. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git installation on Mac OS X - problem
I've recently re-installed all of my MacPorts installations due to a third-party application corrupting my installation. Upon attempting to install git-core via MacPorts, it hangs on the "Building" stage, with no disk activity (clang process is active in Terminal). So, I attempted to download the .pkg install from git-scm.com. This Installer says the installation was complete, but Apple's outdated version of git (1.7.3) is still the active binary. Two observations: 1) The README.txt file says that Git will be installed into /usr/local/bin 2) The installation actually seems to be installed into /usr/bin, according to the installation log, which I have attached. This is the same location as Apple's version of Git. Of particular interest is the PATH variable setting at the very top of the log file, which makes no reference to /usr/local/bin... only /usr/bin. Installer[18576]: Env: PATH=/usr/bin:/bin:/usr/sbin:/sbin I've rarely experienced problems with installing software in the past. Is this an oversight on the part of the package assembler, or am I missing something? ~Ted Sep 2 13:04:36 topdog Installer[18576]: @(#)PROGRAM:Install PROJECT:Install-735 Sep 2 13:04:36 topdog Installer[18576]: @(#)PROGRAM:Installer PROJECT:Installer-617 Sep 2 13:04:36 topdog Installer[18576]: Hardware: MacBookPro3,1 @ 2.20 GHz (x 2), 4096 MB RAM Sep 2 13:04:36 topdog Installer[18576]: Running OS Build: Mac OS X 10.8.4 (12E55) Sep 2 13:04:36 topdog Installer[18576]: Env: PATH=/usr/bin:/bin:/usr/sbin:/sbin Sep 2 13:04:36 topdog Installer[18576]: Env: TMPDIR=/var/folders/h9/d7vc3w7j7jg0lsxsf7bvx6hhgn/T/ Sep 2 13:04:36 topdog Installer[18576]: Env: SHELL=/bin/bash Sep 2 13:04:36 topdog Installer[18576]: Env: HOME=/Users/tedwood Sep 2 13:04:36 topdog Installer[18576]: Env: USER=tedwood Sep 2 13:04:36 topdog Installer[18576]: Env: LOGNAME=tedwood Sep 2 13:04:36 topdog Installer[18576]: Env: SSH_AUTH_SOCK=/tmp/launch-dVtpfg/Listeners Sep 2 13:04:36 topdog Installer[18576]: Env: Apple_Ubiquity_Message=/tmp/launch-O5o1Ms/Apple_Ubiquity_Message Sep 2 13:04:36 topdog Installer[18576]: Env: Apple_PubSub_Socket_Render=/tmp/launch-2hUrBG/Render Sep 2 13:04:36 topdog Installer[18576]: Env: DISPLAY=/tmp/launch-AKzVzY/org.macosforge.xquartz:0 Sep 2 13:04:36 topdog Installer[18576]: Env: COMMAND_MODE=unix2003 Sep 2 13:04:36 topdog Installer[18576]: Env: __CF_USER_TEXT_ENCODING=0x1F5:0:0 Sep 2 13:04:37 topdog Installer[18576]: Git 1.8.3.2 Installation Log Sep 2 13:04:37 topdog Installer[18576]: Opened from: /Volumes/Git 1.8.3.2 Snow Leopard Intel Universal/git-1.8.3.2-intel-universal-snow-leopard.pkg Sep 2 13:04:37 topdog Installer[18576]: Product archive /Volumes/Git 1.8.3.2 Snow Leopard Intel Universal/git-1.8.3.2-intel-universal-snow-leopard.pkg trustLevel=100 Sep 2 13:06:23 topdog Installer[18576]: InstallerStatusNotifications plugin loaded Sep 2 13:06:29 topdog runner[18604]: Administrator authorization granted. Sep 2 13:06:29 topdog Installer[18576]: Sep 2 13:06:29 topdog Installer[18576]: User picked Standard Install Sep 2 13:06:29 topdog Installer[18576]: Choices selected for installation: Sep 2 13:06:29 topdog Installer[18576]:Upgrade: "Git 1.8.3.2" Sep 2 13:06:29 topdog Installer[18576]:Upgrade: "Git" Sep 2 13:06:29 topdog Installer[18576]: git-1.8.3.2-intel-universal-snow-leopard.pkg#git.pkg : GitOSX.Installer.git1832.git.pkg : 1 Sep 2 13:06:29 topdog Installer[18576]:Upgrade: "etc" Sep 2 13:06:29 topdog Installer[18576]: git-1.8.3.2-intel-universal-snow-leopard.pkg#etc.pkg : GitOSX.Installer.git1832.etc.pkg : 1 Sep 2 13:06:29 topdog Installer[18576]: Sep 2 13:06:29 topdog Installer[18576]: It took 0.00 seconds to summarize the package selections. Sep 2 13:06:29 topdog Installer[18576]: -[IFDInstallController(Private) _buildInstallPlan]: location = file://localhost Sep 2 13:06:29 topdog Installer[18576]: -[IFDInstallController(Private) _buildInstallPlan]: file://localhost/Volumes/Git%201.8.3.2%20Snow%20Leopard%20Intel%20Universal/git-1.8.3.2-intel-universal-snow-leopard.pkg#git.pkg Sep 2 13:06:29 topdog Installer[18576]: -[IFDInstallController(Private) _buildInstallPlan]: file://localhost/Volumes/Git%201.8.3.2%20Snow%20Leopard%20Intel%20Universal/git-1.8.3.2-intel-universal-snow-leopard.pkg#etc.pkg Sep 2 13:06:29 topdog Installer[18576]: Set authorization level to root for session Sep 2 13:06:29 topdog Installer[18576]: Will use PK session Sep 2 13:06:29 topdog Installer[18576]: Starting installation: Sep 2 13:06:29 topdog Installer[18576]: Configuring volume "TopDogHD" Sep 2 13:06:29 topdog Installer[18576]: Preparing disk for local booted install. Sep 2 13:06:29 topdog Installer[18576]: Free space on "TopDogHD": 13.19 GB (13
[PATCH V2] check-ignore: Add option to ignore index contents
I have updated the original version of this patch to encompass the
feedback comments obtained. Updates include:
1) Rename option to --no-index for consistency with other commands
2) Improved Documentation text
3) Extension to test scripts to include this option
Regarding test scripts I have scoped coverage to ensure correct
behaviour with the new option in all standard cases but without
duplicating every single corner case.
The original patch is at $gmane/233381.
check-ignore currently shows how .gitignore rules would treat untracked
paths. Tracked paths do not generate useful output. This prevents
debugging of why a path became tracked unexpectedly unless that path is
first removed from the index with `git rm --cached `.
This option (-i, --no-index) simply by-passes the check for the path
being in the index and hence allows tracked paths to be checked too.
Whilst this behaviour deviates from the characteristics of `git add` and
`git status` its use case is unlikely to cause any user confusion.
Test scripts are augmented to check this option against the standard
ignores to ensure correct behaviour.
Signed-off-by: Dave Williams
---
Documentation/git-check-ignore.txt | 7
builtin/check-ignore.c | 14 ---
t/t0008-ignores.sh | 77 +-
3 files changed, 83 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-check-ignore.txt
b/Documentation/git-check-ignore.txt
index d2df487..96c591f 100644
--- a/Documentation/git-check-ignore.txt
+++ b/Documentation/git-check-ignore.txt
@@ -45,6 +45,13 @@ OPTIONS
not be possible to distinguish between paths which match a
pattern and those which don't.
+-i, --no-index::
+ Don't look in the index when undertaking the checks. This can
+ be used to debug why a path became tracked by e.g. `git add .`
+ and was not ignored by the rules as expected by the user or when
+ developing patterns including negation to match a path previously
+ added with `git add -f`.
+
OUTPUT
--
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 4a8fc70..2b0c8a6 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -5,7 +5,7 @@
#include "pathspec.h"
#include "parse-options.h"
-static int quiet, verbose, stdin_paths, show_non_matching;
+static int quiet, verbose, stdin_paths, show_non_matching, no_index;
static const char * const check_ignore_usage[] = {
"git check-ignore [options] pathname...",
"git check-ignore [options] --stdin < ",
@@ -24,6 +24,8 @@ static const struct option check_ignore_options[] = {
N_("input paths are terminated by a null character")),
OPT_BOOLEAN('n', "non-matching", &show_non_matching,
N_("show non-matching input paths")),
+ OPT_BOOLEAN('i', "no-index", &no_index,
+ N_("ignore index when checking")),
OPT_END()
};
@@ -67,7 +69,7 @@ static int check_ignore(struct dir_struct *dir,
const char *prefix, const char **pathspec)
{
const char *path, *full_path;
- char *seen;
+ char *seen = NULL;
int num_ignored = 0, dtype = DT_UNKNOWN, i;
struct exclude *exclude;
@@ -82,7 +84,9 @@ static int check_ignore(struct dir_struct *dir,
* should not be ignored, in order to be consistent with
* 'git status', 'git add' etc.
*/
- seen = find_pathspecs_matching_against_index(pathspec);
+ if (!no_index) {
+ seen = find_pathspecs_matching_against_index(pathspec);
+ }
for (i = 0; pathspec[i]; i++) {
path = pathspec[i];
full_path = prefix_path(prefix, prefix
@@ -90,7 +94,7 @@ static int check_ignore(struct dir_struct *dir,
full_path = check_path_for_gitlink(full_path);
die_if_path_beyond_symlink(full_path, prefix);
exclude = NULL;
- if (!seen[i]) {
+ if (no_index || !seen[i]) {
exclude = last_exclude_matching(dir, full_path, &dtype);
}
if (!quiet && (exclude || show_non_matching))
@@ -157,7 +161,7 @@ int cmd_check_ignore(int argc, const char **argv, const
char *prefix)
die(_("--non-matching is only valid with --verbose"));
/* read_cache() is only necessary so we can watch out for submodules. */
- if (read_cache() < 0)
+ if (!no_index && read_cache() < 0)
die(_("index file corrupt"));
memset(&dir, 0, sizeof(dir));
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index c29342d..0ad0534 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -66,11 +66,11 @@ test_check_ignore () {
init_vars &&
rm -f "$HOME/stdout" "$HOME/stderr" "$HOME/cmd" &&
- echo git $global_args check-ignore $quiet_opt $verbose_opt
$non_matching_opt $args \
+ echo git $global_args c
Re: [PATCH v3 07/11] Documentation/replace: tell that -f option bypasses the type check
From: "Christian Couder"
From: "Philip Oakley"
From: "Christian Couder"
Maybe we can show that in an example. But I think the patch is quite
clear as it is and should be enough.
If we really want to correct some false beliefs, the best would be
to
state the truth where those false beliefs are stated.
I've added a sub-comment to the original SO post that started this
thread (my post $gmane/231598 - SO/a/18027030/717355), but given the
guy's blog has comments going back to 2009 I suspect its a bit of a
http://xkcd.com/386/ task, hence my desire that it's explicitly
mentioned in the 'replace' documentation. In addition, if the guy
doesn't correct his post I'll mark it down in a couple of days to
make
it plain to other readers it's in error.
The creation of a (merge?) commit that's equivalent to a graft line
isn't something that jumps out (to me) as an easy couple lines of
bash
script.
A 'graft2replace' script (or 'git-graft' command) which takes an
existing graft file (or command line list) and creates the
replacement
objects and then does the replace, all while still in a dirty tree
would be the holy grail for properly deprecating grafts (which are
sooo easy to create)
You mean something like the following:
$ cat ./graft2replace.sh
#!/bin/bash
while read orig parents
do
printf "%s" "git cat-file commit $orig"
printf "%s" " | perl -n -e 'print unless /^parent /'"
insn=''
for commit in $parents; do insn="$insn print \"parent
$commit\\n\";"; done
printf "%s" " | perl -n -e 'print; if (/^tree /) { $insn }'"
printf "%s\n" " > new_commit.txt"
printf "%s\n" 'REPL=$(git hash-object -t commit -w
new_commit.txt)'
Does `hash-object` do the inverese of `cat-file commit`?
I didn't find the hash-object(1) man page very informative on that
matter and a (very) quick look at its code made me think it was just
hashing the raw contents which wouldn't be what what was wanted.
printf "%s\n" "git replace $orig \$REPL"
done
This generates shell instructions from a graft file. Then you only
need to execute the generated shell instructions.
For example:
$ cat graft_file.txt
5bf34fff3186254d7254583675d10ddf98df989b
79fe155489351e8af829a3106e7150397c57d863
dcfbab6bea3df3166503f3084cec2679f10f9e80
fb5657082148297b61fbca7e64d51c1e7870309a
$ cat graft_file.txt | ./graft2replace.sh
git cat-file commit 5bf34fff3186254d7254583675d10ddf98df989b |
perl -n -e 'print unless /^parent /' | perl -n -e 'print; if (/^tree
/) { print "parent 79fe155489351e8af829a3106e7150397c57d863\n"; print
"parent dcfbab6bea3df3166503f3084cec2679f10f9e80\n"; }' >
new_commit.txt
REPL=$(git hash-object -t commit -w new_commit.txt)
git replace 5bf34fff3186254d7254583675d10ddf98df989b $REPL
git cat-file commit fb5657082148297b61fbca7e64d51c1e7870309a |
perl -n -e 'print unless /^parent /' | perl -n -e 'print; if (/^tree
/) { }' > new_commit.txt
REPL=$(git hash-object -t commit -w new_commit.txt)
git replace fb5657082148297b61fbca7e64d51c1e7870309a $REPL
Note that I haven't really tested it.
Best,
Christian.
--
I think we could call it 'git-graft', being the help function/script
that converts raw grafts to proper object replacements ;-)
Philip
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/11] Documentation/replace: tell that -f option bypasses the type check
Hi, Philip Oakley wrote: > Does `hash-object` do the inverese of `cat-file commit`? > > I didn't find the hash-object(1) man page very informative on that matter Hm. The manpage says: Computes the object ID value for an object with specified type with the contents of the named file [...], and optionally writes the resulting object into the object database. And then: -w Actually write the object into the object database. Any ideas for making this clearer? Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/11] Documentation/replace: tell that -f option bypasses the type check
From: "Jonathan Nieder" Hi, Philip Oakley wrote: Does `hash-object` do the inverese of `cat-file commit`? I didn't find the hash-object(1) man page very informative on that matter Hm. The manpage says: Computes the object ID value for an object with specified type with the contents of the named file [...], and optionally writes the resulting object into the object database. And then: -w Actually write the object into the object database. Any ideas for making this clearer? The problem is the file format, in the sense that the earlier `git cat-file commit $orig` has a human readable output which is a description of the commit header, rather than the specific binary content. I couldn't tell if the command would cope with such a human readable file. In Christian's quick untested script he'd matched the two commands as opposites, which doesn't appear to be the case, and I'm somewhat ignorant of what hash-object is doing - I'd expect that it simply hashed the already constructed data, but my ignorance starts showing at this point ;-) regards Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] check-ignore: Add option to ignore index contents
On Mon, Sep 2, 2013 at 5:20 PM, Dave Williams
wrote:
> I have updated the original version of this patch to encompass the
> feedback comments obtained. Updates include:
> 1) Rename option to --no-index for consistency with other commands
> 2) Improved Documentation text
> 3) Extension to test scripts to include this option
>
> Regarding test scripts I have scoped coverage to ensure correct
> behaviour with the new option in all standard cases but without
> duplicating every single corner case.
>
> The original patch is at $gmane/233381.
This commentary, which is not intended as part of the commit message,
normally goes below the '---' line after your sign-off just before the
diffstat. When the project maintainer applies a patch with git-am,
such commentary is stripped out automatically, otherwise he has to
strip it manually. (Alternately, you could use a -->8-- line to
separate the above commentary from the commit message.)
One more minor issue below...
> check-ignore currently shows how .gitignore rules would treat untracked
> paths. Tracked paths do not generate useful output. This prevents
> debugging of why a path became tracked unexpectedly unless that path is
> first removed from the index with `git rm --cached `.
>
> This option (-i, --no-index) simply by-passes the check for the path
> being in the index and hence allows tracked paths to be checked too.
>
> Whilst this behaviour deviates from the characteristics of `git add` and
> `git status` its use case is unlikely to cause any user confusion.
>
> Test scripts are augmented to check this option against the standard
> ignores to ensure correct behaviour.
>
> Signed-off-by: Dave Williams
> ---
> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
> index c29342d..0ad0534 100755
> --- a/t/t0008-ignores.sh
> +++ b/t/t0008-ignores.sh
> @@ -87,6 +87,9 @@ test_check_ignore () {
> # check-ignore --verbose output is the same as normal output except
> # for the extra first column.
> #
> +# A parameter is used to determine if the tests are run with the
> +# normal case (using the index), or with the -i or --no_index option.
s/--no_index/--no-index/
> +#
> # Arguments:
> # - (optional) prereqs for this test, e.g. 'SYMLINKS'
> # - test name
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] Preparation for non-ff pulls by default
It is very typical for Git newcomers to inadvertently create merges and worst: inadvertently pushing them. This is one of the reasons many experienced users prefer to avoid 'git pull', and recommend newcomers to avoid it as well. To avoid these problems and keep 'git pull' useful, it has been suggested that 'git pull' barfs by default if the merge is non-fast-forward, which unfortunately would break backwards compatibility. This patch series leaves everything in place to enable this new mode, but it only gets enabled if the user specifically configures it; pull.mode = merge-ff-only. Later on this mode can be enabled by default (e.g. in v2.0). To achieve that first some configurations are renamed: for example: pull.rebase => pull.mode = rebase, but the old ones remain functional, thus there are no functional changes. Felipe Contreras (6): merge: simplify ff-only option t: replace pulls with merges pull: rename pull.rename to pull.mode pull: refactor $rebase variable into $mode pull: add --merge option pull: add merge-ff-only option Documentation/config.txt | 24 +++-- Documentation/git-pull.txt | 10 -- branch.c | 4 +-- builtin/merge.c| 20 +-- git-pull.sh| 49 --- t/annotate-tests.sh| 2 +- t/t3200-branch.sh | 40 +++--- t/t4200-rerere.sh | 2 +- t/t5520-pull.sh| 62 ++ t/t5601-clone.sh | 4 +-- t/t9114-git-svn-dcommit-merge.sh | 2 +- t/t9500-gitweb-standalone-no-errors.sh | 2 +- 12 files changed, 157 insertions(+), 64 deletions(-) -- 1.8.4-338-gefd7fa6 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] pull: add merge-ff-only option
It is very typical for Git newcomers to inadvertently create merges and
worst, inadvertently pushing them. This is one of the reasons many
experienced users prefer to avoid 'git pull', and recommend newcomers to
avoid it as well.
To avoid these problems and keep 'git pull' useful it has been suggested
that 'git pull' barfs by default if the merge is non-fast-forward, which
unfortunately would break backwards compatibility.
This patch leaves everything in place to enable this new mode, but it
only gets enabled if the user specifically configures it; pull.mode =
merge-ff-only.
Later on this mode can be enabled by default (e.g. in v2.0).
For the full discussion you can read:
http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=225305
Signed-off-by: Felipe Contreras
---
Documentation/config.txt | 8 +---
builtin/merge.c | 9 -
git-pull.sh | 7 +++
t/t5520-pull.sh | 36
4 files changed, 56 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9489a59..13635e0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1822,9 +1822,11 @@ pretty.::
pull.mode::
When "git pull" is run, this determines if it would either merge or
- rebase the fetched branch. The possible values are 'merge' and
- 'rebase'. See "branch..pullmode" for doing this in a non
- branch-specific manner.
+ rebase the fetched branch. The possible values are 'merge',
+ 'rebase', and 'merge-ff-only'. If 'merge-ff-only' is specified, the
+ merge will only succeed if it's fast-forward. See
+ "branch..pullmode" for doing this in a non branch-specific
+ manner.
+
*NOTE*: this is a possibly dangerous operation; do *not* use
it unless you understand the implications (see linkgit:git-rebase[1]
diff --git a/builtin/merge.c b/builtin/merge.c
index da9fc08..97b4205 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1437,8 +1437,15 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
}
}
- if (fast_forward == FF_ONLY)
+ if (fast_forward == FF_ONLY) {
+ const char *msg = getenv("GIT_MERGE_FF_ONLY_HELP");
+ if (msg) {
+ fprintf(stderr, "%s\n", msg);
+ ret = 1;
+ goto done;
+ }
die(_("Not possible to fast-forward, aborting."));
+ }
/* We are going to make a new commit. */
git_committer_info(IDENT_STRICT);
diff --git a/git-pull.sh b/git-pull.sh
index fbb8a9b..a9cf7ac 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -62,6 +62,7 @@ then
echo "Please use pull.mode and branch..pullmode instead."
fi
fi
+test -z "$mode" && mode=merge
dry_run=
while :
do
@@ -307,6 +308,12 @@ then
fi
fi
+if test "$mode" = merge-ff-only -a -z "$no_ff$ff_only${squash#--no-squash}"
+then
+ ff_only=--ff-only
+ export GIT_MERGE_FF_ONLY_HELP="The pull was not fast-forward, please
either merge or rebase."
+fi
+
merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
case "$mode" in
rebase)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 978a3c1..798ae2f 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -310,4 +310,40 @@ test_expect_success 'branch.to-rebase.pullmode should
override pull.mode' '
test new = $(git show HEAD:file2)
'
+test_expect_success 'git pull fast-forward' '
+ test_when_finished "git checkout master && git branch -D other test" &&
+ test_config pull.mode merge-ff-only &&
+ git checkout -b other master &&
+ >new &&
+ git add new &&
+ git commit -m new &&
+ git checkout -b test -t other &&
+ git reset --hard master &&
+ git pull
+'
+
+test_expect_success 'git pull non-fast-forward' '
+ test_when_finished "git checkout master && git branch -D other test" &&
+ test_config pull.mode merge-ff-only &&
+ git checkout -b other master^ &&
+ >new &&
+ git add new &&
+ git commit -m new &&
+ git checkout -b test -t other &&
+ git reset --hard master &&
+ test_must_fail git pull
+'
+
+test_expect_success 'git pull non-fast-forward (merge)' '
+ test_when_finished "git checkout master && git branch -D other test" &&
+ test_config pull.mode merge-ff-only &&
+ git checkout -b other master^ &&
+ >new &&
+ git add new &&
+ git commit -m new &&
+ git checkout -b test -t other &&
+ git reset --hard master &&
+ git pull --merge
+'
+
test_done
--
1.8.4-338-gefd7fa6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] pull: add --merge option
Also, deprecate --no-rebase since there's no need for it any more. Signed-off-by: Felipe Contreras --- Documentation/git-pull.txt | 8 ++-- git-pull.sh| 6 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index 991352f..e09f004 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -119,8 +119,12 @@ It rewrites history, which does not bode well when you published that history already. Do *not* use this option unless you have read linkgit:git-rebase[1] carefully. ---no-rebase:: - Override earlier --rebase. +-m:: +--merge:: + Force a merge. ++ +See `pull.mode`, `branch..pullmode` in linkgit:git-config[1] if you want +to make `git pull` always use `--merge`. Options related to fetching ~~~ diff --git a/git-pull.sh b/git-pull.sh index f53d193..fbb8a9b 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -127,8 +127,12 @@ do -r|--r|--re|--reb|--reba|--rebas|--rebase) mode=rebase ;; + -m|--m|--me|--mer|--merg|--merge) + mode=merge + ;; --no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase) - mode= + mode=merge + echo "--no-rebase is deprecated, please use --merge instead" ;; --recurse-submodules) recurse_submodules=--recurse-submodules -- 1.8.4-338-gefd7fa6 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] pull: rename pull.rename to pull.mode
Also 'branch..rebase' to 'branch..pullmode'.
This way 'pull.mode' can be set to 'merge', and the default can be
something else.
The old configurations still work, but get deprecated.
Signed-off-by: Felipe Contreras
---
Documentation/config.txt | 22 +++---
Documentation/git-pull.txt | 2 +-
branch.c | 4 ++--
git-pull.sh| 18 --
t/t3200-branch.sh | 40
t/t5520-pull.sh| 26 ++
t/t5601-clone.sh | 4 ++--
7 files changed, 78 insertions(+), 38 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ec57a15..9489a59 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -706,7 +706,7 @@ branch.autosetupmerge::
branch.autosetuprebase::
When a new branch is created with 'git branch' or 'git checkout'
that tracks another branch, this variable tells Git to set
- up pull to rebase instead of merge (see "branch..rebase").
+ up pull to rebase instead of merge (see "branch..pullmode").
When `never`, rebase is never automatically set to true.
When `local`, rebase is set to true for tracked branches of
other local branches.
@@ -760,11 +760,11 @@ branch..mergeoptions::
option values containing whitespace characters are currently not
supported.
-branch..rebase::
- When true, rebase the branch on top of the fetched branch,
- instead of merging the default branch from the default remote when
- "git pull" is run. See "pull.rebase" for doing this in a non
- branch-specific manner.
+branch..pullmode::
+ When "git pull" is run, this determines if it would either merge or
+ rebase the fetched branch. The possible values are 'merge' and
+ 'rebase'. See "pull.mode" for doing this in a non branch-specific
+ manner.
+
*NOTE*: this is a possibly dangerous operation; do *not* use
it unless you understand the implications (see linkgit:git-rebase[1]
@@ -1820,11 +1820,11 @@ pretty.::
Note that an alias with the same name as a built-in format
will be silently ignored.
-pull.rebase::
- When true, rebase branches on top of the fetched branch, instead
- of merging the default branch from the default remote when "git
- pull" is run. See "branch..rebase" for setting this on a
- per-branch basis.
+pull.mode::
+ When "git pull" is run, this determines if it would either merge or
+ rebase the fetched branch. The possible values are 'merge' and
+ 'rebase'. See "branch..pullmode" for doing this in a non
+ branch-specific manner.
+
*NOTE*: this is a possibly dangerous operation; do *not* use
it unless you understand the implications (see linkgit:git-rebase[1]
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 6ef8d59..991352f 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -109,7 +109,7 @@ include::merge-options.txt[]
fetched, the rebase uses that information to avoid rebasing
non-local changes.
+
-See `pull.rebase`, `branch..rebase` and `branch.autosetuprebase` in
+See `pull.mode`, `branch..pullmode` and `branch.autosetuprebase` in
linkgit:git-config[1] if you want to make `git pull` always use
`--rebase` instead of merging.
+
diff --git a/branch.c b/branch.c
index c5c6984..c6b70c2 100644
--- a/branch.c
+++ b/branch.c
@@ -71,8 +71,8 @@ void install_branch_config(int flag, const char *local, const
char *origin, cons
if (rebasing) {
strbuf_reset(&key);
- strbuf_addf(&key, "branch.%s.rebase", local);
- git_config_set(key.buf, "true");
+ strbuf_addf(&key, "branch.%s.pullmode", local);
+ git_config_set(key.buf, "rebase");
}
strbuf_release(&key);
diff --git a/git-pull.sh b/git-pull.sh
index f0df41c..de57c1d 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -43,10 +43,24 @@ log_arg= verbosity= progress= recurse_submodules=
verify_signatures=
merge_args= edit=
curr_branch=$(git symbolic-ref -q HEAD)
curr_branch_short="${curr_branch#refs/heads/}"
-rebase=$(git config --bool branch.$curr_branch_short.rebase)
+mode=$(git config branch.${curr_branch_short}.pullmode)
+if test -z "$mode"
+then
+ mode=$(git config pull.mode)
+fi
+test "$mode" == 'rebase' && rebase="true"
if test -z "$rebase"
then
- rebase=$(git config --bool pull.rebase)
+ rebase=$(git config --bool branch.$curr_branch_short.rebase)
+ if test -z "$rebase"
+ then
+ rebase=$(git config --bool pull.rebase)
+ fi
+ if test "$rebase" = 'true'
+ then
+ echo "The configurations pull.rebase and branch..rebase
are deprecated."
+ echo "Please use pull.mode and branch..pullmode instead."
+ fi
fi
dry_run=
while :
diff --git a/t/t3200-bran
[PATCH 1/6] merge: simplify ff-only option
No functional changes.
Signed-off-by: Felipe Contreras
---
builtin/merge.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 34a6166..da9fc08 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -186,13 +186,6 @@ static int option_parse_n(const struct option *opt,
return 0;
}
-static int option_parse_ff_only(const struct option *opt,
- const char *arg, int unset)
-{
- fast_forward = FF_ONLY;
- return 0;
-}
-
static struct option builtin_merge_options[] = {
{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
N_("do not show a diffstat at the end of the merge"),
@@ -210,9 +203,9 @@ static struct option builtin_merge_options[] = {
OPT_BOOL('e', "edit", &option_edit,
N_("edit message before committing")),
OPT_SET_INT(0, "ff", &fast_forward, N_("allow fast-forward (default)"),
FF_ALLOW),
- { OPTION_CALLBACK, 0, "ff-only", NULL, NULL,
+ { OPTION_SET_INT, 0, "ff-only", &fast_forward, NULL,
N_("abort if fast-forward is not possible"),
- PARSE_OPT_NOARG | PARSE_OPT_NONEG, option_parse_ff_only },
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },
OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
OPT_BOOL(0, "verify-signatures", &verify_signatures,
N_("Verify that the named commit has a valid GPG signature")),
--
1.8.4-338-gefd7fa6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] pull: refactor $rebase variable into $mode
Signed-off-by: Felipe Contreras
---
git-pull.sh | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index de57c1d..f53d193 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -48,8 +48,7 @@ if test -z "$mode"
then
mode=$(git config pull.mode)
fi
-test "$mode" == 'rebase' && rebase="true"
-if test -z "$rebase"
+if test -z "$mode"
then
rebase=$(git config --bool branch.$curr_branch_short.rebase)
if test -z "$rebase"
@@ -58,6 +57,7 @@ then
fi
if test "$rebase" = 'true'
then
+ mode="rebase"
echo "The configurations pull.rebase and branch..rebase
are deprecated."
echo "Please use pull.mode and branch..pullmode instead."
fi
@@ -125,10 +125,10 @@ do
merge_args="$merge_args$xx "
;;
-r|--r|--re|--reb|--reba|--rebas|--rebase)
- rebase=true
+ mode=rebase
;;
--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
- rebase=false
+ mode=
;;
--recurse-submodules)
recurse_submodules=--recurse-submodules
@@ -171,7 +171,7 @@ error_on_no_merge_candidates () {
esac
done
- if test true = "$rebase"
+ if test "$mode" = rebase
then
op_type=rebase
op_prep=against
@@ -185,7 +185,7 @@ error_on_no_merge_candidates () {
remote=$(git config "branch.$curr_branch.remote")
if [ $# -gt 1 ]; then
- if [ "$rebase" = true ]; then
+ if [ "$mode" = rebase ]; then
printf "There is no candidate for rebasing against "
else
printf "There are no candidates for merging "
@@ -208,7 +208,7 @@ error_on_no_merge_candidates () {
exit 1
}
-test true = "$rebase" && {
+test "$mode" = rebase && {
if ! git rev-parse -q --verify HEAD >/dev/null
then
# On an unborn branch
@@ -273,7 +273,7 @@ case "$merge_head" in
then
die "$(gettext "Cannot merge multiple branches into empty
head")"
fi
- if test true = "$rebase"
+ if test "$mode" = rebase
then
die "$(gettext "Cannot rebase onto multiple branches")"
fi
@@ -294,7 +294,7 @@ then
exit
fi
-if test true = "$rebase"
+if test "$mode" = rebase
then
o=$(git show-branch --merge-base $curr_branch $merge_head $oldremoteref)
if test "$oldremoteref" = "$o"
@@ -304,8 +304,8 @@ then
fi
merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
-case "$rebase" in
-true)
+case "$mode" in
+rebase)
eval="git-rebase $diffstat $strategy_args $merge_args $verbosity"
eval="$eval --onto $merge_head ${oldremoteref:-$merge_head}"
;;
--
1.8.4-338-gefd7fa6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] t: replace pulls with merges
This is what the code intended. No functional changes. Signed-off-by: Felipe Contreras --- t/annotate-tests.sh| 2 +- t/t4200-rerere.sh | 2 +- t/t9114-git-svn-dcommit-merge.sh | 2 +- t/t9500-gitweb-standalone-no-errors.sh | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index d4e7f47..01deece 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -92,7 +92,7 @@ test_expect_success 'blame 2 authors + 1 branch2 author' ' ' test_expect_success 'merge branch1 & branch2' ' - git pull . branch1 + git merge branch1 ' test_expect_success 'blame 2 authors + 2 merged-in authors' ' diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 7ff..cf19eb7 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -172,7 +172,7 @@ test_expect_success 'first postimage wins' ' git show second^:a1 | sed "s/To die: t/To die! T/" >a1 && git commit -q -a -m third && - test_must_fail git pull . first && + test_must_fail git merge first && # rerere kicked in ! grep "^===\$" a1 && test_cmp expect a1 diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh index f524d2f..d33d714 100755 --- a/t/t9114-git-svn-dcommit-merge.sh +++ b/t/t9114-git-svn-dcommit-merge.sh @@ -62,7 +62,7 @@ test_expect_success 'setup git mirror and merge' ' echo friend > README && cat tmp >> README && git commit -a -m "friend" && - git pull . merge + git merge merge ' test_debug 'gitk --all & sleep 1' diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh index 6fca193..3864388 100755 --- a/t/t9500-gitweb-standalone-no-errors.sh +++ b/t/t9500-gitweb-standalone-no-errors.sh @@ -328,7 +328,7 @@ test_expect_success \ git add b && git commit -a -m "On branch" && git checkout master && -git pull . b && +git merge b && git tag merge_commit' test_expect_success \ -- 1.8.4-338-gefd7fa6 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/11] Documentation/replace: tell that -f option bypasses the type check
Philip Oakley wrote: > The problem is the file format, in the sense that the earlier `git cat-file > commit $orig` has a human readable output which is a description of the > commit header, rather than the specific binary content. Ah. That's the actual "raw" commit object format, though. The manpage for git-cat-file(1) says: SYNOPSIS git cat-file (-t | -s | -e | -p | | --textconv ) git cat-file (--batch | --batch-check) < DESCRIPTION In its first form, the command provides the content or the type of an object in the repository. [...] OUTPUT ... If is specified, the raw (though uncompressed) contents of the will be returned. I agree that this isn't as clear as it should be. I see a few problems: (1) The synopsis treats "git cat-file -t/-s/-e/-p ", "git cat-file --textconv :", and "git cat-file " as the same form of the command. It would be easier to explain these as three different forms. (2) There is no EXAMPLES section and no examples. (3) There is no pointer to the git object formats. A pointer to a new gitobject(5) manpage would presumably make everything clearer. https://www.kernel.org/pub/software/scm/git/docs/user-manual.html#examining-the-data might be a good source of text to start from for solving (1), since it explains the command a little better. Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/11] Documentation/replace: tell that -f option bypasses the type check
From: "Jonathan Nieder" Philip Oakley wrote: The problem is the file format, in the sense that the earlier `git cat-file commit $orig` has a human readable output which is a description of the commit header, rather than the specific binary content. Ah. That's the actual "raw" commit object format, though. Aha.. Sudden realisation that the cat-file _is_ the 'raw' format and that the sha1's etc are shown in ascii hex, rather than being in a compact binary format (same for 'unix' dates etc.) So the 'human readable' output is exactly the 'type_name' field followed by a single space (SP) followed by the sha1 in ascii hex (i.e. tree/parent), or appropriate data in the well defined format (for author/committer) SP 'email' SP date. etc. It was the Human readable == Machine readable that I'd missed. The manpage for git-cat-file(1) says: SYNOPSIS git cat-file (-t | -s | -e | -p | | --textconv ) git cat-file (--batch | --batch-check) < DESCRIPTION In its first form, the command provides the content or the type of an object in the repository. [...] OUTPUT ... If is specified, the raw (though uncompressed) contents of the will be returned. I agree that this isn't as clear as it should be. I see a few problems: (1) The synopsis treats "git cat-file -t/-s/-e/-p ", "git cat-file --textconv :", and "git cat-file " as the same form of the command. It would be easier to explain these as three different forms. (2) There is no EXAMPLES section and no examples. (3) There is no pointer to the git object formats. A pointer to a new gitobject(5) manpage would presumably make everything clearer. https://www.kernel.org/pub/software/scm/git/docs/user-manual.html#examining-the-data might be a good source of text to start from for solving (1), since it explains the command a little better. A quick run of the example "git cat-file commit HEAD", seen in the context of your email helped me appreciate the situation. Thanks, Jonathan Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] revision: introduce --exclude= to tame wildcards
On Mon, Sep 2, 2013 at 3:11 PM, Johannes Sixt wrote:
> Am 31.08.2013 01:55, schrieb Junio C Hamano:
>> People often find "git log --branches" etc. that includes _all_
>> branches is cumbersome to use when they want to grab most but except
>> some. The same applies to --tags, --all and --glob.
>>
>> Teach the revision machinery to remember patterns, and then upon the
>> next such a globbing option, exclude those that match the pattern.
>>
>> With this, I can view only my integration branches (e.g. maint,
>> master, etc.) without topic branches, which are named after two
>> letters from primary authors' names, slash and topic name.
>>
>> git rev-list --no-walk --exclude=??/* --branches |
>> git name-rev --refs refs/heads/* --stdin
>>
>> This one shows things reachable from local and remote branches that
>> have not been merged to the integration branches.
>>
>> git log --remotes --branches --not --exclude=??/* --branches
>>
>> It may be a bit rough around the edges, in that the pattern to give
>> the exclude option depends on what globbing option follows. In
>> these examples, the pattern "??/*" is used, not "refs/heads/??/*",
>> because the globbing option that follows the -"-exclude="
>> is "--branches". As each use of globbing option resets previously
>> set "--exclude", this may not be such a bad thing, though.
>
> I argued "--except should trump everything" earlier, but the case
> involving --not
>
> --branches --except maint --not master
>
> to mean the same as
>
> --branches --except maint master
>
> just does not make sense.
No, but this could make sense:
--branches ^master --except maint --not master
==
--branches --except maint
> An alternative would be that --not would divide the command line
> arguments into ranges within which one --except would subtract
> subsequent refs from earlier globbing arguments in the same range.
> That's not simpler to explain than your current proposal.
Something like that can be easily done in my approach:
--- a/revision.c
+++ b/revision.c
@@ -1984,6 +1984,7 @@ static int handle_revision_pseudo_opt(const char
*submodule,
handle_reflog(revs, *flags);
} else if (!strcmp(arg, "--not")) {
*flags ^= UNINTERESTING | BOTTOM;
+ *flags &= ~SKIP;
} else if (!strcmp(arg, "--except")) {
*flags |= SKIP;
} else if (!strcmp(arg, "--no-walk")) {
@@ -2628,7 +2629,8 @@ int prepare_revision_walk(struct rev_info *revs)
for (i = 0; i < revs->cmdline.nr; i++) {
struct rev_cmdline_entry *ce;
ce = &revs->cmdline.rev[i];
- if ((ce->flags & SKIP) && !refcmp(ce->name, e->name)) {
+ if ((ce->flags & SKIP) && !refcmp(ce->name, e->name) &&
+ ((ce->flags & UNINTERESTING)
== (e->item->flags & UNINTERESTING))) {
e->item->flags =
recalculate_flag(revs, e->item->sha1, ce->name);
goto next;
}
diff --git a/t/t6112-rev-list-except.sh b/t/t6112-rev-list-except.sh
index a40a641..441e1da 100755
--- a/t/t6112-rev-list-except.sh
+++ b/t/t6112-rev-list-except.sh
@@ -57,4 +57,21 @@ test_expect_success 'rev-list --except and --not
with proper flags' '
test_cmp expect actual
'
+test_expect_success 'rev-list --not ranges' '
+
+ git rev-list --topo-order test --not master --except master
test > actual &&
+ git rev-list --topo-order test > expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'rev-list multiple --not ranges' '
+
+ git checkout -b extra test &&
+ echo five > content &&
+ git commit -a -m five &&
+ git rev-list --topo-order test --not master --except master
test --not extra > actual &&
+ git rev-list --topo-order test extra > expect &&
+ test_cmp expect actual
+'
+
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: the pager
> From: Matthieu Moy
> > const char *git_pager(int stdout_is_tty)
> > {
> > const char *pager;
> >
> > if (!stdout_is_tty)
> > return NULL;
> >
> > pager = getenv("GIT_PAGER");
> > if (!pager) {
> > if (!pager_program)
> > git_config(git_default_config, NULL);
> > pager = pager_program;
> > }
> > if (!pager)
> > pager = getenv("PAGER");
> > if (!pager)
> > pager = DEFAULT_PAGER;
> > else if (!*pager || !strcmp(pager, "cat"))
> > pager = NULL;
>
> I guess the "else" could and should be dropped. If you do so (and
> possibly insert a blank line between the DEFAULT_PAGER case and the
> "pager = NULL" case), you get a nice pattern
>
> if (!pager)
> try_something();
> if (!pager)
> try_next_option();
That's true, but it would change the effect of using "cat" as a value:
"cat" as a value of DEFAULT_PAGER would cause git_pager() to return
NULL, whereas now it causes git_pager() to return "cat". (All other
places where "cat" can be a value are translated to NULL already.)
This is why I want to know what the *intended* behavior is, because we
might be changing Git's behavior, and I want to know that if we do
that, we're changing it to what it should be. And I haven't seen
anyone venture an opinion on what the intended behavior is.
> I agree that a comment like this would help, though:
>
> --- a/cache.h
> +++ b/cache.h
> @@ -1266,7 +1266,7 @@ static inline ssize_t write_str_in_full(int fd, const
> char *str)
>
> /* pager.c */
> extern void setup_pager(void);
> -extern const char *pager_program;
> +extern const char *pager_program; /* value read from git_config() */
> extern int pager_in_use(void);
> extern int pager_use_color;
> extern int term_columns(void);
First off, the wording is wrong, it should be "value set by
git_config()".
But that doesn't tell the reader what the significance of the value
is. I suspect that a number of global variables need to be marked:
> /* The pager program name, or "cat" if there is no pager.
> * Can be overridden by the pager. configuration value for a
> * single command, or suppressed by the --no-pager option.
> * Set by calling git_config().
> * NULL if hasn't been set yet by calling git_config(). */
> extern const char *pager_program;
Dale
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: the pager
> I've noticed that Git by default puts long output through "less" as a > pager. I don't like that, but this is not the time to change > established behavior. But while tracking that down, I noticed that > the paging behavior is controlled by at least 5 things: > > the -p/--paginate/--no-pager options > the GIT_PAGER environment variable > the PAGER environment variable > the core.pager Git configuration variable > the build-in default (which seems to usually be "less") One complication is the meaning of -p/--no-pager: With the remaining sources, we assume that there is a priority sequence, and that is used to determine what the pager is. There is a somewhat independent question of when the pager is activated. What I know so far is that some commands use the pager by default and some by default do not. My expectation is that --no-pager can be used to suppress the pager for *any* command. Is it also true that -p can force the pager for *any* command, or are there commands which will not page even with -p? I assume that if -p is specified but the "which pager" selection is "cat" (or some other specification of no pager), then there is no paging operation. Dale -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: the pager
Hi, Dale R. Worley wrote: > That's true, but it would change the effect of using "cat" as a value: > "cat" as a value of DEFAULT_PAGER would cause git_pager() to return > NULL, whereas now it causes git_pager() to return "cat". (All other > places where "cat" can be a value are translated to NULL already.) > > This is why I want to know what the *intended* behavior is, because we > might be changing Git's behavior, and I want to know that if we do > that, we're changing it to what it should be. And I haven't seen > anyone venture an opinion on what the intended behavior is. I don't really follow. For all practical purposes, "cat" is equivalent to no pager at all, no? And the git-var(1) manpage describes the intended order of precedence, as far as I can tell, except that it was written before v1.7.4-rc0~76^2 (allow command-specific pagers in pager., 2010-11-17) which forgot to update some documentation. Suggested wording for improving the documentation or its organization would of course be welcome. And I agree with Matthieu that the name of the pager_program global variable is needlessly confusing --- perhaps it should be called config_pager_program or similar. Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] revision: introduce --exclude= to tame wildcards
On 09/02/2013 10:11 PM, Johannes Sixt wrote:
> Am 31.08.2013 01:55, schrieb Junio C Hamano:
>> People often find "git log --branches" etc. that includes _all_
>> branches is cumbersome to use when they want to grab most but except
>> some. The same applies to --tags, --all and --glob.
>>
>> Teach the revision machinery to remember patterns, and then upon the
>> next such a globbing option, exclude those that match the pattern.
>>
>> With this, I can view only my integration branches (e.g. maint,
>> master, etc.) without topic branches, which are named after two
>> letters from primary authors' names, slash and topic name.
>>
>> git rev-list --no-walk --exclude=??/* --branches |
>> git name-rev --refs refs/heads/* --stdin
>>
>> This one shows things reachable from local and remote branches that
>> have not been merged to the integration branches.
>>
>> git log --remotes --branches --not --exclude=??/* --branches
>>
>> It may be a bit rough around the edges, in that the pattern to give
>> the exclude option depends on what globbing option follows. In
>> these examples, the pattern "??/*" is used, not "refs/heads/??/*",
>> because the globbing option that follows the -"-exclude="
>> is "--branches". As each use of globbing option resets previously
>> set "--exclude", this may not be such a bad thing, though.
>
> I argued "--except should trump everything" earlier, but the case
> involving --not
>
> --branches --except maint --not master
>
> to mean the same as
>
> --branches --except maint master
>
> just does not make sense.
>
> An alternative would be that --not would divide the command line
> arguments into ranges within which one --except would subtract
> subsequent refs from earlier globbing arguments in the same range.
> That's not simpler to explain than your current proposal.
>
> So I like the relative simplicity of this approach. Here is a bit of
> documentation.
>
> --- 8< ---
> Subject: [PATCH] document --exclude option
>
> Signed-off-by: Johannes Sixt
> ---
> Documentation/rev-list-options.txt | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/rev-list-options.txt
> b/Documentation/rev-list-options.txt
> index 5bdfb42..650c252 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -174,6 +174,21 @@ parents) and `--max-parents=-1` (negative numbers denote
> no upper limit).
> is automatically prepended if missing. If pattern lacks '?',
> '{asterisk}',
> or '[', '/{asterisk}' at the end is implied.
>
> +--exclude=::
> +
> + Do not include refs matching '' that the next `--all`,
> + `--branches`, `--tags`, `--remotes`, or `--glob` would otherwise
> + consider. Repetitions of this option accumulate exclusion patterns
> + up to the next `--all`, `--branches`, `--tags`, `--remotes`, or
> + `--glob` option (other options or arguments do not clear
> + accumlated patterns).
> ++
> +The patterns given should not begin with `refs/heads`, `refs/tags`, or
> +`refs/remotes` when applied to `--branches`, `--tags`, or `--remotes`,
> +restrictively, and they must begin with `refs/` when applied to `--glob`
s/restrictively/respectively/
> +or `--all`. If a trailing '/{asterisk}' is intended, it must be given
> +explicitly.
> +
> --ignore-missing::
>
> Upon seeing an invalid object name in the input, pretend as if
>
It seems to me that this is growing into a language for expressing
boolean expressions without allowing terms to be combined in the full
generality that, say, a real programming language would allow. Maybe
instead of trying to decide on the "perfect" grouping and precedence
rules, it would be clearer to allow the user to specify them. I almost
hate to suggest it, but have you considered making the expression
"syntax" a little bit more flexible by allowing parentheses, à la
find(1), or something analogous?:
'(' --tags --except='v[0-9]*' ')' -o '(' --branches --except='mh/*' ')'
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/8] refs: add update_refs for multiple simultaneous updates
On 09/02/2013 07:20 PM, Brad King wrote:
> On 09/01/2013 02:08 AM, Junio C Hamano wrote:
>>> Though the refs themeselves cannot be modified together in a single
>>
>> "themselves".
>
> Fixed.
>
>> I notice that we are using an array of structures and letting qsort
>> swap 50~64 bytes of data
>
> Michael suggested this too, so fixed.
Hmmm, I see that you changed the signature of update_refs() to take an
array of pointers. My suggestion was unclear, but I didn't mean that
the function signature had to be changed. Rather, I meant that *within*
the function, you could have created an array of pointers to the
structures in the input array and thereafter accessed it via the pointers:
int update_refs(const char *action, const struct ref_update *updates_orig,
int n, enum action_on_err onerr)
{
[...]
struct ref_update **updates;
[...]
updates = xcalloc(n, sizeof(*updates));
for (i = 0; i < n; i++)
updates[i] = &updates_orig[i];
[...]
}
However, your approach is also fine. It will typically involve more
malloc()s but smaller memcpy()s (i.e., via ALLOC_GROW()) at the caller,
and since usually the number of ref_updates being done at one time will
be limited anyway, I don't see a reason to prefer one version over the
other.
Thanks for making the change.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Document pack v4 format
On Sat, 31 Aug 2013, Nguyễn Thái Ngọc Duy wrote: > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > Incorporated suggestions by Nico and Junio. I went ahead and added > escape hatches for converting thin packs to full ones so the document > does not really match the code (I've been watching Nico's repository, > commit reading is added, good stuff!) Now tree reading is added. multiple encoding bug fixes trickled down to their originating commits as well. Something is still wrong with deltas though. Nicolas
Re: [PATCH 23/23] initial pack index v3 support on the read side
On Sat, 31 Aug 2013, Duy Nguyen wrote: > On Tue, Aug 27, 2013 at 11:26 AM, Nicolas Pitre wrote: > > A bit crud but good enough for now. > > I wonder if we should keep a short SHA-1 table in .idx. An entry in > the original .idx v1 table is + then offset moved out > to make the table more compact for binary search. I observe that we > don't always need 20 byte SHA-1 to uniquely identify an entry in a > pack, so the SHA-1 table could be split in two: one table contain the > first part of SHA-1, long enough to identify any entry in the pack; > the second table contains either full SHA-1 or the remaining part. > Binary search is done on the first table, if matched, full sha-1 from > the second table is compared. We already have the second sha-1 table > in .pack v4. We could add the first table in .idx v3. > > On linux-2.6 even in one-pack configuration, we only need the first 6 > bytes of sha-1 to identify an object. That would cut the bsearch sha-1 > table to 1/4 full sha-1 table size. I don't see the point though. Why having two tables when only one suffice? If the argument is about making the SHA1 binary search more efficient given a smaller memory footprint, that comes with extra complexity coming from the variable length SHA1 strings in the second table. So I'm not sure there is much to gain. Furthermore, one of the design of pack v4 is to avoid the SHA1 binary search entirely. You will need the binary search only once to locate the top commit object, but from there the entire object tree can be walked simply by using the sha1ref index and looking up the corresponding offset into the pack index file to locate the next object. Same thing for walking tree objects: the pack v4 tree deltas are meant to be walked inline without having to _apply_ any delta. Nicolas -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/23] pack v4: commit object encoding
On Tue, 3 Sep 2013, Duy Nguyen wrote: > On Tue, Aug 27, 2013 at 11:25 AM, Nicolas Pitre wrote: > > This goes as follows: > > > > - Tree reference: either variable length encoding of the index > > into the SHA1 table or the literal SHA1 prefixed by 0 (see > > add_sha1_ref()). > > > > - Parent count: variable length encoding of the number of parents. > > This is normally going to occupy a single byte but doesn't have to. > > > > - List of parent references: a list of add_sha1_ref() encoded references, > > or nothing if the parent count was zero. > > With .pack v3 it's impossible to create delta cycles (3b910d0 add > tests for indexing packs with delta cycles - 2013-08-23) but it is > possible with .pack v4 (and therefore at least index-pack needs to > detect and reject them), correct? Some malicious user can create > commit A with parent ref 1, then make the SHA-1 table so that ref 1 is > A. The same with the new tree representation. pack-index should validate the SHA1 of the object being pointed at. In that case I doubt you'll be able to actually construct an object which contains a SHA1 parent reference and make the SHA1 of this very object resolve to the same value. Nicolas -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git send-email: include [anything]-by: signatures
On Sat, Aug 31, 2013 at 10:22:50PM +0300, Michael S. Tsirkin wrote: > On Mon, Aug 26, 2013 at 07:57:47PM +0300, Michael S. Tsirkin wrote: > > Consider [anything]-by: a valid signature. > > This includes Tested-by: Acked-by: Reviewed-by: etc. > > > > Signed-off-by: Michael S. Tsirkin > > Ping. > Any opinion on whether this change is acceptable? I was left confused by your commit message, as it wasn't clear to me what a "signature" is. But the point of it seems to be that people mention others in commit messages using "X-by:" pseudo-headers besides "signed-off-by", and you want to cc them along with the usual S-O-B. That seems like a reasonable goal, but I have two concerns. One, I would think the utility of this would be per-project, depending on what sorts of things people in a particular project put in pseudo-headers. Grepping the kernel history shows that most X-by headers have a person on the right-hand side, though quite often it is not a valid email address (on the other hand, quite a few s-o-b lines in the kernel do not have a valid email). And two, the existing options for enabling/disabling this code all explicitly mention signed-off-by, which becomes awkward. You did not update the documentation in your patch, but I think you would end up having to explain that "--supress-cc=sob" and "--signed-off-by-cc" really mean "all pseudo-header lines ending in -by". So I think it might be a nicer approach to introduce a new "suppress-cc" class that means "all pseudo-header tokens ending in -by" or similar. We might even want the new behavior on by default, but it would at least give the user an escape hatch if their project generates a lot of false positives. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Document pack v4 format
On Tue, 3 Sep 2013, Nicolas Pitre wrote: > On Sat, 31 Aug 2013, Nguyễn Thái Ngọc Duy wrote: > > > > > Signed-off-by: Nguyễn Thái Ngọc Duy > > --- > > Incorporated suggestions by Nico and Junio. I went ahead and added > > escape hatches for converting thin packs to full ones so the document > > does not really match the code (I've been watching Nico's repository, > > commit reading is added, good stuff!) > > Now tree reading is added. multiple encoding bug fixes trickled down to > their originating commits as well. > > Something is still wrong with deltas though. Deltas fixed now. So... looks like pack v4 is now "functional". However something is still wrong as it operates about 6 times slower than pack v3. Anyone wishes to investigate? Nicolas
Re: [PATCH] {fetch,receive}-pack: drop unpack-objects, delay loosing objects until the end
On Mon, Sep 02, 2013 at 10:05:07AM +0700, Nguyen Thai Ngoc Duy wrote: > Current code peaks into the transfered pack's header, if the number of > objects is under a limit, unpack-objects is called to handle the rest, > otherwise index-pack is. This patch makes fetch-pack use index-pack > unconditionally, then turn objects loose and remove the pack at the > end. unpack-objects is deprecated and may be removed in future. I do like consolidating the object-receiving code paths, but there is a downside to this strategy: we increase the I/O in cases where we end up unpacking, as we spool the tmpfile to disk, and then force objects loose (whereas with the current code, unpack-objects reads straight from the network into loose objects). I think that is what you're saying here: > - by going through index-pack first, then unpack, we pay extra cost >for completing a thin pack into a full one. But compared to fetch's >total time, it should not be noticeable because unpack-objects is >only called when the pack contains a small number of objects. ...but the cost is paid by total pack size, not number of objects. So if I am pushing up a commit with a large uncompressible blob, I've effectively doubled my disk I/O. It would make more sense to me for index-pack to learn command-line options specifying the limits, and then to operate on the pack as it streams in. E.g., to decide after seeing the header to unpack rather than index, or to drop large blobs from the pack (and put them in their own pack directly) as we are streaming into it (we do not know the blob size ahead of time, but we can make a good guess if it has a large on-disk size in the pack). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
