Re: [PATCH 2/5] am: teach StGit patch parser how to read from stdin
On Tue, Jun 9, 2015 at 3:57 AM, Junio C Hamano gits...@pobox.com wrote:
Paul Tan pyoka...@gmail.com writes:
git-mailsplit, which splits mbox patches, will read the patch from stdin
when the filename is - or there are no files listed on the
command-line.
To be consistent with this behavior, teach the StGit patch parser to
read from stdin if the filename is - or no files are listed on the
command-line.
Hmm, doesn't
perl -ne 'processing for each line'
with or without a BEGIN {} block, read from the standard input (if
no filename is given) or the given file (if given), and more
importantly, doesn't it treat a lone - as STDIN anyway?
That is, wouldn't it make more sense to do something like:
test $# != 0 || set -- -
for stgit
do
...
@@PERL@@ -ne 'BEGIN { $subject = 0 }
...
' $stgit $dotest/$msgnum || clean_abort
done
Same for patch 5/5.
Ah yes, this makes more sense.
Thanks,
Paul
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/13] delete_ref(): handle special case more explicitly
On 06/08/2015 06:48 PM, Stefan Beller wrote: On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote: delete_ref() uses a different convention for its old_sha1 parameter than, say, ref_transaction_delete(): NULL_SHA1 means not to check the old value. Make this fact a little bit clearer in the code by handling it in explicit, commented code rather than burying it in a conditional expression. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index b575bb8..f9d87b6 100644 --- a/refs.c +++ b/refs.c @@ -2795,10 +2795,13 @@ int delete_ref(const char *refname, const unsigned char *old_sha1, struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; + /* Treat NULL_SHA1 as don't care */ and by don't care you mean we still care about getting it deleted, the part we don't care about is the particular sha1 (could be a bogus ref). Correct. I will to change the comment to /* * Treat NULL_SHA1 and NULL alike, to mean we don't care what * the old value of the reference was (or even if it didn't * exist): */ to make that clearer. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
Hi,
On 2015-06-08 23:00, Michael Rappazzo wrote:
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..b92375e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -977,7 +977,9 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+format=$(git config --get rebase.instructionFormat)
+# the 'rev-list .. | sed' requires %m to parse; the instruction
requires %H to parse
+git rev-list $merges_option --format=%m%H ${format-%s} --reverse
--left-right --topo-order \
These two lines are too long (longer than 80 columns)...
Besides, are you sure you don't want to substitute an empty
'rebase.instructionFormat' by '%s'? I would have expected to read
`${format:-%s}` (note the colon), but then, this was Junio's suggestion...
Junio, what do you think, should we not rather substitute empty values by `%s`
as if the config setting was unset?
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n s/^//p |
while read -r sha1 rest
Ciao,
Johannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/13] delete_ref(): move declaration to refs.h
On 06/08/2015 06:43 PM, Stefan Beller wrote:
On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
[...]
+/*
+ * Delete the specified reference. If old_sha1 is non-NULL and not
+ * NULL_SHA1, then verify that the current value of the reference is
+ * old_sha1 before deleting it.
And here I wondered what the distinction between NULL and non-NULL,
but NULL_SHA1
is and digging into the code, there is none. (As you can also see in
this patch above with
(old_sha1 !is_null_sha1(old_sha1)) ? old_sha1 : NULL,
but when digging deeper, the !is_null_sha1(old_sha1) is an arbitrary
limitation (i.e.
ref_transaction_delete and ref_transaction_update don't differ between
those two cases.)
The distinction comes in at lock_ref_sha1_basic, where I think we may
want to get rid of
the is_null_sha1 check and depend on NULL/non-NULL as the difference
for valid and invalid
sha1's?
I'm having a little trouble understanding your comment.
The convention for ref_transaction_update() and friends is that
* old_sha1 == NULL
We don't care whether the reference existed prior to the
update, nor what its value was.
* *old_sha1 is NULL_SHA1
(by which I mean that old_sha1 points at 20 zero bytes; I hope
that's clear even though NULL_SHA1 is not actually defined
anywhere): The reference must *not* have existed prior to the
update.
* old_sha1 has some other value
The reference must have had that value prior to the update.
lock_ref_sha1_basic() distinguishes between { NULL vs. NULL_SHA1 vs.
other values } in the same way that ref_transaction_update() does.
The delete_ref() function doesn't follow the same convention. It treats
NULL and NULL_SHA1 identically, as don't care.
It probably makes sense to change delete_ref() use the same convention
as ref_transaction_update(), but there are quite a few callers and I
didn't have the energy to review them all as part of this patch series.
So I left it unchanged and just documented the status quo better.
That said, do we want to add another sentence to the doc here saying
non-NULL and not
NULL_SHA1 are treated the same or is it clear enough?
With or without this question addressed:
Reviewed-by: Stefan Beller sbel...@google.com
In set notation,
non-NULL =
non-NULL and not NULL_SHA1 ∪
non-NULL and equal to NULL_SHA1
The latter two are *not* treated the same, so I don't see how we can
claim that non-NULL and not NULL_SHA1 are treated the same. I must
be misunderstanding you.
Would it help if I changed the comment to
Delete the specified reference. If old_sha1 is non-NULL and not
NULL_SHA1, then verify that the current value of the reference is
old_sha1 before deleting it. If old_sha1 is NULL or NULL_SHA1,
delete the reference it it exists, regardless of its old value.
?
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/13] delete_refs(): convert error message to lower case
On 06/08/2015 06:51 PM, Stefan Beller wrote: On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote: This string is going to have to be re-internationalized anyway because of the previous commit. So while we're at it, we might as well convert it to lower case as per our usual practice. Although the previous patch and this are addressing two slightly different things, we may want to squash this into the previous without dropping any of the commit message? (It might make reviewing easier, I'd assume) OK, I will squash them together. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] am --abort: support aborting to unborn branch
On Tue, Jun 9, 2015 at 4:10 AM, Junio C Hamano gits...@pobox.com wrote: Paul Tan pyoka...@gmail.com writes: When git-am is first run on an unborn branch, no ORIG_HEAD is created. As such, any applied commits will remain even after a git am --abort. I think this answered my question on 4/6; that may indicate that these two are either done in a wrong order or perhaps should be a single change. Ah right, patch 4/6 was too sneaky in that it tried to do the support unborn branch thing as well, which should only be handled in this patch. I still think the patches should be separate though since they are conceptually different. 4/6 modifies the index clean up function, while 5/6 modifies the reset HEAD function. Thanks, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1]: git-completion.tcsh fails w/ noclobber
The subject is better but still not quite there. I suggest: [PATCH] git-completion.tcsh: fix redirect with noclobber On Tue, Jun 9, 2015 at 1:01 AM, Ariel Faigon github.2...@yendor.com wrote: tcsh users who happen to have 'set noclobber' elsewhere in their ~/.tcshrc or ~/.cshrc startup files get a 'File exist' error, and the tcsh completion file doesn't get generated/updated. Adding a `!` in the redirect works correctly for both clobber (default) and 'set noclobber' users. Helped-by: Junio C Hamano notificati...@github.com The email address look wrong. If he really helped you, he probably emailed you using another address. Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: is used by Google Summer of Code students. If someone helps you and you want to acknowledge that, you can add an Helped-by: trailer (unless your helper tells you that you can add a Reviewed-by: or maybe a Signed-off-by:). Signed-off-by: Ariel Faigon github.2...@yendor.com --- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
I see your point, and I'll explore that avenue.
Personally, I like the idea that one could also use the short hash if
the custom instruction started with %h , but I see the value in
leaving the variable blank.
After running the tests with a custom format enabled, I did find that
autosquash doesn't work, so I am working to correct that.
On Tue, Jun 9, 2015 at 5:36 AM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
Hi,
On 2015-06-08 23:00, Michael Rappazzo wrote:
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..b92375e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -977,7 +977,9 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+format=$(git config --get rebase.instructionFormat)
+# the 'rev-list .. | sed' requires %m to parse; the instruction
requires %H to parse
+git rev-list $merges_option --format=%m%H ${format-%s} --reverse
--left-right --topo-order \
These two lines are too long (longer than 80 columns)...
Besides, are you sure you don't want to substitute an empty
'rebase.instructionFormat' by '%s'? I would have expected to read
`${format:-%s}` (note the colon), but then, this was Junio's suggestion...
Junio, what do you think, should we not rather substitute empty values by
`%s` as if the config setting was unset?
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n s/^//p |
while read -r sha1 rest
Ciao,
Johannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] status: give more information during rebase -i
Junio C Hamano gits...@pobox.com writes:
Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr writes:
git status gives more information during rebase -i, about the list of
command that are done during the rebase. It displays the two last
commands executed and the two next lines to be executed. It also gives
hints to find the whole files in .git directory.
---
Without 1/4 and 2/4 in the same thread, it is hard to guess what you
wanted to do with these two patches. Remember, reviewers review not
just your patches but those from many others.
I would in the meantime assume these are replacement patches for the
ones in
http://thread.gmane.org/gmane.comp.version-control.git/270743/focus=270744
You assumed correctly, I didn't wanted to flood the mailing list and I
assumed it would be linked correctly with the previous since. Looks like
I was wrong.
diff --git a/wt-status.c b/wt-status.c
index c83eca5..7f88470 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1026,12 +1026,73 @@ static int split_commit_in_progress(struct wt_status
*s)
return split_in_progress;
}
+static void show_rebase_information(struct wt_status *s,
+ struct wt_status_state *state,
+ const char *color)
+{
+ if (state-rebase_interactive_in_progress) {
+ int i, begin;
+ int lines_to_show_nr = 2;
lines_to_show = 2 or nr_lines_to_show = 2 would be easier to read.
Done
+
+ struct strbuf buf = STRBUF_INIT;
+ struct string_list have_done = STRING_LIST_INIT_DUP;
+ struct string_list yet_to_do = STRING_LIST_INIT_DUP;
+
+ strbuf_read_file(buf, git_path(rebase-merge/done), 0);
+ stripspace(buf, 1);
+ have_done.nr = string_list_split(have_done, buf.buf, '\n', -1);
+ string_list_remove_empty_items(have_done, 1);
+ strbuf_release(buf);
+
+ strbuf_read_file(buf, git_path(rebase-merge/git-rebase-todo), 0);
+ stripspace(buf, 1);
+ string_list_split(yet_to_do, buf.buf, '\n', -1);
+ string_list_remove_empty_items(yet_to_do, 1);
+ strbuf_release(buf);
+
+ if (have_done.nr == 0)
+ status_printf_ln(s, color, _(No commands done.));
Do the users even need to be told that, I wonder?
I guess it removes the ambiguity of being told nothing.
+ else{
Style: else {
Ok thanks.
+ status_printf_ln(s, color,
+ Q_(Last command done (%d command done):,
+ Last commands done (%d commands done):,
+ have_done.nr),
+ have_done.nr);
+ begin = (have_done.nr lines_to_show_nr) ? have_done.nr-lines_to_show_nr
: 0;
+ for (i = begin; i have_done.nr; i++) {
+ status_printf_ln(s, color, %s, have_done.items[i].string);
+ }
Hmm, perhaps fold lines like this (and you do not need begin)?
for (i = (lines_to_show_nr have_done.nr)
? have_done.nr - lines_to_show_nr : 0;
i have_done.nr;
i++)
status_printf_ln(...);
+ if (have_done.nr lines_to_show_nr s-hints)
+ status_printf_ln(s, color,
+ _( (see more in file %s)), git_path(rebase-merge/done));
That's a nice touch ;-)
+ }
+ if (yet_to_do.nr == 0)
+ status_printf_ln(s, color,
+ _(No commands remaining.));
This I can see why we may want to say it.
+ else{
+
+ status_printf_ln(s, color,
+ Q_(Next command to do (%d remaining command):,
+ Next commands to do (%d remaining commands):,
+ yet_to_do.nr),
+ yet_to_do.nr);
+ for (i = 0; i lines_to_show_nr i yet_to_do.nr; i++) {
+ status_printf_ln(s, color, %s, yet_to_do.items[i].string);
+ }
+ if (s-hints)
+ status_printf_ln(s, color,
+ _( (use \git rebase --edit-todo\ to view and edit)));
+ }
Make sure you do not leak memory used by two string lists here...
Done, thanks again.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: GNU diff and git diff - difference on myers algorithm?
Hi Luis, On 2015-06-08 20:34, Luis R. Rodriguez wrote: Based on a cursory review of the git code I get the impression that GNU diff and git 'diff' do not share any code for the possible diff algorithms. Indeed, Git's diff machinery is based[*1*] ofn libxdiff[*2*], not on GNU diff. I'm in particularly curious more about the default myers algorithm. Are you looking for a freely available implementation of the Myers algorithm? Or are you interested in understanding it? Please note that Myers' algorithm is just one first step in most diff implementations (and that other diff algorithms have become popular, in particular because comparing strings can be accelerated by hashing the text lines first, and those hashes can also be used to identify matching pairs of unique lines, giving rise to yet another huge performance boost for typical uses). The reason why Myers' algorithm is not sufficient for diff implementations is that it only optimizes the edit distance, i.e. the amount of added/removed lines, while patches should be readable, too, i.e. prefer *consecutive* edits to disjunct ones. Just to mention one post-processing technique that is so useful that I implemented it for Git[*3*]: the patience diff algorithm of Bram Cohen (of BitTorrent fame) finds matching pairs of unique lines -- think of a function from which another function is refactored, for example, intuitively you want the diff to keep the signature of the original function as a context line. Disclaimer: While it is true that Gene and I shared an office for one month, and that I am once again working in the same institute as he does, all my knowledge about this algorithm stems from my reading his paper and implementing the algorithm in Java for use in JGit[*3*]. I can take time to do a precise code review of the algorithms used on both GNU diff and git but if someone can already vet for any differences that'd be appreciated as it would save time. Again, I am curious what your goal is? I am sure I can support your quest better when I understand what the purpose of this code review should be. Ciao, Johannes Footnote *1*: https://github.com/git/git/commit/3443546f6ef57fe28ea5cca232df8e400bfc3883 Footnote *2*: http://www.xmailserver.org/xdiff-lib.html Footnote *3*: https://github.com/git/git/blob/master/xdiff/xpatience.c Footnote *4*: https://github.com/eclipse/jgit/blob/master/org.eclipse.jgit/src/org/eclipse/jgit/diff/MyersDiff.java -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] bisect: simplify the add of new bisect terms
On Tue, Jun 9, 2015 at 9:01 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Subject: Re: [PATCH 3/4] bisect: simplify the add of new bisect terms
s/add/addition/
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes:
+static const char *name_bad;
+static const char *name_good;
Same remark as PATCH 2.
As for patch 2, I think name_bad and name_good are better than
name_new and name_old.
[...]
+ name_bad = bad;
+ name_good = good;
+ } else {
+ strbuf_getline(str, fp, '\n');
+ name_bad = strbuf_detach(str, NULL);
+ strbuf_getline(str, fp, '\n');
+ name_good = strbuf_detach(str, NULL);
+ }
I would have kept just
name_bad = bad;
name_good = good;
in this patch, and introduce BISECT_TERMS in a separate one.
Yeah I agree that it is more logical to have first a patch that does
on bisect.c the same thing as patch 2 does on git-bisect.sh.
For example the patch series could be for now:
1) bisect: typo fix
2) bisect: replace hardcoded bad|good with variables
3) git-bisect: replace hardcoded bad|good with variables
4) bisect: simplify adding new terms
5) bisect: add old/new terms
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] bisect: replace hardcoded bad|good by variables
On Tue, Jun 9, 2015 at 8:45 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes: --- a/git-bisect.sh +++ b/git-bisect.sh @@ -32,6 +32,8 @@ OPTIONS_SPEC= _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' _x40=$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40 +NAME_BAD=bad +NAME_GOOD=good I would have written NAME_NEW=bad NAME_OLD=good old/new are the generic wording, so I think it would make more sense for the codebase to use it when we don't hardcode old/new. I don't agree with NAME_NEW and NAME_OLD instead of NAME_BAD and NAME_OLD, for me it is easier when reasonning about the code to always think as if we want to find a bug. This is especially true when thinking about cases when we are given a good commit that is not an ancestor of the bad commit (and we have to find the merge base and so on), because in this case the good commit might be newer than the bad commit. old/new is not more generic than good/bad. It just has a different kind of drawbacks, and as good/bad is older and is the default we should keep that in the names. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] am --abort: keep unrelated commits on unborn branch
On Tue, Jun 9, 2015 at 4:13 AM, Junio C Hamano gits...@pobox.com wrote: Paul Tan pyoka...@gmail.com writes: Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure and warn, 2010-12-21), git-am would refuse to rewind HEAD if commits were made since the last git-am failure. This check was implemented in safe_to_abort(), which checked to see if HEAD's hash matched the abort-safety file. However, this check was skipped if the abort-safety file was empty, which can happen if git-am failed while on an unborn branch. Shouldn't we then be checking that the HEAD is still unborn if this file is found and says that there was no history in the beginning, in order to give the am on top of unborn workflow the same degree of safety? We do already check to see if the HEAD is still unborn: abort_safety=$(cat $dotest/abort-safety) if test z$(git rev-parse --verify -q HEAD) = z$abort_safety then return 0 fi gettextln You seem to have moved HEAD since the last 'am' failure. Not rewinding to ORIG_HEAD 2 If HEAD is unborn, then git rev-parse will not print anything, so we would be comparing an empty string to an empty string. Thanks, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] prune_remote(): use delete_refs()
On 06/08/2015 07:12 PM, Jeff King wrote: On Mon, Jun 08, 2015 at 09:57:04AM -0700, Stefan Beller wrote: On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote: This will result in errors being emitted for references that can't be deleted, but that is a good thing. This sounds a bit like hand-waving to me. Trust me, I'm an engineer!. I think the argument is we failed to do that the user asked for, but never reported the reason why. But I don't see how that is the case. We already complain if repack_without_refs fail, and AFAICT the original call to delete_ref() would emit an error, as well. The old and new code report errors that come from repack_without_refs() the same way. The difference is how they report errors within delete_ref(). The old code only allowed delete_ref() to emit its error. The new code (in delete_refs()) allows delete_ref() to emit its error, but then follows it up with error(_(could not remove reference %s), refname) The could not remove reference error originally came from a similar message in remove_branches() (from builtin/remote.c). I *think* this is an improvement, because the error from delete_ref() (which usually comes from ref_transaction_commit()) can be pretty low-level, like Cannot lock ref '%s': unable to resolve reference %s: %s where the last %s is the original strerror. I would be happy to change the behavior if somebody has a concrete wish. At the same time I don't think we need to sweat the details too much, because these errors should only ever be seen in the case of a broken repository or a race between two processes; i.e., only in pretty rare and anomalous situations. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/9] tag: libify parse_opt_points_at()
On 06/09/2015 12:30 AM, Junio C Hamano wrote: This feels way too specialized to live as part of parse_options infrastructure. The existing caller(s) may want to use this callback for parsing points-at option they have, but is that the only plausible use of this callback? It looks to be usable by any future caller that wants to take and accumulate any object names into an sha1-array, so perhaps rename it to be a bit more generic to represent its nature better? parse_opt_object_name() or something? This makes sense! Will change. I also wonder if we can (and want to) refactor the users of with-commit callback. Have them use this to obtain an sha1-array and then convert what they received into a commit-list themselves. But wouldn't that be too much of an overhead to iterate through the sha1-array and convert it to a commit-list? -- Regards, Karthik -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] prune_remote(): use delete_refs()
On Tue, Jun 09, 2015 at 12:50:13PM +0200, Michael Haggerty wrote: The new code (in delete_refs()) allows delete_ref() to emit its error, but then follows it up with error(_(could not remove reference %s), refname) The could not remove reference error originally came from a similar message in remove_branches() (from builtin/remote.c). I *think* this is an improvement, because the error from delete_ref() (which usually comes from ref_transaction_commit()) can be pretty low-level, like Cannot lock ref '%s': unable to resolve reference %s: %s where the last %s is the original strerror. I would be happy to change the behavior if somebody has a concrete wish. At the same time I don't think we need to sweat the details too much, because these errors should only ever be seen in the case of a broken repository or a race between two processes; i.e., only in pretty rare and anomalous situations. Thanks for the explanation. I agree it probably doesn't matter much either way. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option
On 06/09/2015 12:42 AM, Junio C Hamano wrote:
Is this intended? I would have expected if I did
git for-each-ref --points-at master
I would get refs/heads/master and any other refs that exactly points
at that commit.
Thats to be changed, thanks!
FIELD NAMES
---
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 4d2d024..b9d180a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -7,6 +7,7 @@
static char const * const for_each_ref_usage[] = {
N_(git for-each-ref [options] [pattern]),
+ N_(git for-each-ref [--points-at object]),
NULL
};
@@ -17,6 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char
*prefix)
struct ref_sorting *sorting = NULL, **sorting_tail = sorting;
int maxcount = 0, quote_style = 0;
struct ref_filter_cbdata ref_cbdata;
+ memset(ref_cbdata, 0, sizeof(ref_cbdata));
struct option opts[] = {
OPT_BIT('s', shell, quote_style,
@@ -33,6 +35,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char
*prefix)
OPT_STRING( 0 , format, format, N_(format), N_(format to use
for the output)),
OPT_CALLBACK(0 , sort, sorting_tail, N_(key),
N_(field name to sort on),
parse_opt_ref_sorting),
+ OPT_CALLBACK(0, points-at, ref_cbdata.filter.points_at,
+N_(object), N_(print only tags of the object),
+parse_opt_points_at),
OPT_END(),
};
@@ -54,7 +59,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char
*prefix)
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
- memset(ref_cbdata, 0, sizeof(ref_cbdata));
I cannot quite see how this change relates to the addition of the
new option.
Well if we memset() after calling parse_opt_points_at(), we loose all
the information we would have obtained.
So the memset() is moved to an earlier location.
--
Regards,
Karthik
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'
Karthik Nayak karthik@gmail.com writes: On 06/08/2015 10:51 PM, Matthieu Moy wrote: We could introduce ref-filter.h earlier, indeed. To me, the current solution is good enough, but introducing ref-filter.h early and adding function definition there in the same commit as you drop the static keyword for them would clearly be an improvement. But that would break the flow, wouldn't it? I wanted ref-filter to be introduced together, hence right after ref-filter.h we move code to ref-filter.c That's why I find the current solution good enough: it also has advantages. But in the current series, when you say make functions public, you are not actually doing so since they're not exported in a .h file. Conversely, PATCH 07 does two things: move code from for-each-ref.c and introduce new declarations. Had you introduced these declarations earlier, this patch would have been pure code movement. In both cases, you have intermediate states that are not fully consistant: either you have public functions in the builtin/ directory (which sometimes happen in Git's codebase, but we try to avoid it), or you have non-static functions that are not declared in a .h. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] status: factor two rebase-related messages together
Junio C Hamano gits...@pobox.com writes: Hmmm, it obviously does not break anything but it is not obvious why this is a good change. Is it that you wanted to have a single instance of if on a branch, we say 'you are rebasing that branch', otherwise we say 'you are rebasing'? Even then, I am not sure if this code movement was the best way to do so (an obvious alternative is to use a shared helper function and call from the two arms of if/elseif/... chain). I made this change because at first sight, this piece of code was difficult to read for me. There was two long branches very similar and I had to spot the differences, and the actual differences were at the very end of the branches so I had to check back what the condition was about. It seems now much more natural to me: the part in common of both branches is in OR-condition and the differences between branches are gathered with the test on the variable they depend. By the way, I agree that this change is not absolutely necessary. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] bisect: simplify the add of new bisect terms
Subject: Re: [PATCH 3/4] bisect: simplify the add of new bisect terms
s/add/addition/
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes:
+static const char *name_bad;
+static const char *name_good;
Same remark as PATCH 2.
} else if (starts_with(refname, good-)) {
Did you forget this one?
-
- fprintf(stderr, The merge base %s is bad.\n
- This means the bug has been fixed
- between %s and [%s].\n,
- bad_hex, bad_hex, good_hex);
-
+ if (!strcmp(name_bad, bad)) {
+ fprintf(stderr, The merge base %s is bad.\n
+ This means the bug has been fixed
+ between %s and [%s].\n,
+ bad_hex, bad_hex, good_hex);
+ }
You need an else here. Maybe it comes later, but as a reviewer, I want
to check that you did not forget it now (because I don't trust myself to
remember that it must be added later).
@@ -890,6 +894,31 @@ static void show_diff_tree(const char *prefix, struct
commit *commit)
}
/*
+ * The terms used for this bisect session are stocked in
+ * BISECT_TERMS: it can be bad/good or new/old.
+ * We read them and stock them to adapt the messages
s/stock/store/ (two instances)
+ * accordingly. Default is bad/good.
+ */
+void read_bisect_terms(void)
+{
+ struct strbuf str = STRBUF_INIT;
+ const char *filename = git_path(BISECT_TERMS);
+ FILE *fp = fopen(filename, r);
+
+ if (!fp) {
I think you would want to error out if errno is not ENOENT.
+ name_bad = bad;
+ name_good = good;
+ } else {
+ strbuf_getline(str, fp, '\n');
+ name_bad = strbuf_detach(str, NULL);
+ strbuf_getline(str, fp, '\n');
+ name_good = strbuf_detach(str, NULL);
+ }
I would have kept just
name_bad = bad;
name_good = good;
in this patch, and introduce BISECT_TERMS in a separate one.
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,6 +77,7 @@ bisect_start() {
orig_args=$(git rev-parse --sq-quote $@)
bad_seen=0
eval=''
+ start_bad_good=0
if test z$(git rev-parse --is-bare-repository) != zfalse
then
mode=--no-checkout
@@ -101,6 +102,9 @@ bisect_start() {
die $(eval_gettext '\$arg' does not appear to
be a valid revision)
break
}
+
+ start_bad_good=1
+
Why do you need this variable? It seems to me that you are hardcoding
once more that terms can be either good/bad or old/new, which you
tried to eliminate from the previous round.
+ if test $start_bad_good -eq 1 -a ! -s $GIT_DIR/BISECT_TERMS
Avoid test -a (not strictly POSIX, and sometimes ambiguous). Use
test ... test ...
instead.
+ then
+ echo $NAME_BAD $GIT_DIR/BISECT_TERMS
+ echo $NAME_GOOD $GIT_DIR/BISECT_TERMS
+ fi
Why not do this unconditionnally? Whether terms are good/bad or old/new,
you can write them to BISECT_TERMS.
- gettextln You need to give me at least one good and
one bad revision.
-(You can use \git bisect bad\ and \git bisect good\ for that.) 2
+ gettextln You need to give me at least one
$(bisect_voc bad) and one $(bisect_voc good) revision.
+(You can use \git bisect $(bisect_voc bad)\ and \git bisect $(bisect_voc
good)\ for that.) 2
I suspect you broke the i18n here too.
+get_terms () {
+ if test -s $GIT_DIR/BISECT_TERMS
+ then
+ NAME_BAD=$(sed -n 1p $GIT_DIR/BISECT_TERMS)
+ NAME_GOOD=$(sed -n 2p $GIT_DIR/BISECT_TERMS)
+ fi
+}
+
+check_and_set_terms () {
+ cmd=$1
+ case $cmd in
+ bad|good)
+ if test -s $GIT_DIR/BISECT_TERMS -a $cmd != $NAME_BAD -a
$cmd != $NAME_GOOD
+ then
+ die $(eval_gettext Invalid command : you're currently
in a \$NAME_BAD/\$NAME_GOOD bisect.)
No space before :
+ fi
+ case $cmd in
+ bad|good)
+ if test ! -s $GIT_DIR/BISECT_TERMS
+ then
+ echo bad $GIT_DIR/BISECT_TERMS
+ echo good $GIT_DIR/BISECT_TERMS
+ fi
+ NAME_BAD=bad
+ NAME_GOOD=good ;;
+ esac ;;
+ esac
+}
+
+bisect_voc () {
+ case $1 in
+ bad) echo bad ;;
+ good) echo good ;;
+ esac
+}
It's weird to have these hardcoded bad/good when you already have
BISECT_TERMS in the same patch.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH 2/4] bisect: replace hardcoded bad|good by variables
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes:
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -32,6 +32,8 @@ OPTIONS_SPEC=
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
_x40=$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40
+NAME_BAD=bad
+NAME_GOOD=good
I would have written
NAME_NEW=bad
NAME_OLD=good
old/new are the generic wording, so I think it would make more sense
for the codebase to use it when we don't hardcode old/new.
(and the double-quotes are not needed)
@@ -249,8 +254,8 @@ bisect_state() {
bisect_write $state $rev
done
check_expected_revs $hash_list ;;
- *,bad)
- die $(gettext 'git bisect bad' can take only one argument.)
;;
+ *,$NAME_BAD)
+ die $(gettext 'git bisect $NAME_BAD' can take only one
argument.) ;;
As Junio mentionned, you'll need an eval_gettext here.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On undoing a forced push
On 06/09/2015 05:42 PM, Duy Nguyen wrote: From a thread on Hacker News. It seems that if a user does not have access to the remote's reflog and accidentally forces a push to a ref, how does he recover it? In order to force push again to revert it back, he would need to know the remote's old SHA-1. Local reflog does not help because remote refs are not updated during a push. This patch prints the latest SHA-1 before the forced push in full. He then can do git push remote +old-sha1:ref He does not even need to have the objects that old-sha1 refers to. We could simply push an empty pack and the the remote will happily accept the force, assuming garbage collection has not happened. But that's another and a little more complex patch. If I am not mistaken, we actively prevent people from downloading an unreferenced SHA (such as would happen if you overwrote refs that contained sensitive information like passwords). Wouldn't allowing the kind of push you just described, require negating that protection? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] status: differentiate interactive from non-interactive rebases
Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr writes: Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr --- t/t7512-status-help.sh | 28 ++-- wt-status.c| 5 - Are there any change since the last version? Please, help reviewers by anticipating this question and versionning your patches ([PATCH v3 2/4] in the subject, git send-email -v3 does this for you). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On undoing a forced push
On 06/09/2015 07:55 PM, Jeff King wrote: On Tue, Jun 09, 2015 at 07:36:20PM +0530, Sitaram Chamarty wrote: This patch prints the latest SHA-1 before the forced push in full. He then can do git push remote +old-sha1:ref He does not even need to have the objects that old-sha1 refers to. We could simply push an empty pack and the the remote will happily accept the force, assuming garbage collection has not happened. But that's another and a little more complex patch. If I am not mistaken, we actively prevent people from downloading an unreferenced SHA (such as would happen if you overwrote refs that contained sensitive information like passwords). Wouldn't allowing the kind of push you just described, require negating that protection? No, this has always worked. If you have write access to a repository, you can fetch anything from it with this trick. Even if we blocked this, there are other ways to leak information. For instance, I can push up objects that are similar to the target object, claim to have the target object, and then hope git will make a delta between my similar object and the target. Iterate on the similar object and you can eventually figure out what is in the target object. aah ok; I must have mis-remembered something. Thanks! -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
How to compile without iconv?
I tried to compile git 2.4.3 on Solaris 10. I used the following configuration: $ ./configure --without-iconv $ grep -i iconv config.status ac_cs_config='--without-iconv' set X /bin/bash './configure' '--without-iconv' $ac_configure_extra_args --no-create --no-recursion OLD_ICONV=UnfortunatelyYes But when I try to compile it, I get an error that libiconv is missing: LINK git-credential-store Undefined first referenced symbol in file libintl_gettext libgit.a(lockfile.o) libiconv_close libgit.a(utf8.o) libiconv_open libgit.a(utf8.o) libintl_ngettextlibgit.a(date.o) libiconvlibgit.a(utf8.o) ld: fatal: symbol referencing errors. No output written to git-credential-store collect2: ld returned 1 exit status gmake: *** [git-credential-store] Error 1 How can I work around this? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] status: factor two rebase-related messages together
Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr writes: Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr --- wt-status.c | 31 +++ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/wt-status.c b/wt-status.c index 33452f1..c239132 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1025,6 +1025,19 @@ static int split_commit_in_progress(struct wt_status *s) free(rebase_orig_head); return split_in_progress; } +static void print_rebase_state(struct wt_status *s, Please, skip a line between functions. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On undoing a forced push
On Tue, Jun 09, 2015 at 07:12:21PM +0700, Duy Nguyen wrote:
diff --git a/transport.c b/transport.c
index f080e93..6bd6a64 100644
--- a/transport.c
+++ b/transport.c
@@ -657,16 +657,17 @@ static void print_ok_ref_status(struct ref *ref, int
porcelain)
[new branch]),
ref, ref-peer_ref, NULL, porcelain);
else {
- char quickref[84];
+ char quickref[104];
You've increased this by 20, but you're adding 40 characters to the
strcpy. Are you sure that's enough?
Also, you might consider writing this in terms of GIT_SHA1_HEXSZ, as it
will be more obvious that this depends on that value. If you don't now,
I will later.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
signature.asc
Description: Digital signature
[PATCH 3/4] status: give more information during rebase -i
git status gives more information during rebase -i, about the list of command that are done during the rebase. It displays the two last commands executed and the two next lines to be executed. It also gives hints to find the whole files in .git directory. Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr --- t/t7512-status-help.sh | 111 + wt-status.c| 64 2 files changed, 175 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 190656d..0c889fa 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -134,9 +134,13 @@ test_expect_success 'prepare for rebase_i_conflicts' ' test_expect_success 'status during rebase -i when conflicts unresolved' ' test_when_finished git rebase --abort ONTO=$(git rev-parse --short rebase_i_conflicts) + LAST_COMMIT=$(git rev-parse rebase_i_conflicts_second) test_must_fail git rebase -i rebase_i_conflicts cat expected EOF interactive rebase in progress; onto $ONTO +Last command done (1 command done): + pick $LAST_COMMIT one_second +No commands remaining. You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) @@ -159,10 +163,14 @@ test_expect_success 'status during rebase -i after resolving conflicts' ' git reset --hard rebase_i_conflicts_second test_when_finished git rebase --abort ONTO=$(git rev-parse --short rebase_i_conflicts) + LAST_COMMIT=$(git rev-parse rebase_i_conflicts_second) test_must_fail git rebase -i rebase_i_conflicts git add main.txt cat expected EOF interactive rebase in progress; onto $ONTO +Last command done (1 command done): + pick $LAST_COMMIT one_second +No commands remaining. You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (all conflicts fixed: run git rebase --continue) @@ -183,7 +191,9 @@ test_expect_success 'status when rebasing -i in edit mode' ' git checkout -b rebase_i_edit test_commit one_rebase_i main.txt one test_commit two_rebase_i main.txt two + COMMIT2=$(git rev-parse rebase_i_edit) test_commit three_rebase_i main.txt three + COMMIT3=$(git rev-parse rebase_i_edit) FAKE_LINES=1 edit 2 export FAKE_LINES test_when_finished git rebase --abort @@ -191,6 +201,10 @@ test_expect_success 'status when rebasing -i in edit mode' ' git rebase -i HEAD~2 cat expected EOF interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_rebase_i + edit $COMMIT3 three_rebase_i +No commands remaining. You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -207,8 +221,11 @@ test_expect_success 'status when splitting a commit' ' git checkout -b split_commit test_commit one_split main.txt one test_commit two_split main.txt two + COMMIT2=$(git rev-parse split_commit) test_commit three_split main.txt three + COMMIT3=$(git rev-parse split_commit) test_commit four_split main.txt four + COMMIT4=$(git rev-parse split_commit) FAKE_LINES=1 edit 2 3 export FAKE_LINES test_when_finished git rebase --abort @@ -217,6 +234,12 @@ test_expect_success 'status when splitting a commit' ' git reset HEAD^ cat expected EOF interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_split + edit $COMMIT3 three_split +Next command to do (1 remaining command): + pick $COMMIT4 four_split + (use git rebase --edit-todo to view and edit) You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -239,7 +262,9 @@ test_expect_success 'status after editing the last commit with --amend during a test_commit one_amend main.txt one test_commit two_amend main.txt two test_commit three_amend main.txt three + COMMIT3=$(git rev-parse amend_last) test_commit four_amend main.txt four + COMMIT4=$(git rev-parse amend_last) FAKE_LINES=1 2 edit 3 export FAKE_LINES test_when_finished git rebase --abort @@ -248,6 +273,11 @@ test_expect_success 'status after editing the last commit with --amend during a git commit --amend -m foo cat expected EOF interactive rebase in progress; onto $ONTO +Last commands done (3 commands done): + pick $COMMIT3 three_amend + edit $COMMIT4 four_amend + (see more in
[PATCH 1/4] status: factor two rebase-related messages together
Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr
---
wt-status.c | 31 +++
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 33452f1..c239132 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1025,6 +1025,19 @@ static int split_commit_in_progress(struct wt_status *s)
free(rebase_orig_head);
return split_in_progress;
}
+static void print_rebase_state(struct wt_status *s,
+ struct wt_status_state *state,
+ const char *color)
+{
+ if (state-branch)
+ status_printf_ln(s, color,
+_(You are currently rebasing branch
'%s' on '%s'.),
+state-branch,
+state-onto);
+ else
+ status_printf_ln(s, color,
+_(You are currently rebasing.));
+}
static void show_rebase_in_progress(struct wt_status *s,
struct wt_status_state *state,
@@ -1033,14 +1046,7 @@ static void show_rebase_in_progress(struct wt_status *s,
struct stat st;
if (has_unmerged(s)) {
- if (state-branch)
- status_printf_ln(s, color,
-_(You are currently rebasing branch
'%s' on '%s'.),
-state-branch,
-state-onto);
- else
- status_printf_ln(s, color,
-_(You are currently rebasing.));
+ print_rebase_state(s, state, color);
if (s-hints) {
status_printf_ln(s, color,
_( (fix conflicts and then run \git rebase
--continue\)));
@@ -1050,14 +1056,7 @@ static void show_rebase_in_progress(struct wt_status *s,
_( (use \git rebase --abort\ to check out
the original branch)));
}
} else if (state-rebase_in_progress || !stat(git_path(MERGE_MSG),
st)) {
- if (state-branch)
- status_printf_ln(s, color,
-_(You are currently rebasing branch
'%s' on '%s'.),
-state-branch,
-state-onto);
- else
- status_printf_ln(s, color,
-_(You are currently rebasing.));
+ print_rebase_state(s, state, color);
if (s-hints)
status_printf_ln(s, color,
_( (all conflicts fixed: run \git rebase
--continue\)));
--
2.4.2.342.ga3499d3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] status: differentiate interactive from non-interactive rebases
Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr --- t/t7512-status-help.sh | 28 ++-- wt-status.c| 5 - 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 68ad2d7..190656d 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -136,7 +136,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' ' ONTO=$(git rev-parse --short rebase_i_conflicts) test_must_fail git rebase -i rebase_i_conflicts cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) @@ -162,7 +162,7 @@ test_expect_success 'status during rebase -i after resolving conflicts' ' test_must_fail git rebase -i rebase_i_conflicts git add main.txt cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (all conflicts fixed: run git rebase --continue) @@ -190,7 +190,7 @@ test_expect_success 'status when rebasing -i in edit mode' ' ONTO=$(git rev-parse --short HEAD~2) git rebase -i HEAD~2 cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -216,7 +216,7 @@ test_expect_success 'status when splitting a commit' ' git rebase -i HEAD~3 git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -247,7 +247,7 @@ test_expect_success 'status after editing the last commit with --amend during a git rebase -i HEAD~3 git commit --amend -m foo cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -277,7 +277,7 @@ test_expect_success 'status: (continue first edit) second edit' ' git rebase -i HEAD~3 git rebase --continue cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -299,7 +299,7 @@ test_expect_success 'status: (continue first edit) second edit and split' ' git rebase --continue git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -326,7 +326,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' ' git rebase --continue git commit --amend -m foo cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -348,7 +348,7 @@ test_expect_success 'status: (amend first edit) second edit' ' git commit --amend -m a git rebase --continue cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -371,7 +371,7 @@ test_expect_success 'status: (amend first edit) second edit and split' ' git rebase --continue git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -399,7 +399,7 @@
[PATCH 4/4] status: add new tests for status during rebase -i
Expand test coverage with one or more than two commands done and with zero, one or more than two commands remaining. Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr --- t/t7512-status-help.sh | 87 ++ 1 file changed, 87 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 0c889fa..cbe27f9 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -856,4 +856,91 @@ EOF test_i18ncmp expected actual ' +test_expect_success 'prepare for different number of commits rebased' ' + git reset --hard master + git checkout -b several_commits + test_commit one_commit main.txt one + test_commit two_commit main.txt two + test_commit three_commit main.txt three + test_commit four_commit main.txt four +' + +test_expect_success 'status: one command done nothing remaining' ' + FAKE_LINES=exec_exit_15 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~3) + test_must_fail git rebase -i HEAD~3 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last command done (1 command done): + exec exit 15 +No commands remaining. +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + +test_expect_success 'status: two commands done with some white lines in done file' ' + FAKE_LINES=1 exec_exit_15 2 3 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~3) + COMMIT4=$(git rev-parse HEAD) + COMMIT3=$(git rev-parse HEAD^) + COMMIT2=$(git rev-parse HEAD^^) + test_must_fail git rebase -i HEAD~3 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_commit + exec exit 15 +Next commands to do (2 remaining commands): + pick $COMMIT3 three_commit + pick $COMMIT4 four_commit + (use git rebase --edit-todo to view and edit) +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + +test_expect_success 'status: two remaining commands with some white lines in todo file' ' + FAKE_LINES=1 2 exec_exit_15 3 4 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~4) + COMMIT4=$(git rev-parse HEAD) + COMMIT3=$(git rev-parse HEAD^) + COMMIT2=$(git rev-parse HEAD^^) + test_must_fail git rebase -i HEAD~4 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last commands done (3 commands done): + pick $COMMIT2 two_commit + exec exit 15 + (see more in file .git/rebase-merge/done) +Next commands to do (2 remaining commands): + pick $COMMIT3 three_commit + pick $COMMIT4 four_commit + (use git rebase --edit-todo to view and edit) +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + test_done -- 2.4.2.342.ga3499d3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On undoing a forced push
On Tue, Jun 09, 2015 at 07:36:20PM +0530, Sitaram Chamarty wrote: This patch prints the latest SHA-1 before the forced push in full. He then can do git push remote +old-sha1:ref He does not even need to have the objects that old-sha1 refers to. We could simply push an empty pack and the the remote will happily accept the force, assuming garbage collection has not happened. But that's another and a little more complex patch. If I am not mistaken, we actively prevent people from downloading an unreferenced SHA (such as would happen if you overwrote refs that contained sensitive information like passwords). Wouldn't allowing the kind of push you just described, require negating that protection? No, this has always worked. If you have write access to a repository, you can fetch anything from it with this trick. Even if we blocked this, there are other ways to leak information. For instance, I can push up objects that are similar to the target object, claim to have the target object, and then hope git will make a delta between my similar object and the target. Iterate on the similar object and you can eventually figure out what is in the target object. This is one of the reasons we do not share objects between public and private forks at GitHub. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: gitscm vs. git-scm
On Tue, Jun 09, 2015 at 03:08:46PM +0200, Matthieu Moy wrote: I guess gitscm.com should just redirect to git-scm.com (sending the Location: field, and/or with stg like meta http-equiv=Refresh content=0; URL=http://git-scm.com; / We (the git project) don't own gitscm.com. I don't recognize the name on the whois: $ whois gitscm.com | grep ^Registrant Registrant Name: Jimmy Ho Registrant Organization: Jimhoyd LLC Registrant Street: PO Box 182 Registrant City: Walnut Creek Registrant State/Province: California Registrant Postal Code: 94597 Registrant Country: United States Registrant Phone: +1.4158468899 Registrant Phone Ext: Registrant Fax: Registrant Fax Ext: Registrant Email: supp...@jimhoyd.com Doesn't seem like a domain squatter, as it redirects to git-scm.com (albeit weirdly) and does not seem to inject ads or other crap. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On undoing a forced push
Hi, On 2015-06-09 16:06, Sitaram Chamarty wrote: On 06/09/2015 05:42 PM, Duy Nguyen wrote: From a thread on Hacker News. It seems that if a user does not have access to the remote's reflog and accidentally forces a push to a ref, how does he recover it? In order to force push again to revert it back, he would need to know the remote's old SHA-1. Local reflog does not help because remote refs are not updated during a push. This patch prints the latest SHA-1 before the forced push in full. He then can do git push remote +old-sha1:ref He does not even need to have the objects that old-sha1 refers to. We could simply push an empty pack and the the remote will happily accept the force, assuming garbage collection has not happened. But that's another and a little more complex patch. If I am not mistaken, we actively prevent people from downloading an unreferenced SHA (such as would happen if you overwrote refs that contained sensitive information like passwords). Wouldn't allowing the kind of push you just described, require negating that protection? I believe that to be the case. Sorry to chime in so late in the discussion, but I think that the `--force-with-lease` option is what you are looking for. It allows you to force-push *but only* if the forced push would overwrite the ref we expect, i.e. (simplified, but you get the idea) `git push --force-with-lease remote ref` will *only* succeed if the remote's ref agrees with the local `refs/remotes/remote/ref`. If you use `--force-with-lease`, you simply cannot force-forget anything on the remote side that you cannot undo (because you have everything locally you need to undo it). Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8] Convert struct ref to use object_id.
Use struct object_id in three fields in struct ref and convert all the
necessary places that use it.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
builtin/clone.c| 16 +++---
builtin/fetch-pack.c | 4 ++--
builtin/fetch.c| 50 +--
builtin/ls-remote.c| 2 +-
builtin/receive-pack.c | 2 +-
builtin/remote.c | 12 +--
connect.c | 2 +-
fetch-pack.c | 18
http-push.c| 46 +++
http.c | 2 +-
remote-curl.c | 10 -
remote.c | 58 +-
remote.h | 6 +++---
send-pack.c| 16 +++---
transport-helper.c | 18
transport.c| 32 ++--
transport.h| 8 +++
walker.c | 2 +-
18 files changed, 152 insertions(+), 152 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index b878252..c909574 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -505,7 +505,7 @@ static void write_remote_refs(const struct ref *local_refs)
for (r = local_refs; r; r = r-next) {
if (!r-peer_ref)
continue;
- add_packed_ref(r-peer_ref-name, r-old_sha1);
+ add_packed_ref(r-peer_ref-name, r-old_oid.hash);
}
if (commit_packed_refs())
@@ -520,9 +520,9 @@ static void write_followtags(const struct ref *refs, const
char *msg)
continue;
if (ends_with(ref-name, ^{}))
continue;
- if (!has_sha1_file(ref-old_sha1))
+ if (!has_object_file(ref-old_oid))
continue;
- update_ref(msg, ref-name, ref-old_sha1,
+ update_ref(msg, ref-name, ref-old_oid.hash,
NULL, 0, UPDATE_REFS_DIE_ON_ERR);
}
}
@@ -542,7 +542,7 @@ static int iterate_ref_map(void *cb_data, unsigned char
sha1[20])
if (!ref)
return -1;
- hashcpy(sha1, ref-old_sha1);
+ hashcpy(sha1, ref-old_oid.hash);
*rm = ref-next;
return 0;
}
@@ -591,12 +591,12 @@ static void update_head(const struct ref *our, const
struct ref *remote,
/* Local default branch link */
create_symref(HEAD, our-name, NULL);
if (!option_bare) {
- update_ref(msg, HEAD, our-old_sha1, NULL, 0,
+ update_ref(msg, HEAD, our-old_oid.hash, NULL, 0,
UPDATE_REFS_DIE_ON_ERR);
install_branch_config(0, head, option_origin,
our-name);
}
} else if (our) {
- struct commit *c = lookup_commit_reference(our-old_sha1);
+ struct commit *c = lookup_commit_reference(our-old_oid.hash);
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
update_ref(msg, HEAD, c-object.sha1,
NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
@@ -606,7 +606,7 @@ static void update_head(const struct ref *our, const struct
ref *remote,
* HEAD points to a branch but we don't know which one.
* Detach HEAD in all these cases.
*/
- update_ref(msg, HEAD, remote-old_sha1,
+ update_ref(msg, HEAD, remote-old_oid.hash,
NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
}
}
@@ -957,7 +957,7 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
* remote HEAD check.
*/
for (ref = refs; ref; ref = ref-next)
- if (is_null_sha1(ref-old_sha1)) {
+ if (is_null_oid(ref-old_oid)) {
complete_refs_before_fetch = 0;
break;
}
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 4a6b340..19215b3 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -17,7 +17,7 @@ static void add_sought_entry_mem(struct ref ***sought, int
*nr, int *alloc,
unsigned char sha1[20];
if (namelen 41 name[40] == ' ' !get_sha1_hex(name, sha1)) {
- hashcpy(ref-old_sha1, sha1);
+ hashcpy(ref-old_oid.hash, sha1);
name += 41;
namelen -= 41;
}
@@ -210,7 +210,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char
*prefix)
while (ref) {
printf(%s %s\n,
- sha1_to_hex(ref-old_sha1), ref-name);
+ oid_to_hex(ref-old_oid), ref-name);
ref = ref-next;
}
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8d5b2db..af42cde 100644
---
[PATCH 2/8] sha1_file: introduce has_object_file helper.
Add has_object_file, which is a wrapper around has_sha1_file, but for
struct object_id.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
cache.h | 3 +++
sha1_file.c | 5 +
2 files changed, 8 insertions(+)
diff --git a/cache.h b/cache.h
index 571c98f..fa1f067 100644
--- a/cache.h
+++ b/cache.h
@@ -946,6 +946,9 @@ extern int has_sha1_pack(const unsigned char *sha1);
*/
extern int has_sha1_file(const unsigned char *sha1);
+/* Same as the above, except for struct object_id. */
+extern int has_object_file(const struct object_id *oid);
+
/*
* Return true iff an alternate object database has a loose object
* with the specified name. This function does not respect replace
diff --git a/sha1_file.c b/sha1_file.c
index 7e38148..09f7f03 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3173,6 +3173,11 @@ int has_sha1_file(const unsigned char *sha1)
return find_pack_entry(sha1, e);
}
+int has_object_file(const struct object_id *oid)
+{
+ return has_sha1_file(oid-hash);
+}
+
static void check_tree(const void *buf, size_t size)
{
struct tree_desc desc;
--
2.4.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] ref_newer: convert to use struct object_id
Convert ref_newer and its caller to use struct object_id instead of
unsigned char *.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
builtin/remote.c | 2 +-
http-push.c | 4 ++--
remote.c | 8
remote.h | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index fa4d04c..0efc388 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -411,7 +411,7 @@ static int get_push_ref_states(const struct ref
*remote_refs,
else if (is_null_oid(ref-old_oid))
info-status = PUSH_STATUS_CREATE;
else if (has_object_file(ref-old_oid)
-ref_newer(ref-new_oid.hash, ref-old_oid.hash))
+ref_newer(ref-new_oid, ref-old_oid))
info-status = PUSH_STATUS_FASTFORWARD;
else
info-status = PUSH_STATUS_OUTOFDATE;
diff --git a/http-push.c b/http-push.c
index d054fdb..0e688a7 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1899,8 +1899,8 @@ int main(int argc, char **argv)
!is_null_oid(ref-old_oid)
!ref-force) {
if (!has_object_file(ref-old_oid) ||
- !ref_newer(ref-peer_ref-new_oid.hash,
- ref-old_oid.hash)) {
+ !ref_newer(ref-peer_ref-new_oid,
+ ref-old_oid)) {
/*
* We do not have the remote ref, or
* we know that the remote ref is not
diff --git a/remote.c b/remote.c
index 706d2fb..675cb23 100644
--- a/remote.c
+++ b/remote.c
@@ -1626,7 +1626,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int
send_mirror,
else if
(!lookup_commit_reference_gently(ref-old_oid.hash, 1) ||
!lookup_commit_reference_gently(ref-new_oid.hash, 1))
reject_reason = REF_STATUS_REJECT_NEEDS_FORCE;
- else if (!ref_newer(ref-new_oid.hash,
ref-old_oid.hash))
+ else if (!ref_newer(ref-new_oid, ref-old_oid))
reject_reason =
REF_STATUS_REJECT_NONFASTFORWARD;
}
@@ -1982,7 +1982,7 @@ static void unmark_and_free(struct commit_list *list,
unsigned int mark)
}
}
-int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1)
+int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
{
struct object *o;
struct commit *old, *new;
@@ -1993,12 +1993,12 @@ int ref_newer(const unsigned char *new_sha1, const
unsigned char *old_sha1)
* Both new and old must be commit-ish and new is descendant of
* old. Otherwise we require --force.
*/
- o = deref_tag(parse_object(old_sha1), NULL, 0);
+ o = deref_tag(parse_object(old_oid-hash), NULL, 0);
if (!o || o-type != OBJ_COMMIT)
return 0;
old = (struct commit *) o;
- o = deref_tag(parse_object(new_sha1), NULL, 0);
+ o = deref_tag(parse_object(new_oid-hash), NULL, 0);
if (!o || o-type != OBJ_COMMIT)
return 0;
new = (struct commit *) o;
diff --git a/remote.h b/remote.h
index 163ea5e..4a039ba 100644
--- a/remote.h
+++ b/remote.h
@@ -150,7 +150,7 @@ extern struct ref **get_remote_heads(int in, char *src_buf,
size_t src_len,
struct sha1_array *shallow);
int resolve_remote_symref(struct ref *ref, struct ref *list);
-int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);
+int ref_newer(const struct object_id *new_oid, const struct object_id
*old_oid);
/*
* Remove and free all but the first of any entries in the input list
--
2.4.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] object_id part 2
This is another series of conversions to struct object_id. This series converts more of the refs code and struct object to use struct object_id. It introduces two additional helper functions. One is has_object_file, which is the equivalent of has_sha1_file. The name was chosen to be slightly more logical than has_oid_file, although it can be changed if desired. The second helper function is parse_oid_hex. It works much like get_oid_hex, but it takes an optional length argument and returns the number of bytes parsed on success (40) and 0 on error. It's designed to be more useful in conditionals and to enable us to avoid having to write GIT_SHA1_HEXSZ in favor of simply using the return value. The final piece in this series is the conversion of struct object to use struct object_id. This is a necessarily large patch because of the large number of places this code is used. brian m. carlson (8): refs: convert some internal functions to use object_id sha1_file: introduce has_object_file helper. Convert struct ref to use object_id. Add a utility function to make parsing hex values easier. add_sought_entry_mem: convert to struct object_id parse_fetch: convert to use struct object_id ref_newer: convert to use struct object_id Convert struct object to object_id archive.c| 6 +-- bisect.c | 10 ++--- branch.c | 2 +- builtin/blame.c | 46 ++-- builtin/branch.c | 2 +- builtin/checkout.c | 24 +-- builtin/clone.c | 18 builtin/commit-tree.c| 4 +- builtin/commit.c | 8 ++-- builtin/describe.c | 20 - builtin/diff-tree.c | 12 +++--- builtin/diff.c | 12 +++--- builtin/fast-export.c| 34 +++ builtin/fetch-pack.c | 14 --- builtin/fetch.c | 54 builtin/fmt-merge-msg.c | 6 +-- builtin/for-each-ref.c | 12 +++--- builtin/fsck.c | 34 +++ builtin/grep.c | 6 +-- builtin/index-pack.c | 10 ++--- builtin/log.c| 24 +-- builtin/ls-remote.c | 2 +- builtin/merge-base.c | 8 ++-- builtin/merge-tree.c | 6 +-- builtin/merge.c | 60 +-- builtin/name-rev.c | 12 +++--- builtin/notes.c | 2 +- builtin/pack-objects.c | 16 +++ builtin/receive-pack.c | 2 +- builtin/reflog.c | 4 +- builtin/remote.c | 12 +++--- builtin/replace.c| 2 +- builtin/reset.c | 6 +-- builtin/rev-list.c | 18 builtin/rev-parse.c | 4 +- builtin/shortlog.c | 2 +- builtin/show-branch.c| 8 ++-- builtin/tag.c| 4 +- builtin/unpack-objects.c | 10 ++--- bundle.c | 10 ++--- cache-tree.c | 2 +- cache.h | 12 ++ combine-diff.c | 4 +- commit.c | 32 +++--- connect.c| 2 +- decorate.c | 2 +- diff-lib.c | 2 +- fetch-pack.c | 24 +-- fsck.c | 10 ++--- hex.c| 7 http-backend.c | 2 +- http-push.c | 88 +++ http.c | 2 +- line-log.c | 6 +-- list-objects.c | 4 +- log-tree.c | 32 +++--- merge-blobs.c| 4 +- merge-recursive.c| 22 +- merge.c | 2 +- notes-merge.c| 24 +-- object.c | 8 ++-- object.h | 2 +- pack-bitmap-write.c | 16 +++ pack-bitmap.c| 34 +++ patch-ids.c | 6 +-- pretty.c | 18 refs.c | 106 +++ remote-curl.c| 21 +- remote.c | 68 +++--- remote.h | 8 ++-- revision.c | 48 ++--- send-pack.c | 16 +++ sequencer.c | 38 - server-info.c| 2 +- sha1_file.c | 5 +++ sha1_name.c | 20 - shallow.c| 6 +-- submodule.c | 8 ++-- tag.c| 10 ++--- test-match-trees.c | 2 +- transport-helper.c | 18 transport.c | 32 +++--- transport.h | 8 ++-- tree.c | 10 ++--- upload-pack.c| 26 ++-- walker.c | 18 wt-status.c | 2 +- 87 files changed, 706 insertions(+), 679 deletions(-) -- 2.4.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to
[PATCH 1/8] refs: convert some internal functions to use object_id
Convert several internal functions in refs.c to use struct object_id,
and use the GIT_SHA1_HEXSZ constants in parse_ref_line.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
refs.c | 104 -
1 file changed, 52 insertions(+), 52 deletions(-)
diff --git a/refs.c b/refs.c
index a742d79..b391a0f 100644
--- a/refs.c
+++ b/refs.c
@@ -1158,7 +1158,7 @@ static const char PACKED_REFS_HEADER[] =
* Return a pointer to the refname within the line (null-terminated),
* or NULL if there was a problem.
*/
-static const char *parse_ref_line(struct strbuf *line, unsigned char *sha1)
+static const char *parse_ref_line(struct strbuf *line, struct object_id *oid)
{
const char *ref;
@@ -1170,15 +1170,15 @@ static const char *parse_ref_line(struct strbuf *line,
unsigned char *sha1)
* +1 (space in between hex and name)
* +1 (newline at the end of the line)
*/
- if (line-len = 42)
+ if (line-len = GIT_SHA1_HEXSZ + 2)
return NULL;
- if (get_sha1_hex(line-buf, sha1) 0)
+ if (get_oid_hex(line-buf, oid) 0)
return NULL;
- if (!isspace(line-buf[40]))
+ if (!isspace(line-buf[GIT_SHA1_HEXSZ]))
return NULL;
- ref = line-buf + 41;
+ ref = line-buf + GIT_SHA1_HEXSZ + 1;
if (isspace(*ref))
return NULL;
@@ -1223,7 +1223,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
enum { PEELED_NONE, PEELED_TAGS, PEELED_FULLY } peeled = PEELED_NONE;
while (strbuf_getwholeline(line, f, '\n') != EOF) {
- unsigned char sha1[20];
+ struct object_id oid;
const char *refname;
const char *traits;
@@ -1236,17 +1236,17 @@ static void read_packed_refs(FILE *f, struct ref_dir
*dir)
continue;
}
- refname = parse_ref_line(line, sha1);
+ refname = parse_ref_line(line, oid);
if (refname) {
int flag = REF_ISPACKED;
if (check_refname_format(refname,
REFNAME_ALLOW_ONELEVEL)) {
if (!refname_is_safe(refname))
die(packed refname is dangerous: %s,
refname);
- hashclr(sha1);
+ oidclr(oid);
flag |= REF_BAD_NAME | REF_ISBROKEN;
}
- last = create_ref_entry(refname, sha1, flag, 0);
+ last = create_ref_entry(refname, oid.hash, flag, 0);
if (peeled == PEELED_FULLY ||
(peeled == PEELED_TAGS starts_with(refname,
refs/tags/)))
last-flag |= REF_KNOWS_PEELED;
@@ -1257,8 +1257,8 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
line.buf[0] == '^'
line.len == PEELED_LINE_LENGTH
line.buf[PEELED_LINE_LENGTH - 1] == '\n'
- !get_sha1_hex(line.buf + 1, sha1)) {
- hashcpy(last-u.value.peeled.hash, sha1);
+ !get_oid_hex(line.buf + 1, oid)) {
+ oidcpy(last-u.value.peeled, oid);
/*
* Regardless of what the file header said,
* we definitely know the value of *this*
@@ -1352,7 +1352,7 @@ static void read_loose_refs(const char *dirname, struct
ref_dir *dir)
strbuf_add(refname, dirname, dirnamelen);
while ((de = readdir(d)) != NULL) {
- unsigned char sha1[20];
+ struct object_id oid;
struct stat st;
int flag;
const char *refdir;
@@ -1374,27 +1374,27 @@ static void read_loose_refs(const char *dirname, struct
ref_dir *dir)
refname.len, 1));
} else {
if (*refs-name) {
- hashclr(sha1);
+ oidclr(oid);
flag = 0;
- if (resolve_gitlink_ref(refs-name,
refname.buf, sha1) 0) {
- hashclr(sha1);
+ if (resolve_gitlink_ref(refs-name,
refname.buf, oid.hash) 0) {
+ oidclr(oid);
flag |= REF_ISBROKEN;
}
} else if (read_ref_full(refname.buf,
RESOLVE_REF_READING,
-sha1, flag)) {
- hashclr(sha1);
+oid.hash, flag)) {
+
[PATCH 6/8] parse_fetch: convert to use struct object_id
Convert the parse_fetch function to use struct object_id. Switch from
get_sha1_hex to parse_oid_hex to avoid hard-coding constants. Remove
the strlen check as parse_oid_hex will fail safely on receiving a
too-short NUL-terminated string.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
remote-curl.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 80cb4c7..46206a0 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -802,19 +802,20 @@ static void parse_fetch(struct strbuf *buf)
if (skip_prefix(buf-buf, fetch , p)) {
const char *name;
struct ref *ref;
- unsigned char old_sha1[20];
+ struct object_id old_oid;
+ int hexlen;
- if (strlen(p) 40 || get_sha1_hex(p, old_sha1))
+ if (!(hexlen = parse_oid_hex(p, -1, old_oid)))
die(protocol error: expected sha/ref, got
%s', p);
- if (p[40] == ' ')
- name = p + 41;
- else if (!p[40])
+ if (p[hexlen] == ' ')
+ name = p + hexlen + 1;
+ else if (!p[hexlen])
name = ;
else
die(protocol error: expected sha/ref, got
%s', p);
ref = alloc_ref(name);
- hashcpy(ref-old_oid.hash, old_sha1);
+ oidcpy(ref-old_oid, old_oid);
*list = ref;
list = ref-next;
--
2.4.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] Add a utility function to make parsing hex values easier.
get_oid_hex is already available for parsing hex object IDs into struct
object_id, but parsing code still must hard-code the number of bytes
read. Introduce parse_oid_hex, which accepts an optional length, and
also returns the number of bytes parsed on success, or 0 on failure.
This makes it easier for code not to assume fixed values when parsing,
and to move to larger hash functions in the future.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
cache.h | 9 +
hex.c | 7 +++
2 files changed, 16 insertions(+)
diff --git a/cache.h b/cache.h
index fa1f067..f3b829f 100644
--- a/cache.h
+++ b/cache.h
@@ -1012,6 +1012,15 @@ extern int for_each_abbrev(const char *prefix,
each_abbrev_fn, void *);
extern int get_sha1_hex(const char *hex, unsigned char *sha1);
extern int get_oid_hex(const char *hex, struct object_id *sha1);
+/*
+ * Like get_oid_hex, but accepts an optional length argument, which may be -1
+ * if the string is terminated by a non-hex character. As with get_oid_hex,
+ * reading stops if a NUL is encountered. Returns the number of characters
+ * read (40) on success and 0 on failure. This is designed to be easier to
+ * use for parsing data than get_oid_hex.
+ */
+extern int parse_oid_hex(const char *hex, int len, struct object_id *oid);
+
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer
result! */
extern char *oid_to_hex(const struct object_id *oid); /* same static buffer
as sha1_to_hex */
extern int read_ref_full(const char *refname, int resolve_flags,
diff --git a/hex.c b/hex.c
index 899b74a..ba196d7 100644
--- a/hex.c
+++ b/hex.c
@@ -61,6 +61,13 @@ int get_oid_hex(const char *hex, struct object_id *oid)
return get_sha1_hex(hex, oid-hash);
}
+int parse_oid_hex(const char *hex, int len, struct object_id *oid)
+{
+ if (len != -1 len GIT_SHA1_HEXSZ)
+ return 0;
+ return get_sha1_hex(hex, oid-hash) ? GIT_SHA1_HEXSZ : 0;
+}
+
char *sha1_to_hex(const unsigned char *sha1)
{
static int bufno;
--
2.4.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] add_sought_entry_mem: convert to struct object_id
Convert this function to use struct object_id. Use parse_oid_hex to
avoid having to hard-code the number of bytes to be parsed.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
builtin/fetch-pack.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 19215b3..f3b89a9 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -14,12 +14,14 @@ static void add_sought_entry_mem(struct ref ***sought, int
*nr, int *alloc,
const char *name, int namelen)
{
struct ref *ref = xcalloc(1, sizeof(*ref) + namelen + 1);
- unsigned char sha1[20];
+ struct object_id oid;
+ int hexlen;
- if (namelen 41 name[40] == ' ' !get_sha1_hex(name, sha1)) {
- hashcpy(ref-old_oid.hash, sha1);
- name += 41;
- namelen -= 41;
+ if ((hexlen = parse_oid_hex(name, namelen, oid)) namelen hexlen +
1
+ name[hexlen] == ' ') {
+ oidcpy(ref-old_oid, oid);
+ name += hexlen + 1;
+ namelen -= hexlen + 1;
}
memcpy(ref-name, name, namelen);
--
2.4.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/13] delete_ref(): move declaration to refs.h
On Tue, Jun 9, 2015 at 3:10 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
On 06/08/2015 06:43 PM, Stefan Beller wrote:
On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu
wrote:
[...]
+/*
+ * Delete the specified reference. If old_sha1 is non-NULL and not
+ * NULL_SHA1, then verify that the current value of the reference is
+ * old_sha1 before deleting it.
And here I wondered what the distinction between NULL and non-NULL,
but NULL_SHA1
is and digging into the code, there is none. (As you can also see in
this patch above with
(old_sha1 !is_null_sha1(old_sha1)) ? old_sha1 : NULL,
but when digging deeper, the !is_null_sha1(old_sha1) is an arbitrary
limitation (i.e.
ref_transaction_delete and ref_transaction_update don't differ between
those two cases.)
The distinction comes in at lock_ref_sha1_basic, where I think we may
want to get rid of
the is_null_sha1 check and depend on NULL/non-NULL as the difference
for valid and invalid
sha1's?
I'm having a little trouble understanding your comment.
The convention for ref_transaction_update() and friends is that
* old_sha1 == NULL
We don't care whether the reference existed prior to the
update, nor what its value was.
* *old_sha1 is NULL_SHA1
(by which I mean that old_sha1 points at 20 zero bytes; I hope
that's clear even though NULL_SHA1 is not actually defined
anywhere): The reference must *not* have existed prior to the
update.
Ok that's what I was missing.
* old_sha1 has some other value
The reference must have had that value prior to the update.
lock_ref_sha1_basic() distinguishes between { NULL vs. NULL_SHA1 vs.
other values } in the same way that ref_transaction_update() does.
The delete_ref() function doesn't follow the same convention. It treats
NULL and NULL_SHA1 identically, as don't care.
It probably makes sense to change delete_ref() use the same convention
as ref_transaction_update(), but there are quite a few callers and I
didn't have the energy to review them all as part of this patch series.
So I left it unchanged and just documented the status quo better.
That said, do we want to add another sentence to the doc here saying
non-NULL and not
NULL_SHA1 are treated the same or is it clear enough?
With or without this question addressed:
Reviewed-by: Stefan Beller sbel...@google.com
In set notation,
non-NULL =
non-NULL and not NULL_SHA1 ∪
non-NULL and equal to NULL_SHA1
The latter two are *not* treated the same, so I don't see how we can
claim that non-NULL and not NULL_SHA1 are treated the same. I must
be misunderstanding you.
Would it help if I changed the comment to
Delete the specified reference. If old_sha1 is non-NULL and not
NULL_SHA1, then verify that the current value of the reference is
old_sha1 before deleting it. If old_sha1 is NULL or NULL_SHA1,
delete the reference it it exists, regardless of its old value.
?
This is very clear to me.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On undoing a forced push
On Tue, Jun 9, 2015 at 9:29 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi, On 2015-06-09 16:06, Sitaram Chamarty wrote: On 06/09/2015 05:42 PM, Duy Nguyen wrote: From a thread on Hacker News. It seems that if a user does not have access to the remote's reflog and accidentally forces a push to a ref, how does he recover it? In order to force push again to revert it back, he would need to know the remote's old SHA-1. Local reflog does not help because remote refs are not updated during a push. This patch prints the latest SHA-1 before the forced push in full. He then can do git push remote +old-sha1:ref He does not even need to have the objects that old-sha1 refers to. We could simply push an empty pack and the the remote will happily accept the force, assuming garbage collection has not happened. But that's another and a little more complex patch. If I am not mistaken, we actively prevent people from downloading an unreferenced SHA (such as would happen if you overwrote refs that contained sensitive information like passwords). Wouldn't allowing the kind of push you just described, require negating that protection? I believe that to be the case. Sorry to chime in so late in the discussion, but I think that the `--force-with-lease` option is what you are looking for. It allows you to force-push *but only* if the forced push would overwrite the ref we expect, i.e. (simplified, but you get the idea) `git push --force-with-lease remote ref` will *only* succeed if the remote's ref agrees with the local `refs/remotes/remote/ref`. Yeah that was my first thought as well. It's unfortunate that --force-with-lease is not as well known though (it wasn't there first, so many people picked it up and it's good enough to not pickup other --force-with-foo options). Maybe we should add the option receive.denyNonFastForwards = onlyWithLease instead? Thanks, Stefan If you use `--force-with-lease`, you simply cannot force-forget anything on the remote side that you cannot undo (because you have everything locally you need to undo it). Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git lock files (Was: GIT for Microsoft Access projects)
Just because Git allows distributed workflows, doesn't mean we
should only focus on being distributed IMHO.
The question for content not being mergable easily pops up all
the time. (Game/Graphics designers, documents, all this binary
stuff, where there is no good merge driver).
I could imagine a git lock command which looks like this:
git config lock.centralServer origin
git config lock.defaultBranch master
git lock add [branch] [--] path/to/file
git lock remove [branch] [--] path/to/file
git lock ls [branch]
And the way this is implemented is roughly (unoptimized, just showing
how you would achieve this with todays command set):
git fetch --depth=1 $(git config --get lock.centralServer) refs/locks/$(git
config --get lock.defaultBranch)
git checkout refs/locks/$(git config --get lock.centralServer)/$(git config
--get lock.defaultBranch)
switch(option) {
case add:
if exist path/to/file
return -1
else
echo $(git config --get user.name) $(date) path/to/file
git add path/to/file git commit add new lock
fi
case remove:
if exist path/to/file
# todo: check if the same user locked it before
rm path/to/file
else
return -1
fi
case ls:
ls -R .
}
git push $(git config --get lock.centralServer) refs/locks/$(git config
--get lock.defaultBranch)
git restore working tree, branch
That said you could just manipulate the git objects directly, no need
to check out to the working dir.
The server would only need to allow pushes to a refs/locks directory and be
done.
the client side would need to have a plumbing command, so you could easily
integrate
a git locking to your application if you don't want to provide a merge driver.
Thanks,
Stefan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/7] t9001-send-email: move script creation in a setup test
Move the creation of the scripts used in to-cmd and cc-cmd tests in a setup test to make them available for later tests. This will be used in the next commit. Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr --- t/t9001-send-email.sh | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 7be14a4..e63fc83 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -312,13 +312,19 @@ test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email' ) ' +test_expect_success $PREREQ 'setup tocmd and cccmd scripts' ' + write_script tocmd-sed -\EOF + sed -n -e s/^tocmd--//p $1 + EOF + write_script cccmd-sed -\EOF + sed -n -e s/^cccmd--//p $1 + EOF +' + test_expect_success $PREREQ 'tocmd works' ' clean_fake_sendmail cp $patches tocmd.patch echo tocmd--to...@example.com tocmd.patch - write_script tocmd-sed -\EOF - sed -n -e s/^tocmd--//p $1 - EOF git send-email \ --from=Example nob...@example.com \ --to-cmd=./tocmd-sed \ @@ -332,9 +338,6 @@ test_expect_success $PREREQ 'cccmd works' ' clean_fake_sendmail cp $patches cccmd.patch echo cccmd-- cc...@example.com cccmd.patch - write_script cccmd-sed -\EOF - sed -n -e s/^cccmd--//p $1 - EOF git send-email \ --from=Example nob...@example.com \ --to=nob...@example.com \ -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] changes from last version
CHANGES (since last submitted version) 1/7 : identical to previous 1/5 2/7 : identical to previous 2/5 3/7 : identical to previous 3/5 4/7 : Modification previously done by 5/5 (refactoring address process) 5/7 : identical modulo a minor change at hunk @@ -1023,8 +1009,13 @@ in the function process_address_list due to 4/7 6/7 : new commit 7/7 : new commit CONFLICT (if merging to pu) There will be a trivial conflict if merging to pu (with 514554cf, 3169e06d and 6be02640 from Eric Sunshine and Allen Hubbe patch series). Suppressing conflict marks resolve it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] index-pack: avoid excessive re-reading of pack directory
On Tue, Jun 09, 2015 at 01:24:36PM -0400, Jeff King wrote: I tested this on my system, and confirmed that for a git clone --no-local --bare git.git: 1. It cuts the number of openat/getdents/close syscalls by several orders of magnitude. 2. The overall time drops from ~11.4s to ~10.5s. I suppose if I timed only the `index-pack` process, it would be even higher (as a percentage improvement). Just for fun, I did a git pack-objects --all --stdout from linux.git, and then timed git index-pack --stdin on it in an empty repo. With a configured alternate pointing to another empty repo, just to make it more unfair. And then I stored it all on a ramdisk to emphasize the cost of the syscalls versus hitting the disk. The numbers I got were: [before] real2m13.093s user3m31.884s sys 0m55.208s [after] real1m40.389s user3m10.776s sys 0m26.012s That's sort of a ridiculous test, but it does show that this was having some impact even on normal systems without insane syscall latencies. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] object name: introduce '^{/!-negative pattern}' notation
On Mon, Jun 8, 2015 at 5:39 PM, Junio C Hamano gits...@pobox.com wrote:
Will Palmer wmpal...@gmail.com writes:
diff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh
index e0fe102..8a5983f 100755
--- a/t/t1511-rev-parse-caret.sh
+++ b/t/t1511-rev-parse-caret.sh
@@ -19,13 +19,17 @@ test_expect_success 'setup' '
echo modified a-blob
git add -u
git commit -m Modified
+ git branch modref
This probably belongs to the previous step, no?
As it isn't referenced until the negative tests, I didn't bother adding
it in the verify the way things are tests. Funny that it was mentioned,
as I *did* originally have it in the first commit, but I moved it to the
commit in which it was first used, so that it would be easier to notice.
+test_expect_success 'ref^{/!-}' '
+ test_must_fail git rev-parse master^{/!-}
+'
H, we must fail because...? We are looking for something that
does not contain an empty string, which by definition does not
exist.
Funny, but is correct ;-).
This is left-over from the original patch's logic, which included a
short-circuit to avoid an empty regex (as per 4322842 get_sha1: handle
special case $commit^{/})... which I now realise perhaps should
have been simply rephrased, rather than ommitted entirely.
I feel like adding something like:
8--
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -737,11 +737,15 @@ static int peel_onion(const char *name, int len,
unsigned char *sha1)
/*
* $commit^{/}. Some regex implementation may reject.
-* We don't need regex anyway. '' pattern always matches.
+* We don't need regex anyway. '' pattern always matches,
+* and '!' pattern never matches.
*/
if (sp[1] == '}')
return 0;
+ if (sp[1] == '!' sp[2] == '-' sp[3] == '}')
+ return -1;
+
prefix = xstrndup(sp + 1, name + len - 1 - (sp + 1));
commit_list_insert((struct commit *)o, list);
ret = get_sha1_oneline(prefix, sha1, list);
--8
...would be the wrong place for this short-circuit check, in light of
discussion around extensibility; so, I'll see how it looks moving that
into get_sha1_oneline(...)
+test_expect_success 'ref^{/!-.}' '
+ test_must_fail git rev-parse master^{/!-.}
+'
Likewise. I however wonder if we catch a commit without any message
(which you probably have to craft with either commit-tree or
hash-object), but that falls into the curiosity not the
practicality category.
A commit with no message should indeed by returned by 'master^{/!-.}',
or at least, that is the intent. This test is only meant to cover the
result of there being no matching commit, however.
In summary: it looks like I'll be sending another one.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option
Karthik Nayak karthik@gmail.com writes:
@@ -54,7 +59,6 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
- memset(ref_cbdata, 0, sizeof(ref_cbdata));
I cannot quite see how this change relates to the addition of the
new option.
Well if we memset() after calling parse_opt_points_at(), we loose all
the information we would have obtained.
So the memset() is moved to an earlier location.
which I did not see, because I expected the code to follow the usual
no decl-after-statement pattern. IOW
int maxcount = 0, quote_style = 0;
struct ref_filter_cbdata ref_cbdata;
+memset(ref_cbdata, 0, sizeof(ref_cbdata));
struct option opts[] = {
OPT_BIT('s', shell, quote_style,
Don't do that. Always start your function like so:
type funcname(args)
{
declarations;
first statement;
...
with no blank line within declarations block and a blank line after
the declarations block.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 1/5] setup: add gentle version of read_gitfile
read_gitfile will die on most error cases. This makes it unsuitable
for speculative calls. Extract the core logic and provide a gentle
version that returns NULL on failure.
The first usecase of the new gentle version will be to probe for
submodules during git clean.
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Jeff King p...@peff.net
Signed-off-by: Erik Elfström erik.elfst...@gmail.com
---
cache.h | 11 -
setup.c | 84 ++---
2 files changed, 75 insertions(+), 20 deletions(-)
diff --git a/cache.h b/cache.h
index 571c98f..25578cb 100644
--- a/cache.h
+++ b/cache.h
@@ -446,7 +446,16 @@ extern int get_common_dir(struct strbuf *sb, const char
*gitdir);
extern const char *get_git_namespace(void);
extern const char *strip_namespace(const char *namespaced_ref);
extern const char *get_git_work_tree(void);
-extern const char *read_gitfile(const char *path);
+
+#define READ_GITFILE_ERR_STAT_FAILED 1
+#define READ_GITFILE_ERR_NOT_A_FILE 2
+#define READ_GITFILE_ERR_OPEN_FAILED 3
+#define READ_GITFILE_ERR_READ_FAILED 4
+#define READ_GITFILE_ERR_INVALID_FORMAT 5
+#define READ_GITFILE_ERR_NO_PATH 6
+#define READ_GITFILE_ERR_NOT_A_REPO 7
+extern const char *read_gitfile_gently(const char *path, int
*return_error_code);
+#define read_gitfile(path) read_gitfile_gently((path), NULL)
extern const char *resolve_gitdir(const char *suspect);
extern void set_git_work_tree(const char *tree);
diff --git a/setup.c b/setup.c
index 863ddfd..4748b63 100644
--- a/setup.c
+++ b/setup.c
@@ -406,35 +406,53 @@ static void update_linked_gitdir(const char *gitfile,
const char *gitdir)
/*
* Try to read the location of the git directory from the .git file,
* return path to git directory if found.
+ *
+ * On failure, if return_error_code is not NULL, return_error_code
+ * will be set to an error code and NULL will be returned. If
+ * return_error_code is NULL the function will die instead (for most
+ * cases).
*/
-const char *read_gitfile(const char *path)
+const char *read_gitfile_gently(const char *path, int *return_error_code)
{
- char *buf;
- char *dir;
+ int error_code = 0;
+ char *buf = NULL;
+ char *dir = NULL;
const char *slash;
struct stat st;
int fd;
ssize_t len;
- if (stat(path, st))
- return NULL;
- if (!S_ISREG(st.st_mode))
- return NULL;
+ if (stat(path, st)) {
+ error_code = READ_GITFILE_ERR_STAT_FAILED;
+ goto cleanup_return;
+ }
+ if (!S_ISREG(st.st_mode)) {
+ error_code = READ_GITFILE_ERR_NOT_A_FILE;
+ goto cleanup_return;
+ }
fd = open(path, O_RDONLY);
- if (fd 0)
- die_errno(Error opening '%s', path);
+ if (fd 0) {
+ error_code = READ_GITFILE_ERR_OPEN_FAILED;
+ goto cleanup_return;
+ }
buf = xmalloc(st.st_size + 1);
len = read_in_full(fd, buf, st.st_size);
close(fd);
- if (len != st.st_size)
- die(Error reading %s, path);
+ if (len != st.st_size) {
+ error_code = READ_GITFILE_ERR_READ_FAILED;
+ goto cleanup_return;
+ }
buf[len] = '\0';
- if (!starts_with(buf, gitdir: ))
- die(Invalid gitfile format: %s, path);
+ if (!starts_with(buf, gitdir: )) {
+ error_code = READ_GITFILE_ERR_INVALID_FORMAT;
+ goto cleanup_return;
+ }
while (buf[len - 1] == '\n' || buf[len - 1] == '\r')
len--;
- if (len 9)
- die(No path in gitfile: %s, path);
+ if (len 9) {
+ error_code = READ_GITFILE_ERR_NO_PATH;
+ goto cleanup_return;
+ }
buf[len] = '\0';
dir = buf + 8;
@@ -448,14 +466,42 @@ const char *read_gitfile(const char *path)
free(buf);
buf = dir;
}
-
- if (!is_git_directory(dir))
- die(Not a git repository: %s, dir);
-
+ if (!is_git_directory(dir)) {
+ error_code = READ_GITFILE_ERR_NOT_A_REPO;
+ goto cleanup_return;
+ }
update_linked_gitdir(path, dir);
path = real_path(dir);
+cleanup_return:
free(buf);
+
+ if (return_error_code)
+ *return_error_code = error_code;
+
+ if (error_code) {
+ if (return_error_code)
+ return NULL;
+
+ switch (error_code) {
+ case READ_GITFILE_ERR_STAT_FAILED:
+ case READ_GITFILE_ERR_NOT_A_FILE:
+ return NULL;
+ case READ_GITFILE_ERR_OPEN_FAILED:
+ die_errno(Error opening '%s', path);
+ case READ_GITFILE_ERR_READ_FAILED:
+ die(Error reading %s, path);
+ case READ_GITFILE_ERR_INVALID_FORMAT:
[PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion
As alias file formats supported by git send-email doesn't take
whitespace into account, it is useless to consider whitespaces in
alias name. remove leading and trailing whitespace before expanding
allow to recognize strings like alias or alias\t passed by --to,
--cc, --bcc options or by the git send-email prompt.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
git-send-email.perl | 1 +
t/t9001-send-email.sh | 24
2 files changed, 25 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index 3d144bd..34c8b8b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -787,6 +787,7 @@ sub expand_aliases {
my %EXPANDED_ALIASES;
sub expand_one_alias {
my $alias = shift;
+ $alias =~ s/^\s+|\s+$//g;
if ($EXPANDED_ALIASES{$alias}) {
die fatal: alias '$alias' expands to itself\n;
}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 9aee474..bbfed56 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1692,4 +1692,28 @@ test_expect_success $PREREQ 'aliases work with email
list' '
test_cmp expected-list actual-list
'
+test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '
+ echo alias to2 t...@example.com .mutt
+ echo alias cc1 Cc 1 c...@example.com .mutt
+ test_config sendemail.aliasesfile .mutt
+ test_config sendemail.aliasfiletype mutt
+ TO1=$(echo QTo 1 t...@example.com | q_to_tab)
+ TO2=$(echo QZto2 | qz_to_tab_space)
+ CC1=$(echo cc1 | append_cr)
+ BCC1=$(echo Q b...@example.com Q | q_to_nul)
+ git send-email \
+ --dry-run \
+ --from=Example f...@example.com \
+ --to=$TO1 \
+ --to=$TO2 \
+ --to= t...@example.com\
+ --cc=$CC1 \
+ --cc=Cc2 c...@example.com \
+ --bcc=$BCC1 \
+ --bcc=b...@example.com \
+ 0001-add-master.patch | replace_variable_fields \
+ actual-list
+ test_cmp expected-list actual-list
+'
+
test_done
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/13] Improve refs module encapsulation
Michael Haggerty mhag...@alum.mit.edu writes: Add functions to the reference API to * Delete a bunch of references at once, but *without* failing the whole transaction if one of the deletions fails. This functionality is used by `git remote remove` and `git remote prune`. * Create initial references during `git clone`. (During clone, references are written directly to the `packed-refs` file without any locking.) Also move the remaining refs function declarations from `cache.h` to `refs.h`. This improves the encapsulation of the refs module. Especially, it means that code outside of the refs module should no longer need to care about the distinction between loose and packed references. Yay. The subject of the series matches this primary value of the topic, even though the cover text makes it sound as if it was just a while we are at adding two functions with Also move... ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] bisect: simplify the add of new bisect terms
Hi,
Thanks for the review,
Junio C Hamano gits...@pobox.com writes:
/*
+ * The terms used for this bisect session are stocked in
+ * BISECT_TERMS: it can be bad/good or new/old.
+ * We read them and stock them to adapt the messages
+ * accordingly. Default is bad/good.
+ */
s/stock/store/ perhaps? I think the idea is not to have this file
in the default case so that absence of it would mean you would be
looking for a transition from (older) good to (more recent) bad.
Yes it is nice but a bisect_terms file is a very useful tool for
verifications at a little cost. For instance, if the user type this:
git bisect start
git bisect bad HEAD
git bisect old HEAD~10
We created the bisect_terms file after the bad then the error is
directly detected when the user type git bisect old.
And I think having we should limit the impact of this good/bad
default case as we would prefer old/new.
+void read_bisect_terms(void)
+{
+struct strbuf str = STRBUF_INIT;
+const char *filename = git_path(BISECT_TERMS);
+FILE *fp = fopen(filename, r);
+
+if (!fp) {
We might want to see why fopen() failed here. If it is because the
file did not exist, great. But otherwise?
Should we display a specific message and cancel the last command?
diff --git a/git-bisect.sh b/git-bisect.sh
index 1f16aaf..529bb43 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,6 +77,7 @@ bisect_start() {
orig_args=$(git rev-parse --sq-quote $@)
bad_seen=0
eval=''
+start_bad_good=0
if test z$(git rev-parse --is-bare-repository) != zfalse
then
mode=--no-checkout
@@ -101,6 +102,9 @@ bisect_start() {
die $(eval_gettext '\$arg' does not
appear to be a valid revision)
break
}
+
+start_bad_good=1
+
It is unclear what this variable means, or what it means to have
zero or one as its value.
This has been done by our elders and we kept because it was
useful. We are however trying to remove it. It is in case of
a 'git bisect start bad_rev good_rev', our function that creates
the bisect_terms is not called. Thus we need to do it manually
in the code.
+get_terms () {
+if test -s $GIT_DIR/BISECT_TERMS
+then
+NAME_BAD=$(sed -n 1p $GIT_DIR/BISECT_TERMS)
+NAME_GOOD=$(sed -n 2p $GIT_DIR/BISECT_TERMS)
It is sad that we need to open the file twice. Can't we do
something using read perhaps?
The cost of it is quite low and we see directly what we meant. We didn't
found a pretty way to read two lines with read.
Don't we want to make sure these two names are sensible? We do not
want an empty-string, for example. I suspect you do not want to
take anything that check-ref-format does not like.
Same comment applies to the C code.
Yes, for now only bad/good/old/new are allowed. But in the future
it will be necessary.
+bisect_voc () {
+case $1 in
+bad) echo bad ;;
+good) echo good ;;
+esac
+}
What is voc?
What if $1 is neither bad/good?
Did you mean to translate 'bad' to $NAME_BAD and 'good' to $NAME_GOOD?
voc stands for vocabulary.
This fonction is mainly used after to display all the builtins possibility. It
is only called internally and if the argument is bad it returns the synonyms of
bad (bad|new in the next patch).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 3/5] t7300: add tests to document behavior of clean and nested git
Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/t7300-clean.sh | 142 +++ 1 file changed, 142 insertions(+) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 99be5d9..fbfdf2d 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh @@ -455,6 +455,148 @@ test_expect_success 'nested git work tree' ' ! test -d bar ' +test_expect_failure 'should clean things that almost look like git but are not' ' + rm -fr almost_git almost_bare_git almost_submodule + mkdir -p almost_git/.git/objects + mkdir -p almost_git/.git/refs + cat almost_git/.git/HEAD -\EOF + garbage + EOF + cp -r almost_git/.git/ almost_bare_git + mkdir almost_submodule/ + cat almost_submodule/.git -\EOF + garbage + EOF + test_when_finished rm -rf almost_* + ## This will fail due to die(Invalid gitfile format: %s, path); in + ## setup.c:read_gitfile. + git clean -f -d + test_path_is_missing almost_git + test_path_is_missing almost_bare_git + test_path_is_missing almost_submodule +' + +test_expect_success 'should not clean submodules' ' + rm -fr repo to_clean sub1 sub2 + mkdir repo to_clean + ( + cd repo + git init + test_commit msg hello.world + ) + git submodule add ./repo/.git sub1 + git commit -m sub1 + git branch before_sub2 + git submodule add ./repo/.git sub2 + git commit -m sub2 + git checkout before_sub2 + to_clean/should_clean.this + git clean -f -d + test_path_is_file repo/.git/index + test_path_is_file repo/hello.world + test_path_is_file sub1/.git + test_path_is_file sub1/hello.world + test_path_is_file sub2/.git + test_path_is_file sub2/hello.world + test_path_is_missing to_clean +' + +test_expect_failure 'should avoid cleaning possible submodules' ' + rm -fr to_clean possible_sub1 + mkdir to_clean possible_sub1 + test_when_finished rm -rf possible_sub* + echo gitdir: foo possible_sub1/.git + possible_sub1/hello.world + chmod 0 possible_sub1/.git + to_clean/should_clean.this + git clean -f -d + test_path_is_file possible_sub1/.git + test_path_is_file possible_sub1/hello.world + test_path_is_missing to_clean +' + +test_expect_failure 'nested (empty) git should be kept' ' + rm -fr empty_repo to_clean + git init empty_repo + mkdir to_clean + to_clean/should_clean.this + git clean -f -d + test_path_is_file empty_repo/.git/HEAD + test_path_is_missing to_clean +' + +test_expect_success 'nested bare repositories should be cleaned' ' + rm -fr bare1 bare2 subdir + git init --bare bare1 + git clone --local --bare . bare2 + mkdir subdir + cp -r bare2 subdir/bare3 + git clean -f -d + test_path_is_missing bare1 + test_path_is_missing bare2 + test_path_is_missing subdir +' + +test_expect_success 'nested (empty) bare repositories should be cleaned even when in .git' ' + rm -fr strange_bare + mkdir strange_bare + git init --bare strange_bare/.git + git clean -f -d + test_path_is_missing strange_bare +' + +test_expect_failure 'nested (non-empty) bare repositories should be cleaned even when in .git' ' + rm -fr strange_bare + mkdir strange_bare + git clone --local --bare . strange_bare/.git + git clean -f -d + test_path_is_missing strange_bare +' + +test_expect_success 'giving path in nested git work tree will remove it' ' + rm -fr repo + mkdir repo + ( + cd repo + git init + mkdir -p bar/baz + test_commit msg bar/baz/hello.world + ) + git clean -f -d repo/bar/baz + test_path_is_file repo/.git/HEAD + test_path_is_dir repo/bar/ + test_path_is_missing repo/bar/baz +' + +test_expect_success 'giving path to nested .git will not remove it' ' + rm -fr repo + mkdir repo untracked + ( + cd repo + git init + test_commit msg hello.world + ) + git clean -f -d repo/.git + test_path_is_file repo/.git/HEAD + test_path_is_dir repo/.git/refs + test_path_is_dir repo/.git/objects + test_path_is_dir untracked/ +' + +test_expect_success 'giving path to nested .git/ will remove contents' ' + rm -fr repo untracked + mkdir repo untracked + ( + cd repo + git init + test_commit msg hello.world + ) + git clean -f -d repo/.git/ + test_path_is_dir repo/.git + test_dir_is_empty repo/.git + test_path_is_dir untracked/ +' + test_expect_success 'force removal
[PATCH v7 2/5] setup: sanity check file size in read_gitfile_gently
read_gitfile_gently will allocate a buffer to fit the entire file that
should be read. Add a sanity check of the file size before opening to
avoid allocating a potentially huge amount of memory if we come across
a large file that someone happened to name .git. The limit is set to
a sufficiently unreasonable size that should never be exceeded by a
genuine .git file.
Signed-off-by: Erik Elfström erik.elfst...@gmail.com
---
cache.h | 1 +
setup.c | 7 +++
2 files changed, 8 insertions(+)
diff --git a/cache.h b/cache.h
index 25578cb..858d9b3 100644
--- a/cache.h
+++ b/cache.h
@@ -454,6 +454,7 @@ extern const char *get_git_work_tree(void);
#define READ_GITFILE_ERR_INVALID_FORMAT 5
#define READ_GITFILE_ERR_NO_PATH 6
#define READ_GITFILE_ERR_NOT_A_REPO 7
+#define READ_GITFILE_ERR_TOO_LARGE 8
extern const char *read_gitfile_gently(const char *path, int
*return_error_code);
#define read_gitfile(path) read_gitfile_gently((path), NULL)
extern const char *resolve_gitdir(const char *suspect);
diff --git a/setup.c b/setup.c
index 4748b63..e76955e 100644
--- a/setup.c
+++ b/setup.c
@@ -414,6 +414,7 @@ static void update_linked_gitdir(const char *gitfile, const
char *gitdir)
*/
const char *read_gitfile_gently(const char *path, int *return_error_code)
{
+ static const int one_MB = 1 20;
int error_code = 0;
char *buf = NULL;
char *dir = NULL;
@@ -430,6 +431,10 @@ const char *read_gitfile_gently(const char *path, int
*return_error_code)
error_code = READ_GITFILE_ERR_NOT_A_FILE;
goto cleanup_return;
}
+ if (st.st_size one_MB) {
+ error_code = READ_GITFILE_ERR_TOO_LARGE;
+ goto cleanup_return;
+ }
fd = open(path, O_RDONLY);
if (fd 0) {
error_code = READ_GITFILE_ERR_OPEN_FAILED;
@@ -489,6 +494,8 @@ cleanup_return:
return NULL;
case READ_GITFILE_ERR_OPEN_FAILED:
die_errno(Error opening '%s', path);
+ case READ_GITFILE_ERR_TOO_LARGE:
+ die(Too large to be a .git file: '%s', path);
case READ_GITFILE_ERR_READ_FAILED:
die(Error reading %s, path);
case READ_GITFILE_ERR_INVALID_FORMAT:
--
2.4.3.373.gc496bfb
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 4/5] p7300: add performance tests for clean
The tests are run in dry-run mode to avoid having to restore the test directories for each timed iteration. Using dry-run is an acceptable compromise since we are mostly interested in the initial computation of what to clean and not so much in the cleaning it self. Signed-off-by: Erik Elfström erik.elfst...@gmail.com --- t/perf/p7300-clean.sh | 31 +++ 1 file changed, 31 insertions(+) create mode 100755 t/perf/p7300-clean.sh diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh new file mode 100755 index 000..ec94cdd --- /dev/null +++ b/t/perf/p7300-clean.sh @@ -0,0 +1,31 @@ +#!/bin/sh + +test_description=Test git-clean performance + +. ./perf-lib.sh + +test_perf_default_repo +test_checkout_worktree + +test_expect_success 'setup untracked directory with many sub dirs' ' + rm -rf 500_sub_dirs 10_sub_dirs clean_test_dir + mkdir 500_sub_dirs 10_sub_dirs clean_test_dir + for i in $(test_seq 1 500) + do + mkdir 500_sub_dirs/dir$i || return $? + done + for i in $(test_seq 1 200) + do + cp -r 500_sub_dirs 10_sub_dirs/dir$i || return $? + done +' + +test_perf 'clean many untracked sub dirs, check for nested git' ' + git clean -n -q -f -d 10_sub_dirs/ +' + +test_perf 'clean many untracked sub dirs, ignore nested git' ' + git clean -n -q -f -f -d 10_sub_dirs/ +' + +test_done -- 2.4.3.373.gc496bfb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] status: differentiate interactive from non-interactive rebases
Matthieu Moy matthieu@grenoble-inp.fr writes: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr writes: Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr --- t/t7512-status-help.sh | 28 ++-- wt-status.c| 5 - Are there any change since the last version? Please, help reviewers by anticipating this question and versionning your patches ([PATCH v3 2/4] in the subject, git send-email -v3 does this for you). Plus, it would help to say something like no changes since v2 after that three-dash line before the diffstat graph. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/13] delete_refs(): improve error message
Michael Haggerty mhag...@alum.mit.edu writes: Change the error message from Could not remove branch %s to Could not remove reference %s This change makes sense even for the existing caller, which uses the function to delete remote-tracking branches. I am 80% convinced ;-) The existing caller never used this for removing tags, so 'could not remove branch' was equally correct for it and was more specific than 'could not remove reference'. If you change it to 'could not remove that thing %s', it would still be correct for the existing caller; it would be even less specific for them, though ;-) The new callers you will add in later patch of course cannot live with 'could not remove branch', so I think that this is an acceptable compromise we can live with. If somebody later wants to make the message more specific, they can add code that switches on the prefix of the ref when coming up with the error message (and use that code consistently in other error messages e.g. 'could not add reference'). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index c413282..2a2a06d 100644 --- a/refs.c +++ b/refs.c @@ -2827,7 +2827,7 @@ int delete_refs(struct string_list *refnames) const char *refname = refnames-items[i].string; if (delete_ref(refname, NULL, 0)) - result |= error(_(Could not remove branch %s), refname); + result |= error(_(Could not remove reference %s), refname); } return result; -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-completion.tcsh: fix redirect with noclobber
Ariel Faigon github.2...@yendor.com writes:
tcsh users who happen to have 'set noclobber' elsewhere in their
~/.tcshrc or ~/.cshrc startup files get a 'File exist' error, and
the tcsh completion file doesn't get generated/updated.
Adding a `!` in the redirect works correctly for both clobber (default)
and 'set noclobber' users.
Helped-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Christian Couder christian.cou...@gmail.com
Signed-off-by: Ariel Faigon github.2...@yendor.com
---
Thanks for enduring three iterations for a single-liner. This
versio will show nicely in git log -p output ;-)
In case anybody is wondering, that Helped-by: notifications@github
was merely because I said this change makes sense, care to send it
over to us at git@vger? from github UI on Ariel's commit. I do not
deserve helped-by for merely encouraging participation.
Will queue. Thanks again.
contrib/completion/git-completion.tcsh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.tcsh
b/contrib/completion/git-completion.tcsh
index 6104a42..4a790d8 100644
--- a/contrib/completion/git-completion.tcsh
+++ b/contrib/completion/git-completion.tcsh
@@ -41,7 +41,7 @@ if ( ! -e ${__git_tcsh_completion_original_script} ) then
exit
endif
-cat EOF ${__git_tcsh_completion_script}
+cat EOF ! ${__git_tcsh_completion_script}
#!bash
#
# This script is GENERATED and will be overwritten automatically.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: format-patch and submodules
Am 05.06.2015 um 01:20 schrieb Christopher Dunn: (Seen in git versions: 2.1.0 and 1.9.3 et al.) $ git format-patch --stdout X^..X | git apply check - fatal: unrecognized input This fails when the commit consists of nothing but a submodule change (as in 'git add submodule foo'), but it passes when a file change is added to the same commit. There used to be a similar problem for empty commits, but that was fixed around git-1.8: http://stackoverflow.com/questions/20775132/cannot-apply-git-patch-replacing-a-file-with-a-link Now, 'git format-patch' outputs nothing for an empty commit. I suppose that needs to be the behavior also when only submodules are changed, since in that case there is no 'diff' section from 'format-patch'. Use-case: git-p4 Of course, we do not plan to add the submodule into Perforce, but we would like this particular command to behave the same whether there are other diffs or not. Hmm, I'm not sure that this is a bug. It looks to me like doing a $ git format-patch --stdout X^..X | git apply check - when nothing is changed except submodules and expecting it to work is the cause of the problem. I get the same error when I do: $git format-patch --stdout master..master | git apply --check - fatal: unrecognized input No submodules involved, just an empty patch. I assume you want to ignore all submodule changes, so you should check if e.g. git diff --ignore-submodules X^..X returns anything before applying that? (From the command you ran I assume you might be able to drop the --ignore-submodules because you already did set diff.ignoreSubmodules to all?) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/7] send-email: allow aliases in patch header and command script outputs
Interpret aliases in:
- Header fields of patches generated by git format-patch
(using --to, --cc, --add-header for example) or
manually modified. Example of fields in header:
To: alias1
Cc: alias2
Cc: alias3
- Outputs of command scripts specified by --cc-cmd and
--to-cmd. Example of script:
#!/bin/sh
echo alias1
echo alias2
--
Note that the --from option of git format-patch takes an argument which
must be formated as .*.* (if not, git format-patch will be aborted with
fatal: invalid ident line). For this reason, passing an alias as an
argument of --from is not possible, but modifying the from field in
the patch manually is.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
git-send-email.perl | 2 ++
t/t9001-send-email.sh | 60 +++
2 files changed, 62 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index e1e9b14..0cac4b0 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1535,7 +1535,9 @@ foreach my $t (@files) {
($confirm =~ /^(?:auto|compose)$/ $compose $message_num
== 1));
$needs_confirm = inform if ($needs_confirm $confirm_unconfigured
@cc);
+ @to = expand_aliases(@to);
@to = validate_address_list(sanitize_address_list(@to));
+ @cc = expand_aliases(@cc);
@cc = validate_address_list(sanitize_address_list(@cc));
@to = (@initial_to, @to);
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index e63fc83..062c703 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1552,6 +1552,66 @@ test_expect_success $PREREQ
'sendemail.aliasfile=~/.mailrc' '
grep ^!someone@example\.org!$ commandline1
'
+test_expect_success $PREREQ 'alias support in To header' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 --to=sbd aliased.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --smtp-server=$(pwd)/fake.sendmail \
+ aliased.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
+test_expect_success $PREREQ 'alias support in Cc header' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 --cc=sbd aliased.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --smtp-server=$(pwd)/fake.sendmail \
+ aliased.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
+test_expect_success $PREREQ 'tocmd works with aliases' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 tocmd.patch
+ echo tocmd--sbd tocmd.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --to-cmd=./tocmd-sed \
+ --smtp-server=$(pwd)/fake.sendmail \
+ tocmd.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
+test_expect_success $PREREQ 'cccmd works with aliases' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 cccmd.patch
+ echo cccmd--sbd cccmd.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --cc-cmd=./cccmd-sed \
+ --smtp-server=$(pwd)/fake.sendmail \
+ cccmd.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
do_xmailer_test () {
expected=$1 params=$2
git format-patch -1
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/7] t9001-send-email: refactor header variable fields replacement
Create a function which replaces Date, Message-Id and
X-Mailer lines generated by git-send-email by a specific string
Date:.*$ - Date: DATE-STRING
Message-Id:.*$ - Message-Id: MESSAGE-ID-STRING
X-Mailer:.*$ - X-Mailer: X-MAILER-STRING
This is a preparatory for the next commit.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
t/t9001-send-email.sh | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 062c703..806f066 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -522,6 +522,12 @@ Result: OK
EOF
+replace_variable_fields () {
+ sed -e s/^\(Date:\).*/\1 DATE-STRING/ \
+ -e s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/ \
+ -e s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/
+}
+
test_suppression () {
git send-email \
--dry-run \
@@ -529,10 +535,7 @@ test_suppression () {
--from=Example f...@example.com \
--to=t...@example.com \
--smtp-server relay.example.com \
- $patches |
- sed -e s/^\(Date:\).*/\1 DATE-STRING/ \
- -e s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/ \
- -e s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/ \
+ $patches | replace_variable_fields \
actual-suppress-$1${2+-$2}
test_cmp expected-suppress-$1${2+-$2} actual-suppress-$1${2+-$2}
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/7] send-email: allow multiple emails using --cc, --to and --bcc
From: Jorge Juan Garcia Garcia jorge-juan.garcia-gar...@ensimag.imag.fr
Accept a list of emails separated by commas in flags --cc, --to and
--bcc. Multiple addresses can already be given by using these options
multiple times, but it is more convenient to allow cutting-and-pasting
a list of addresses from the header of an existing e-mail message,
which already lists them as comma-separated list, as a value to a
single parameter.
The following format can now be used:
$ git send-email --to='Jane j...@example.com, m...@example.com'
However format using commas in names doesn't work:
$ git send-email --to='Jane, Doe j...@example.com'
Remove the limitation imposed by 79ee555b (Check and document the
options to prevent mistakes, 2006-06-21) which rejected every argument
with comma in --cc, --to and --bcc.
Helped-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
Documentation/git-send-email.txt | 21 +--
git-send-email.perl | 21 ++-
t/t9001-send-email.sh| 44
3 files changed, 65 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 8045546..0146164 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -49,17 +49,23 @@ Composing
of 'sendemail.annotate'. See the CONFIGURATION section for
'sendemail.multiEdit'.
---bcc=address::
+--bcc=address,...::
Specify a Bcc: value for each email. Default is the value of
'sendemail.bcc'.
+
-The --bcc option must be repeated for each user you want on the bcc list.
+Addresses containing commas (Doe, Jane foo...@example.com) are not
+currently supported.
++
+This option may be specified multiple times.
---cc=address::
+--cc=address,...::
Specify a starting Cc: value for each email.
Default is the value of 'sendemail.cc'.
+
-The --cc option must be repeated for each user you want on the cc list.
+Addresses containing commas (Doe, Jane foo...@example.com) are not
+currently supported.
++
+This option may be specified multiple times.
--compose::
Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
@@ -110,13 +116,16 @@ is not set, this will be prompted for.
Only necessary if --compose is also set. If --compose
is not set, this will be prompted for.
---to=address::
+--to=address,...::
Specify the primary recipient of the emails generated. Generally, this
will be the upstream maintainer of the project involved. Default is the
value of the 'sendemail.to' configuration value; if that is unspecified,
and --to-cmd is not specified, this will be prompted for.
+
-The --to option must be repeated for each user you want on the to list.
+Addresses containing commas (Doe, Jane foo...@example.com) are not
+currently supported.
++
+This option may be specified multiple times.
--8bit-encoding=encoding::
When encountering a non-ASCII message or subject that does not
diff --git a/git-send-email.perl b/git-send-email.perl
index 59dcfe3..ea03308 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -460,20 +460,6 @@ my ($repoauthor, $repocommitter);
($repoauthor) = Git::ident_person(@repo, 'author');
($repocommitter) = Git::ident_person(@repo, 'committer');
-# Verify the user input
-
-foreach my $entry (@initial_to) {
- die Comma in --to entry: $entry'\n unless $entry !~ m/,/;
-}
-
-foreach my $entry (@initial_cc) {
- die Comma in --cc entry: $entry'\n unless $entry !~ m/,/;
-}
-
-foreach my $entry (@bcclist) {
- die Comma in --bcclist entry: $entry'\n unless $entry !~ m/,/;
-}
-
sub parse_address_line {
if ($have_mail_address) {
return map { $_-format } Mail::Address-parse($_[0]);
@@ -1023,8 +1009,13 @@ sub sanitize_address_list {
return (map { sanitize_address($_) } @_);
}
+sub split_at_commas {
+ return (map { split /\s*,\s*/, $_ } @_);
+}
+
sub process_address_list {
-my @addr_list = expand_aliases(@_);
+my @addr_list = split_at_commas(@_);
+@addr_list = expand_aliases(@addr_list);
@addr_list = sanitize_address_list(@addr_list);
@addr_list = validate_address_list(@addr_list);
return @addr_list;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 806f066..9aee474 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1648,4 +1648,48 @@ test_expect_success $PREREQ '--[no-]xmailer with
sendemail.xmailer=false' '
do_xmailer_test 1 --xmailer
'
+test_expect_success $PREREQ 'setup expected-list' '
+ git send-email \
+ --dry-run \
[PATCH v3 4/7] send-email: refactor address list process
Simplify code by creating a funct (comma separated, with aliases ...)
into a simple list of valid email addresses.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
git-send-email.perl | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 0cac4b0..59dcfe3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -808,12 +808,9 @@ sub expand_one_alias {
return $aliases{$alias} ? expand_aliases(@{$aliases{$alias}}) : $alias;
}
-@initial_to = expand_aliases(@initial_to);
-@initial_to = validate_address_list(sanitize_address_list(@initial_to));
-@initial_cc = expand_aliases(@initial_cc);
-@initial_cc = validate_address_list(sanitize_address_list(@initial_cc));
-@bcclist = expand_aliases(@bcclist);
-@bcclist = validate_address_list(sanitize_address_list(@bcclist));
+@initial_to = process_address_list(@initial_to);
+@initial_cc = process_address_list(@initial_cc);
+@bcclist = process_address_list(@bcclist);
if ($thread !defined $initial_reply_to $prompting) {
$initial_reply_to = ask(
@@ -1026,6 +1023,13 @@ sub sanitize_address_list {
return (map { sanitize_address($_) } @_);
}
+sub process_address_list {
+my @addr_list = expand_aliases(@_);
+@addr_list = sanitize_address_list(@addr_list);
+@addr_list = validate_address_list(@addr_list);
+return @addr_list;
+}
+
# Returns the local Fully Qualified Domain Name (FQDN) if available.
#
# Tightly configured MTAa require that a caller sends a real DNS
@@ -1535,10 +1539,8 @@ foreach my $t (@files) {
($confirm =~ /^(?:auto|compose)$/ $compose $message_num
== 1));
$needs_confirm = inform if ($needs_confirm $confirm_unconfigured
@cc);
- @to = expand_aliases(@to);
- @to = validate_address_list(sanitize_address_list(@to));
- @cc = expand_aliases(@cc);
- @cc = validate_address_list(sanitize_address_list(@cc));
+ @to = process_address_list(@to);
+ @cc = process_address_list(@cc);
@to = (@initial_to, @to);
@cc = (@initial_cc, @cc);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/5] Improving performance of git clean
Here is a reroll of this series (after much delay). Changes in v7: * changed order of file size and file open error check in read_gitfile * resolved conflicts with nd/multiple-work-trees. This removed the need for is_git_directory_gently that was added in v6 and simplified some error cases. Erik Elfström (5): setup: add gentle version of read_gitfile setup: sanity check file size in read_gitfile_gently t7300: add tests to document behavior of clean and nested git p7300: add performance tests for clean clean: improve performance when removing lots of directories builtin/clean.c | 30 +-- cache.h | 12 - setup.c | 91 +--- t/perf/p7300-clean.sh | 31 +++ t/t7300-clean.sh | 140 ++ 5 files changed, 280 insertions(+), 24 deletions(-) create mode 100755 t/perf/p7300-clean.sh -- 2.4.3.373.gc496bfb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses
Remove leading and trailing whitespaces when sanitizing addresses so
that git send-email give the same output when passing arguments like
j...@example.comor \t j...@example.com as with
j...@example.com.
The next commit will introduce a test for this aswell.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
git-send-email.perl | 3 +++
1 file changed, 3 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index ea03308..3d144bd 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -978,6 +978,9 @@ sub sanitize_address {
# remove garbage after email address
$recipient =~ s/(.*).*$/$1/;
+ # remove leading and trailing whitespace
+ $recipient =~ s/^\s+|\s+$//g;
+
my ($recipient_name, $recipient_addr) = ($recipient =~
/^(.*?)\s*(.*)/);
if (not $recipient_name) {
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] index-pack: avoid excessive re-reading of pack directory
On Fri, Jun 05, 2015 at 08:29:21AM -0400, Jeff King wrote:
On Fri, Jun 05, 2015 at 08:18:17AM -0400, Jeff King wrote:
1. Devise some torture to tests to see whether my patch series is in
fact racy on Linux.
2. Assuming it is, scrap it and make a has_sha1_file_quick() which
might sometimes return a false negative (if somebody else is
repacking). Use that in index-pack (and possibly other places, but
we can start with index-pack).
If we skip step 1 out of pessimism (which I think is a reasonable thing
to do), then step 2 should not be all that much work. I'm going to be
offline for a few days, though, so I won't get to it until next week at
the earliest. If you (or someone else) wants to take a stab at it,
please feel free.
Actually, it really is a tiny bit of work. I knocked this out in less
than 10 minutes. It passes the test suite, but it's entirely possible
that I did something boneheaded that does not fix your problem. ;)
Please test and confirm that it makes things better on your system.
I tested this on my system, and confirmed that for a git clone
--no-local --bare git.git:
1. It cuts the number of openat/getdents/close syscalls by several
orders of magnitude.
2. The overall time drops from ~11.4s to ~10.5s. I suppose if I timed
only the `index-pack` process, it would be even higher (as a
percentage improvement).
So I expect it should fix the problem on your NFS system.
Here's the patch again. Same as before, but I'm cc-ing Junio, as I think
it is good to apply.
-- 8 --
Subject: index-pack: avoid excessive re-reading of pack directory
Since 45e8a74 (has_sha1_file: re-check pack directory before
giving up, 2013-08-30), we spend extra effort for
has_sha1_file to give the right answer when somebody else is
repacking. Usually this effort does not matter, because
after finding that the object does not exist, the next step
is usually to die().
However, some code paths make a large number of
has_sha1_file checks which are _not_ expected to return 1.
The collision test in index-pack.c is such a case. On a
local system, this can cause a performance slowdown of
around 5%. But on a system with high-latency system calls
(like NFS), it can be much worse.
This patch introduces a quick flag to has_sha1_file which
callers can use when they would prefer high performance at
the cost of false negatives during repacks. There may be
other code paths that can use this, but the index-pack one
is the most obviously critical, so we'll start with
switching that one.
Signed-off-by: Jeff King p...@peff.net
---
builtin/index-pack.c | 2 +-
cache.h | 11 ++-
sha1_file.c | 4 +++-
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 7ea2020..0761dfd 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -784,7 +784,7 @@ static void sha1_object(const void *data, struct
object_entry *obj_entry,
assert(data || obj_entry);
read_lock();
- collision_test_needed = has_sha1_file(sha1);
+ collision_test_needed = has_sha1_file_with_flags(sha1, HAS_SHA1_QUICK);
read_unlock();
if (collision_test_needed !data) {
diff --git a/cache.h b/cache.h
index 571c98f..98edc2c 100644
--- a/cache.h
+++ b/cache.h
@@ -943,8 +943,17 @@ extern int has_sha1_pack(const unsigned char *sha1);
* Return true iff we have an object named sha1, whether local or in
* an alternate object database, and whether packed or loose. This
* function does not respect replace references.
+ *
+ * If the QUICK flag is set, do not re-check the pack directory
+ * when we cannot find the object (this means we may give a false
+ * negative answer if another process is simultaneously repacking).
*/
-extern int has_sha1_file(const unsigned char *sha1);
+#define HAS_SHA1_QUICK 0x1
+extern int has_sha1_file_with_flags(const unsigned char *sha1, int flags);
+static inline int has_sha1_file(const unsigned char *sha1)
+{
+ return has_sha1_file_with_flags(sha1, 0);
+}
/*
* Return true iff an alternate object database has a loose object
diff --git a/sha1_file.c b/sha1_file.c
index 7e38148..dc8746b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3161,7 +3161,7 @@ int has_sha1_pack(const unsigned char *sha1)
return find_pack_entry(sha1, e);
}
-int has_sha1_file(const unsigned char *sha1)
+int has_sha1_file_with_flags(const unsigned char *sha1, int flags)
{
struct pack_entry e;
@@ -3169,6 +3169,8 @@ int has_sha1_file(const unsigned char *sha1)
return 1;
if (has_loose_object(sha1))
return 1;
+ if (flags HAS_SHA1_QUICK)
+ return 0;
reprepare_packed_git();
return find_pack_entry(sha1, e);
}
--
2.4.2.752.geeb594a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More
[PATCH] git-completion.tcsh: fix redirect with noclobber
tcsh users who happen to have 'set noclobber' elsewhere in their
~/.tcshrc or ~/.cshrc startup files get a 'File exist' error, and
the tcsh completion file doesn't get generated/updated.
Adding a `!` in the redirect works correctly for both clobber (default)
and 'set noclobber' users.
Helped-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Christian Couder christian.cou...@gmail.com
Signed-off-by: Ariel Faigon github.2...@yendor.com
---
contrib/completion/git-completion.tcsh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.tcsh
b/contrib/completion/git-completion.tcsh
index 6104a42..4a790d8 100644
--- a/contrib/completion/git-completion.tcsh
+++ b/contrib/completion/git-completion.tcsh
@@ -41,7 +41,7 @@ if ( ! -e ${__git_tcsh_completion_original_script} ) then
exit
endif
-cat EOF ${__git_tcsh_completion_script}
+cat EOF ! ${__git_tcsh_completion_script}
#!bash
#
# This script is GENERATED and will be overwritten automatically.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git lock files (Was: GIT for Microsoft Access projects)
Thanks folks, I am digesting all you have said.
Now the command line I can do (I'm a programmer) but the secretary here I doubt.
So is there at GUI interface for this? Does it work on Windows systems?
Thanks,
Paul
Stefan Beller sbel...@google.com wrote:
Just because Git allows distributed workflows, doesn't mean we
should only focus on being distributed IMHO.
The question for content not being mergable easily pops up all
the time. (Game/Graphics designers, documents, all this binary
stuff, where there is no good merge driver).
I could imagine a git lock command which looks like this:
git config lock.centralServer origin
git config lock.defaultBranch master
git lock add [branch] [--] path/to/file
git lock remove [branch] [--] path/to/file
git lock ls [branch]
And the way this is implemented is roughly (unoptimized, just showing
how you would achieve this with todays command set):
git fetch --depth=1 $(git config --get lock.centralServer)
refs/locks/$(git config --get lock.defaultBranch)
git checkout refs/locks/$(git config --get lock.centralServer)/$(git
config --get lock.defaultBranch)
switch(option) {
case add:
if exist path/to/file
return -1
else
echo $(git config --get user.name) $(date) path/to/file
git add path/to/file git commit add new lock
fi
case remove:
if exist path/to/file
# todo: check if the same user locked it before
rm path/to/file
else
return -1
fi
case ls:
ls -R .
}
git push $(git config --get lock.centralServer) refs/locks/$(git config
--get lock.defaultBranch)
git restore working tree, branch
That said you could just manipulate the git objects directly, no need
to check out to the working dir.
The server would only need to allow pushes to a refs/locks directory and be
done.
the client side would need to have a plumbing command, so you could easily
integrate
a git locking to your application if you don't want to provide a merge driver.
Thanks,
Stefan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git lock files (Was: GIT for Microsoft Access projects)
On Tue, 9 Jun 2015 13:21:44 -0500 hack...@suddenlink.net wrote: Thanks folks, I am digesting all you have said. Now the command line I can do (I'm a programmer) but the secretary here I doubt. So is there at GUI interface for this? Does it work on Windows systems? That's why I asked whether the thing you do really want is a document management system, not a version control system. Yes, Git works on Windows thanks to folks behind the Git for Windows project (often and errorneously called msysGit in the internets) and yes there do exist mature Windows GUI front-ends to it, with TortoiseGit and Git Extensions being supposedly the most visible picks. But there's such thing as an irreducible complexity: while these tools strive to be user-friendly, and TortoiseGit even tries to make you think you're using Subversion rather than Git, they won't hide all the underlying complexity of a DVCS tool, which Git is, from the user. So... I bet for your random user, it would be much easier to switch to the browser window and upload another version of their document there, with a short note describing what they do and why. This is how a typical DMS works. You won't get all that awesomness Git gives you to fiddle with your source code files but in return you'll get a system which requires next to zero training for any layman to use it. Please rememeber about [1]. Many of the statements that post does are outdated but its essense remains to be true when it comes to handing off Git to users not possessing ninja-level computer skills. I especially recommend to think through this particular passage: | They often struggle to use version control at all; are you now going | to teach them the difference between “pull” and “update”, between | “commit” and “push”? Look me in the eyes and say that with a straight | face. I also wonder how do you intend to explain them why they can't push because someone else had just did that, and what to do about this, and why. (And whose version should win, in the end, as the files you intend to work with are not subject for merging in the usual sense of this word -- when it comes to plain text files.) 1. http://blog.red-bean.com/sussman/?p=79 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 5/5] clean: improve performance when removing lots of directories
git clean uses resolve_gitlink_ref() to check for the presence of
nested git repositories, but it has the drawback of creating a
ref_cache entry for every directory that should potentially be
cleaned. The linear search through the ref_cache list causes a massive
performance hit for large number of directories.
Modify clean.c:remove_dirs to use setup.c:is_git_directory and
setup.c:read_gitfile_gently instead.
Both these functions will open files and parse contents when they find
something that looks like a git repository. This is ok from a
performance standpoint since finding repository candidates should be
comparatively rare.
Using is_git_directory and read_gitfile_gently should give a more
standardized check for what is and what isn't a git repository but
also gives three behavioral changes.
The first change is that we will now detect and avoid cleaning empty
nested git repositories (only init run). This is desirable.
Second, we will no longer die when cleaning a file named .git with
garbage content (it will be cleaned instead). This is also desirable.
The last change is that we will detect and avoid cleaning empty bare
repositories that have been placed in a directory named .git. This
is not desirable but should have no real user impact since we already
fail to clean non-empty bare repositories in the same scenario. This
is thus deemed acceptable.
On top of this we add some extra precautions. If read_gitfile_gently
fails to open the git file, read the git file or verify the path in
the git file we assume that the path with the git file is a valid
repository and avoid cleaning.
Update t7300 to reflect these changes in behavior.
The time to clean an untracked directory containing 10 sub
directories went from 61s to 1.7s after this change.
Helped-by: Jeff King p...@peff.net
Signed-off-by: Erik Elfström erik.elfst...@gmail.com
---
builtin/clean.c | 30 ++
t/t7300-clean.sh | 10 --
2 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 6dcb72e..df53def 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -10,7 +10,6 @@
#include cache.h
#include dir.h
#include parse-options.h
-#include refs.h
#include string-list.h
#include quote.h
#include column.h
@@ -148,6 +147,31 @@ static int exclude_cb(const struct option *opt, const char
*arg, int unset)
return 0;
}
+/*
+ * Return 1 if the given path is the root of a git repository or
+ * submodule else 0. Will not return 1 for bare repositories with the
+ * exception of creating a bare repository in foo/.git and calling
+ * is_git_repository(foo).
+ */
+static int is_git_repository(struct strbuf *path)
+{
+ int ret = 0;
+ int gitfile_error;
+ size_t orig_path_len = path-len;
+ assert(orig_path_len != 0);
+ if (path-buf[orig_path_len - 1] != '/')
+ strbuf_addch(path, '/');
+ strbuf_addstr(path, .git);
+ if (read_gitfile_gently(path-buf, gitfile_error) ||
is_git_directory(path-buf))
+ ret = 1;
+ if (gitfile_error == READ_GITFILE_ERR_OPEN_FAILED ||
+ gitfile_error == READ_GITFILE_ERR_READ_FAILED)
+ ret = 1; /* This could be a real .git file, take the
+ * safe option and avoid cleaning */
+ strbuf_setlen(path, orig_path_len);
+ return ret;
+}
+
static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
int dry_run, int quiet, int *dir_gone)
{
@@ -155,13 +179,11 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
struct strbuf quoted = STRBUF_INIT;
struct dirent *e;
int res = 0, ret = 0, gone = 1, original_len = path-len, len;
- unsigned char submodule_head[20];
struct string_list dels = STRING_LIST_INIT_DUP;
*dir_gone = 1;
- if ((force_flag REMOVE_DIR_KEEP_NESTED_GIT)
- !resolve_gitlink_ref(path-buf, HEAD,
submodule_head)) {
+ if ((force_flag REMOVE_DIR_KEEP_NESTED_GIT)
is_git_repository(path)) {
if (!quiet) {
quote_path_relative(path-buf, prefix, quoted);
printf(dry_run ? _(msg_would_skip_git_dir) :
_(msg_skip_git_dir),
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index fbfdf2d..32e96da 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -455,7 +455,7 @@ test_expect_success 'nested git work tree' '
! test -d bar
'
-test_expect_failure 'should clean things that almost look like git but are
not' '
+test_expect_success 'should clean things that almost look like git but are
not' '
rm -fr almost_git almost_bare_git almost_submodule
mkdir -p almost_git/.git/objects
mkdir -p almost_git/.git/refs
@@ -468,8 +468,6 @@ test_expect_failure 'should clean things that almost look
like git but are not'
garbage
EOF
test_when_finished rm
Re: git lock files (Was: GIT for Microsoft Access projects)
On Tue, Jun 9, 2015 at 11:21 AM, hack...@suddenlink.net wrote: Thanks folks, I am digesting all you have said. I did not intend to answer your original question, but to start a discussion on the feasibility of a dedicated git lock command. There are lots of things which are checked in alongside the code (The code is which is why you want to use Git in the first place), such as Graphics, CAD, design documents (maybe in binary format such as ODF, MS Excel). All I did was proposing a new command and laying out its behavior. By being careful at what to use as the porcelain command line and what to use as a stable plumbing command, other programs could rely on that. (Some proprietary CAD tools such as Altium have a subversion integration [1], maybe Git wants to offer an easy interface to allow for Git integration as easily?) Now the command line I can do (I'm a programmer) but the secretary here I doubt. So is there at GUI interface for this? Does it work on Windows systems? As all I was saying is dreaming out new concepts, there is currently no code. Thanks, Paul [1] http://techdocs.altium.com/display/ADOH/Version+Control+and+Altium+Designer -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
Johannes Schindelin johannes.schinde...@gmx.de writes:
Besides, are you sure you don't want to substitute an empty
rebase.instructionFormat' by '%s'? I would have expected to read
${format:-%s}` (note the colon), but then, this was Junio's
suggestion...
That was me simply being sloppy myself, expecting people not to copy
and paste literally without thinking. Thanks for noticing.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
I have since reworked this script to support the short hash in the
custom format as a special case:
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+format=$(git config --get rebase.instructionFormat)
+no_format=$?
+if test ${no_format} -ne 0
+then
+ format=%H %s
+elif test ${format:0:3} != %H test ${format:0:3} != %h
+then
+ format=%H ${format}
+fi
+# the 'rev-list .. | sed' requires %m to parse; the instruction
requires %H to parse
+git rev-list $merges_option --format=%m${format} \
+ --reverse --left-right --topo-order \
I also use the $no_format variable later on in the autosquash
re-ordering, and have the tests passing. I want to add some new tests
on the custom format, and will send a new patch when that is complete.
On Tue, Jun 9, 2015 at 3:23 PM, Junio C Hamano gits...@pobox.com wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
Besides, are you sure you don't want to substitute an empty
rebase.instructionFormat' by '%s'? I would have expected to read
${format:-%s}` (note the colon), but then, this was Junio's
suggestion...
That was me simply being sloppy myself, expecting people not to copy
and paste literally without thinking. Thanks for noticing.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] bisect: simplify the add of new bisect terms
Matthieu Moy matthieu@grenoble-inp.fr wrote: I think you would want to error out if errno is not ENOENT. Junio C Hamano jch2...@gmail.com wrote: We might want to see why fopen() failed here. If it is because the file did not exist, great. But otherwise? This file is always supposed to exist when the function is called unless the user has manually deleted it (or a bug in our programs). Maybe we should just return an error if the file cannot be opened, regardless the reason. We kept it like it is, with the default case, because it was what our elders did, and that aborting because of BISECT_TERMS is not good for backward compatibility. But then, in both cases, there is no reason to differenciate ENOENT. Is that right ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] bisect: replace hardcoded bad|good by variables
Matthieu Moy matthieu@grenoble-inp.fr writes: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes: --- a/git-bisect.sh +++ b/git-bisect.sh @@ -32,6 +32,8 @@ OPTIONS_SPEC= _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' _x40=$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40 +NAME_BAD=bad +NAME_GOOD=good I would have written NAME_NEW=bad NAME_OLD=good old/new are the generic wording, so I think it would make more sense for the codebase to use it when we don't hardcode old/new. Yeah, I would think so, especially if we envision that the new/old will not be the only pair we will ever allow in place for the traditional bad/good. Being bad is just a special case of being new only when you are hunting for a regression. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/19] parse-options-cb: implement parse_opt_pass_strbuf()
Paul Tan pyoka...@gmail.com writes:
diff --git a/parse-options-cb.c b/parse-options-cb.c
index be8c413..5b1dbcf 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -134,3 +134,32 @@ int parse_opt_noop_cb(const struct option *opt, const
char *arg, int unset)
{
return 0;
}
+
+/**
+ * For an option opt, recreates the command-line option in opt-value which
+ * must be an strbuf. This is useful when we need to pass the command-line
+ * option to another command.
+ */
This is to be used to create a single argument, not an entire
command line to be executed via run_command() or a shell, right?
It wasn't very clear from the above command.
If the same option is given more than once, the earlier ones are
discarded and the last one wins. That may be OK for your expected
callers, and I do not think this needs to have an option to instead
accumulate them, but it needs to be made clear in the comment and
the API documentation for future developers who want to use the
parse-options API.
+int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int
unset)
+{
+ struct strbuf *sb = opt-value;
+
+ strbuf_reset(sb);
+
+ if (opt-long_name) {
+ strbuf_addstr(sb, unset ? --no- : --);
+ strbuf_addstr(sb, opt-long_name);
+ if (arg) {
+ strbuf_addch(sb, '=');
+ strbuf_addstr(sb, arg);
+ }
+ } else if (opt-short_name !unset) {
+ strbuf_addch(sb, '-');
+ strbuf_addch(sb, opt-short_name);
+ if (arg)
+ strbuf_addstr(sb, arg);
+ } else
+ return -1;
+
+ return 0;
+}
diff --git a/parse-options.h b/parse-options.h
index c71e9da..1d21398 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -224,6 +224,7 @@ extern int parse_opt_with_commit(const struct option *,
const char *, int);
extern int parse_opt_tertiary(const struct option *, const char *, int);
extern int parse_opt_string_list(const struct option *, const char *, int);
extern int parse_opt_noop_cb(const struct option *, const char *, int);
+extern int parse_opt_pass_strbuf(const struct option *, const char *, int);
#define OPT__VERBOSE(var, h) OPT_COUNTUP('v', verbose, (var), (h))
#define OPT__QUIET(var, h)OPT_COUNTUP('q', quiet, (var), (h))
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()
Paul Tan pyoka...@gmail.com writes:
Certain git commands, such as git-pull, are simply wrappers around other
git commands like git-fetch, git-merge and git-rebase. As such, these
wrapper commands will typically need to pass through command-line
options of the commands they wrap.
Implement the parse_opt_pass_argv_array() parse-options callback, which
will reconstruct all the provided command-line options into an
argv_array, such that it can be passed to another git command. This is
useful for passing command-line options that can be specified multiple
times.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
Notes:
v2
* This function is a requirement for the rewrite of git-am to handle
passing git-apply's options to git-apply. Since it would be
implemented anyway I thought it would be good if git-pull could take
advantage of it as well to handle --strategy and --strategy-option.
parse-options-cb.c | 32
parse-options.h| 1 +
2 files changed, 33 insertions(+)
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 5b1dbcf..7330506 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -4,6 +4,7 @@
#include commit.h
#include color.h
#include string-list.h
+#include argv-array.h
/*- some often used options -*/
@@ -163,3 +164,34 @@ int parse_opt_pass_strbuf(const struct option *opt,
const char *arg, int unset)
return 0;
}
+
+/**
+ * For an option opt, recreate the command-line option, appending it to
+ * opt-value which must be a argv_array. This is useful when we need to pass
+ * the command-line option, which can be specified multiple times, to another
+ * command.
+ */
Almost the same comment as 01/19 applies to this comment.
I think it makes good sense to have two variants, one that lets the
last one win and pass only that last one (i.e. 01/19) and the other
that accumulates them into an argv_array (i.e. this one). But it
feels iffy, given that the acculate version essentially creates an
array of (char *), to make the last one wins, leaving a single
string to use strbuf. I'd find it much more understandable if 01/19
took (char **) as opt-value instead of a strbuf.
In any case, these two need to be added as a related pair to the API
documentation.
Thanks.
+int parse_opt_pass_argv_array(const struct option *opt, const char *arg, int
unset)
+{
+ struct strbuf sb = STRBUF_INIT;
+ struct argv_array *opt_value = opt-value;
+
+ if (opt-long_name) {
+ strbuf_addstr(sb, unset ? --no- : --);
+ strbuf_addstr(sb, opt-long_name);
+ if (arg) {
+ strbuf_addch(sb, '=');
+ strbuf_addstr(sb, arg);
+ }
+ } else if (opt-short_name !unset) {
+ strbuf_addch(sb, '-');
+ strbuf_addch(sb, opt-short_name);
+ if (arg)
+ strbuf_addstr(sb, arg);
+ } else
+ return -1;
+
+ argv_array_push(opt_value, sb.buf);
+ strbuf_release(sb);
+ return 0;
+}
diff --git a/parse-options.h b/parse-options.h
index 1d21398..b663f87 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *,
const char *, int);
extern int parse_opt_string_list(const struct option *, const char *, int);
extern int parse_opt_noop_cb(const struct option *, const char *, int);
extern int parse_opt_pass_strbuf(const struct option *, const char *, int);
+extern int parse_opt_pass_argv_array(const struct option *, const char *,
int);
#define OPT__VERBOSE(var, h) OPT_COUNTUP('v', verbose, (var), (h))
#define OPT__QUIET(var, h)OPT_COUNTUP('q', quiet, (var), (h))
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fw: sort of a bug report - git rebase dropping empty commits
Argh, thanks a lot! Should have read the man page better. OTOH, I expect 'git commit --allow-empty' being needed, but 'git rebase --keep-empty' comes somewhat as a surprise - I wasn't expecting git rebase to commit each in turn, but of course that's what it does. On 7 May 2015 at 01:47, Mikael Magnusson mika...@gmail.com wrote: On Thu, May 7, 2015 at 2:19 AM, Hin-Tak Leung hintak.le...@gmail.com wrote: Repost from another account. vger.kernel.org seems not to like postings from my other alias (which goes through yahoo). (please cc - I am not a subscriber) Recently I have started to keep some notes in git repo's with --allow-empty - i.e. the meaningful content is the commit message and the date itself, not the diff. Use the git rebase --keep-empty option? -- Mikael Magnusson -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On undoing a forced push
On Tue, Jun 9, 2015 at 11:29 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Sorry to chime in so late in the discussion, but I think that the `--force-with-lease` option is what you are looking for. It allows you to force-push *but only* if the forced push would overwrite the ref we expect, i.e. (simplified, but you get the idea) `git push --force-with-lease remote ref` will *only* succeed if the remote's ref agrees with the local `refs/remotes/remote/ref`. If you use `--force-with-lease`, you simply cannot force-forget anything on the remote side that you cannot undo (because you have everything locally you need to undo it). Yeah I recall Junio did something about pushes.. I was about to suggest that we promote force-with-lease to default --force and current --force becomes --force --force. But there's this from commit 2233ad4 (Merge branch 'jc/push-cas' - 2013-09-09) that makes me hesitate The logic to choose the default implemented here is fragile (e.g. git fetch after seeing a failure will update the remote-tracking branch and will make the next push pass, defeating the safety pretty easily). It is suitable only for the simplest workflows, and it may hurt users more than it helps them. Either way I still want to provide an escape hatch for --force as it's good to reduce the number of unrecoverable operations down. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 09/19] pull: error on no merge candidates
Paul Tan pyoka...@gmail.com writes:
/**
+ * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
+ * into merge_heads.
+ */
Hmph, I vaguely recall doing that in C elsewhere already, even
though I do not remember where offhand...
+static void get_merge_heads(struct sha1_array *merge_heads)
+{
+ const char *filename = git_path(FETCH_HEAD);
+ FILE *fp;
+ struct strbuf sb = STRBUF_INIT;
+ unsigned char sha1[GIT_SHA1_RAWSZ];
+
+ if (!(fp = fopen(filename, r)))
+ die_errno(_(could not open '%s' for reading), filename);
+ while(strbuf_getline(sb, fp, '\n') != EOF) {
Missing SP after while
+ if (get_sha1_hex(sb.buf, sha1))
+ continue; /* invalid line: does not start with SHA1 */
+ if (starts_with(sb.buf + GIT_SHA1_HEXSZ, \tnot-for-merge))
Look for \tnot-for-merge\t instead?
The patch overall looks good. Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/19] pull: support pull.ff config
Paul Tan pyoka...@gmail.com writes:
* Yup, I did mean strcmp(value, only)
I may be missing some backstory behind this comment; in the patch, I
instead see !strcmp(value, only) and I think it makes sense.
+ if (git_config_get_value(pull.ff, value))
+ return;
+ switch (git_config_maybe_bool(pull.ff, value)) {
+ case 0:
+ strbuf_addstr(sb, --no-ff);
+ return;
Align switch and case at the same indent level; the body of switch
is overindented.
+ case 1:
+ strbuf_addstr(sb, --ff);
+ return;
+ }
+ if (!strcmp(value, only)) {
+ strbuf_addstr(sb, --ff-only);
+ return;
+ }
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 05/19] pull: implement fetch + merge
Paul Tan pyoka...@gmail.com writes:
+/**
+ * Parses argv into [repo [refspecs...]], returning their values in
`repo`
+ * as a string and `refspecs` as a null-terminated array of strings. If
`repo`
+ * is not provided in argv, it is set to NULL.
+ */
+static void parse_repo_refspecs(int argc, const char **argv, const char
**repo,
+ const char ***refspecs)
+{
+ if (argc 0) {
+ *repo = *argv++;
+ argc--;
+ } else
+ *repo = NULL;
+ *refspecs = argv;
+}
+
+/**
+ * Runs git-fetch, returning its exit status. `repo` and `refspecs` are the
+ * repository and refspecs to fetch, or NULL if they are not provided.
+ */
+static int run_fetch(const char *repo, const char **refspecs)
+{
+ struct argv_array args = ARGV_ARRAY_INIT;
+ int ret;
+
+ argv_array_pushl(args, fetch, --update-head-ok, NULL);
+ if (repo)
+ argv_array_push(args, repo);
+ while (*refspecs)
+ argv_array_push(args, *refspecs++);
As you cannot say git pull refspecs..., the above might be more
clear if you spelled it like this instead:
if (repo) {
argv_array_push(args, repo);
argv_array_pushv(args, refspecs);
} else if (*refspecs) {
die(BUG: refspec without repo?);
}
+/**
+ * Runs git-merge, returning its exit status.
+ */
+static int run_merge(void)
+{
+ int ret;
+ struct argv_array args = ARGV_ARRAY_INIT;
+
+ argv_array_pushl(args, merge, NULL);
+ argv_array_push(args, FETCH_HEAD);
+ ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
+ argv_array_clear(args);
+ return ret;
+}
No frills yet, which is a good way to start with and show the
overall structure.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/19] pull: pass verbosity, --progress flags to fetch and merge
Paul Tan pyoka...@gmail.com writes:
Re-implement support for this flag by introducing the option callback
handler parse_opt_passthru(). This callback is used to pass the
--progress or --no-progress command-line switch to git-fetch and
git-merge.
Forgot to rephrase? parse-opt-passthru() is a good name for pass
the single string, last one wins, but is implemented in another
patch.
Use parse_opt_passthru() implemented earlier to pass the
--[no-]progress command line options to git-fetch and
git-merge.
or something like that.
diff --git a/builtin/pull.c b/builtin/pull.c
index 0ca23a3..c9c2cc0 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -16,11 +16,35 @@ static const char * const pull_usage[] = {
NULL
};
+/* Shared options */
+static int opt_verbosity;
+static struct strbuf opt_progress = STRBUF_INIT;
+
static struct option pull_options[] = {
+ /* Shared options */
+ OPT__VERBOSITY(opt_verbosity),
+ { OPTION_CALLBACK, 0, progress, opt_progress, NULL,
+ N_(force progress reporting),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf},
+
OPT_END()
};
Seeing this use case convinces me that the new parse-opt callback
parse-opt-pass-single-string-last-one-wins() in 01/19 should not be
strbuf based interface but should be (const char *) based one.
Other than that the change looks nicely done. So far, the series
has been a very pleasant read up to this step.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/9] parse-options: add parse_opt_merge_filter()
On 06/09/2015 12:50 AM, Junio C Hamano wrote:
Karthik Nayak karthik@gmail.com writes:
+int parse_opt_merge_filter(const struct option *opt, const char *arg, int
unset)
+{
+ struct ref_filter *rf = opt-value;
+ unsigned char sha1[20];
+
+ rf-merge = opt-long_name[0] == 'n'
+ ? REF_FILTER_MERGED_OMIT
+ : REF_FILTER_MERGED_INCLUDE;
+
+ if (!arg)
+ arg = HEAD;
+ if (get_sha1(arg, sha1))
+ die(_(malformed object name %s), arg);
+
+ rf-merge_commit = lookup_commit_reference_gently(sha1, 0);
+ if (!rf-merge_commit)
+ return opterror(opt, must point to a commit, 0);
+
+ return 0;
+}
Again, this smells too specific to live as a part of parse-options
infrastructure. If we want to have two helper callbacks, one that
gives the results in an sha1-array (because there is no guarantee
that you want only commits) and in a commit-list, I am fine with
having parse_opt_object_name() and parse_opt_with_commit(). Perhaps
rename the latter which was named too specifically to something more
sensible (e.g. parse_opt_commit_object_name()) and use it from the
caller you wanted to use parse_opt_merge_filter()? The caller, if
it is not prepared to see more than one commits specified, may have
to check if (!list || !list-next) { die(I want one and only one) }
or something, though.
Having it in ref-filter.h as parse_opt_merge_filter() is fine,
though. After all, you would be sharing it with for-each-ref,
branch and tag and nobody else anyway.
I guess it's better left in ref-filter.h. We could like you said make
it depend on parse_opt_with_commit() but that again means we need to
check if more than one commits are specified. So I think it would be
better to have it in ref-filter.h
diff --git a/parse-options.h b/parse-options.h
index 3ae16a1..7bcf0f3 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -221,6 +221,7 @@ extern int parse_opt_expiry_date_cb(const struct option *,
const char *, int);
extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
extern int parse_opt_points_at(const struct option *, const char *, int);
+extern int parse_opt_merge_filter(const struct option *, const char *, int);
extern int parse_opt_with_commit(const struct option *, const char *, int);
extern int parse_opt_tertiary(const struct option *, const char *, int);
extern int parse_opt_string_list(const struct option *, const char *, int);
@@ -243,5 +244,15 @@ extern int parse_opt_noop_cb(const struct option *, const
char *, int);
OPT_COLOR_FLAG(0, color, (var), (h))
#define OPT_COLUMN(s, l, v, h) \
{ OPTION_CALLBACK, (s), (l), (v), N_(style), (h), PARSE_OPT_OPTARG,
parseopt_column_callback }
+#define OPT_NO_MERGED(filter, h) \
+ { OPTION_CALLBACK, 0, no-merged, (filter), N_(commit), (h), \
+ PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
+ parse_opt_merge_filter, (intptr_t) HEAD \
+ }
+#define OPT_MERGED(filter, h) \
+ { OPTION_CALLBACK, 0, merged, (filter), N_(commit), (h), \
+ PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
+ parse_opt_merge_filter, (intptr_t) HEAD \
+ }
Likewise.
This too would be better off in ref-filter.h
--
Regards,
Karthik
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 7/9] parse-options.h: add macros for '--contains' option
On 06/09/2015 01:02 AM, Junio C Hamano wrote:
Karthik Nayak karthik@gmail.com writes:
+#define OPT_CONTAINS(filter, h) \
+ { OPTION_CALLBACK, 0, contains, (filter), N_(commit), (h), \
+ PARSE_OPT_LASTARG_DEFAULT, \
+ parse_opt_with_commit, (intptr_t) HEAD \
+ }
+#define OPT_WITH(filter, h) \
+ { OPTION_CALLBACK, 0, with, (filter), N_(commit), (h), \
+ PARSE_OPT_LASTARG_DEFAULT, \
+ parse_opt_with_commit, (intptr_t) HEAD \
+ }
The redundancy bothers me. Can't we do a bit better than that,
perhaps like this?
#define _OPT_CONTAINS_OR_WITH(name, variable, help) \
{ OPTION_CALLBACK, 0, name, (variable), N_(commit), (help), \
PARSE_OPT_LASTARG_DEFAULT, \
parse_opt_with_commit, (intptr_t) HEAD \
}
#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(contains, v, h)
This seems better, thanks!
--
Regards,
Karthik
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
gitscm vs. git-scm
Hi there, I (mis-) remembered the git site address and noticed that gitscm.com returns empty while git-scm.com is our beloved home. I thought, though, that we have a couple domains with redirects but I may be misremembering that also. Or DNS is hicking up. Cheers, Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: Release candidate of Git for Windows 2.x is out
Hi, On 2015-06-09 14:10, QbProg wrote: I reproduce it using the windows command prompt (cmd.exe) using any repository. I tryed with bash and it works correctly. Please note that you removed enough context that the mail does not make sense anymore if read individually. At this point it might be best to open a ticket, as I suggested in my announcement: just log into GitHub (or sign up for free) and direct your web browser to https://github.com/git-for-windows/git/issues/new. For the record: I tried again, with `Git CMD`, this time I called ```cmd git clone https://github.com/dscho/images.git octo2 ``` ... and octo2/ is created correctly and contains the `.git/` subdirectory (which is hidden by default, but you can call `cd octo2/.git`). Read: I still cannot reproduce the issue. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On undoing a forced push
From a thread on Hacker News. It seems that if a user does not have
access to the remote's reflog and accidentally forces a push to a ref,
how does he recover it? In order to force push again to revert it
back, he would need to know the remote's old SHA-1. Local reflog does
not help because remote refs are not updated during a push.
This patch prints the latest SHA-1 before the forced push in full. He
then can do
git push remote +old-sha1:ref
He does not even need to have the objects that old-sha1 refers
to. We could simply push an empty pack and the the remote will happily
accept the force, assuming garbage collection has not happened. But
that's another and a little more complex patch.
Is there any other way to undo a forced push?
-- 8 --
diff --git a/transport.c b/transport.c
index f080e93..6bd6a64 100644
--- a/transport.c
+++ b/transport.c
@@ -657,16 +657,17 @@ static void print_ok_ref_status(struct ref *ref, int
porcelain)
[new branch]),
ref, ref-peer_ref, NULL, porcelain);
else {
- char quickref[84];
+ char quickref[104];
char type;
const char *msg;
- strcpy(quickref, status_abbrev(ref-old_sha1));
if (ref-forced_update) {
+ strcpy(quickref, sha1_to_hex(ref-old_sha1));
strcat(quickref, ...);
type = '+';
msg = forced update;
} else {
+ strcpy(quickref, status_abbrev(ref-old_sha1));
strcat(quickref, ..);
type = ' ';
msg = NULL;
-- 8 --
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: Release candidate of Git for Windows 2.x is out
Hi, On 2015-06-09 10:43, Qb wrote: I'm trying the release candidate on Win 8.1. Everything's working now, but when I clone a repository git clone http://./name.git CustomFolder it creates CustomFolder with the checkout files, but the .git folder is created inside CustomFolder\CustomFolder\ I cannot reproduce this behavior. This is what I did to test: 1. install Git-2.4.2.1-release-candidate-64.exe (default options) 2. run Git Bash 3. git clone https://github.com/dscho/images.git octocats This results in a correct checkout: `$HOME/octocats/.git/` exists and contains the local repository. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] bisect: replace hardcoded bad|good by variables
Christian Couder christian.cou...@gmail.com writes: old/new is not more generic than good/bad. I disagree with this. In any case, we're looking for a pair of commits where one is a direct parent of the other. So in the end, there's always the old behavior and the new behavior in the end. In natural language, I can write terms good/bad correspond to the situation where the new behavior is a bug and the old behavior was correct and terms fixed/unfixed correspond to the situation where the new behavior does not have a bug and the old one does, so I can describe several pairs of terms with old/new. When looking for a bugfix, saying NAME_GOOD=new seems backward. I would read this as the good behavior is to be new, while I would expect the new behavior is to be good. and as good/bad is older and is the default we should keep that in the names. I agree with this part though. If people working with the bisect codebase (which includes you) are more comfortable with good/bad, that's a valid reason to keep it. IOW, I still think old/new is more generic, but that is not a strong objection and should not block the patch. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
Mike Rappazzo rappa...@gmail.com writes:
I have since reworked this script to support the short hash in the
custom format as a special case:
I thought that we always give short form when presenting it to the
end user to edit, but for internal bookkeeping purposes we make sure
that we use the full SHA-1, so that new objects created during the
rebase session will not cause used to be unique but not anymore
ambiguity in the commit object names in the instruction sheet.
I have been assuming that the rev-list we have been mucking with
was to prepare the latter, the internal bookkeeping data, which must
always spell 40-hex (that is why the default oneline is not
--oneline but --pretty=oneline).
Why is it necessary to make %m%H part customizable in the first
place?
Puzzled
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+format=$(git config --get rebase.instructionFormat)
+no_format=$?
+if test ${no_format} -ne 0
+then
+ format=%H %s
+elif test ${format:0:3} != %H test ${format:0:3} != %h
Do not use bash-ism substring.
+then
+ format=%H ${format}
+fi
+# the 'rev-list .. | sed' requires %m to parse; the instruction
requires %H to parse
+git rev-list $merges_option --format=%m${format} \
+ --reverse --left-right --topo-order \
I also use the $no_format variable later on in the autosquash
re-ordering, and have the tests passing. I want to add some new tests
on the custom format, and will send a new patch when that is complete.
On Tue, Jun 9, 2015 at 3:23 PM, Junio C Hamano gits...@pobox.com wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
Besides, are you sure you don't want to substitute an empty
rebase.instructionFormat' by '%s'? I would have expected to read
${format:-%s}` (note the colon), but then, this was Junio's
suggestion...
That was me simply being sloppy myself, expecting people not to copy
and paste literally without thinking. Thanks for noticing.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On undoing a forced push
On Tue, Jun 9, 2015 at 10:00 PM, brian m. carlson
sand...@crustytoothpaste.net wrote:
On Tue, Jun 09, 2015 at 07:12:21PM +0700, Duy Nguyen wrote:
diff --git a/transport.c b/transport.c
index f080e93..6bd6a64 100644
--- a/transport.c
+++ b/transport.c
@@ -657,16 +657,17 @@ static void print_ok_ref_status(struct ref *ref, int
porcelain)
[new branch]),
ref, ref-peer_ref, NULL, porcelain);
else {
- char quickref[84];
+ char quickref[104];
You've increased this by 20, but you're adding 40 characters to the
strcpy. Are you sure that's enough?
Also, you might consider writing this in terms of GIT_SHA1_HEXSZ, as it
will be more obvious that this depends on that value. If you don't now,
I will later.
It's a demonstration patch and I didn't pay much attention. I think
converting this quickref to strbuf may be better though, when you
convert this file to object_id.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/19] pull: check if in unresolved merge state
Paul Tan pyoka...@gmail.com writes: @@ -422,6 +423,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix) parse_repo_refspecs(argc, argv, repo, refspecs); + git_config(git_default_config, NULL); + + if (read_cache_unmerged()) + die_resolve_conflict(Pull); + + if (file_exists(git_path(MERGE_HEAD))) + die_conclude_merge(); + if (!opt_ff.len) config_get_ff(opt_ff); Hmph. If you are going to do the git_config() call yourself, it might make more sense to define git_pull_config() callback and parse the pull.ff yourself, updating the use of the lazy git_config_get_value() API you introduced in patch 10/19. The above might is stronger than my usual might; I am undecided yet before reading the remainder of the series. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/19] pull: implement skeletal builtin pull
Paul Tan pyoka...@gmail.com writes:
+int cmd_pull(int argc, const char **argv, const char *prefix)
+{
+ if (!getenv(_GIT_USE_BUILTIN_PULL)) {
+ const char *path = mkpath(%s/git-pull, git_exec_path());
+
+ if (sane_execvp(path, (char**) argv) 0)
Style: (char **)argv.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] bisect: simplify the add of new bisect terms
Louis-Alexandre Stuber stub...@ensimag.grenoble-inp.fr writes: Matthieu Moy matthieu@grenoble-inp.fr wrote: I think you would want to error out if errno is not ENOENT. Junio C Hamano jch2...@gmail.com wrote: We might want to see why fopen() failed here. If it is because the file did not exist, great. But otherwise? This file is always supposed to exist when the function is called unless the user has manually deleted it (or a bug in our programs). Maybe we should just return an error if the file cannot be opened, regardless the reason. We kept it like it is, with the default case, because it was what our elders did, and that aborting because of BISECT_TERMS is not good for backward compatibility. But then, in both cases, there is no reason to differenciate ENOENT. Is that right ? One thing that immediately comes to mind is when you (perhaps accidentally, or perhaps you were asked to) cd into somebody else's repository and take a look or help, the file exists but cannot be read by you and you get EPERM. That is very different from ENOENT, which is an expected error when you are not using a customized terms. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 13/19] pull: implement pulling into an unborn branch
Paul Tan pyoka...@gmail.com writes:
/**
+ * Pulls into void by branching off merge_head.
+ */
+static int pull_into_void(unsigned char merge_head[GIT_SHA1_RAWSZ],
+ unsigned char curr_head[GIT_SHA1_RAWSZ])
+{
It is not wrong per-se, but is rather unusual (and misleading) to
specify the array-ness and array-size of parameters. Our codebase
tends to prefer spelling it like so instead:
static int pull_into_void(unsigned char *merge_head,
unsigned char *curr_head)
{
+ /*
+ * Two-way merge: we claim the index is based on an empty tree,
s/claim/treat/ perhaps?
+ * and try to fast-forward to HEAD. This ensures we will not lose
+ * index/worktree changes that the user already made on the unborn
+ * branch.
+ */
+ if (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head, 0))
+ return 1;
+
+ if (update_ref(initial pull, HEAD, merge_head, curr_head, 0,
UPDATE_REFS_DIE_ON_ERR))
+ return 1;
+
+ return 0;
+}
+
+/**
* Runs git-merge, returning its exit status.
*/
static int run_merge(void)
@@ -475,5 +497,10 @@ int cmd_pull(int argc, const char **argv, const char
*prefix)
if (!merge_heads.nr)
die_no_merge_candidates(repo, refspecs);
- return run_merge();
+ if (is_null_sha1(orig_head)) {
+ if (merge_heads.nr 1)
+ die(_(Cannot merge multiple branches into empty
head.));
+ return pull_into_void(*merge_heads.sha1, curr_head);
+ } else
+ return run_merge();
}
Sensible.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 15/19] pull: teach git pull about --rebase
Paul Tan pyoka...@gmail.com writes:
+enum rebase_type {
+ REBASE_INVALID = -1,
+ REBASE_FALSE = 0,
+ REBASE_TRUE,
+ REBASE_PRESERVE
+};
+
+/**
+ * Parses the value of --rebase, branch.*.rebase or pull.rebase. If value is
a
+ * false value, returns REBASE_FALSE. If value is a true value, returns
+ * REBASE_TRUE. If value is preserve, returns REBASE_PRESERVE. Otherwise,
+ * returns -1 to signify an invalid value.
+ */
+static enum rebase_type parse_config_rebase(const char *value)
+{
+ int v = git_config_maybe_bool(pull.rebase, value);
+ if (!v)
+ return REBASE_FALSE;
+ else if (v = 0)
+ return REBASE_TRUE;
It is somewhat misleading to say v = 0 when you already use !v to
signal something else. Perhaps else if (v 0) is better?
+/**
+ * Returns remote's upstream branch for the current branch. If remote is
NULL,
+ * the current branch's configured default remote is used. Returns NULL if
+ * `remote` does not name a valid remote, HEAD does not point to a branch,
+ * remote is not the branch's configured remote or the branch does not have
any
+ * configured upstream branch.
+ */
+static char *get_upstream_branch(const char *remote)
+{
+ struct remote *rm;
+ struct branch *curr_branch;
+
+ rm = remote_get(remote);
+ if (!rm)
+ return NULL;
+
+ curr_branch = branch_get(HEAD);
+ if (!curr_branch)
+ return NULL;
+
+ if (curr_branch-remote != rm)
+ return NULL;
+
+ if (!curr_branch-merge_nr)
+ return NULL;
+
+ return xstrdup(curr_branch-merge[0]-dst);
+}
Hmph, it is somewhat surprising that we do not have such a helper
already. Wouldn't we need this logic to implement $branch@{upstream}
syntax?
+/**
+ * Derives the remote tracking branch from the remote and refspec.
+ *
+ * FIXME: The current implementation assumes the default mapping of
+ * refs/heads/branch_name to refs/remotes/remote_name/branch_name.
+ */
+static char *get_tracking_branch(const char *remote, const char *refspec)
+{
This does smell like an incomplete reimplementation of what
get_fetch_map() knows how to do.
+/**
+ * Given the repo and refspecs, sets fork_point to the point at which the
+ * current branch forked from its remote tracking branch. Returns 0 on
success,
+ * -1 on failure.
+ */
+static int get_rebase_fork_point(unsigned char fork_point[GIT_SHA1_RAWSZ],
+ const char *repo, const char *refspec)
+{
+...
+}
This function looks OK (the two get_*_branch() helpers it uses I am
not sure about though).
Same comment on fork_point[] parameter's type applies here,
though. While I do not mind if you used struct object_id to
represent these object names, if you are sticking to the traditional
unsigned char [20], then these should be unsigned char * to be
consistent with others.
+/**
+ * Sets merge_base to the octopus merge base of curr_head, merge_head and
+ * fork_point. Returns 0 if a merge base is found, 1 otherwise.
+ */
+static int get_octopus_merge_base(unsigned char merge_base[GIT_SHA1_HEXSZ],
+ unsigned char curr_head[GIT_SHA1_RAWSZ],
+ unsigned char merge_head[GIT_SHA1_RAWSZ],
+ unsigned char fork_point[GIT_SHA1_RAWSZ])
+{
+...
+}
OK (and everything after this point looks good).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Using clean/smudge scripts from repository
I'm setting up a clean/smudge filter for a repository. In local testing it seemed to work well. To allow for the clean/smudge scripts to be updated as the source changes, I put the clean/smudge scripts into the repository, and configured the filter in my ~/.gitconfig file to be simply ./filter --clean and ./filter --smudge. However, when cloning a fresh repository instance, that's breaking because a file with the filter set by .gitattributes is getting created BEFORE the filter script itself exists. Playing around with things, I realized that alphabetically the filter script came after the filename being filtered. So I renamed the filter to __filter, and that appears to have changed to order in which contents are created during git clone. Is this a proper solution, or did I just luck out? Am I perhaps doing something foolish? Thanks, Bob -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
