* Re: color.diff.whitespace unused on removed lines
From: Jeff King @ 2016-10-04 15:29 UTC (permalink / raw)
To: Sandro Santilli; +Cc: git
In-Reply-To: <20161004081429.GC17002@localhost>
On Tue, Oct 04, 2016 at 10:14:29AM +0200, Sandro Santilli wrote:
> The color.diff.whitespace configuration is not used on
> removed lines, but only on added lines.
Right. The original purpose was to warn you when you were introducing
whitespace breakages. Getting rid of other people's whitespace breakages
is OK.
We later did b8767f7 (diff.c: --ws-error-highlight=<kind> option,
2015-05-26) to let you see them on other lines, though. I think that
would do what you want.
-Peff
^ permalink raw reply
* Re: Regression: git no longer works with musl libc's regex impl
From: Jeff King @ 2016-10-04 15:27 UTC (permalink / raw)
To: Rich Felker; +Cc: git, musl
In-Reply-To: <20161004150848.GA7949@brightrain.aerifal.cx>
On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:
> This commit broke support for using git with musl libc:
>
> https://github.com/git/git/commit/2f8952250a84313b74f96abb7b035874854cf202
Yep. The idea is that you would compile git with NO_REGEX=1, and it
would use the included compat routines.
Is there something in particular you want to get out of using musl's
regex that is not supported in the compat library?
> Rather than depending on non-portable GNU regex extensions, there is a
> simple portable fix for the issue this code was added to work around:
> When a text file is being mmapped for use with string functions which
> depend on null termination, if the file size:
>
> 1. is nonzero mod page size, it just works; the remainder of the last
> page reads as zero bytes when mmapped.
Is that a portable assumption?
> 2. if an exact multiple of the page size, then instead of directly
> mmapping the file, first mmap a mapping 1 byte (thus 1 page) larger
> with MAP_ANON, then use MAP_FIXED to map the file over top of all
> but the last page. Now the mmapped buffer can safely be used as a C
> string.
I'm not sure whether all of our compat layers for mmap would be happy
with that (e.g., see compat/win32mmap.c).
So it seems like any mmap-related solutions would have to be
conditional, too. And then regexec_buf() would have to become something
like:
int regexec_buf(...)
{
#if defined(REG_STARTEND)
... set up match ...
return regexec(..., REG_STARTEND);
#elif defined(MMAP_ALWAYS_HAS_NUL)
/*
* We assume that every buffer we see is always NUL-terminated
* eventually, either because it comes from xmallocz() or our
* mmap layer always ensures an extra NUL.
*/
return regexec(...);
#else
#error "Nope, you need either NO_REGEX or USE_MMAP_NUL"
#endif
}
The assumption in the middle case feels pretty hacky, though. It fails
if we get a buffer from somewhere besides those two sources. It fails if
somebody calls regexec_buf() on a subset of a string.
It also doesn't handle matching past embedded NULs in the string. That's
not something we're relying on yet, but it would be nice to support
consistently in the long run.
If there's a compelling reason, it might be worth making that tradeoff.
But I am not sure what the compelling reason is to use musl's regex
(aside from the obvious of "less code in the resulting executable").
-Peff
^ permalink raw reply
* Regression: git no longer works with musl libc's regex impl
From: Rich Felker @ 2016-10-04 15:08 UTC (permalink / raw)
To: git; +Cc: musl
This commit broke support for using git with musl libc:
https://github.com/git/git/commit/2f8952250a84313b74f96abb7b035874854cf202
Rather than depending on non-portable GNU regex extensions, there is a
simple portable fix for the issue this code was added to work around:
When a text file is being mmapped for use with string functions which
depend on null termination, if the file size:
1. is nonzero mod page size, it just works; the remainder of the last
page reads as zero bytes when mmapped.
2. if an exact multiple of the page size, then instead of directly
mmapping the file, first mmap a mapping 1 byte (thus 1 page) larger
with MAP_ANON, then use MAP_FIXED to map the file over top of all
but the last page. Now the mmapped buffer can safely be used as a C
string.
If such a solution is acceptable I can try to prepare a patch.
Rich
^ permalink raw reply
* [PATCH v2] http: http.emptyauth should allow empty (not just NULL) usernames
From: David Turner @ 2016-10-04 14:53 UTC (permalink / raw)
To: git, sandals; +Cc: David Turner
When using Kerberos authentication with newer versions of libcurl,
CURLOPT_USERPWD must be set to a value, even if it is an empty value.
The value is never sent to the server. Previous versions of libcurl
did not require this variable to be set. One way that some users
express the empty username/password is http://:@gitserver.example.com,
which http.emptyauth was designed to support. Another, equivalent,
URL is http://@gitserver.example.com. The latter leads to a username
of zero-length, rather than a NULL username, but CURLOPT_USERPWD still
needs to be set (if http.emptyauth is set). Do so.
Signed-off-by: David Turner <dturner@twosigma.com>
---
Same patch, different message.
---
http.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/http.c b/http.c
index 82ed542..bd0dba2 100644
--- a/http.c
+++ b/http.c
@@ -351,7 +351,7 @@ static int http_options(const char *var, const char *value, void *cb)
static void init_curl_http_auth(CURL *result)
{
- if (!http_auth.username) {
+ if (!http_auth.username || !*http_auth.username) {
if (curl_empty_auth)
curl_easy_setopt(result, CURLOPT_USERPWD, ":");
return;
--
2.8.0.rc4.22.g8ae061a
^ permalink raw reply related
* Re: [PATCH 18/18] alternates: use fspathcmp to detect duplicates
From: Jeff King @ 2016-10-04 14:10 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list, René Scharfe
In-Reply-To: <CA+P7+xqhuYmp-H=b-SrNdZjN5urWGHPuNkWbeVgCBF1UuhQZKQ@mail.gmail.com>
On Mon, Oct 03, 2016 at 11:51:59PM -0700, Jacob Keller wrote:
> On Mon, Oct 3, 2016 at 1:36 PM, Jeff King <peff@peff.net> wrote:
> > On a case-insensitive filesystem, we should realize that
> > "a/objects" and "A/objects" are the same path. We already
> > use fspathcmp() to check against the main object directory,
> > but until recently we couldn't use it for comparing against
> > other alternates (because their paths were not
> > NUL-terminated strings). But now we can, so let's do so.
> >
>
> Yep, makes sense.
>
> > Note that we also need to adjust count-objects to load the
> > config, so that it can see the setting of core.ignorecase
> > (this is required by the test, but is also a general bugfix
> > for users of count-objects).
>
> Also makes sense.
BTW, I tested this on a vfat loopback device, but I was surprised to see
that quite a few other tests failed on that device.
At least one of the problems is that symlinks are not supported, but
lib-httpd.sh wants to use them for its Apache setup. I guess people on
Windows just don't run the httpd tests at all, which is not too
surprising.
Likewise, credential-cache fails because it cannot create a Unix socket
(and the flag for that is in the build, not a run-time filesystem
check).
Some of the other failures seemed to be due to lack of an executable bit
on the filesystem. I'm not sure if we could or should do better run-time
detection of that sort of thing. I think some of the checks are tied to
the build, and that's generally good enough in practice because people
don't use vfat on their Linux machines. So tracking down each of them
may just be pedantic make-work that nobody cares about.
I did wonder if there was another good filesystem to use for
case-insensitive experiments on Linux. At the time I didn't think there
was good support for making HFS+ filesystems, but it looks Debian cares
mkfs.hfs. That's probably a better choice for such experiments.
-Peff
^ permalink raw reply
* Re: [PATCH 17/18] sha1_file: always allow relative paths to alternates
From: Jeff King @ 2016-10-04 14:00 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list, René Scharfe
In-Reply-To: <CA+P7+xr9ugWWcoyQ6dToFacwff8rGJhYJNxy+E5_iGjubONLPQ@mail.gmail.com>
On Mon, Oct 03, 2016 at 11:50:30PM -0700, Jacob Keller wrote:
> > Note that our normalization doesn't actually look at the
> > filesystem, so it can still be fooled by crossing symbolic
> > links. But that's also true of absolute paths, so it's not a
> > good reason to disallow only relative paths (it's
> > potentially a reason to switch to real_path(), but that's a
> > separate and non-trivial change).
>
> Hmm, ya using real_path would fix that but I definitely agree that's
> not trivial and can be done in the future if we think it is or becomes
> necessary.
I did look into this briefly. The trick is that real_path() assumes
relative paths are relative from the current directory (and does chdir()
trickery to get the filesystem to resolve things for us). So you'd
really need a "real_path_from" that chdirs to the relative base, issues
the real_path() from there, and then chdirs back to the original cwd.
Which I guess is no less gross than what real_path() is doing itself
internally, but it's definitely something for another patch. Given the
fact that we don't check it now and nobody has complained leads me to
believe that nobody really cares.
Actually, given the fact that we didn't allow relative bases in
recursive alternates, I suspect that very few people are using
complicated alternate setups in the first place.
-Peff
^ permalink raw reply
* Re: [PATCH 16/18] count-objects: report alternates via verbose mode
From: Jeff King @ 2016-10-04 13:56 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list, René Scharfe
In-Reply-To: <CA+P7+xoR-14fmzrYU_sGoTRgcfP8fe5y-+kSkoM-2=E3Jb56FQ@mail.gmail.com>
On Mon, Oct 03, 2016 at 11:46:32PM -0700, Jacob Keller wrote:
> \On Mon, Oct 3, 2016 at 1:36 PM, Jeff King <peff@peff.net> wrote:
> > There's no way to get the list of alternates that git
> > computes internally; our tests only infer it based on which
> > objects are available. In addition to testing, knowing this
> > list may be helpful for somebody debugging their alternates
> > setup.
> >
> > Let's add it to the "count-objects -v" output. We could give
> > it a separate flag, but there's not really any need.
> > "count-objects -v" is already a debugging catch-all for the
> > object database, its output is easily extensible to new data
> > items, and printing the alternates is not expensive (we
> > already had to find them to count the objects).
> >
>
> Makes sense. Unless there's a compelling reason you'd want to print
> out these alternates *without* anything else from -v, but you can just
> use grep like the test does so this seems fine to me.
Yeah. I could definitely be persuaded otherwise, but I just couldn't see
these being used for anything useful beyond debugging.
-Peff
^ permalink raw reply
* Re: [PATCH 08/18] link_alt_odb_entry: refactor string handling
From: Jeff King @ 2016-10-04 13:53 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list, René Scharfe
In-Reply-To: <CA+P7+xrUOnDebwZnfu-xv-GuTJka4-eNUAfBudQf5ZhnkczU6w@mail.gmail.com>
On Mon, Oct 03, 2016 at 11:05:42PM -0700, Jacob Keller wrote:
> This definitely makes reading the following function much easier,
> though the diff is a bit funky. I think the end result is much
> clearer.
Yeah, it's really hard to see that all of the "ent" setup is kept,
because it moves _and_ changes its content (from pfxlen to pathbuf.len).
I actually tried to split this into two patches to make the diff easier
to read, but there are two mutually dependent changes: moving to
pathbuf.len everywhere requires not-freeing pathbuf in the early code
path. But if you do that and don't move all of "is it usable" checks up,
then you have to add a bunch of new error-handling code that would just
get ripped out in the next patch.
There's definitely _some_ of that in this series already (e.g., the
counting logic in alt_sha1_path() added by patch 14 that just gets
ripped out in patch 15 when fill_sha1_path() learns to use a strbuf). I
tried to balance "show each individual obvious step" with "don't make
people review a bunch of scaffolding that's not going to be in the final
product".
-Peff
^ permalink raw reply
* Re: [PATCH 06/18] t5613: clarify "too deep" recursion tests
From: Jeff King @ 2016-10-04 13:48 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list, René Scharfe
In-Reply-To: <CA+P7+xok5PoNKO+8R6zF9SXYfDq6BboDTDz9WZYEczs0pFK+pw@mail.gmail.com>
On Mon, Oct 03, 2016 at 10:57:48PM -0700, Jacob Keller wrote:
> > diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
> > index 7bc1c3c..b393613 100755
> > --- a/t/t5613-info-alternate.sh
> > +++ b/t/t5613-info-alternate.sh
> > @@ -39,6 +39,18 @@ test_expect_success 'preparing third repository' '
> > )
> > '
> >
> > +# Note: These tests depend on the hard-coded value of 5 as "too deep". We start
> > +# the depth at 0 and count links, not repositories, so in a chain like:
> > +#
> > +# A -> B -> C -> D -> E -> F -> G -> H
> > +# 0 1 2 3 4 5 6
> > +#
>
> Ok so we count links, but wouldn't we have 5 links when we hit F, and
> not G? Or am I missing something here?
This is what I was trying to get at with the "start the depth at 0". We
disallow a depth greater than 5, but because we start at 0-counting,
it's really six links. I guess saying "5 as too deep" is really the
misleading part. It should be "5 as the maximum depth".
-Peff
^ permalink raw reply
* Re: [PATCH 04/18] t5613: whitespace/style cleanups
From: Jeff King @ 2016-10-04 13:47 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list, René Scharfe
In-Reply-To: <CA+P7+xoX6XsKRFUCx+vhUo7ARYMEXktzcbFS=zh1NUTgRdhdUA@mail.gmail.com>
On Mon, Oct 03, 2016 at 10:52:39PM -0700, Jacob Keller wrote:
> On Mon, Oct 3, 2016 at 1:34 PM, Jeff King <peff@peff.net> wrote:
> > Our normal test style these days puts the opening quote of
> > the body on the description line, and indents the body with
> > a single tab. This ancient test did not follow this.
> >
>
> I was surprised you didn't do this first, but it doesn't really make a
> difference either way. This is also a pretty straight forward
> improvement, and I can see why you'd want to split this out to review
> separately.
I was trying to leave it to the end, to move the substantive changes up
front (and because there _isn't_ a correct style for some of the things
it was doing). But it just got too painful to do the "don't chdir" patch
without updating the style. I agree it might have made more sense at the
very beginning, but I didn't think it mattered enough to go through the
trouble of rebasing the earlier patches (which would essentially be
rewriting them).
-Peff
^ permalink raw reply
* Re: [PATCH 01/18] t5613: drop reachable_via function
From: Jeff King @ 2016-10-04 13:43 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list, René Scharfe
In-Reply-To: <CA+P7+xq3G=CNHNNQ5YpkHkud_5SUrTBOwZ3y7d8DvM9nKyXV9g@mail.gmail.com>
On Mon, Oct 03, 2016 at 10:48:31PM -0700, Jacob Keller wrote:
> On Mon, Oct 3, 2016 at 1:33 PM, Jeff King <peff@peff.net> wrote:
> > This function was never used since its inception in dd05ea1
> > (test case for transitive info/alternates, 2006-05-07).
> > Which is just as well, since it mutates the repo state in a
> > way that would invalidate further tests, without cleaning up
> > after itself. Let's get rid of it so that nobody is tempted
> > to use it.
> >
>
> Makes sense. It wouldn't be a good idea to leave this around since it
> didn't clean up after itself. Curious why no test actually used it
> though..
I wondered that, too. Sometimes in cases like this a call got dropped
during a re-roll on the list. But the original email:
http://public-inbox.org/git/20060507181947.GE23738@admingilde.org/
has the problem, too. So probably it was just leftover cruft that was
made obsolete even before it hit the list.
-Peff
^ permalink raw reply
* Re: [PATCH 0/18] alternate object database cleanups
From: Jeff King @ 2016-10-04 13:41 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list, René Scharfe
In-Reply-To: <CA+P7+xob7ohj1MxuLGGLwwJyi4RfqUTeLkbw86u+VvbU=uEyAw@mail.gmail.com>
On Mon, Oct 03, 2016 at 10:47:31PM -0700, Jacob Keller wrote:
> > The number of patches is a little intimidating, but I tried hard to
> > break the refactoring down into a sequence of obviously-correct steps.
> > You can be the judge of my success.
>
> I read through them once. I'm going to re-read through them again and
> leave any comments I had.
Thanks for having the fortitude to read them all. :) After looking at
your comments, I don't _think_ there's anything that necessitates a
re-roll, but I'll respond to a few of them individually.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #01; Mon, 3)
From: Lars Schneider @ 2016-10-04 13:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqtwct3w0p.fsf@gitster.mtv.corp.google.com>
> On 04 Oct 2016, at 00:31, Junio C Hamano <gitster@pobox.com> wrote:
>
>
> * ls/filter-process (2016-09-23) 11 commits
> - convert: add filter.<driver>.process option
> - convert: make apply_filter() adhere to standard Git error handling
> - convert: modernize tests
> - convert: quote filter names in error messages
> - pkt-line: add functions to read/write flush terminated packet streams
> - pkt-line: add packet_write_gently()
> - pkt-line: add packet_flush_gently()
> - pkt-line: add packet_write_fmt_gently()
> - run-command: move check_pipe() from write_or_die to run_command
> - pkt-line: extract set_packet_header()
> - pkt-line: rename packet_write() to packet_write_fmt()
>
> The smudge/clean filter API expect an external process is spawned
> to filter the contents for each path that has a filter defined. A
> new type of "process" filter API has been added to allow the first
> request to run the filter for a path to spawn a single process, and
> all filtering need is served by this single process for multiple
> paths, reducing the process creation overhead.
>
> Somehow I thought this was getting ready for 'next' but it seems
> at least another round of reroll is coming?
I thought so, too, but t0021 was flaky and Jakub had a number of good
suggestions. I just posted v9:
http://public-inbox.org/git/20161004125947.67104-1-larsxschneider@gmail.com/
- Lars
^ permalink raw reply
* [PATCH v3 6/6] wt-status: begin error messages with lower-case
From: Johannes Schindelin @ 2016-10-04 13:06 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1475586229.git.johannes.schindelin@gmx.de>
The previous code still followed the old git-pull.sh code which did not
adhere to our new convention.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/pull.c | 2 +-
wt-status.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index c639167..0bf9802 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -810,7 +810,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
if (!autostash)
require_clean_work_tree(N_("pull with rebase"),
- "Please commit or stash them.", 1, 0);
+ "please commit or stash them.", 1, 0);
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
diff --git a/wt-status.c b/wt-status.c
index 061597a..010276b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2265,15 +2265,15 @@ int require_clean_work_tree(const char *action, const char *hint, int ignore_sub
if (has_unstaged_changes(ignore_submodules)) {
/* TRANSLATORS: the action is e.g. "pull with rebase" */
- error(_("Cannot %s: You have unstaged changes."), _(action));
+ error(_("cannot %s: You have unstaged changes."), _(action));
err = 1;
}
if (has_uncommitted_changes(ignore_submodules)) {
if (err)
- error(_("Additionally, your index contains uncommitted changes."));
+ error(_("additionally, your index contains uncommitted changes."));
else
- error(_("Cannot %s: Your index contains uncommitted changes."),
+ error(_("cannot %s: Your index contains uncommitted changes."),
_(action));
err = 1;
}
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 5/6] wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
From: Johannes Schindelin @ 2016-10-04 13:05 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1475586229.git.johannes.schindelin@gmx.de>
Sometimes we are *actually* interested in those changes... For
example when an interactive rebase wants to continue with a staged
submodule update.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/pull.c | 2 +-
wt-status.c | 16 +++++++++-------
wt-status.h | 7 ++++---
3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 14ef8b5..c639167 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -810,7 +810,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
if (!autostash)
require_clean_work_tree(N_("pull with rebase"),
- "Please commit or stash them.", 0);
+ "Please commit or stash them.", 1, 0);
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
diff --git a/wt-status.c b/wt-status.c
index f4103e4..061597a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2214,13 +2214,14 @@ void wt_status_print(struct wt_status *s)
/**
* Returns 1 if there are unstaged changes, 0 otherwise.
*/
-int has_unstaged_changes(void)
+int has_unstaged_changes(int ignore_submodules)
{
struct rev_info rev_info;
int result;
init_revisions(&rev_info, NULL);
- DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
+ if (ignore_submodules)
+ DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(&rev_info.diffopt, QUICK);
diff_setup_done(&rev_info.diffopt);
result = run_diff_files(&rev_info, 0);
@@ -2230,7 +2231,7 @@ int has_unstaged_changes(void)
/**
* Returns 1 if there are uncommitted changes, 0 otherwise.
*/
-int has_uncommitted_changes(void)
+int has_uncommitted_changes(int ignore_submodules)
{
struct rev_info rev_info;
int result;
@@ -2239,7 +2240,8 @@ int has_uncommitted_changes(void)
return 0;
init_revisions(&rev_info, NULL);
- DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
+ if (ignore_submodules)
+ DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(&rev_info.diffopt, QUICK);
add_head_to_pending(&rev_info);
diff_setup_done(&rev_info.diffopt);
@@ -2251,7 +2253,7 @@ int has_uncommitted_changes(void)
* If the work tree has unstaged or uncommitted changes, dies with the
* appropriate message.
*/
-int require_clean_work_tree(const char *action, const char *hint, int gently)
+int require_clean_work_tree(const char *action, const char *hint, int ignore_submodules, int gently)
{
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
int err = 0;
@@ -2261,13 +2263,13 @@ int require_clean_work_tree(const char *action, const char *hint, int gently)
update_index_if_able(&the_index, lock_file);
rollback_lock_file(lock_file);
- if (has_unstaged_changes()) {
+ if (has_unstaged_changes(ignore_submodules)) {
/* TRANSLATORS: the action is e.g. "pull with rebase" */
error(_("Cannot %s: You have unstaged changes."), _(action));
err = 1;
}
- if (has_uncommitted_changes()) {
+ if (has_uncommitted_changes(ignore_submodules)) {
if (err)
error(_("Additionally, your index contains uncommitted changes."));
else
diff --git a/wt-status.h b/wt-status.h
index 68e367a..54fec77 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -129,8 +129,9 @@ __attribute__((format (printf, 3, 4)))
void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);
/* The following functions expect that the caller took care of reading the index. */
-int has_unstaged_changes(void);
-int has_uncommitted_changes(void);
-int require_clean_work_tree(const char *action, const char *hint, int gently);
+int has_unstaged_changes(int ignore_submodules);
+int has_uncommitted_changes(int ignore_submodules);
+int require_clean_work_tree(const char *action, const char *hint,
+ int ignore_submodules, int gently);
#endif /* STATUS_H */
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 4/6] Export also the has_un{staged,committed}_changed() functions
From: Johannes Schindelin @ 2016-10-04 13:05 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1475586229.git.johannes.schindelin@gmx.de>
They will be used in the upcoming rebase helper.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
wt-status.c | 4 ++--
wt-status.h | 4 +++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index b92c54d..f4103e4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2214,7 +2214,7 @@ void wt_status_print(struct wt_status *s)
/**
* Returns 1 if there are unstaged changes, 0 otherwise.
*/
-static int has_unstaged_changes(void)
+int has_unstaged_changes(void)
{
struct rev_info rev_info;
int result;
@@ -2230,7 +2230,7 @@ static int has_unstaged_changes(void)
/**
* Returns 1 if there are uncommitted changes, 0 otherwise.
*/
-static int has_uncommitted_changes(void)
+int has_uncommitted_changes(void)
{
struct rev_info rev_info;
int result;
diff --git a/wt-status.h b/wt-status.h
index 03ecf53..68e367a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -128,7 +128,9 @@ void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, .
__attribute__((format (printf, 3, 4)))
void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);
-/* The following function expect that the caller took care of reading the index. */
+/* The following functions expect that the caller took care of reading the index. */
+int has_unstaged_changes(void);
+int has_uncommitted_changes(void);
int require_clean_work_tree(const char *action, const char *hint, int gently);
#endif /* STATUS_H */
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 3/6] Make the require_clean_work_tree() function reusable
From: Johannes Schindelin @ 2016-10-04 13:05 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1475586229.git.johannes.schindelin@gmx.de>
It is remarkable that libgit.a did not sport this function yet... Let's
move it into a more prominent (and into an actually reusable) spot:
wt-status.[ch].
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/pull.c | 77 +---------------------------------------------------------
wt-status.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
wt-status.h | 3 +++
3 files changed, 80 insertions(+), 76 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index d1e093c..14ef8b5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -17,6 +17,7 @@
#include "revision.h"
#include "tempfile.h"
#include "lockfile.h"
+#include "wt-status.h"
enum rebase_type {
REBASE_INVALID = -1,
@@ -326,82 +327,6 @@ static int git_pull_config(const char *var, const char *value, void *cb)
}
/**
- * Returns 1 if there are unstaged changes, 0 otherwise.
- */
-static int has_unstaged_changes(void)
-{
- struct rev_info rev_info;
- int result;
-
- init_revisions(&rev_info, NULL);
- DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
- DIFF_OPT_SET(&rev_info.diffopt, QUICK);
- diff_setup_done(&rev_info.diffopt);
- result = run_diff_files(&rev_info, 0);
- return diff_result_code(&rev_info.diffopt, result);
-}
-
-/**
- * Returns 1 if there are uncommitted changes, 0 otherwise.
- */
-static int has_uncommitted_changes(void)
-{
- struct rev_info rev_info;
- int result;
-
- if (is_cache_unborn())
- return 0;
-
- init_revisions(&rev_info, NULL);
- DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
- DIFF_OPT_SET(&rev_info.diffopt, QUICK);
- add_head_to_pending(&rev_info);
- diff_setup_done(&rev_info.diffopt);
- result = run_diff_index(&rev_info, 1);
- return diff_result_code(&rev_info.diffopt, result);
-}
-
-/**
- * If the work tree has unstaged or uncommitted changes, dies with the
- * appropriate message.
- */
-static int require_clean_work_tree(const char *action, const char *hint,
- int gently)
-{
- struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
- int err = 0;
-
- hold_locked_index(lock_file, 0);
- refresh_cache(REFRESH_QUIET);
- update_index_if_able(&the_index, lock_file);
- rollback_lock_file(lock_file);
-
- if (has_unstaged_changes()) {
- /* TRANSLATORS: the action is e.g. "pull with rebase" */
- error(_("Cannot %s: You have unstaged changes."), _(action));
- err = 1;
- }
-
- if (has_uncommitted_changes()) {
- if (err)
- error(_("Additionally, your index contains uncommitted changes."));
- else
- error(_("Cannot %s: Your index contains uncommitted changes."),
- _(action));
- err = 1;
- }
-
- if (err) {
- if (hint)
- error("%s", hint);
- if (!gently)
- exit(err);
- }
-
- return err;
-}
-
-/**
* Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
* into merge_heads.
*/
diff --git a/wt-status.c b/wt-status.c
index 9628c1d..b92c54d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -16,6 +16,7 @@
#include "strbuf.h"
#include "utf8.h"
#include "worktree.h"
+#include "lockfile.h"
static const char cut_line[] =
"------------------------ >8 ------------------------\n";
@@ -2209,3 +2210,78 @@ void wt_status_print(struct wt_status *s)
break;
}
}
+
+/**
+ * Returns 1 if there are unstaged changes, 0 otherwise.
+ */
+static int has_unstaged_changes(void)
+{
+ struct rev_info rev_info;
+ int result;
+
+ init_revisions(&rev_info, NULL);
+ DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
+ DIFF_OPT_SET(&rev_info.diffopt, QUICK);
+ diff_setup_done(&rev_info.diffopt);
+ result = run_diff_files(&rev_info, 0);
+ return diff_result_code(&rev_info.diffopt, result);
+}
+
+/**
+ * Returns 1 if there are uncommitted changes, 0 otherwise.
+ */
+static int has_uncommitted_changes(void)
+{
+ struct rev_info rev_info;
+ int result;
+
+ if (is_cache_unborn())
+ return 0;
+
+ init_revisions(&rev_info, NULL);
+ DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
+ DIFF_OPT_SET(&rev_info.diffopt, QUICK);
+ add_head_to_pending(&rev_info);
+ diff_setup_done(&rev_info.diffopt);
+ result = run_diff_index(&rev_info, 1);
+ return diff_result_code(&rev_info.diffopt, result);
+}
+
+/**
+ * If the work tree has unstaged or uncommitted changes, dies with the
+ * appropriate message.
+ */
+int require_clean_work_tree(const char *action, const char *hint, int gently)
+{
+ struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
+ int err = 0;
+
+ hold_locked_index(lock_file, 0);
+ refresh_cache(REFRESH_QUIET);
+ update_index_if_able(&the_index, lock_file);
+ rollback_lock_file(lock_file);
+
+ if (has_unstaged_changes()) {
+ /* TRANSLATORS: the action is e.g. "pull with rebase" */
+ error(_("Cannot %s: You have unstaged changes."), _(action));
+ err = 1;
+ }
+
+ if (has_uncommitted_changes()) {
+ if (err)
+ error(_("Additionally, your index contains uncommitted changes."));
+ else
+ error(_("Cannot %s: Your index contains uncommitted changes."),
+ _(action));
+ err = 1;
+ }
+
+ if (err) {
+ if (hint)
+ error("%s", hint);
+ if (!gently)
+ exit(err);
+ }
+
+ return err;
+}
diff --git a/wt-status.h b/wt-status.h
index e401837..03ecf53 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -128,4 +128,7 @@ void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, .
__attribute__((format (printf, 3, 4)))
void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);
+/* The following function expect that the caller took care of reading the index. */
+int require_clean_work_tree(const char *action, const char *hint, int gently);
+
#endif /* STATUS_H */
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 2/6] pull: make code more similar to the shell script again
From: Johannes Schindelin @ 2016-10-04 13:05 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1475586229.git.johannes.schindelin@gmx.de>
When converting the pull command to a builtin, the
require_clean_work_tree() function was renamed and the pull-specific
parts hard-coded.
This makes it impossible to reuse the code, so let's modify the code to
make it more similar to the original shell script again.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/pull.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index d4bd635..d1e093c 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -365,10 +365,11 @@ static int has_uncommitted_changes(void)
* If the work tree has unstaged or uncommitted changes, dies with the
* appropriate message.
*/
-static void die_on_unclean_work_tree(void)
+static int require_clean_work_tree(const char *action, const char *hint,
+ int gently)
{
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
- int do_die = 0;
+ int err = 0;
hold_locked_index(lock_file, 0);
refresh_cache(REFRESH_QUIET);
@@ -376,20 +377,28 @@ static void die_on_unclean_work_tree(void)
rollback_lock_file(lock_file);
if (has_unstaged_changes()) {
- error(_("Cannot pull with rebase: You have unstaged changes."));
- do_die = 1;
+ /* TRANSLATORS: the action is e.g. "pull with rebase" */
+ error(_("Cannot %s: You have unstaged changes."), _(action));
+ err = 1;
}
if (has_uncommitted_changes()) {
- if (do_die)
+ if (err)
error(_("Additionally, your index contains uncommitted changes."));
else
- error(_("Cannot pull with rebase: Your index contains uncommitted changes."));
- do_die = 1;
+ error(_("Cannot %s: Your index contains uncommitted changes."),
+ _(action));
+ err = 1;
}
- if (do_die)
- exit(1);
+ if (err) {
+ if (hint)
+ error("%s", hint);
+ if (!gently)
+ exit(err);
+ }
+
+ return err;
}
/**
@@ -875,7 +884,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
die(_("Updating an unborn branch with changes added to the index."));
if (!autostash)
- die_on_unclean_work_tree();
+ require_clean_work_tree(N_("pull with rebase"),
+ "Please commit or stash them.", 0);
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 1/6] pull: drop confusing prefix parameter of die_on_unclean_work_tree()
From: Johannes Schindelin @ 2016-10-04 13:05 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1475586229.git.johannes.schindelin@gmx.de>
In cmd_pull(), when verifying that there are no changes preventing a
rebasing pull, we diligently pass the prefix parameter to the
die_on_unclean_work_tree() function which in turn diligently passes it
to the has_unstaged_changes() and has_uncommitted_changes() functions.
The casual reader might now be curious (as this developer was) whether
that means that calling `git pull --rebase` in a subdirectory will
ignore unstaged changes in other parts of the working directory. And be
puzzled that `git pull --rebase` (correctly) complains about those
changes outside of the current directory.
The puzzle is easily resolved: while we take pains to pass around the
prefix and even pass it to init_revisions(), the fact that no paths are
passed to init_revisions() ensures that the prefix is simply ignored.
That, combined with the fact that we will *always* want a *full* working
directory check before running a rebasing pull, is reason enough to
simply do away with the actual prefix parameter and to pass NULL
instead, as if we were running this from the top-level working directory
anyway.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/pull.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 398aae1..d4bd635 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -328,12 +328,12 @@ static int git_pull_config(const char *var, const char *value, void *cb)
/**
* Returns 1 if there are unstaged changes, 0 otherwise.
*/
-static int has_unstaged_changes(const char *prefix)
+static int has_unstaged_changes(void)
{
struct rev_info rev_info;
int result;
- init_revisions(&rev_info, prefix);
+ init_revisions(&rev_info, NULL);
DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(&rev_info.diffopt, QUICK);
diff_setup_done(&rev_info.diffopt);
@@ -344,7 +344,7 @@ static int has_unstaged_changes(const char *prefix)
/**
* Returns 1 if there are uncommitted changes, 0 otherwise.
*/
-static int has_uncommitted_changes(const char *prefix)
+static int has_uncommitted_changes(void)
{
struct rev_info rev_info;
int result;
@@ -352,7 +352,7 @@ static int has_uncommitted_changes(const char *prefix)
if (is_cache_unborn())
return 0;
- init_revisions(&rev_info, prefix);
+ init_revisions(&rev_info, NULL);
DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(&rev_info.diffopt, QUICK);
add_head_to_pending(&rev_info);
@@ -365,7 +365,7 @@ static int has_uncommitted_changes(const char *prefix)
* If the work tree has unstaged or uncommitted changes, dies with the
* appropriate message.
*/
-static void die_on_unclean_work_tree(const char *prefix)
+static void die_on_unclean_work_tree(void)
{
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
int do_die = 0;
@@ -375,12 +375,12 @@ static void die_on_unclean_work_tree(const char *prefix)
update_index_if_able(&the_index, lock_file);
rollback_lock_file(lock_file);
- if (has_unstaged_changes(prefix)) {
+ if (has_unstaged_changes()) {
error(_("Cannot pull with rebase: You have unstaged changes."));
do_die = 1;
}
- if (has_uncommitted_changes(prefix)) {
+ if (has_uncommitted_changes()) {
if (do_die)
error(_("Additionally, your index contains uncommitted changes."));
else
@@ -875,7 +875,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
die(_("Updating an unborn branch with changes added to the index."));
if (!autostash)
- die_on_unclean_work_tree(prefix);
+ die_on_unclean_work_tree();
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 0/6] Pull out require_clean_work_tree() functionality from builtin/pull.c
From: Johannes Schindelin @ 2016-10-04 13:04 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1473580914.git.johannes.schindelin@gmx.de>
This is the 5th last patch series of my work to accelerate interactive
rebases in particular on Windows.
Basically, all it does is to make reusable some functions that were
ported over from git-pull.sh but made private to builtin/pull.c.
Changes since v2:
- added a hint for translators.
- changed the existing error messages to start with a lower-case, as per
our current convention (the previous error messages were inherited
from code written before that convention was in place).
- struck the "truly" adjective from the commit message, as it did not
get Junio's consent.
Johannes Schindelin (6):
pull: drop confusing prefix parameter of die_on_unclean_work_tree()
pull: make code more similar to the shell script again
Make the require_clean_work_tree() function reusable
Export also the has_un{staged,committed}_changed() functions
wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
wt-status: begin error messages with lower-case
builtin/pull.c | 71 +++-------------------------------------------------
wt-status.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
wt-status.h | 6 +++++
3 files changed, 87 insertions(+), 68 deletions(-)
Published-As: https://github.com/dscho/git/releases/tag/require-clean-work-tree-v3
Fetch-It-Via: git fetch https://github.com/dscho/git require-clean-work-tree-v3
Interdiff vs v2:
diff --git a/builtin/pull.c b/builtin/pull.c
index c639167..0bf9802 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -810,7 +810,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
if (!autostash)
require_clean_work_tree(N_("pull with rebase"),
- "Please commit or stash them.", 1, 0);
+ "please commit or stash them.", 1, 0);
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
diff --git a/wt-status.c b/wt-status.c
index a86918a..010276b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2264,15 +2264,16 @@ int require_clean_work_tree(const char *action, const char *hint, int ignore_sub
rollback_lock_file(lock_file);
if (has_unstaged_changes(ignore_submodules)) {
- error(_("Cannot %s: You have unstaged changes."), _(action));
+ /* TRANSLATORS: the action is e.g. "pull with rebase" */
+ error(_("cannot %s: You have unstaged changes."), _(action));
err = 1;
}
if (has_uncommitted_changes(ignore_submodules)) {
if (err)
- error(_("Additionally, your index contains uncommitted changes."));
+ error(_("additionally, your index contains uncommitted changes."));
else
- error(_("Cannot %s: Your index contains uncommitted changes."),
+ error(_("cannot %s: Your index contains uncommitted changes."),
_(action));
err = 1;
}
--
2.10.0.windows.1.325.ge6089c1
base-commit: 0cf36115dce7438a0eafad54a81cc57175e8fb54
^ permalink raw reply
* [PATCH v9 05/14] pkt-line: rename packet_write() to packet_write_fmt()
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
packet_write() should be called packet_write_fmt() because it is a
printf-like function that takes a format string as first parameter.
packet_write_fmt() should be used for text strings only. Arbitrary
binary data should use a new packet_write() function that is introduced
in a subsequent patch.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/archive.c | 4 ++--
builtin/receive-pack.c | 4 ++--
builtin/remote-ext.c | 4 ++--
builtin/upload-archive.c | 4 ++--
connect.c | 2 +-
daemon.c | 2 +-
http-backend.c | 2 +-
pkt-line.c | 2 +-
pkt-line.h | 2 +-
shallow.c | 2 +-
upload-pack.c | 30 +++++++++++++++---------------
11 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/builtin/archive.c b/builtin/archive.c
index a1e3b94..49f4914 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -47,10 +47,10 @@ static int run_remote_archiver(int argc, const char **argv,
if (name_hint) {
const char *format = archive_format_from_filename(name_hint);
if (format)
- packet_write(fd[1], "argument --format=%s\n", format);
+ packet_write_fmt(fd[1], "argument --format=%s\n", format);
}
for (i = 1; i < argc; i++)
- packet_write(fd[1], "argument %s\n", argv[i]);
+ packet_write_fmt(fd[1], "argument %s\n", argv[i]);
packet_flush(fd[1]);
buf = packet_read_line(fd[0], NULL);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 011db00..1ce7682 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -218,7 +218,7 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
static void show_ref(const char *path, const unsigned char *sha1)
{
if (sent_capabilities) {
- packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
+ packet_write_fmt(1, "%s %s\n", sha1_to_hex(sha1), path);
} else {
struct strbuf cap = STRBUF_INIT;
@@ -233,7 +233,7 @@ static void show_ref(const char *path, const unsigned char *sha1)
if (advertise_push_options)
strbuf_addstr(&cap, " push-options");
strbuf_addf(&cap, " agent=%s", git_user_agent_sanitized());
- packet_write(1, "%s %s%c%s\n",
+ packet_write_fmt(1, "%s %s%c%s\n",
sha1_to_hex(sha1), path, 0, cap.buf);
strbuf_release(&cap);
sent_capabilities = 1;
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index 88eb8f9..11b48bf 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -128,9 +128,9 @@ static void send_git_request(int stdin_fd, const char *serv, const char *repo,
const char *vhost)
{
if (!vhost)
- packet_write(stdin_fd, "%s %s%c", serv, repo, 0);
+ packet_write_fmt(stdin_fd, "%s %s%c", serv, repo, 0);
else
- packet_write(stdin_fd, "%s %s%chost=%s%c", serv, repo, 0,
+ packet_write_fmt(stdin_fd, "%s %s%chost=%s%c", serv, repo, 0,
vhost, 0);
}
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 2caedf1..dc872f6 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -88,11 +88,11 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
writer.git_cmd = 1;
if (start_command(&writer)) {
int err = errno;
- packet_write(1, "NACK unable to spawn subprocess\n");
+ packet_write_fmt(1, "NACK unable to spawn subprocess\n");
die("upload-archive: %s", strerror(err));
}
- packet_write(1, "ACK\n");
+ packet_write_fmt(1, "ACK\n");
packet_flush(1);
while (1) {
diff --git a/connect.c b/connect.c
index 722dc3f..5330d9c 100644
--- a/connect.c
+++ b/connect.c
@@ -730,7 +730,7 @@ struct child_process *git_connect(int fd[2], const char *url,
* Note: Do not add any other headers here! Doing so
* will cause older git-daemon servers to crash.
*/
- packet_write(fd[1],
+ packet_write_fmt(fd[1],
"%s %s%chost=%s%c",
prog, path, 0,
target_host, 0);
diff --git a/daemon.c b/daemon.c
index 425aad0..afce1b9 100644
--- a/daemon.c
+++ b/daemon.c
@@ -281,7 +281,7 @@ static int daemon_error(const char *dir, const char *msg)
{
if (!informative_errors)
msg = "access denied or repository not exported";
- packet_write(1, "ERR %s: %s", msg, dir);
+ packet_write_fmt(1, "ERR %s: %s", msg, dir);
return -1;
}
diff --git a/http-backend.c b/http-backend.c
index adc8c8c..eef0a36 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -464,7 +464,7 @@ static void get_info_refs(struct strbuf *hdr, char *arg)
hdr_str(hdr, content_type, buf.buf);
end_headers(hdr);
- packet_write(1, "# service=git-%s\n", svc->name);
+ packet_write_fmt(1, "# service=git-%s\n", svc->name);
packet_flush(1);
argv[0] = svc->name;
diff --git a/pkt-line.c b/pkt-line.c
index 62fdb37..0a9b61c 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -118,7 +118,7 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)
packet_trace(out->buf + orig_len + 4, n - 4, 1);
}
-void packet_write(int fd, const char *fmt, ...)
+void packet_write_fmt(int fd, const char *fmt, ...)
{
static struct strbuf buf = STRBUF_INIT;
va_list args;
diff --git a/pkt-line.h b/pkt-line.h
index 3cb9d91..1902fb3 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -20,7 +20,7 @@
* side can't, we stay with pure read/write interfaces.
*/
void packet_flush(int fd);
-void packet_write(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
diff --git a/shallow.c b/shallow.c
index 54e2db7..d666e24 100644
--- a/shallow.c
+++ b/shallow.c
@@ -260,7 +260,7 @@ static int advertise_shallow_grafts_cb(const struct commit_graft *graft, void *c
{
int fd = *(int *)cb;
if (graft->nr_parent == -1)
- packet_write(fd, "shallow %s\n", oid_to_hex(&graft->oid));
+ packet_write_fmt(fd, "shallow %s\n", oid_to_hex(&graft->oid));
return 0;
}
diff --git a/upload-pack.c b/upload-pack.c
index ca7f941..cd47de6 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -393,13 +393,13 @@ static int get_common_commits(void)
if (multi_ack == 2 && got_common
&& !got_other && ok_to_give_up()) {
sent_ready = 1;
- packet_write(1, "ACK %s ready\n", last_hex);
+ packet_write_fmt(1, "ACK %s ready\n", last_hex);
}
if (have_obj.nr == 0 || multi_ack)
- packet_write(1, "NAK\n");
+ packet_write_fmt(1, "NAK\n");
if (no_done && sent_ready) {
- packet_write(1, "ACK %s\n", last_hex);
+ packet_write_fmt(1, "ACK %s\n", last_hex);
return 0;
}
if (stateless_rpc)
@@ -416,20 +416,20 @@ static int get_common_commits(void)
const char *hex = sha1_to_hex(sha1);
if (multi_ack == 2) {
sent_ready = 1;
- packet_write(1, "ACK %s ready\n", hex);
+ packet_write_fmt(1, "ACK %s ready\n", hex);
} else
- packet_write(1, "ACK %s continue\n", hex);
+ packet_write_fmt(1, "ACK %s continue\n", hex);
}
break;
default:
got_common = 1;
memcpy(last_hex, sha1_to_hex(sha1), 41);
if (multi_ack == 2)
- packet_write(1, "ACK %s common\n", last_hex);
+ packet_write_fmt(1, "ACK %s common\n", last_hex);
else if (multi_ack)
- packet_write(1, "ACK %s continue\n", last_hex);
+ packet_write_fmt(1, "ACK %s continue\n", last_hex);
else if (have_obj.nr == 1)
- packet_write(1, "ACK %s\n", last_hex);
+ packet_write_fmt(1, "ACK %s\n", last_hex);
break;
}
continue;
@@ -437,10 +437,10 @@ static int get_common_commits(void)
if (!strcmp(line, "done")) {
if (have_obj.nr > 0) {
if (multi_ack)
- packet_write(1, "ACK %s\n", last_hex);
+ packet_write_fmt(1, "ACK %s\n", last_hex);
return 0;
}
- packet_write(1, "NAK\n");
+ packet_write_fmt(1, "NAK\n");
return -1;
}
die("git upload-pack: expected SHA1 list, got '%s'", line);
@@ -650,7 +650,7 @@ static void receive_needs(void)
while (result) {
struct object *object = &result->item->object;
if (!(object->flags & (CLIENT_SHALLOW|NOT_SHALLOW))) {
- packet_write(1, "shallow %s",
+ packet_write_fmt(1, "shallow %s",
oid_to_hex(&object->oid));
register_shallow(object->oid.hash);
shallow_nr++;
@@ -662,7 +662,7 @@ static void receive_needs(void)
struct object *object = shallows.objects[i].item;
if (object->flags & NOT_SHALLOW) {
struct commit_list *parents;
- packet_write(1, "unshallow %s",
+ packet_write_fmt(1, "unshallow %s",
oid_to_hex(&object->oid));
object->flags &= ~CLIENT_SHALLOW;
/* make sure the real parents are parsed */
@@ -741,7 +741,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
struct strbuf symref_info = STRBUF_INIT;
format_symref_info(&symref_info, cb_data);
- packet_write(1, "%s %s%c%s%s%s%s%s agent=%s\n",
+ packet_write_fmt(1, "%s %s%c%s%s%s%s%s agent=%s\n",
oid_to_hex(oid), refname_nons,
0, capabilities,
(allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?
@@ -753,11 +753,11 @@ static int send_ref(const char *refname, const struct object_id *oid,
git_user_agent_sanitized());
strbuf_release(&symref_info);
} else {
- packet_write(1, "%s %s\n", oid_to_hex(oid), refname_nons);
+ packet_write_fmt(1, "%s %s\n", oid_to_hex(oid), refname_nons);
}
capabilities = NULL;
if (!peel_ref(refname, peeled.hash))
- packet_write(1, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons);
+ packet_write_fmt(1, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons);
return 0;
}
--
2.10.0
^ permalink raw reply related
* [PATCH v9 03/14] run-command: move check_pipe() from write_or_die to run_command
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
Move check_pipe() to run_command and make it public. This is necessary
to call the function from pkt-line in a subsequent patch.
While at it, make async_exit() static to run_command.c as it is no
longer used from outside.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
run-command.c | 17 +++++++++++++++--
run-command.h | 2 +-
write_or_die.c | 13 -------------
3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/run-command.c b/run-command.c
index 5a4dbb6..3269362 100644
--- a/run-command.c
+++ b/run-command.c
@@ -634,7 +634,7 @@ int in_async(void)
return !pthread_equal(main_thread, pthread_self());
}
-void NORETURN async_exit(int code)
+static void NORETURN async_exit(int code)
{
pthread_exit((void *)(intptr_t)code);
}
@@ -684,13 +684,26 @@ int in_async(void)
return process_is_async;
}
-void NORETURN async_exit(int code)
+static void NORETURN async_exit(int code)
{
exit(code);
}
#endif
+void check_pipe(int err)
+{
+ if (err == EPIPE) {
+ if (in_async())
+ async_exit(141);
+
+ signal(SIGPIPE, SIG_DFL);
+ raise(SIGPIPE);
+ /* Should never happen, but just in case... */
+ exit(141);
+ }
+}
+
int start_async(struct async *async)
{
int need_in, need_out;
diff --git a/run-command.h b/run-command.h
index 5066649..cf29a31 100644
--- a/run-command.h
+++ b/run-command.h
@@ -139,7 +139,7 @@ struct async {
int start_async(struct async *async);
int finish_async(struct async *async);
int in_async(void);
-void NORETURN async_exit(int code);
+void check_pipe(int err);
/**
* This callback should initialize the child process and preload the
diff --git a/write_or_die.c b/write_or_die.c
index 0734432..eab8c8d 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -1,19 +1,6 @@
#include "cache.h"
#include "run-command.h"
-static void check_pipe(int err)
-{
- if (err == EPIPE) {
- if (in_async())
- async_exit(141);
-
- signal(SIGPIPE, SIG_DFL);
- raise(SIGPIPE);
- /* Should never happen, but just in case... */
- exit(141);
- }
-}
-
/*
* Some cases use stdio, but want to flush after the write
* to get error handling (and to get better interactive
--
2.10.0
^ permalink raw reply related
* [PATCH v9 04/14] run-command: add wait_on_exit
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
The flag 'clean_on_exit' kills child processes spawned by Git on exit.
A hard kill like this might not be desired in all cases.
Add 'wait_on_exit' which closes the child's stdin on Git exit and waits
until the child process has terminated.
The flag is used in a subsequent patch.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
run-command.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--------
run-command.h | 3 +++
2 files changed, 50 insertions(+), 8 deletions(-)
diff --git a/run-command.c b/run-command.c
index 3269362..96c54fe 100644
--- a/run-command.c
+++ b/run-command.c
@@ -21,6 +21,9 @@ void child_process_clear(struct child_process *child)
struct child_to_clean {
pid_t pid;
+ char *name;
+ int stdin;
+ int wait;
struct child_to_clean *next;
};
static struct child_to_clean *children_to_clean;
@@ -28,12 +31,33 @@ static int installed_child_cleanup_handler;
static void cleanup_children(int sig, int in_signal)
{
+ int status;
+ struct child_to_clean *p = children_to_clean;
+
+ /* Close the the child's stdin as indicator that Git will exit soon */
+ while (p) {
+ if (p->wait)
+ if (p->stdin > 0)
+ close(p->stdin);
+ p = p->next;
+ }
+
while (children_to_clean) {
- struct child_to_clean *p = children_to_clean;
+ p = children_to_clean;
children_to_clean = p->next;
+
+ if (p->wait) {
+ fprintf(stderr, _("Waiting for '%s' to finish..."), p->name);
+ while ((waitpid(p->pid, &status, 0)) < 0 && errno == EINTR)
+ ; /* nothing */
+ fprintf(stderr, _("done!\n"));
+ }
+
kill(p->pid, sig);
- if (!in_signal)
+ if (!in_signal) {
+ free(p->name);
free(p);
+ }
}
}
@@ -49,10 +73,16 @@ static void cleanup_children_on_exit(void)
cleanup_children(SIGTERM, 0);
}
-static void mark_child_for_cleanup(pid_t pid)
+static void mark_child_for_cleanup(pid_t pid, const char *name, int stdin, int wait)
{
struct child_to_clean *p = xmalloc(sizeof(*p));
p->pid = pid;
+ p->wait = wait;
+ p->stdin = stdin;
+ if (name)
+ p->name = xstrdup(name);
+ else
+ p->name = "process";
p->next = children_to_clean;
children_to_clean = p;
@@ -63,6 +93,13 @@ static void mark_child_for_cleanup(pid_t pid)
}
}
+#ifdef NO_PTHREADS
+static void mark_child_for_cleanup_no_wait(pid_t pid, const char *name, int timeout, int stdin)
+{
+ mark_child_for_cleanup(pid, NULL, 0, 0);
+}
+#endif
+
static void clear_child_for_cleanup(pid_t pid)
{
struct child_to_clean **pp;
@@ -421,8 +458,9 @@ int start_command(struct child_process *cmd)
}
if (cmd->pid < 0)
error_errno("cannot fork() for %s", cmd->argv[0]);
- else if (cmd->clean_on_exit)
- mark_child_for_cleanup(cmd->pid);
+ else if (cmd->clean_on_exit || cmd->wait_on_exit)
+ mark_child_for_cleanup(
+ cmd->pid, cmd->argv[0], cmd->in, cmd->wait_on_exit);
/*
* Wait for child's execvp. If the execvp succeeds (or if fork()
@@ -482,8 +520,9 @@ int start_command(struct child_process *cmd)
failed_errno = errno;
if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT))
error_errno("cannot spawn %s", cmd->argv[0]);
- if (cmd->clean_on_exit && cmd->pid >= 0)
- mark_child_for_cleanup(cmd->pid);
+ if ((cmd->clean_on_exit || cmd->wait_on_exit) && cmd->pid >= 0)
+ mark_child_for_cleanup(
+ cmd->pid, cmd->argv[0], cmd->in, cmd->clean_on_exit_timeout);
argv_array_clear(&nargv);
cmd->argv = sargv;
@@ -765,7 +804,7 @@ int start_async(struct async *async)
exit(!!async->proc(proc_in, proc_out, async->data));
}
- mark_child_for_cleanup(async->pid);
+ mark_child_for_cleanup_no_wait(async->pid);
if (need_in)
close(fdin[0]);
diff --git a/run-command.h b/run-command.h
index cf29a31..f7b9907 100644
--- a/run-command.h
+++ b/run-command.h
@@ -42,7 +42,10 @@ struct child_process {
unsigned silent_exec_failure:1;
unsigned stdout_to_stderr:1;
unsigned use_shell:1;
+ /* kill the child on Git exit */
unsigned clean_on_exit:1;
+ /* close the child's stdin on Git exit and wait until it terminates */
+ unsigned wait_on_exit:1;
};
#define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }
--
2.10.0
^ permalink raw reply related
* [PATCH v9 14/14] contrib/long-running-filter: add long running filter example
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
Add a simple pass-thru filter as example implementation for the Git
filter protocol version 2. See Documentation/gitattributes.txt, section
"Filter Protocol" for more info.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
Documentation/gitattributes.txt | 4 +-
contrib/long-running-filter/example.pl | 127 +++++++++++++++++++++++++++++++++
2 files changed, 130 insertions(+), 1 deletion(-)
create mode 100755 contrib/long-running-filter/example.pl
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 5868f00..a182ef2 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -514,7 +514,9 @@ the next "key=value" list containing a command. Git will close
the command pipe on exit. The filter is expected to detect EOF
and exit gracefully on its own.
-If you develop your own long running filter
+A long running filter demo implementation can be found in
+`contrib/long-running-filter/example.pl` located in the Git
+core repository. If you develop your own long running filter
process then the `GIT_TRACE_PACKET` environment variables can be
very helpful for debugging (see linkgit:git[1]).
diff --git a/contrib/long-running-filter/example.pl b/contrib/long-running-filter/example.pl
new file mode 100755
index 0000000..f4102d2
--- /dev/null
+++ b/contrib/long-running-filter/example.pl
@@ -0,0 +1,127 @@
+#!/usr/bin/perl
+#
+# Example implementation for the Git filter protocol version 2
+# See Documentation/gitattributes.txt, section "Filter Protocol"
+#
+# Please note, this pass-thru filter is a minimal skeleton. No proper
+# error handling was implemented.
+#
+
+use strict;
+use warnings;
+
+my $MAX_PACKET_CONTENT_SIZE = 65516;
+
+sub packet_bin_read {
+ my $buffer;
+ my $bytes_read = read STDIN, $buffer, 4;
+ if ( $bytes_read == 0 ) {
+
+ # EOF - Git stopped talking to us!
+ exit();
+ }
+ elsif ( $bytes_read != 4 ) {
+ die "invalid packet: '$buffer'";
+ }
+ my $pkt_size = hex($buffer);
+ if ( $pkt_size == 0 ) {
+ return ( 1, "" );
+ }
+ elsif ( $pkt_size > 4 ) {
+ my $content_size = $pkt_size - 4;
+ $bytes_read = read STDIN, $buffer, $content_size;
+ if ( $bytes_read != $content_size ) {
+ die "invalid packet ($content_size bytes expected; $bytes_read bytes read)";
+ }
+ return ( 0, $buffer );
+ }
+ else {
+ die "invalid packet size: $pkt_size";
+ }
+}
+
+sub packet_txt_read {
+ my ( $res, $buf ) = packet_bin_read();
+ unless ( $buf =~ s/\n$// ) {
+ die "A non-binary line MUST be terminated by an LF.";
+ }
+ return ( $res, $buf );
+}
+
+sub packet_bin_write {
+ my $buf = shift;
+ print STDOUT sprintf( "%04x", length($buf) + 4 );
+ print STDOUT $buf;
+ STDOUT->flush();
+}
+
+sub packet_txt_write {
+ packet_bin_write( $_[0] . "\n" );
+}
+
+sub packet_flush {
+ print STDOUT sprintf( "%04x", 0 );
+ STDOUT->flush();
+}
+
+( packet_txt_read() eq ( 0, "git-filter-client" ) ) || die "bad initialize";
+( packet_txt_read() eq ( 0, "version=2" ) ) || die "bad version";
+( packet_bin_read() eq ( 1, "" ) ) || die "bad version end";
+
+packet_txt_write("git-filter-server");
+packet_txt_write("version=2");
+
+( packet_txt_read() eq ( 0, "clean=true" ) ) || die "bad capability";
+( packet_txt_read() eq ( 0, "smudge=true" ) ) || die "bad capability";
+( packet_bin_read() eq ( 1, "" ) ) || die "bad capability end";
+
+packet_txt_write("clean=true");
+packet_txt_write("smudge=true");
+packet_flush();
+
+while (1) {
+ my ($command) = packet_txt_read() =~ /^command=([^=]+)$/;
+ my ($pathname) = packet_txt_read() =~ /^pathname=([^=]+)$/;
+
+ packet_bin_read();
+
+ my $input = "";
+ {
+ binmode(STDIN);
+ my $buffer;
+ my $done = 0;
+ while ( !$done ) {
+ ( $done, $buffer ) = packet_bin_read();
+ $input .= $buffer;
+ }
+ }
+
+ my $output;
+ if ( $command eq "clean" ) {
+ ### Perform clean here ###
+ $output = $input;
+ }
+ elsif ( $command eq "smudge" ) {
+ ### Perform smudge here ###
+ $output = $input;
+ }
+ else {
+ die "bad command '$command'";
+ }
+
+ packet_txt_write("status=success");
+ packet_flush();
+ while ( length($output) > 0 ) {
+ my $packet = substr( $output, 0, $MAX_PACKET_CONTENT_SIZE );
+ packet_bin_write($packet);
+ if ( length($output) > $MAX_PACKET_CONTENT_SIZE ) {
+ $output = substr( $output, $MAX_PACKET_CONTENT_SIZE );
+ }
+ else {
+ $output = "";
+ }
+ }
+ packet_flush(); # flush content!
+ packet_flush(); # empty list, keep "status=success" unchanged!
+
+}
--
2.10.0
^ permalink raw reply related
* [PATCH v9 08/14] pkt-line: add packet_flush_gently()
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
packet_flush() would die in case of a write error even though for some
callers an error would be acceptable. Add packet_flush_gently() which
writes a pkt-line flush packet like packet_flush() but does not die in
case of an error. The function is used in a subsequent patch.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pkt-line.c | 8 ++++++++
pkt-line.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/pkt-line.c b/pkt-line.c
index 56915f0..286eb09 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -91,6 +91,14 @@ void packet_flush(int fd)
write_or_die(fd, "0000", 4);
}
+int packet_flush_gently(int fd)
+{
+ packet_trace("0000", 4, 1);
+ if (write_in_full(fd, "0000", 4) == 4)
+ return 0;
+ return error("flush packet write failed");
+}
+
void packet_buf_flush(struct strbuf *buf)
{
packet_trace("0000", 4, 1);
diff --git a/pkt-line.h b/pkt-line.h
index 3caea77..3fa0899 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -23,6 +23,7 @@ void packet_flush(int fd);
void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+int packet_flush_gently(int fd);
int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
/*
--
2.10.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox