* Re: [PATCHv2 1/8] test-lib: allow negation of prerequisites
From: Jeff King @ 2012-11-15 16:42 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115074658.GA8429@elie.Belkin>
On Wed, Nov 14, 2012 at 11:46:58PM -0800, Jonathan Nieder wrote:
> > +test_expect_success !LAZY_TRUE 'missing lazy prereqs skip tests' '
>
> I have a visceral nervousness when reading this code, from too much
> unpleasant experience of bash's csh-style !history expansion. Luckily
> bash does not treat ! specially in the '-o sh' mode used by tests.
I can understand, having been bitten by it many times myself (not only
is it counter-intuitively expanded inside double quotes, and not only
does it not do what you wanted, but it does not insert the command into
the history, so you cannot even go back to fix it easily).
> Does this feature work when running a test explicitly using
> "bash <name of test>"? That's something I do from time to time to
> figure out whether a weird behavior is shell-specific.
Yes. You can test it yourself with "bash t0000-basic.sh". The reason is
that the "!" is part of history expansion, which is only enabled by
default for interactive shells.
> If it works everywhere, this patch would help me conquer my fear of
> exclamation points in git's tests, which would be a comfort to me and
> a very good thing.
As far as I know, "!" should be safe in a non-interactive shell script.
-Peff
^ permalink raw reply
* Re: [PATCHv2 1/8] test-lib: allow negation of prerequisites
From: Jonathan Nieder @ 2012-11-15 16:49 UTC (permalink / raw)
To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115164228.GA18108@sigill.intra.peff.net>
Jeff King wrote:
> Yes. You can test it yourself with "bash t0000-basic.sh". The reason is
> that the "!" is part of history expansion, which is only enabled by
> default for interactive shells.
Nice to hear. Thanks much for looking into it.
> On Wed, Nov 14, 2012 at 11:46:58PM -0800, Jonathan Nieder wrote:
>> If it works everywhere, this patch would help me conquer my fear of
>> exclamation points in git's tests, which would be a comfort to me and
>> a very good thing.
^ permalink raw reply
* Re: [PATCH v3 0/5] push: update remote tags only with force
From: Junio C Hamano @ 2012-11-15 16:50 UTC (permalink / raw)
To: Angelo Borsotti
Cc: Chris Rorvick, git, Drew Northup, Michael Haggerty, Philip Oakley,
Johannes Sixt, Kacper Kornet, Jeff King, Felipe Contreras
In-Reply-To: <CAB9Jk9DK9AWBe_cf6=-v0pyD9xdhSoYPsKmRnUOwjuAm=hewfA@mail.gmail.com>
Angelo Borsotti <angelo.borsotti@gmail.com> writes:
>> I am *not* convinced that the "refs/tags/ is the only special
>> hierarchy whose contents should not move" is a bad limitation we
>> should avoid, but if it indeed is a bad limitation, the above is one
>> possible way to think about avoiding it.
>
> What other hierarchy besides branches and tags is there? Do you have
> in mind some other that should not move?
People use their own hierarchies for various purposes that are not
pre-defined by git-core, e.g. refs/changes/, refs/pull/, etc.
Depending on the semantics the projects want out of these
hierarchies, some of them may well be considered "create-only".
^ permalink raw reply
* Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
From: Junio C Hamano @ 2012-11-15 16:56 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, git, Thomas Rast, Jonathan Nieder
In-Reply-To: <20121115111334.GA1879@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I think a much more compelling argument/commit message for your
> suggested patch would be:
>
> We currently prompt the user for the "From" address. This is an
> inconvenience in the common case that the user has configured their
> identity in the environment, but is meant as a safety check for when
> git falls back to an implicitly generated identity (which may or may
> not be valid).
>
> That safety check is not really necessary, though, as by default
> send-email will prompt the user for a final confirmation before
> sending out any message. The likelihood that a user has both bothered
> to turn off this default _and_ not configured any identity (nor
> checked that the automatic identity is valid) is rather low.
This somehow reminds me of the first paragraph of f20f387 (commit:
check committer identity more strictly, 2012-07-23).
I never use "send-email driving format-patch" workflow myself, but I
suspect there are people among who do so who are using --compose to
do the cover letter of their series. Does the "confirmation as the
last step" help them, or would they have to retype their message?
^ permalink raw reply
* Re: creation of empty branches
From: Jeff King @ 2012-11-15 17:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrew Ardill, Angelo Borsotti, git
In-Reply-To: <7vmwyjan96.fsf@alter.siamese.dyndns.org>
On Wed, Nov 14, 2012 at 01:27:33PM -0800, Junio C Hamano wrote:
> > Instead of
> > fatal: Not a valid object name: 'master'.
> > perhaps
> > fatal: Cannot create branch 'foo' from empty branch 'master'. To
> > rename 'master' use 'git branch -m master foo'.
>
> The first new sentence is a definite improvement, but I do not think
> the advice in the second sentence is necessarily a good idea,
> because it is dubious that the user is likely to have wanted to
> rename 'master' to something else. "git branch foo master" (or its
> moral equivalent "git checkout -b foo" while on master) is a wish to
> have a history that ends in 'foo' *forked* from history of 'master',
> but because you do not even have anything on 'master' yet, you
> cannot fork the history, as you explained earlier (snipped). In
> that sense, 'empty branch' is a slight misnomer---as far as "git
> branch foo master" is concerned, the 'master' branch does not yet
> exist (and that is why we often call it an "unborn branch", not
> "empty").
>
> fatal: cannot fork master's history that does not exist yet.
>
> would be more accurate description of the situation.
I agree with most of your reasoning. I think simply using Andrew's first
sentence is a little more clear to a new user, even though it may be
less technically precise, but I don't have a strong opinion.
That still leaves "checkout -b". I do not see it as a problem that "git
branch foo" does not work whereas "git checkout -b foo" does; we
special-case the latter because it can do something sensible, whereas
there is nothing sensible for "git branch foo" to do. However, I think
it is missing one piece:
-- >8 --
Subject: checkout: print a message when switching unborn branches
When we switch to a new branch using checkout, we usually output a
message indicating what happened. However, when we switch from an unborn
branch to a new branch, we do not print anything, which may leave the
user wondering what happened.
The reason is that the unborn branch is a special case (see abe1998),
and does not follow the usual switch_branches code path. Let's add a
similar informational message to the special case to match the usual
code path.
Signed-off-by: Jeff King <peff@peff.net>
---
Two possible tweaks:
1. The message is the same as "git checkout -b" when we are actually
moving to a new branch. We could optionally mention that the branch
is empty or unborn.
2. We do not check whether the old unborn branch has the same name as
the new one. So you get "Switched to a new branch..." if you try to
checkout the same branch. We'd have to pass more information into
the special case to detect this. I don't know if we care.
builtin/checkout.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 781295b..a9c1b5a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -951,6 +951,9 @@ static int switch_unborn_to_new_branch(const struct checkout_opts *opts)
strbuf_addf(&branch_ref, "refs/heads/%s", opts->new_branch);
status = create_symref("HEAD", branch_ref.buf, "checkout -b");
strbuf_release(&branch_ref);
+ if (!opts->quiet)
+ fprintf(stderr, _("Switched to a new branch '%s'\n"),
+ opts->new_branch);
return status;
}
^ permalink raw reply related
* Re: [PATCH] wildmatch: correct isprint and isspace
From: "Jan H. Schönherr" @ 2012-11-15 17:13 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Junio C Hamano, rene.scharfe, Johannes Sixt, torvalds
In-Reply-To: <1352981983-22005-1-git-send-email-pclouds@gmail.com>
Am 15.11.2012 13:19, schrieb Nguyễn Thái Ngọc Duy:
> On Thu, Nov 15, 2012 at 2:30 AM, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> > Nevertheless, it's unfortunate that we have an isspace() that *almost* does
> > what the widely known thing of the same name does. I'd shy away from
> > changing git's version directly, because it's used more than a hundred times
> > in the code, and estimating the impact of adding \v and \f to it.
> > Perhaps renaming it to isgitspace() is a good first step, followed by
> > adding a "standard" version of isspace() for wildmatch?
>
> There are just too many call sites of isspace() and there is a risk
> of new call sites coming in independently. So I think keeping isspace()
> as-is and using a different name for the standard version is probably
> a better choice.
After having a closer look, where wildmatch is actually used -- matching
filenames -- and I've not yet seen \v or \f in a filename, it's possibly
unnecessary to do anything about isspace() right now.
(It's probably more an issue that filenames can be localized, and we only
support unlocalized character classes.)
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 02f48f6..d4c3fda 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -486,6 +486,7 @@ extern const unsigned char sane_ctype[256];
> #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)
> #define isascii(x) (((x) & ~0x7f) == 0)
> #define isspace(x) sane_istest(x,GIT_SPACE)
> +#define isspace_posix(x) (((x) >= 9 && (x) <= 13) || (x) == 32)
> #define isdigit(x) sane_istest(x,GIT_DIGIT)
> #define isalpha(x) sane_istest(x,GIT_ALPHA)
> #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)
> @@ -499,7 +500,8 @@ extern const unsigned char sane_ctype[256];
> #define isxdigit(x) (hexval_table[x] != -1)
This was from a previous patch, but maybe: "hexval_table[(unsigned char)x]"
> #define isprint(x) (sane_istest(x, GIT_ALPHA | GIT_DIGIT | GIT_SPACE | \
> GIT_PUNCT | GIT_REGEX_SPECIAL | GIT_GLOB_SPECIAL | \
> - GIT_PATHSPEC_MAGIC))
> + GIT_PATHSPEC_MAGIC) && \
> + (x) >= 32)
May I suggest the current is_print() implementation in master:
#define isprint(x) ((x) >= 0x20 && (x) <= 0x7e)
To summarize my opinion:
I no longer see a reason to correct isspace() (unless somebody with an actual
use case complains), and a more POSIXly isprint() is already in master.
=> Nothing to do. :)
Regards
Jan
^ permalink raw reply
* Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
From: Jeff King @ 2012-11-15 17:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Felipe Contreras, git, Thomas Rast, Jonathan Nieder
In-Reply-To: <7vvcd6954q.fsf@alter.siamese.dyndns.org>
On Thu, Nov 15, 2012 at 08:56:37AM -0800, Junio C Hamano wrote:
> > I think a much more compelling argument/commit message for your
> > suggested patch would be:
> >
> > We currently prompt the user for the "From" address. This is an
> > inconvenience in the common case that the user has configured their
> > identity in the environment, but is meant as a safety check for when
> > git falls back to an implicitly generated identity (which may or may
> > not be valid).
> >
> > That safety check is not really necessary, though, as by default
> > send-email will prompt the user for a final confirmation before
> > sending out any message. The likelihood that a user has both bothered
> > to turn off this default _and_ not configured any identity (nor
> > checked that the automatic identity is valid) is rather low.
>
> This somehow reminds me of the first paragraph of f20f387 (commit:
> check committer identity more strictly, 2012-07-23).
>
> I never use "send-email driving format-patch" workflow myself, but I
> suspect there are people among who do so who are using --compose to
> do the cover letter of their series. Does the "confirmation as the
> last step" help them, or would they have to retype their message?
That is a good question. That confirmation step does come after they
have typed their cover letter. However, if they are using --compose,
they are dumped in their editor with something like:
From Jeff King <peff@peff.net> # This line is ignored.
GIT: Lines beginning in "GIT:" will be removed.
GIT: Consider including an overall diffstat or table of contents
GIT: for the patch you are writing.
GIT:
GIT: Clear the body content if you don't wish to send a summary.
From: Jeff King <peff@peff.net>
Subject:
In-Reply-To:
which I think would count as sufficient notice of the address being
used.
-Peff
^ permalink raw reply
* Re: [PATCH] status: add advice on how to push/pull to tracking branch
From: Junio C Hamano @ 2012-11-15 17:53 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <1352976300-20159-1-git-send-email-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> I thought this was obvious enough not to deserve an advice, but a
> colleague of mine had troubles with "commited but not pushed" changes.
> Maybe an additional advice would have helped. After all, it's an
> advice, and can be deactivated ...
>
> remote.c | 13 ++++++++++---
> t/t2020-checkout-detach.sh | 1 +
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 04fd9ea..9c19689 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1627,13 +1627,15 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
>
> base = branch->merge[0]->dst;
> base = shorten_unambiguous_ref(base, 0);
> - if (!num_theirs)
> + if (!num_theirs) {
> strbuf_addf(sb,
> Q_("Your branch is ahead of '%s' by %d commit.\n",
> "Your branch is ahead of '%s' by %d commits.\n",
> num_ours),
> base, num_ours);
> - else if (!num_ours)
> + strbuf_addf(sb,
> + _(" (use \"git push\" to publish your local commits)\n"));
> + } else if (!num_ours) {
The message should make it clear that the two words in double quotes
only hint what command is used to "publish your local commits" and
not to be taken as literal "here is what you exactly would type",
but I do not think that is what I would get from this if I were a
total newbie who would need this advise.
It is even more true given that this is showing an arbitrary, and
more likely than not a non-current branch, especially with the
recent move from "matching" to "simple" where a naive use of "git
push" is to push the branch that is currently checked out and no
other branches.
see 'git push --help' to learn how to publish your local commits
might be more appropriate.
> + strbuf_addf(sb,
> + _(" (use \"git pull\" to update your local branch)\n"));
> + } else {
Likewise, and the non-currentness of the branch being described is
even worse in here, as unlike "git push" that can still be used to
push a non-current branch, "git pull" is never to be used to update
local branch that is not current, which means the advice must mention
"git checkout" somewhere.
^ permalink raw reply
* [PATCH v2 4/5] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
From: Brandon Casey @ 2012-11-15 17:55 UTC (permalink / raw)
To: git; +Cc: Brandon Casey, Brandon Casey
In-Reply-To: <1352943474-15573-4-git-send-email-drafnel@gmail.com>
Currently, if the s-o-b footer of a commit message contains a
"(cherry picked from ..." line that was added by a previous cherry-pick -x,
it is not recognized as a s-o-b footer and will cause a newline to be
inserted before an additional s-o-b is added.
So, rework ends_rfc2822_footer to recognize the "(cherry picked from ..."
string as part of the footer. Plus mark the test in t3511 as fixed.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
Declare cherry_picked_prefix variable as static. This is the only change
with respect to v1.
-Brandon
sequencer.c | 44 +++++++++++++++++++++++++++++---------------
t/t3511-cherry-pick-x.sh | 2 +-
2 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 01edec2..213fa4f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -18,6 +18,7 @@
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
const char sign_off_header[] = "Signed-off-by: ";
+static const char cherry_picked_prefix[] = "(cherry picked from commit ";
static void remove_sequencer_state(void)
{
@@ -492,7 +493,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
if (opts->record_origin) {
- strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+ strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
}
@@ -1017,13 +1018,34 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
+static int is_rfc2822_line(const char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ int ch = buf[i];
+ if (ch == ':')
+ break;
+ if (isalnum(ch) || (ch == '-'))
+ continue;
+ return 0;
+ }
+
+ return 1;
+}
+
+static int is_cherry_pick_from_line(const char *buf, int len)
+{
+ return (strlen(cherry_picked_prefix) + 41) <= len &&
+ !prefixcmp(buf, cherry_picked_prefix);
+}
+
static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
{
- int ch;
int hit = 0;
- int i, j, k;
+ int i, k;
int len = sb->len - ignore_footer;
- int first = 1;
+ int last_was_rfc2822 = 0;
const char *buf = sb->buf;
for (i = len - 1; i > 0; i--) {
@@ -1040,20 +1062,12 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
; /* do nothing */
k++;
- if ((buf[i] == ' ' || buf[i] == '\t') && !first)
+ if (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\t'))
continue;
- first = 0;
-
- for (j = 0; i + j < len; j++) {
- ch = buf[i + j];
- if (ch == ':')
- break;
- if (isalnum(ch) ||
- (ch == '-'))
- continue;
+ if (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||
+ is_cherry_pick_from_line(buf+i, k-i)))
return 0;
- }
}
return 1;
}
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index b2098e0..785486e 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -63,7 +63,7 @@ test_expect_success 'cherry-pick -s not confused by rfc2822 continuation line' '
test_cmp expect actual
'
-test_expect_failure 'cherry-pick treats -s "(cherry picked from..." line as part of footer' '
+test_expect_success 'cherry-pick treats -s "(cherry picked from..." line as part of footer' '
pristine_detach initial &&
git cherry-pick -s rfc2822-cherry-base &&
cat <<-EOF >expect &&
--
1.8.0
^ permalink raw reply related
* [PATCH] config: don't segfault when given --path with a missing value
From: Carlos Martín Nieto @ 2012-11-15 18:10 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121115161758.GC6157@sigill.intra.peff.net>
When given a variable without a value, such as '[section] var' and
asking git-config to treat it as a path, git_config_pathname returns
an error and doesn't modify its output parameter. show_config assumes
that the call is always successful and sets a variable to indicate
that vptr should be freed. In case of an error however, trying to do
this will cause the program to be killed, as it's pointing to memory
in the stack.
Detect the error and return immediately to avoid freeing or accessing
the uninitialed memory in the stack.
Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
On Thu, Nov 15, 2012 at 08:11:50AM -0800, Jeff King wrote:
> Hmm, actually, we should probably propagate the error (I was thinking
> for some reason this was in the listing code, but it is really about
> getting a specific variable, and that variable does not have a sane
> format. We'll already have printed the non-bool error, so we should
> probably die. So more like:
>
> if (git_config_pathname(&vptr, key_, value_) < 0)
> return -1;
> must_free_vptr = 1;
Yeah, that's more sensible. I didn't notice that the buffer never gets
written to in this codepath, and the trying to print it out is silly
when we know that there is nothing valid to print. Thanks for the
review. I've included your test as well, which really makes all of
this your code. Do we have some equivalent of a Basically-writen-by
line?
builtin/config.c | 3 ++-
t/t1300-repo-config.sh | 5 +++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index 442ccc2..4dc5ffa 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -129,7 +129,8 @@ static int show_config(const char *key_, const char *value_, void *cb)
else
sprintf(value, "%d", v);
} else if (types == TYPE_PATH) {
- git_config_pathname(&vptr, key_, value_);
+ if (git_config_pathname(&vptr, key_, value_) < 0)
+ return -1;
must_free_vptr = 1;
} else if (value_) {
vptr = value_;
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index a477453..17272e0 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -803,6 +803,11 @@ test_expect_success NOT_MINGW 'get --path copes with unset $HOME' '
test_cmp expect result
'
+test_expect_success 'get --path barfs on boolean variable' '
+ echo "[path]bool" >.git/config &&
+ test_must_fail git config --get --path path.bool
+'
+
cat > expect << EOF
[quote]
leading = " test"
--
1.8.0.316.g291341c
^ permalink raw reply related
* Re: [PATCH] config: don't segfault when given --path with a missing value
From: Jeff King @ 2012-11-15 18:15 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git
In-Reply-To: <1353003001-22600-1-git-send-email-cmn@elego.de>
On Thu, Nov 15, 2012 at 10:10:01AM -0800, Carlos Martín Nieto wrote:
> When given a variable without a value, such as '[section] var' and
> asking git-config to treat it as a path, git_config_pathname returns
> an error and doesn't modify its output parameter. show_config assumes
> that the call is always successful and sets a variable to indicate
> that vptr should be freed. In case of an error however, trying to do
> this will cause the program to be killed, as it's pointing to memory
> in the stack.
>
> Detect the error and return immediately to avoid freeing or accessing
> the uninitialed memory in the stack.
>
> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
Acked-by: Jeff King <peff@peff.net>
> Yeah, that's more sensible. I didn't notice that the buffer never gets
> written to in this codepath, and the trying to print it out is silly
> when we know that there is nothing valid to print.
> Thanks for the review. I've included your test as well, which really
> makes all of this your code.
Eh, I guess so. You did the hard part of finding it, though. ;)
> Do we have some equivalent of a Basically-writen-by line?
Nothing structured. But I am comfortable enough with the number of times
I am mentioned in "git log" already, so don't worry about it.
-Peff
^ permalink raw reply
* `git mv` has ambiguous error message for non-existing target
From: Patrick Lehner @ 2012-11-15 18:54 UTC (permalink / raw)
To: git
Hey guys,
as was brought up on #github today, the "git mv" command has a bit of a
little-helping output message when the target directory (or any
intermediate directories) dont exist.
To reproduce:
- cd into a git repo
- assuming "filea.txt" is an existing file in the CWD, and "dirb" is
neither a file nor a directory in the CWD, use the command "git mv
filea.txt dirb/filea.txt"
- this will produce an error message like `fatal: renaming 'filea.sh'
failed: No such file or directory`
It does not mention that the problem is, in fact, the target directory
not existing. This seems to be mostly a problem for users unfamiliar
with bash/*nix console commands. Although it is documented that git mv
will not create intermediate folders (which is fine, because neither
does mv), the error message might lead to believe a problem exists with
the source file.
Expanding the error message to "No such file or directory: 'dirb/' "
would probably clear this up.
Best regards and thanks to anyone who could improve this,
Patrick
^ permalink raw reply
* Re: [regression] Newer gits cannot clone any remote repos
From: Ramsay Jones @ 2012-11-15 18:55 UTC (permalink / raw)
To: Douglas Mencken; +Cc: git, Nguyen Thai Ngoc Duy, Junio C Hamano
In-Reply-To: <CACYvZ7jMC5xw4LxiuG5m+=grpQEg+wZb_7BaU4Xn-r7ix=S-bw@mail.gmail.com>
Douglas Mencken wrote:
>> Could you try re-building git with the
>> NO_THREAD_SAFE_PREAD build variable set?
>
> Yeah! It works!!!
>
> --- evil/Makefile
> +++ good/Makefile
> @@ -957,6 +957,7 @@
> HAVE_PATHS_H = YesPlease
> LIBC_CONTAINS_LIBINTL = YesPlease
> HAVE_DEV_TTY = YesPlease
> + NO_THREAD_SAFE_PREAD = YesPlease
> endif
> ifeq ($(uname_S),GNU/kFreeBSD)
> NO_STRLCPY = YesPlease
>
> With this, I do have correctly working git clone.
OK, good.
You didn't mention which platform you are on; from the above Makefile
hunk, however, I can deduce that you are on *some* version of Linux.
Hmm, it doesn't seem too likely that your pread() is thread-unsafe
(possible, just unlikely), so it could be a more fundamental problem
with the threaded index-pack code (ie commit b8a2486f1 et. seq.).
However, as a first step could you try running the test program
(given below) on your system to determine if your pread() is thread-safe
or not. (gcc -I. -o test-pread test-pread.c; ./test-pread)
Also, what is the output of "uname -a".
ATB,
Ramsay Jones
-- >8 --
#include "git-compat-util.h"
#include "thread-utils.h"
#define DATA_FILE "junk.data"
#define MAX_DATA 256 * 1024
#define NUM_THREADS 3
#define TRIALS 500000
struct thread_data {
pthread_t t;
int fd;
int cnt;
int fails;
unsigned long n;
};
static struct thread_data t[NUM_THREADS+1];
int create_data_file(void)
{
int i, fd = open(DATA_FILE, O_CREAT | O_TRUNC | O_WRONLY, 0600);
if (fd < 0)
return -1;
for (i = 0; i < MAX_DATA; i++)
if (write(fd, &i, sizeof(int)) < 0) {
close(fd);
unlink(DATA_FILE);
return -1;
}
close(fd);
return 0;
}
void *read_thread(void *data)
{
struct thread_data *d = (struct thread_data *)data;
int i, j, rd;
for (i = 0; i < TRIALS; i += MAX_DATA) {
for (j = 0; j < MAX_DATA; j++) {
ssize_t sz = read(d->fd, &rd, sizeof(int));
if (sz < 0 || rd != j)
d->fails++;
d->cnt++;
}
lseek(d->fd, 0, SEEK_SET);
}
return NULL;
}
void *pread_thread(void *data)
{
struct thread_data *d = (struct thread_data *)data;
int i, j, rd;
for (i = 0; i < TRIALS; i++) {
ssize_t sz;
d->n = d->n * 1103515245 + 12345;
j = d->n % MAX_DATA;
sz = pread(d->fd, &rd, sizeof(int), j * sizeof(int));
if (sz < 0 || rd != j)
d->fails++;
d->cnt++;
}
return NULL;
}
int main(int argc, char *argv[])
{
int fd, i;
if (create_data_file() < 0) {
printf("can't create data file\n");
return 1;
}
if ((fd = open(DATA_FILE, O_RDONLY)) < 0) {
printf("can't open data file\n");
unlink(DATA_FILE);
return 1;
}
for (i = 0; i < NUM_THREADS+1; i++) {
int ret;
t[i].fd = fd;
t[i].cnt = 0;
t[i].fails = 0;
t[i].n = i * 16381;
ret = pthread_create(&t[i].t, NULL,
(i == 0) ? read_thread : pread_thread,
&t[i]);
if (ret) {
printf("can't create thread %d (%s)\n", i, strerror(ret));
unlink(DATA_FILE);
return 1;
}
}
for (i = 0; i < NUM_THREADS+1; i++)
pthread_join(t[i].t, NULL);
close(fd);
for (i = 0; i < NUM_THREADS+1; i++)
printf("%2d: trials %d, failed %d\n", i, t[i].cnt, t[i].fails);
unlink(DATA_FILE);
return 0;
}
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Ramsay Jones @ 2012-11-15 19:05 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jeff King, mlevedahl, git
In-Reply-To: <50A2B14C.9040608@web.de>
Torsten Bögershausen wrote:
>> * ml/cygwin-mingw-headers (2012-11-12) 1 commit
>> - Update cygwin.c for new mingw-64 win32 api headers
>>
>> Make git work on newer cygwin.
>>
>> Will merge to 'next'.
>
> (Sorry for late answer, I managed to test the original patch minutes before Peff merged it to pu)
> (And thanks for maintaining git)
>
> Is everybody using cygwin happy with this?
I am still on cygwin 1.5.22 and quite happy that this patch does
not (seem) to cause any problems. ;-P
> I managed to compile on a fresh installed cygwin,
> but failed to compile under 1.7.7, see below.
> Is there a way we can achieve to compile git both under "old" and "new" cygwin 1.7 ?
> Or is this not worth the effort?
Did the cygwin project not bump an api version number somewhere?
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [regression] Newer gits cannot clone any remote repos
From: Ramsay Jones @ 2012-11-15 19:17 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Douglas Mencken, git
In-Reply-To: <50A2B2DF.1080704@web.de>
Torsten Bögershausen wrote:
> On 13.11.12 19:55, Ramsay Jones wrote:
>> Douglas Mencken wrote:
>>> *Any* git clone fails with:
>>>
>>> fatal: premature end of pack file, 106 bytes missing
>>> fatal: index-pack failed
>>>
>>> At first, I tried 1.8.0, and it failed. Then I tried to build 1.7.10.5
>>> then, and it worked. Then I tried 1.7.12.2, but it fails the same way
>>> as 1.8.0.
>>> So I decided to git bisect.
>>>
>>> b8a2486f1524947f232f657e9f2ebf44e3e7a243 is the first bad commit
>>> ``index-pack: support multithreaded delta resolving''
>>
>> This looks like the same problem I had on cygwin, which lead to
>> commit c0f86547c ("index-pack: Disable threading on cygwin", 26-06-2012).
>>
>> I didn't notice which platform you are on, but maybe you also have a
>> thread-unsafe pread()? Could you try re-building git with the
>> NO_THREAD_SAFE_PREAD build variable set?
>>
>> HTH.
>>
>> ATB,
>> Ramsay Jones
>
> This is interesting.
> I had the same problem on a PowerPC
> (Old PowerBook G4 running Linux).
>
> Using NO_THREAD_SAFE_PREAD helped, thanks for the hint.
> (After recompiling without NO_THREAD_SAFE_PREAD I could clone
> from this machine again, so the problem is not really reproducable)
Yes, the failures would be intermittent (and often not easily
reproducible). The threaded index-pack code did not fail for
me on cygwin at all during development, including tests, but failed
immediately I installed v1.7.11. On real repositories, it failed
intermittently. On some repos it always failed, on some it never
failed and on some others it would sometimes fail, sometimes not.
ATB,
Ramsay Jones
^ permalink raw reply
* Re: Git does not understand absolute Win'dos' path
From: Ramsay Jones @ 2012-11-15 19:20 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Martin Lichtin, git@vger.kernel.org
In-Reply-To: <50A36844.3090000@viscovery.net>
Johannes Sixt wrote:
> Am 11/14/2012 10:12, schrieb Martin Lichtin:
>> Maven's release plugin prepares a call Git like in this example:
>>
>> cmd.exe /X /C "git commit --verbose -F
>> C:\cygwin\tmp\maven-scm-915771020.commit pom.xml"
>>
>> Git doesn't seem to understand the -F argument and treats it like a
>> relative path (relative to the repository root):
>>
>> $ cmd.exe /X /C "git commit --verbose -F C:\cygwin\tmp\commit pom.xml"
>> fatal: could not read log file 'mytestdir/C:\cygwin\tmp\commit': No
>> such file or directory
>
> According to the code, this should not happen if you are using msysgit.
> For this reason, I guess you are using Cygwin git. Right?
>
> I don't know what Cygwin programs are supposed to do if they receive an
> argument that looks like a Windows style absolute path.
>
> OTOH, it could be argued that Maven should not treat a Cygwin program like
> a DOS program, and it should pass the path in the POSIXy form
> /c/cygwin/tmp/commit or /tmp/commit.
I would argue precisely this! :-D
ATB,
Ramsay Jones
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Torsten Bögershausen @ 2012-11-15 19:35 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Torsten Bögershausen, Jeff King, mlevedahl, git
In-Reply-To: <50A53CF0.7040809@ramsay1.demon.co.uk>
On 11/15/2012 08:05 PM, Ramsay Jones wrote:
> Torsten Bögershausen wrote:
>>> * ml/cygwin-mingw-headers (2012-11-12) 1 commit
>>> - Update cygwin.c for new mingw-64 win32 api headers
>>>
>>> Make git work on newer cygwin.
>>>
>>> Will merge to 'next'.
>>
>> (Sorry for late answer, I managed to test the original patch minutes before Peff merged it to pu)
>> (And thanks for maintaining git)
>>
>> Is everybody using cygwin happy with this?
>
> I am still on cygwin 1.5.22 and quite happy that this patch does
> not (seem) to cause any problems. ;-P
>
>> I managed to compile on a fresh installed cygwin,
>> but failed to compile under 1.7.7, see below.
>> Is there a way we can achieve to compile git both under "old" and "new" cygwin 1.7 ?
>> Or is this not worth the effort?
>
> Did the cygwin project not bump an api version number somewhere?
>
> ATB,
> Ramsay Jones
Ramsay,
you can run uname -r to see the version number.
I myself haven't fully understood all the consequences,
somewhere between 1.7.7 and 1.7.17 the include files had been changed.
If this has consequences for using e.g. winsock2.dll, I want to know
myself ;-)
/Torsten
^ permalink raw reply
* Re: [PATCH] status: add advice on how to push/pull to tracking branch
From: Matthieu Moy @ 2012-11-15 19:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr4nu92hm.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> - if (!num_theirs)
>> + if (!num_theirs) {
>> strbuf_addf(sb,
>> Q_("Your branch is ahead of '%s' by %d commit.\n",
>> "Your branch is ahead of '%s' by %d commits.\n",
>> num_ours),
>> base, num_ours);
>> - else if (!num_ours)
>> + strbuf_addf(sb,
>> + _(" (use \"git push\" to publish your local commits)\n"));
>> + } else if (!num_ours) {
>
> The message should make it clear that the two words in double quotes
> only hint what command is used to "publish your local commits" and
> not to be taken as literal "here is what you exactly would type",
> but I do not think that is what I would get from this if I were a
> total newbie who would need this advise.
>
> It is even more true given that this is showing an arbitrary, and
> more likely than not a non-current branch, especially with the
> recent move from "matching" to "simple" where a naive use of "git
> push" is to push the branch that is currently checked out and no
> other branches.
I don't understand what you mean by "non-current". If you mean a local
branch not pointed to by HEAD, then I don't understand the remark, as
the message is shown by "git status" (looking more closely, it is also
shown by "git checkout", but after switching branch so also showing a
message about the current branch) and precisely talks about the current
branch. If you mean that the upsteam branch has a name different from
the local one, then with "push.default=simple", argumentless "git push"
will fail and show a detailed explanation to the user, which I find
acceptable.
I can tweak the advice to show the full "git push" command with
push.default=matching/current, but first, I'd like to understand your
remark.
>> + strbuf_addf(sb,
>> + _(" (use \"git pull\" to update your local branch)\n"));
>> + } else {
>
> Likewise, and the non-currentness of the branch being described is
> even worse in here, as unlike "git push" that can still be used to
> push a non-current branch, "git pull" is never to be used to update
> local branch that is not current, which means the advice must mention
> "git checkout" somewhere.
I understand this remark even less. We're showing a message about the
current branch and its upstream branch. In which case would "git pull"
not do the right thing?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* gitpacker progress report and a question
From: Eric S. Raymond @ 2012-11-15 21:28 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]
Some days ago I reported that I was attempting to write a tool that could
(a) take a git repo and unpack it into a tarball sequence plus a metadata log,
(b) reverse that operation, packing a tarball and log sequence into a repo.
Thanks in part to advice by Andreas Schwab and in part to looking at the
text of the p4 import script, this effort has succeeded. A proof of
concept is enclosed. It isn't documented yet, and has not been tested
on a repository with branches or merges in the history, but I am confident
that the distance from here to a finished and tested tool is short.
The immediate intended use is for importing older projects that are
available only as sequences of release tarballs, but there are other
sorts of repository surgery that would become easier using it.
I'm still looking for a better name for it and would welcome suggestions.
Before I do much further work, I need to determine how this will be shipped.
I see two possibilities: either I ship it as a small standalone project,
or it becomes a git subcommand shipped with the git suite. How I document
it and set up its tests would differ between these two cases.
Is there a process for submitting new subcommands? What are the
test-suite and documentation requirements?
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
[-- Attachment #2: gitpacker --]
[-- Type: text/plain, Size: 12708 bytes --]
#!/usr/bin/env python
"""
gitpacker - assemble tree sequences into repository histories
Requires git and cpio.
"""
import sys, os, getopt, subprocess, time, tempfile
DEBUG_GENERAL = 1
DEBUG_PROGRESS = 2
DEBUG_COMMANDS = 3
class Fatal(Exception):
"Unrecoverable error."
def __init__(self, msg):
Exception.__init__(self)
self.msg = msg
class Baton:
"Ship progress indications to stdout."
def __init__(self, prompt, endmsg='done', enable=False):
self.prompt = prompt
self.endmsg = endmsg
self.countfmt = None
self.counter = 0
if enable:
self.stream = sys.stdout
else:
self.stream = None
self.count = 0
self.time = 0
def __enter__(self):
if self.stream:
self.stream.write(self.prompt + "...")
if os.isatty(self.stream.fileno()):
self.stream.write(" \010")
self.stream.flush()
self.count = 0
self.time = time.time()
return self
def startcounter(self, countfmt, initial=1):
self.countfmt = countfmt
self.counter = initial
def bumpcounter(self):
if self.stream is None:
return
if os.isatty(self.stream.fileno()):
if self.countfmt:
update = self.countfmt % self.counter
self.stream.write(update + ("\010" * len(update)))
self.stream.flush()
else:
self.twirl()
self.counter = self.counter + 1
def endcounter(self):
if self.stream:
w = len(self.countfmt % self.count)
self.stream.write((" " * w) + ("\010" * w))
self.stream.flush()
self.countfmt = None
def twirl(self, ch=None):
"One twirl of the baton."
if self.stream is None:
return
if os.isatty(self.stream.fileno()):
if ch:
self.stream.write(ch)
self.stream.flush()
return
else:
update = "-/|\\"[self.count % 4]
self.stream.write(update + ("\010" * len(update)))
self.stream.flush()
self.count = self.count + 1
def __exit__(self, extype, value_unused, traceback_unused):
if extype == KeyboardInterrupt:
self.endmsg = "interrupted"
if extype == Fatal:
self.endmsg = "aborted by error"
if self.stream:
self.stream.write("...(%2.2f sec) %s.\n" \
% (time.time() - self.time, self.endmsg))
return False
def do_or_die(dcmd, legend=""):
"Either execute a command or raise a fatal exception."
if legend:
legend = " " + legend
if verbose >= DEBUG_COMMANDS:
sys.stdout.write("executing '%s'%s\n" % (dcmd, legend))
try:
retcode = subprocess.call(dcmd, shell=True)
if retcode < 0:
raise Fatal("child was terminated by signal %d." % -retcode)
elif retcode != 0:
raise Fatal("child returned %d." % retcode)
except (OSError, IOError) as e:
raise Fatal("execution of %s%s failed: %s" % (dcmd, legend, e))
def capture_or_die(dcmd, legend=""):
"Either execute a command and capture its output or die."
if legend:
legend = " " + legend
if verbose >= DEBUG_COMMANDS:
sys.stdout.write("executing '%s'%s\n" % (dcmd, legend))
try:
return subprocess.check_output(dcmd, shell=True)
except subprocess.CalledProcessError as e:
if e.returncode < 0:
raise Fatal("child was terminated by signal %d." % -e.returncode)
elif e.returncode != 0:
sys.stderr.write("gitpacker: child returned %d." % e.returncode)
sys.exit(1)
def git_pack(indir, outdir, quiet=False):
"Pack a tree sequence and associated logfile into a repository"
do_or_die("mkdir %s; git init -q %s" % (outdir, outdir))
logfile = os.path.join(indir, "log")
commit_id = [None]
state = 0
parents = []
comment = committername = authorname = ""
commitdate = authordate = commitemail = authoremail = ""
commitcount = 1;
linecount = 0
with Baton("Packing", enable=not quiet) as baton:
for line in open(logfile):
if verbose > DEBUG_PROGRESS:
print "Looking at: '%s'" % repr(line)
if state == 0:
if line == '\n':
state = 1
else:
try:
space = line.index(' ')
leader = line[:space]
follower = line[space:].strip()
if leader == "commit":
commit = follower
elif leader == "parent":
parents.append(follower)
elif leader not in ("author", "committer"):
raise Fatal("unexpected log attribute at %s" \
% repr(line))
elif leader == "committer":
(committername, committeremail, committerdate) = [x.strip() for x in follower.replace('>','<').split('<')]
elif leader == "author":
(authorname, authoremail, authordate) = [x.strip() for x in follower.replace('>','<').split('<')]
except ValueError:
raise Fatal('"%s", line %d: ill-formed log entry' % (logfile, linecount))
elif state == 1:
if line == ".\n":
if verbose > DEBUG_PROGRESS:
print "Interpretation begins"
os.chdir(outdir)
if commitcount > 1:
do_or_die("rm `git ls-tree --name-only HEAD`")
if verbose > DEBUG_PROGRESS:
print "Copying"
os.chdir("%s/%d" % (indir, commitcount))
do_or_die("find . -print | cpio -pd --quiet %s" % (outdir,))
os.chdir(outdir)
do_or_die("git add -A")
tree_id = capture_or_die("git write-tree").strip()
if verbose > DEBUG_PROGRESS:
print "Tree ID is", tree_id
(_, commentfile) = tempfile.mkstemp()
with open(commentfile, "w") as cfp:
cfp.write(comment)
command = "git commit-tree %s " % tree_id
command += " ".join(map(lambda p: "-p " + commit_id[int(p)],parents))
command += "<'%s'" % commentfile
environment = ""
environment += " GIT_AUTHOR_NAME='%s' " % authorname
environment += " GIT_AUTHOR_EMAIL='%s' " % authoremail
environment += " GIT_AUTHOR_DATE='%s' " % authordate
environment += " GIT_COMMITTER_NAME='%s' " % committername
environment += " GIT_COMMITTER_EMAIL='%s' " % committeremail
environment += " GIT_COMMITTER_DATE='%s' " % committerdate
commit_id.append(capture_or_die(environment + command).strip())
do_or_die("git update-ref HEAD %s" % commit_id[-1])
os.remove(commentfile)
state = 0
parents = []
comment = committername = authorname = ""
committerdate = authordate = committeremail = authoremail = ""
commitcount += 1
baton.twirl()
if maxcommit != 0 and commitcount >= maxcommit:
break
else:
if line.startswith("."):
line = line[1:]
comment += line
def git_unpack(indir, outdir, quiet=False):
"Unpack a repository into a tree sequence and associated logfile."
rawlogfile = os.path.join(outdir, "rawlog")
with Baton("Unpacking", enable=not quiet) as baton:
do_or_die("rm -fr %s; mkdir %s" % (outdir, outdir))
baton.twirl()
do_or_die("cd %s; git log --all --reverse --format=raw >%s" % (indir, rawlogfile))
baton.twirl()
commitcount = 1
commit_map = {}
os.chdir(indir)
try:
for line in open(rawlogfile):
baton.twirl()
if line.startswith("commit "):
commit = line.split()[1]
commit_map[commit] = commitcount
do_or_die("git checkout %s 2>/dev/null; mkdir %s/%d" \
% (commit, outdir, commitcount))
do_or_die("git ls-tree -r --name-only --full-tree %s | cpio -pd --quiet %s/%d"
% (commit, outdir, commitcount))
commitcount += 1
finally:
do_or_die("git reset --hard >/dev/null; git checkout master >/dev/null 2>&1")
cooked = os.path.join(outdir, "log")
body_latch = False
try:
with open(cooked, "w") as wfp:
linecount = 0
for line in open(rawlogfile):
linecount += 1
if line[0].isspace():
if line.startswith(" " * 4):
line = line[4:]
# Old-school byte stuffing.
if line.startswith("."):
line = "." + line
else:
space = line.index(' ')
leader = line[:space]
follower = line[space:].strip()
if leader == "tree":
continue
if leader == "commit" and linecount > 1:
wfp.write(".\n")
# FIXME: Check that log raw emits one parent per line
if leader in ("commit", "parent"):
line = "%s %s\n" % (leader, commit_map[follower])
body_latch = False
elif leader not in ("author", "committer"):
raise Fatal("unexpected log attribute at %s" \
% repr(line))
if line == '\n':
if not body_latch:
body_latch = True
else:
continue
wfp.write(line)
wfp.write(".\n")
except (ValueError, IndexError, KeyError):
raise Fatal("log rewrite failed on %s" % repr(line))
os.remove(rawlogfile)
if __name__ == '__main__':
(options, arguments) = getopt.getopt(sys.argv[1:], "ci:m:o:qxv")
mode = 'auto'
indir = '.'
outdir = None
quiet = False
maxcommit = 0
verbose = 0
for (opt, val) in options:
if opt == '-x':
mode = 'unpack'
elif opt == '-c':
mode = 'pack'
elif opt == '-m':
indir = int(val)
elif opt == '-i':
indir = val
elif opt == '-o':
outdir = val
elif opt == '-q':
quiet = True
elif opt == '-v':
verbose += 1
if not os.path.exists(indir):
sys.stderr.write("gitpacker: input directory %s must exist.\n" % indir)
sys.exit(1)
if mode == 'auto':
if os.path.exists(os.path.join(indir, ".git")):
mode = 'unpack'
else:
mode = 'pack'
assert mode == 'pack' or mode == 'unpack'
if outdir is None:
if mode == 'pack':
outdir = indir + "/packed"
elif mode == 'unpack':
outdir = indir + "/unpacked"
if os.path.exists(outdir):
sys.stderr.write("gitpacker: output directory %s must not exist.\n" % outdir)
sys.exit(1)
indir = os.path.abspath(indir)
outdir = os.path.abspath(outdir)
if verbose >= DEBUG_PROGRESS:
sys.stderr.write("gitpacker: %s from %s to %s.\n" % (mode, indir, outdir))
try:
try:
here = os.getcwd()
if mode == 'pack':
git_pack(indir, outdir, quiet=quiet)
elif mode == 'unpack':
git_unpack(indir, outdir, quiet=quiet)
finally:
os.chdir(here)
except Fatal, e:
sys.stderr.write(e.msg + "\n")
sys.exit(1)
except KeyboardInterrupt:
pass
# end
^ permalink raw reply
* Re: [PATCH] Unify appending signoff in format-patch, commit and sequencer
From: Brandon Casey @ 2012-11-15 20:42 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git@vger.kernel.org
In-Reply-To: <1352982778-28631-1-git-send-email-pclouds@gmail.com>
On Thu, Nov 15, 2012 at 4:32 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> There are two implementations of append_signoff in log-tree.c and
> sequencer.c, which do more or less the same thing. This patch
>
> - teaches sequencer.c's append_signoff() not to append the signoff if
> it's already there but not at the bottom
>
> - removes log-tree.c's
>
> - make sure "Signed-off-by: foo" in the middle of a line is not
> counted as a sign off
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> Interestingly this patch triggers the fault that it fixes.
> I was surprised that there was no blank line before my S-o-b
> and thought I broke something. It turns out I used unmodified
> format-patch and it mistook the S-o-b quote as true S-o-b line.
> The modified one puts the blank line back.
Heh, yeah I noticed this bug yesterday when I submitted my changes
affecting the same area of code (sequence.c). Glad I didn't waste too
much time fixing it.
Have you looked at this:
http://thread.gmane.org/gmane.comp.version-control.git/209781
That series doesn't duplicate your work, but the two series's should
be resolved.
-Brandon
^ permalink raw reply
* Re: [PATCH] status: add advice on how to push/pull to tracking branch
From: Junio C Hamano @ 2012-11-15 21:49 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqmwyid59i.fsf@grenoble-inp.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> I don't understand what you mean by "non-current". If you mean a local
> branch not pointed to by HEAD, then I don't understand the remark, as
> the message is shown by "git status" (looking more closely, it is also
> shown by "git checkout", but after switching branch so also showing a
> message about the current branch) and precisely talks about the current
> branch.
Ah, Ok, I somehow thought that "branch -v" would also use this
information, and/or during my absense this function from remote.c
got linked into "git remote show" ;-)
So it is not an issue right now, but we will have to worry about the
messaging when we start using this to describe a branch that is not
currently checked out.
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #04; Wed, 14)
From: Junio C Hamano @ 2012-11-15 22:47 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s1-bMjr3_XUwkG2XSNzq_aUzHXrs0ZieHDCn+JBYOQMoQ@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Wed, Nov 14, 2012 at 11:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> * fc/completion-test-simplification (2012-10-29) 2 commits
>> - completion: simplify __gitcomp test helper
>> - completion: refactor __gitcomp related tests
>>
>> Clean up completion tests.
>>
>> There were some comments on the list.
>>
>> Expecting a re-roll.
>
> This was already re-rolled
> http://article.gmane.org/gmane.comp.version-control.git/209382
Thanks --- will take a look and replace but not today (I was
spending most of my time on the 'master' integration front).
^ permalink raw reply
* Re: gitpacker progress report and a question
From: Max Horn @ 2012-11-15 22:35 UTC (permalink / raw)
To: esr; +Cc: git
In-Reply-To: <20121115212818.GA21558@thyrsus.com>
On 15.11.2012, at 22:28, Eric S. Raymond wrote:
> Some days ago I reported that I was attempting to write a tool that could
> (a) take a git repo and unpack it into a tarball sequence plus a metadata log,
> (b) reverse that operation, packing a tarball and log sequence into a repo.
Ah, I could have used such a tool a year or so ago. Sounds useful to me, anyway :)
>
> Thanks in part to advice by Andreas Schwab and in part to looking at the
> text of the p4 import script, this effort has succeeded. A proof of
> concept is enclosed. It isn't documented yet, and has not been tested
> on a repository with branches or merges in the history, but I am confident
> that the distance from here to a finished and tested tool is short.
>
> The immediate intended use is for importing older projects that are
> available only as sequences of release tarballs, but there are other
> sorts of repository surgery that would become easier using it.
>
> I'm still looking for a better name for it and would welcome suggestions.
Isn't "gitar" the kind of natural choice? ;) At least for a stand-alone tool, not for a git subcommand.
Cheers,
Max
>
> Before I do much further work, I need to determine how this will be shipped.
> I see two possibilities: either I ship it as a small standalone project,
> or it becomes a git subcommand shipped with the git suite. How I document
> it and set up its tests would differ between these two cases.
>
> Is there a process for submitting new subcommands? What are the
> test-suite and documentation requirements?
> --
> <a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
> <gitpacker.txt>
^ permalink raw reply
* Re: gitpacker progress report and a question
From: Eric S. Raymond @ 2012-11-15 23:05 UTC (permalink / raw)
To: Max Horn; +Cc: git
In-Reply-To: <877E14DE-313D-4863-980B-13D077230F7F@quendi.de>
Max Horn <postbox@quendi.de>:
> > I'm still looking for a better name for it and would welcome suggestions.
>
> Isn't "gitar" the kind of natural choice? ;) At least for a stand-alone tool, not for a git subcommand.
I just renamed it git-weave. I keep talking about tarballs because I keep
thinking about using it archeologically on projects that only exist as
tarball sequences, but the tool actually oacks and unpacks *file tree*
sequences.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* [PATCH 6/5] sequencer.c: refrain from adding duplicate s-o-b lines
From: Brandon Casey @ 2012-11-15 23:24 UTC (permalink / raw)
To: pclouds; +Cc: git, Brandon Casey, Brandon Casey
In-Reply-To: <1352943474-15573-5-git-send-email-drafnel@gmail.com>
Detect whether the s-o-b already exists in the commit footer and refrain
from adding a duplicate.
Update t3511 to test new behavior.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
Hi Duy,
How about this patch on top of my series as a base for your patch to
unify the code paths that append signoff in format-patch, commit, and
sequencer?
-Brandon
sequencer.c | 28 ++++++++++++++++++----------
t/t3511-cherry-pick-x.sh | 20 ++++++++++++++++++--
2 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 7ad1163..546dacb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -42,13 +42,15 @@ static int is_cherry_pick_from_line(const char *buf, int len)
!prefixcmp(buf, cherry_picked_prefix);
}
-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
+static int ends_rfc2822_footer(struct strbuf *sb, struct strbuf *sob,
+ int ignore_footer)
{
int hit = 0;
int i, k;
int len = sb->len - ignore_footer;
int last_was_rfc2822 = 0;
const char *buf = sb->buf;
+ int found_sob = 0;
for (i = len - 1; i > 0; i--) {
if (hit && buf[i] == '\n')
@@ -66,12 +68,15 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
if (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\t'))
continue;
+ if ((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) &&
+ sob && !found_sob &&
+ !strncasecmp(buf+i, sob->buf, sob->len))
+ found_sob = 1;
- if (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||
- is_cherry_pick_from_line(buf+i, k-i)))
+ if (!(last_was_rfc2822 || is_cherry_pick_from_line(buf+i, k-i)))
return 0;
}
- return 1;
+ return 1 + found_sob;
}
static void remove_sequencer_state(void)
@@ -547,7 +552,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
if (opts->record_origin) {
- if (!ends_rfc2822_footer(&msgbuf, 0))
+ if (!ends_rfc2822_footer(&msgbuf, NULL, 0))
strbuf_addch(&msgbuf, '\n');
strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
@@ -1077,6 +1082,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
void append_signoff(struct strbuf *msgbuf, int ignore_footer)
{
struct strbuf sob = STRBUF_INIT;
+ int has_footer = 0;
int i;
strbuf_addstr(&sob, sign_off_header);
@@ -1085,10 +1091,12 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer)
strbuf_addch(&sob, '\n');
for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
; /* do nothing */
- if (prefixcmp(msgbuf->buf + i, sob.buf)) {
- if (!i || !ends_rfc2822_footer(msgbuf, ignore_footer))
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);
- }
+ if (!i || !(has_footer =
+ ends_rfc2822_footer(msgbuf, &sob, ignore_footer)))
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+ "\n", 1);
+ if (has_footer != 2)
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf,
+ sob.len);
strbuf_release(&sob);
}
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index af7a87c..a15b199 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -11,9 +11,10 @@ pristine_detach () {
git clean -d -f -f -q -x
}
-non_rfc2822_mesg='base with footer
+non_rfc2822_mesg="base with footer
-Commit message body is here.'
+Commit message body is here.
+Not an s-o-b Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
rfc2822_mesg="$non_rfc2822_mesg
@@ -25,6 +26,9 @@ rfc2822_cherry_mesg="$rfc2822_mesg
(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
Tested-by: C.U. Thor <cuthor@example.com>"
+rfc2822_cherry_sob_mesg="$rfc2822_cherry_mesg
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+Signed-off-by: C.U. Thor <cuthor@example.com>"
test_expect_success setup '
git config advice.detachedhead false &&
@@ -36,6 +40,8 @@ test_expect_success setup '
test_commit "$rfc2822_mesg" foo b rfc2822-base &&
git reset --hard initial &&
test_commit "$rfc2822_cherry_mesg" foo b rfc2822-cherry-base &&
+ git reset --hard initial &&
+ test_commit "$rfc2822_cherry_sob_mesg" foo b rfc2822-cherry-sob-base &&
pristine_detach initial &&
test_commit conflicting unrelated
'
@@ -151,4 +157,14 @@ test_expect_success 'cherry-pick treats -x -s "(cherry picked from..." line as p
test_cmp expect actual
'
+test_expect_success 'cherry-pick -s detects committer s-o-b already exists' '
+ pristine_detach initial &&
+ git cherry-pick -s rfc2822-cherry-sob-base &&
+ cat <<-EOF >expect &&
+ $rfc2822_cherry_sob_mesg
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.8.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