* Re: What's The Right Way to Do This?
From: Michael Witten @ 2011-09-23 5:08 UTC (permalink / raw)
To: Jon Forrest; +Cc: git
In-Reply-To: <loom.20110923T064720-366@post.gmane.org>
On Fri, Sep 23, 2011 at 04:48, Jon Forrest <nobozo@gmail.com> wrote:
> ... So, I tried to revert using various methods I found by
> googling "git revert"...
> ...
> ...this experience left me wondering what I should have
> done in the first place.
What you should have done in the first place is RTFM; while
not fantastic, there is a decent amount of official git
documentation.
> What happened was that when I tried to revert back to the
> commit before the one I made, the files I had modified
> *and* the files that apparently were modified by other
> people in #3 above were reverted. This wasn't what
> I wanted. I only wanted to revert the changes I had made.
It would be a good idea to reproduce the unexpected behavior
you are experiencing by means of a minimal set of commands
that you can post here for others to try.
> With the help of someone more experienced than me we were
> able to get things back to normal... There's a chance I'm
> going to have to go through all this again as I try to fix
> the problem with my changes.
Why don't you ask that person about it?
^ permalink raw reply
* Re: [PATCH 2/2] diff_index: honor in-index, not working-tree, .gitattributes
From: Jay Soffian @ 2011-09-23 5:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Michael Haggerty
In-Reply-To: <CAG+J_DzUQ3OGfiX=vHVGC7SHvwToVjD7uwFyDa8Tq6t7YwX12Q@mail.gmail.com>
On Thu, Sep 22, 2011 at 8:38 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> On Thu, Sep 22, 2011 at 6:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> I think the logical conclusion of assuming that we will keep the "single
>> source only" semantics (which I think we will, by the way, unless I hear a
>> concrete proposal to how we apply attributes from more than one sources in
>> what way to which side of the diff) is that a patch might be an
>> improvement over the current behaviour if it teaches "diff-tree" to read
>> from the tree and populate the in-core index (never writing it out to
>> $GIT_DIR/index) from the postimage tree (i.e. "diff preimage postimage" or
>> "diff -R postimage preimage") when it is run in a bare repository.
>
> Okay, I can give that a try.
This area of git is still black magic to me. My best guess is
something like this:
diff --git a/tree-diff.c b/tree-diff.c
index b3cc2e4753..6fd84eb2bb 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -5,6 +5,8 @@
#include "diff.h"
#include "diffcore.h"
#include "tree.h"
+#include "attr.h"
+#include "unpack-trees.h"
static void show_entry(struct diff_options *opt, const char *prefix,
struct tree_desc *desc, struct strbuf *base);
@@ -280,6 +282,19 @@ int diff_tree_sha1(const unsigned char *old,
const unsigned char *new, const cha
die("unable to read destination tree (%s)", sha1_to_hex(new));
init_tree_desc(&t1, tree1, size1);
init_tree_desc(&t2, tree2, size2);
+
+ if (is_bare_repository()) {
+ struct unpack_trees_options unpack_opts;
+ memset(&unpack_opts, 0, sizeof(unpack_opts));
+ unpack_opts.index_only = 1;
+ unpack_opts.head_idx = -1;
+ unpack_opts.src_index = &the_index;
+ unpack_opts.dst_index = &the_index;
+ unpack_opts.fn = oneway_merge;
+ if (unpack_trees(1, DIFF_OPT_TST(opt, REVERSE_DIFF) ? &t1 : &t2,
&unpack_opts) == 0)
+ git_attr_set_direction(GIT_ATTR_INDEX, &the_index);
+ }
+
retval = diff_tree(&t1, &t2, base, opt);
if (!*base && DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename()) {
init_tree_desc(&t1, tree1, size1);
(And in case gmail line wraps that -- https://gist.github.com/1236806)
Am I barking up the right tree? (Obviously still needs tests, and
maybe an --[no]-tree-attributes option.)
j.
^ permalink raw reply related
* Re: Bug: git log --numstat counts wrong
From: Tay Ray Chuan @ 2011-09-23 6:30 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Alexander Pepper, git
In-Reply-To: <4E7B5F28.2020204@lsrfire.ath.cx>
On Fri, Sep 23, 2011 at 12:15 AM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> The patch below reverts a part of 27af01d5523 that's not explained in its
> commit message and doesn't seem to contribute to the intended speedup. It
> seems to restore the original diff output. I don't know how it's actually
> doing that, though, as I haven't dug into the code at all.
>
> [snip]
>
> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
> index 5a33d1a..e419f4f 100644
> --- a/xdiff/xprepare.c
> +++ b/xdiff/xprepare.c
> @@ -383,7 +383,7 @@ static int xdl_clean_mmatch(char const *dis, long i, long s, long e) {
> * might be potentially discarded if they happear in a run of discardable.
> */
> static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2) {
> - long i, nm, nreff;
> + long i, nm, nreff, mlim;
> xrecord_t **recs;
> xdlclass_t *rcrec;
> char *dis, *dis1, *dis2;
> @@ -396,16 +396,20 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
> dis1 = dis;
> dis2 = dis1 + xdf1->nrec + 1;
>
> + if ((mlim = xdl_bogosqrt(xdf1->nrec)) > XDL_MAX_EQLIMIT)
> + mlim = XDL_MAX_EQLIMIT;
> for (i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; i <= xdf1->dend; i++, recs++) {
> rcrec = cf->rcrecs[(*recs)->ha];
> nm = rcrec ? rcrec->len2 : 0;
> - dis1[i] = (nm == 0) ? 0: 1;
> + dis1[i] = (nm == 0) ? 0: (nm >= mlim) ? 2: 1;
> }
>
> + if ((mlim = xdl_bogosqrt(xdf2->nrec)) > XDL_MAX_EQLIMIT)
> + mlim = XDL_MAX_EQLIMIT;
> for (i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; i <= xdf2->dend; i++, recs++) {
> rcrec = cf->rcrecs[(*recs)->ha];
> nm = rcrec ? rcrec->len1 : 0;
> - dis2[i] = (nm == 0) ? 0: 1;
> + dis2[i] = (nm == 0) ? 0: (nm >= mlim) ? 2: 1;
> }
>
> for (nreff = 0, i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart];
>
>
>
Thanks for the patch, René.
Sorry for not explaining that part of the change.
My understanding of mlim is that it "caps" how deep the for loop at
around line 387 goes through a hash bucket/record chaing to find a
matching record from side A in side B (and vice-versa in a later
loop), probably to prevent running time from becoming too long.
But with 27af01d, this is no longer a concern. We can get an *exact*,
pre-computed count of matching records in the other side, so we don't
have go through the hash bucket. Thus mlim is no longer needed.
So re-introducing mlim doesn't seem right, even though it may fix this
"bug" (ie restore the old behaviour).
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: What's The Right Way to Do This?
From: Luke Diamand @ 2011-09-23 6:30 UTC (permalink / raw)
To: Jon Forrest; +Cc: git
In-Reply-To: <loom.20110923T064720-366@post.gmane.org>
On 23/09/11 05:48, Jon Forrest wrote:
> I'm just now starting to use git for more than trivial things.
> Today I got myself in trouble. Here's what happened:
>
> 1) I pulled the master branch from the IT repository from our
> main git server.
>
> 2) I created a branch from this called "J" and started making changes.
>
> 3) Other people pulled master from IT and then pushed changes back.
>
> 4) I merged J with my master branch.
Someone will be along soon to correct me, but I think you should have
rebased against the master branch rather than merged. Something like:
% git rebase origin
That way your changes would remain appended to the tip of the master
branch. That will in general make your life much easier for all sorts of
reasons.
>
> 5) I tried pushing my master back to origin but this failed with
> the usual message saying I first needed to pull from origin.
> So, I pulled and then pushed. This worked.
>
> 6) On another server where I was going to use my changes I pulled
> master from IT.
>
> 6) It turned out that my changes were incorrect. So, I tried to revert
> using various methods I found by googling "git revert". What happened
> was that when I tried to revert back to the commit before the one I
> made, the files I had modified *and* the files that apparently were
> modified by other people in #3 above were reverted. This wasn't what
> I wanted. I only wanted to revert the changes I had made.
At a guess, git revert created a commit that undid your _merge_, rather
than your commit. The merge included all those other changes....
It's a good idea to take a look at what a commit does before pushing it
- "git show HEAD" is your friend.
>
> With the help of someone more experienced than me we were able to get
> things back to normal but this experience left me wondering what I
> should have done in the first place. There's a chance I'm going to
> have to go through all this again as I try to fix the problem with
> my changes.
Use git rebase (*) to keep your changes nicely arranged at the top of
the main branch.
Use git show to check the sanity of what you're about to push upstream.
I think you could award yourself a nice cup of tea after an experience
like that though, having been on both sides of it, I can imagine you
need it :-P
Regards!
Luke
(*) But note that rebasing can cause problems for people who are
downstream of _you_. See the git-rebase(1) man page.
^ permalink raw reply
* Re: What's The Right Way to Do This?
From: Johannes Sixt @ 2011-09-23 6:39 UTC (permalink / raw)
To: Jon Forrest; +Cc: git
In-Reply-To: <loom.20110923T064720-366@post.gmane.org>
Am 9/23/2011 6:48, schrieb Jon Forrest:
> I'm just now starting to use git for more than trivial things.
> Today I got myself in trouble. Here's what happened:
>
> 1) I pulled the master branch from the IT repository from our
> main git server.
>
> 2) I created a branch from this called "J" and started making changes.
>
> 3) Other people pulled master from IT and then pushed changes back.
OK, so you are basically using a central repo.
> 4) I merged J with my master branch.
>
> 5) I tried pushing my master back to origin but this failed with
> the usual message saying I first needed to pull from origin.
> So, I pulled and then pushed. This worked.
>
> 6) On another server where I was going to use my changes I pulled
> master from IT.
I assume you mention this because with this step you detected that your
changes were crap.
> 6) It turned out that my changes were incorrect. So, I tried to revert
> using various methods I found by googling "git revert". What happened
> was that when I tried to revert back to the commit before the one I
> made, the files I had modified *and* the files that apparently were
> modified by other people in #3 above were reverted. This wasn't what
> I wanted. I only wanted to revert the changes I had made.
It looks like you had reset the history, not just reverted your changes.
> With the help of someone more experienced than me we were able to get
> things back to normal but this experience left me wondering what I
> should have done in the first place. There's a chance I'm going to
> have to go through all this again as I try to fix the problem with
> my changes.
With a central repository, any push into it marks a "done deal". No matter
how wrong and bogus your changes are, as soon as you push them into the
central repository, they are cast in stone.
At this point, the best you can do is to revert you changes, and by this I
mean that you create and push out another commit on top that backs out the
changes that you made.
If this is not what you (or your team) wants, than the problem is not with
suboptimal use of the git toolset, but with the push policy within you
team. A good policy should forbid that untested stuff can be pushed into
the central repository.
My suggestion is that you find a way to move your changes to the test site
(the "another server" you mention in step 6) without going through the
central repository. For example, if you have ssh access between your
development machine and the test site, you can push or pull your changes
between them directly.
-- Hannes
^ permalink raw reply
* [PATCH] Teach '--with-tree' option to check-attr
From: Jay Soffian @ 2011-09-23 7:25 UTC (permalink / raw)
To: git; +Cc: Jay Soffian, Junio C Hamano, Michael Haggerty, Jakub Narebski
Jakub Narebski writes:
> Nb. the ability to read gitattributes from given commit would be
> useful also for gitweb (the `encoding` gitattribute, etc.).
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
2011/9/22 Jakub Narebski <jnareb@gmail.com>:
> Unfortunately it doesn't seem to be there mechanism to query about
> state of gitattributes at given commit.
>
> There is a slight problem from the UI point of view of git-check-attr,
> namely that there are _three_ pieces of information: a place to read
> .gitattributes from (working tree, index, commit), list of attributes
> to check (or --all) and list of files (list of paths). You can use
> "--" to separate _two_ pieces of information.
>
> Nb. the ability to read gitattributes from given commit would be
> useful also for gitweb (the `encoding` gitattribute, etc.).
How's this? Builds on top of js/check-attr-cached.
Documentation/git-check-attr.txt | 4 ++++
builtin/check-attr.c | 33 ++++++++++++++++++++++++++++++++-
t/t0003-attributes.sh | 7 +++++++
3 files changed, 43 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 5abdbaa51c..06e5d95e0b 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -27,6 +27,10 @@ OPTIONS
--cached::
Consider `.gitattributes` in the index only, ignoring the working tree.
+--with-tree=<tree-ish>::
+ Consider .gitattributes in <tree-ish> only, ignoring the working tree
+ and index.
+
--stdin::
Read file names from stdin instead of from the command-line.
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index ded0d836d3..fe926d3c97 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -3,10 +3,13 @@
#include "attr.h"
#include "quote.h"
#include "parse-options.h"
+#include "tree-walk.h"
+#include "unpack-trees.h"
static int all_attrs;
static int cached_attrs;
static int stdin_paths;
+static const char *with_tree;
static const char * const check_attr_usage[] = {
"git check-attr [-a | --all | attr...] [--] pathname...",
"git check-attr --stdin [-a | --all | attr...] < <list-of-paths>",
@@ -18,6 +21,7 @@ static int null_term_line;
static const struct option check_attr_options[] = {
OPT_BOOLEAN('a', "all", &all_attrs, "report all attributes set on file"),
OPT_BOOLEAN(0, "cached", &cached_attrs, "use .gitattributes only from the index"),
+ OPT_STRING(0, "with-tree", &with_tree, "tree-ish", "use .gitattributes only from <tree-ish>"),
OPT_BOOLEAN(0 , "stdin", &stdin_paths, "read file names from stdin"),
OPT_BOOLEAN('z', NULL, &null_term_line,
"input paths are terminated by a null character"),
@@ -101,8 +105,35 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
die("invalid cache");
}
- if (cached_attrs)
+ if (cached_attrs && with_tree)
+ error_with_usage("Cannot use --cached and --with-tree together");
+
+ if (cached_attrs) {
git_attr_set_direction(GIT_ATTR_INDEX, NULL);
+ } else if (with_tree) {
+ unsigned char sha1[20];
+ struct tree *tree;
+ struct unpack_trees_options opts;
+ struct tree_desc t;
+
+ if (get_sha1(with_tree, sha1))
+ die("Not a valid object name");
+
+ tree = parse_tree_indirect(sha1);
+ if (tree == NULL)
+ die("Not a tree object");
+
+ memset(&opts, 0, sizeof(opts));
+ opts.index_only = 1;
+ opts.head_idx = -1;
+ opts.src_index = &the_index;
+ opts.dst_index = &the_index;
+ opts.fn = oneway_merge;
+ init_tree_desc(&t, tree->buffer, tree->size);
+ if (unpack_trees(1, &t, &opts))
+ return -1;
+ git_attr_set_direction(GIT_ATTR_INDEX, &the_index);
+ }
doubledash = -1;
for (i = 0; doubledash < 0 && i < argc; i++) {
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 46b0736b35..36ac3a02da 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -140,6 +140,7 @@ test_expect_success 'root subdir attribute test' '
'
test_expect_success 'setup bare' '
+ git commit -m ".gitattributes for testing --with-tree below" &&
git clone --bare . bare.git &&
cd bare.git
'
@@ -163,6 +164,12 @@ test_expect_success 'bare repository: check that --cached honors index' '
test_cmp ../specified-all actual
'
+test_expect_success 'bare repository: check --with-tree' '
+ git check-attr --with-tree=HEAD --stdin --all < ../stdin-all |
+ sort > actual &&
+ test_cmp ../specified-all actual
+'
+
test_expect_success 'bare repository: test info/attributes' '
(
echo "f test=f"
--
1.7.7.rc2.5.g12a2f
^ permalink raw reply related
* Re: [PATCH v3 13/22] resolve_ref(): turn buffer into a proper string as soon as possible
From: Thomas Rast @ 2011-09-23 8:17 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <1316121043-29367-14-git-send-email-mhagger@alum.mit.edu>
Hi Michael
Michael Haggerty wrote:
> Immediately strip off trailing spaces and null-terminate the string
> holding the contents of the reference file; this allows the use of
> string functions and avoids the need to keep separate track of the
> string's length. (get_sha1_hex() fails automatically if the string is
> too short.)
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
I'm getting valgrind failures in t1450-fsck and t3800-mktag which
blame to this commit. For t1450 it looks as follows:
ok 5 - object with bad sha1
expecting success:
git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
test_when_finished "git update-ref -d refs/heads/invalid" &&
git fsck 2>out &&
cat out &&
grep "not a commit" out
==19623== Use of uninitialised value of size 8
==19623== at 0x4B6747: hexval (cache.h:798)
==19623== by 0x4B6797: get_sha1_hex (hex.c:42)
==19623== by 0x4DD12A: resolve_ref (refs.c:588)
==19623== by 0x4DC777: get_ref_dir (refs.c:313)
==19623== by 0x4DC6FA: get_ref_dir (refs.c:302)
==19623== by 0x4DC963: get_loose_refs (refs.c:368)
==19623== by 0x4DD556: do_for_each_ref (refs.c:687)
==19623== by 0x4DDA05: for_each_replace_ref (refs.c:806)
==19623== by 0x4E5CE9: prepare_replace_object (replace_object.c:86)
==19623== by 0x4E5D3C: do_lookup_replace_object (replace_object.c:103)
==19623== by 0x4C99BB: lookup_replace_object (cache.h:764)
==19623== by 0x4C9FA6: parse_object (object.c:191)
==19623== Uninitialised value was created by a stack allocation
==19623== at 0x4DCE34: resolve_ref (refs.c:498)
or when I run it at the tip of pu instead of at the commit itself,
line numbers are like so:
==2308== Use of uninitialised value of size 8
==2308== at 0x4ADB8B: get_sha1_hex (cache.h:800)
==2308== by 0x4D4283: resolve_ref (refs.c:629)
==2308== by 0x4D4851: get_ref_dir (refs.c:361)
==2308== by 0x4D48C6: get_ref_dir (refs.c:350)
==2308== by 0x4D4D29: do_for_each_ref (refs.c:412)
==2308== by 0x4DCD93: do_lookup_replace_object (replace_object.c:86)
==2308== by 0x4C31F4: parse_object (cache.h:764)
==2308== by 0x4F2A1D: get_sha1_1 (sha1_name.c:567)
==2308== by 0x4F2D5F: get_sha1_with_context_1 (sha1_name.c:1117)
==2308== by 0x4F3543: get_sha1 (cache.h:822)
==2308== by 0x461B50: cmd_rev_parse (rev-parse.c:723)
==2308== by 0x404B71: run_builtin (git.c:308)
==2308== Uninitialised value was created by a stack allocation
==2308== at 0x4D4006: resolve_ref (refs.c:530)
Can you look into this?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: How to use git attributes to configure server-side checks?
From: Michael Haggerty @ 2011-09-23 8:35 UTC (permalink / raw)
To: Junio C Hamano
Cc: git discussion list, Jay Soffian, Jeff King, Jakub Narebski
In-Reply-To: <7vwrd0xzdc.fsf@alter.siamese.dyndns.org>
On 09/22/2011 07:26 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> I would like the checking configuration to be *versioned* along with the
>> code. For example, suppose my project decides to enforce a rule that
>> all Python code needs to be indented with spaces. It might be that not
>> all of our old code adheres to this rule, and that we only want to clean
>> up the code in master.
>
> You want to sneak in a badly formatted code? Add an entry to the in-tree
> attributes file to disable whitespace checking to cover that file!
No, the scenario that I was trying to describe is a project that wants
to tighten up its code formatting rules after years of laxity. It is
convenient to support legacy branches that still contain nonconforming
code without having to reformat it all, just as it is convenient to fix
the current code incrementally rather than requiring all of the cleanup
to be done in one big bang. Thus it is important that new rules not be
enforced retroactively on old code.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: Bug: git log --numstat counts wrong
From: Tay Ray Chuan @ 2011-09-23 9:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alexander Pepper, git
In-Reply-To: <7vobycxy71.fsf@alter.siamese.dyndns.org>
On Fri, Sep 23, 2011 at 1:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Alexander Pepper <pepper@inf.fu-berlin.de> writes:
>>
>>> Am 21.09.2011 um 14:24 schrieb Junio C Hamano:
>>>>> $ git log --numstat 48a07e7e533f507228e8d1c99d4d48e175e14260
>>>>> [...]
>>>>> 11 10 src/java/voldemort/server/storage/StorageService.java
>>>>
>>>> Didn't we update it this already? I seem to get 10/9 here not 11/10.
>>>
>>> Current 'maint' (cd2b8ae9), 'master' (4b5eac7f)...
>>
>> That's a tad old master you seem to have.
>>
>> Strangely, bisection points at 27af01d5523, which was supposed to be only
>> about performance and never about correctness. There is something fishy
>> going on....
>
> In any case, I think the real issue is that depending on how much context
> you ask, the resulting diff is different (and both are valid diffs). If
> you ask "log -p" (or "diff" or "show") to produce a patch, then we use the
> default 3-line context. And then you feed that to an external diffstat to
> count the number of deleted and added lines to get one set of numbers.
>
> The --numstat (and --diffstat) code seems to be running the internal diff
> machinery with 0-line context and counting the resulting diff internally.
>
> And of course the results between the above two would be different because
> diff can match lines differently when given different number of context
> lines to include in the result.
>
> So perhaps a good sanity-check for you to try (note: not checking your
> sanity, but checking the sanity of the above analysis) would be to do:
>
> $ git show 48a07e7e53 -- $that_path | diffstat
> $ git show -U0 48a07e7e53 -- $that_path | diffstat
> $ git show --numstat 48a07e7e53 -- $that_path
> $ git show --stat 48a07e7e53 -- $that_path
>
> and see how they compare (make sure to use the same version of git for
> these experiments). The first one uses the default 3-lines context, the
> second one forces 0-line context, and the last two uses 0-line context
> hardwired in the code.
>
> Applying the following patch should make the last two use the default
> context or -U$num given from the command line to be consistent with the
> codepath where we generate textual patches.
>
> diff.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 9038f19..302ef33 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2251,6 +2251,8 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
> memset(&xpp, 0, sizeof(xpp));
> memset(&xecfg, 0, sizeof(xecfg));
> xpp.flags = o->xdl_opts;
> + xecfg.ctxlen = o->context;
> + xecfg.interhunkctxlen = o->interhunkcontext;
> xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
> &xpp, &xecfg);
> }
Thanks Junio.
But wait, where does this patch go? Before or after 27af01d? If I'm
understanding the situation correctly, this patch won't change the
reporting 10/9 for --numstat, no?
Anyway, this patch looks right.
Acked-by: Tay Ray Chuan <rctay89@gmail.com>
Interesting to find that we have many xdiff users that don't respect
diff options on the command line (or may not acess to them), like
patch-id, merge. I wonder if there would be less conflicts if merge's
ctxlen could be overriden...
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: [PATCH 3/6] branch: teach --edit-description option
From: Nguyen Thai Ngoc Duy @ 2011-09-23 9:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1316729362-7714-4-git-send-email-gitster@pobox.com>
On Thu, Sep 22, 2011 at 03:09:19PM -0700, Junio C Hamano wrote:
> + if (launch_editor(git_path(edit_description), &buf, NULL)) {
> + strbuf_release(&buf);
> + return -1;
> + }
> + stripspace(&buf, 1);
> +
> + strbuf_addf(&name, "branch.%s.description", branch_name);
> + status = git_config_set(name.buf, buf.buf);
I suppose a Windows editor mave save the description with \r\n
ending. Perhaps a patch like this to avoid messing up config file?
--8<--
Subject: [PATCH] config: quote \r in value
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
config.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/config.c b/config.c
index 4183f80..2e238ac 100644
--- a/config.c
+++ b/config.c
@@ -165,6 +165,9 @@ static char *parse_value(void)
case 'b':
c = '\b';
break;
+ case 'r':
+ c = '\r';
+ break;
case 'n':
c = '\n';
break;
@@ -1048,6 +1051,9 @@ static int store_write_pair(int fd, const char *key, const char *value)
for (i = 0; value[i]; i++)
switch (value[i]) {
+ case '\r':
+ strbuf_addstr(&sb, "\\r");
+ break;
case '\n':
strbuf_addstr(&sb, "\\n");
break;
--
1.7.3.1.256.g2539c.dirty
--8<--
^ permalink raw reply related
* Re: [PATCH 0/6] A handful of "branch description" patches
From: Michael J Gruber @ 2011-09-23 8:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1316729362-7714-1-git-send-email-gitster@pobox.com>
Junio C Hamano venit, vidit, dixit 23.09.2011 00:09:
> Here are a few patches that I have queued in 'pu', redoing some of the
> patches I already sent out to the list, around "branch description".
>
> The original motivation was to make the push/pull workflow appear more
> robust by allowing human-to-human communication to leave audit trail that
> can be verified when it becomes necessary. Namely:
>
> * request-pull message carries the SHA-1 of what is expected to be
> merged; and
>
> * "signed push" leaves the SHA-1 of what was pushed to the remote,
> cryptographically signed.
>
> Linus's reaction, as I understood him, was "if we are spending efforts to
> add more information, the end result should be more informative to humans
> not just to machines", and I agree. An example of piece of information we
> often talk about is branch description---what a particular branch is meant
> to achieve. Both request-pull messages and declarations of what was pushed
> are good places to record that piece of information.
>
> So here is a partially re-rolled series to get us closer.
>
> * The logic to read from an existing branch description was in
> builtin/branch.c in the original series, but the first patch separates
> it out into branch.c as a helper function;
>
> * The second one is a digression; the branch description describes what
> the topic aims to achieve, so it was natural to use it to prime the
> cover letter while preparing a patch series with format-patch;
>
> * The third one that adds "branch --edit-description" is basically
> unchanged modulo small leakfix from the original round;
>
> * And the remainder of the series for request-pull is the same as the
> last round.
I'm afraid I've missed the first installment of the series, or rather the fact that it was about more than just signed pushes. I've been working at (and with) branch and tag annotations for quite a while now and should have probably pushed the WIP rather than just dropping the occasional note. So I'll describe briefly what I have (the branches are in any of my repos[1]), which is notes based:
mjg/vob/branch-notes [mjg/vob/virtual-objects: ahead 4]
Annotations for branches and tags
Show notes for branches and tags when "branch" resp. "tag" is called with "--notes".
The "--notes" argument can take on all usual forms.
mjg/vob/format-patch-branch-note [mjg/vob/refrev-hash: ahead 1]
Cover letter from notes
Fill in the cover letter from a note to ref:HEAD if --notes is given.
TODO: The current branch may not be the one the format-patch arguments refer to.
mjg/vob/refrev-hash [mjg/vob/virtual-objects: ahead 2]
Pseudo revs for refnames
Introduce "ref:foo" to denote the (virtual) refname object for the ref named
"foo". This is handy for now (editing branch and tag notes) but should
become obsoleted by a better ui, such as "git branch --edit foo" or
"git notes --refname edit foo".
Introduce "ref:" as a shortcut to "ref:HEAD" which is the refname object
for the current branch.
mjg/vob/refrev-pretend [mjg/vob/virtual-objects: ahead 1]
Pseudo revs for refnames
An alternative implementation using pretend_sha1...
Currently unused.
mjg/vob/virtual-objects [origin/next: ahead 2, behind 10]
Virtual refname objects
For each existing refname, introduce virtual objects corresponding to a blob
with the refname as the content. "virtual" refers to the fact that these
objects are not written out but exist for all other purposes, such as
attaching notes and keeping them from being pruned.
mjg/vob/virtual-refs-for-rnos
Virtual refs for refname objects
For each ref, pretend that the corresponding refname object is referenced
to keep it from being pruned. This still requires branch note code to
write out these objects.
(Unused earlier approach.)
mjg/vob/virtual-refs-pretend-all
Virtual refs for refname objects
For each ref, pretend that the corresponding refname object is referenced
to keep it from being pruned. This still requires branch note code to
write out these objects.
(Unused earlier approach using pretend_sha1....)
Yes, the above is (with added newlines and removed top commit info) the output of 'git branch -vv --notes --list mjg/vob\*' :)
Open questions:
* Should the refname object for ref "foo" really be identical to a blob with content "foo"? Or content "ref: foo? Or...?
* Should ref (branch and tag) annotations use the same default notes tree as commit notes?
* How best to view annotations on remote branches? This is connected with open questions about notes sharing and the ref namespace structure.
I do think that config based descriptions are a quick solution, but a very non-distributed, non-versioned approach when compared to the notes approach.
Michael
[1]
git://github.com/gitigit/git.git
git://gitorious.org/~mjg/git/mjg.git
git://repo.or.cz/git/mjg.git
^ permalink raw reply
* Re: [PATCH 3/6] branch: teach --edit-description option
From: Michael J Gruber @ 2011-09-23 9:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1316729362-7714-4-git-send-email-gitster@pobox.com>
Junio C Hamano venit, vidit, dixit 23.09.2011 00:09:
> Using branch.$name.description as the configuration key, give users a
> place to write about what the purpose of the branch is and things like
> that, so that various subsystems, e.g. "push -s", "request-pull", and
> "format-patch --cover-letter", can later be taught to use this
> information.
>
> The "-m" option similar to "commit/tag" is deliberately omitted, as the
> whole point of branch description is about giving descriptive information
> (the name of the branch itself is a better place for information that fits
> on a single-line).
I don't think that is the only reason why we should not make "git branch
-m foo bar" set the description "foo" for branch "bar"...
Granted, your argument applies to "--set-description" ("-m"-like) also.
Michael
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/git-branch.txt | 5 +++
> builtin/branch.c | 56 ++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 507b8d0..8871a4e 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -14,6 +14,7 @@ SYNOPSIS
> 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
> 'git branch' (-m | -M) [<oldbranch>] <newbranch>
;)
> 'git branch' (-d | -D) [-r] <branchname>...
> +'git branch' --edit-description [<branchname>]
^ permalink raw reply
* Re: How to use git attributes to configure server-side checks?
From: Michael Haggerty @ 2011-09-23 10:06 UTC (permalink / raw)
To: Jeff King
Cc: Jay Soffian, Junio C Hamano, git discussion list, Jakub Narebski
In-Reply-To: <20110922205856.GA8563@sigill.intra.peff.net>
On 09/22/2011 10:58 PM, Jeff King wrote:
> However, I think this is skirting around a somewhat larger issue, which
> is that gitattributes are sometimes about "this is what the file is like
> at the current state of history", and sometimes about "this is what this
> file is like throughout history".
This is a very dangerous line of thought. When content is read out of a
historical commit, the content must be identical to what was checked
into that commit. For example, the EOL characters that I see in an old
file revision must not depend on my current HEAD or index. This rule
should apply regardless of whether the content is being read to be
checked out, put into an archive, or diffed against some other revision.
So for these purposes, I think there is no choice but to honor the
gitattributes files in the tree/index/directory from which the content
was read.
If the user really wants to say "this is what this file is like
throughout history" (thereby altering history), then this should be a
local decision that should be stored locally in $GIT_DIR/info/attributes.
There are other attributes that affect "two-argument" operations like
diff. Here the policy has to depend on the situation. In the case of
diff, I suggest that the relevant attributes be read from *both*
versions of the file. If they are the same, then obviously use them.
If they differ, then fall back to a "safe" default (for example, the
diff that would be used if *no* attributes had been specified).
However, as a special case, if an attribute is specified in one version
and unspecified in another, it would *usually* be OK to use the
specified version of the attributes and that might be considered a
reasonable alternative.
By the way, it is possible that the two ends of a diff operation have
different filenames (e.g., with -M or -C). In this case the attributes
should be looked up using the filename corresponding to the commit.
> So I think doing it "right" would be a lot more complicated than just
> reading the commits from a particular tree. I think it would mean adding
> more context to all attribute lookups, and having each caller decide how
> the results should be used.
I agree.
> So if the status quo with working trees (i.e., retroactively applying
> the current gitattributes to past commits) is good enough, perhaps the
> bare-repository solution should be modeled the same way? In other words,
> should "git log -p" in a bare repo be looking for attributes not in the
> commits being diffed, but rather in HEAD[2]?
This would be a very poor policy, modeled on a status quo that is *not*
good enough. For example, HEAD might not even be a descendant of the
commit whose content is being interrogated; the commit might have new
files and new gitattributes that would be ignored. I suppose your
suggestion is better than the status quo, so it would be fine as a
starting point, *provided* it is clear that people should not rely on
this behavior not being improved later.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH 2/2] diff_index: honor in-index, not working-tree, .gitattributes
From: Michael Haggerty @ 2011-09-23 10:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jay Soffian, git, Jeff King, Jakub Narebski
In-Reply-To: <7v8vpgxkvb.fsf@alter.siamese.dyndns.org>
On 09/23/2011 12:39 AM, Junio C Hamano wrote:
> [...] It
> would be a regression if the attributes mechanism is used for auditing
> purposes (as we start reading from a tree that is being audited using the
> very attributes it brings in), though.
I'm confused by this comment.
If an auditing system can be subverted by altering .gitattributes, then
I can do just as much harm by changing the .gitattributes in one commit
and making the "nasty" change in a second. So any rigorous auditing
system based on .gitattributes would have to prevent me from committing
modifications to .gitattributes, in which case my commit will be
rejected anyway.
If by "auditing" you mean other less rigorous checks to which exceptions
are *allowed*, then it is preferable to add the exception in the same
commit as the otherwise-offending content, and therefore it is
*required* that the .gitattributes of the new tree be used when checking
the contents of that tree.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: Bug: git log --numstat counts wrong
From: Alexander Pepper @ 2011-09-23 10:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Tay Ray Chuan
In-Reply-To: <7vobycxy71.fsf@alter.siamese.dyndns.org>
Am 22.09.2011 um 19:51 schrieb Junio C Hamano:
> So perhaps a good sanity-check for you to try (note: not checking your
> sanity, but checking the sanity of the above analysis) would be to do:
>
> $ git show 48a07e7e53 -- $that_path | diffstat
> $ git show -U0 48a07e7e53 -- $that_path | diffstat
> $ git show --numstat 48a07e7e53 -- $that_path
> $ git show --stat 48a07e7e53 -- $that_path
[...]
> Applying the following patch should make the last two use the default
> context or -U$num given from the command line to be consistent with the
> codepath where we generate textual patches.
>
> diff.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 9038f19..302ef33 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2251,6 +2251,8 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
> memset(&xpp, 0, sizeof(xpp));
> memset(&xecfg, 0, sizeof(xecfg));
> xpp.flags = o->xdl_opts;
> + xecfg.ctxlen = o->context;
> + xecfg.interhunkctxlen = o->interhunkcontext;
> xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
> &xpp, &xecfg);
> }
First of all: thank you for your extended feedback, your respectful e-mails and your patch!
I did some benachmarking. I compared git version 1.7.6.3 with 1.7.7.rc2 and 1.7.7.rc2 with the above patch.
My test setup:
git version 1.7.6.3: 740a8fc2
git version 1.7.7.rc2: 167a5800
git version 1.7.7.rc2': 167a5800 with the above patch applied.
Tuple (15,07) shows 15 lines added and 7 lines removed
$ git show $rev -- $that_path | diffstat
$ git show -U0 $rev -- $that_path | diffstat
$ git log --numstat -n1 --oneline $rev
$ git show --stat --oneline -n1 $rev -- $that_path
Test 1:
repo='https://github.com/voldemort/voldemort.git'
rev='48a07e7e'
that_path='src/java/voldemort/server/storage/StorageService.java'
Results:
1.7.6.3 1.7.7.rc2 1.7.7.rc2'
(10,09) (10,09) (10,09)
(11,10) (10,09) (10,09)
(11,10) (10,09) (10,09)
(11,10) (10,09) (10,09)
Test 2:
repo='https://github.com/voldemort/voldemort.git'
rev='c21ad764'
that_path='contrib/hadoop-store-builder/src/java/voldemort/store/readonly/mr/HadoopStoreBuilderReducer.java'
Results:
1.7.6.3 1.7.7.rc2 1.7.7.rc2'
(30,27) (25,22) (25,22)
(25,22) (25,22) (25,22)
(25,22) (25,22) (25,22)
(25,22) (25,22) (25,22)
Test 3:
repo='private repo'
rev='bd61f26e'
that_path='[...]JmeterTest/loadtests/JMeterLoadTest.jmx'
Results:
1.7.6.3 1.7.7.rc2 1.7.7.rc2'
(450,3544) (450,3544) (450,3544)
(401,3495) (401,3495) (401,3495)
(401,3495) (401,3495) (450,3544)
(401,3495) (401,3495) (450,3544)
In Test 1 the patch seems to be different formatted from 1.7.7.rc2, so the context doesn't matter with the newer version.
In Test 2 the patch seems to be different formatted from 1.7.7.rc2, so the context doesn't matter with the newer version.
In Test 3 (which is a private repo) where a lot of different lines where changes, some single lines, some multiple lines long makes the biggest difference. With the different contexts different output is observed. I'm sorry that I can not find an open source example for that, but your patch seems to fix this.
So it seems that the patch output in general changed between 1.7.6.3 and 1.7.7.rc2. If I had the right to vote for your patch, I would give it a +1 :-)
Greetings from Berlin
Alex
PS: Will this patch be in the final version of 1.7.7?
^ permalink raw reply
* Re: How to use git attributes to configure server-side checks?
From: Michael Haggerty @ 2011-09-23 10:38 UTC (permalink / raw)
To: Jakub Narebski
Cc: Junio C Hamano, git discussion list, Jay Soffian, Jeff King
In-Reply-To: <m3vcskqjcv.fsf@localhost.localdomain>
On 09/23/2011 12:54 AM, Jakub Narebski wrote:
> There is a slight problem from the UI point of view of git-check-attr,
> namely that there are _three_ pieces of information: a place to read
> .gitattributes from (working tree, index, commit), list of attributes
> to check (or --all) and list of files (list of paths). You can use
> "--" to separate _two_ pieces of information.
An alternative would be to accept <rev>:<path>, :<n>:<path>, and :<path>
in addition to simple <path>.
I also just remembered that currently the paths passed to
git-check-attr, git_check_attr(), and git_all_attrs() do not have to
correspond to existing objects; the paths are compared as strings.
Presumably we should retain this aspect of the behavior even if a
revision or tree is specified explicitly.
There are other problems in the UI of git-check-attr:
1. Its "-z" option affects how standard input is interpreted; there is
no way to cause the output to be generated with NUL separators.
2. Its output does not distinguish between the special statuses
unspecified/unset/set and possible string values
"unspecified"/"unset"/"set".
Maybe a --porcelain option could be used to overcome these shortcomings.
If necessary we could deprecate "git check-attr" and implement an
improved command ("git get-attr"?) that starts from a clean UI slate.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* [PATCH] fix push --quiet: add 'quiet' capability to receive-pack
From: Clemens Buchacher @ 2011-09-23 11:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, tmz, tobiasu, jkeating
In-Reply-To: <7vaa9xyxpf.fsf@alter.siamese.dyndns.org>
Currently, git push --quiet produces some non-error output, e.g.:
$ git push --quiet
Unpacking objects: 100% (3/3), done.
This fixes a bug reported for the fedora git package:
https://bugzilla.redhat.com/show_bug.cgi?id=725593
Commit 90a6c7d4 (propagate --quiet to send-pack/receive-pack)
introduced the --quiet option to receive-pack and made send-pack
pass that option. Older versions of receive-pack do not recognize
the option, however, and terminate immediately. The commit was
therefore reverted.
This change instead adds a 'quiet' capability to receive-pack,
which is a backwards compatible.
In addition, this fixes push --quiet via http: A verbosity of 0
means quiet for remote helpers.
Reported-by: Jesse Keating <jkeating@redhat.com>
Reported-by: Tobias Ulmer <tobiasu@tmux.org>
Cc: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
On Wed, Sep 21, 2011 at 10:04:28PM -0700, Junio C Hamano wrote:
>
> * cb/maint-quiet-push (2011-09-05) 4 commits
> . t5541: avoid TAP test miscounting
> . push: old receive-pack does not understand --quiet
> . fix push --quiet via http
> . tests for push --quiet
>
> Dropped for rerolling after 1.7.7 cycle.
This is the re-rolled version based on current master, without the
backwards incompatible receive-pack --quiet option.
I squashed the tests in and added your unpack(void) fixup.
I have not added Michael's "t5541: avoid TAP test miscounting"
since I cannot reproduce the error:
http://mid.gmane.org/e4e82f1267da3edfc600361de0041f618c31e30c.1315232475.git.git@drmicha.warpmail.net
And there is also this related patch "server_supports(): parse
feature list more carefully", which looked good to me:
http://mid.gmane.org/7vmxejy9od.fsf@alter.siamese.dyndns.org
Does it need more work?
Clemens
builtin/receive-pack.c | 14 ++++++++++++--
builtin/send-pack.c | 13 ++++++++++---
remote-curl.c | 4 +++-
t/t5523-push-upstream.sh | 7 +++++++
t/t5541-http-push.sh | 8 ++++++++
5 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ae164da..4419323 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -31,6 +31,7 @@ static int transfer_unpack_limit = -1;
static int unpack_limit = 100;
static int report_status;
static int use_sideband;
+static int quiet;
static int prefer_ofs_delta = 1;
static int auto_update_server_info;
static int auto_gc = 1;
@@ -114,7 +115,7 @@ static int show_ref(const char *path, const unsigned char *sha1, int flag, void
else
packet_write(1, "%s %s%c%s%s\n",
sha1_to_hex(sha1), path, 0,
- " report-status delete-refs side-band-64k",
+ " report-status delete-refs side-band-64k quiet",
prefer_ofs_delta ? " ofs-delta" : "");
sent_capabilities = 1;
return 0;
@@ -636,6 +637,8 @@ static struct command *read_head_info(void)
report_status = 1;
if (strstr(refname + reflen + 1, "side-band-64k"))
use_sideband = LARGE_PACKET_MAX;
+ if (strstr(refname + reflen + 1, "quiet"))
+ quiet = 1;
}
cmd = xcalloc(1, sizeof(struct command) + len - 80);
hashcpy(cmd->old_sha1, old_sha1);
@@ -684,8 +687,10 @@ static const char *unpack(void)
if (ntohl(hdr.hdr_entries) < unpack_limit) {
int code, i = 0;
- const char *unpacker[4];
+ const char *unpacker[5];
unpacker[i++] = "unpack-objects";
+ if (quiet)
+ unpacker[i++] = "-q";
if (receive_fsck_objects)
unpacker[i++] = "--strict";
unpacker[i++] = hdr_arg;
@@ -799,6 +804,11 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
const char *arg = *argv++;
if (*arg == '-') {
+ if (!strcmp(arg, "--quiet")) {
+ quiet = 1;
+ continue;
+ }
+
if (!strcmp(arg, "--advertise-refs")) {
advertise_refs = 1;
continue;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index c1f6ddd..a8d6b4c 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -263,6 +263,8 @@ int send_pack(struct send_pack_args *args,
args->use_ofs_delta = 1;
if (server_supports("side-band-64k"))
use_sideband = 1;
+ if (!server_supports("quiet"))
+ args->quiet = 0;
if (!remote_refs) {
fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
@@ -301,11 +303,12 @@ int send_pack(struct send_pack_args *args,
char *old_hex = sha1_to_hex(ref->old_sha1);
char *new_hex = sha1_to_hex(ref->new_sha1);
- if (!cmds_sent && (status_report || use_sideband)) {
- packet_buf_write(&req_buf, "%s %s %s%c%s%s",
+ if (!cmds_sent && (status_report || use_sideband || args->quiet)) {
+ packet_buf_write(&req_buf, "%s %s %s%c%s%s%s",
old_hex, new_hex, ref->name, 0,
status_report ? " report-status" : "",
- use_sideband ? " side-band-64k" : "");
+ use_sideband ? " side-band-64k" : "",
+ args->quiet ? " quiet" : "");
}
else
packet_buf_write(&req_buf, "%s %s %s",
@@ -439,6 +442,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
args.force_update = 1;
continue;
}
+ if (!strcmp(arg, "--quiet")) {
+ args.quiet = 1;
+ continue;
+ }
if (!strcmp(arg, "--verbose")) {
args.verbose = 1;
continue;
diff --git a/remote-curl.c b/remote-curl.c
index b8cf45a..2341106 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -762,7 +762,9 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
argv[argc++] = "--thin";
if (options.dry_run)
argv[argc++] = "--dry-run";
- if (options.verbosity > 1)
+ if (options.verbosity == 0)
+ argv[argc++] = "--quiet";
+ else if (options.verbosity > 1)
argv[argc++] = "--verbose";
argv[argc++] = url;
for (i = 0; i < nr_spec; i++)
diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
index c229fe6..9ee52cf 100755
--- a/t/t5523-push-upstream.sh
+++ b/t/t5523-push-upstream.sh
@@ -108,4 +108,11 @@ test_expect_failure TTY 'push --no-progress suppresses progress' '
! grep "Writing objects" err
'
+test_expect_success TTY 'quiet push' '
+ ensure_fresh_upstream &&
+
+ test_terminal git push --quiet --no-progress upstream master 2>&1 | tee output &&
+ test_cmp /dev/null output
+'
+
test_done
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index a73c826..e756a08 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -5,6 +5,7 @@
test_description='test smart pushing over http via http-backend'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh
if test -n "$NO_CURL"; then
skip_all='skipping test, git built without http support'
@@ -154,5 +155,12 @@ test_expect_success 'push (chunked)' '
test $HEAD = $(git rev-parse --verify HEAD))
'
+test_expect_success TTY 'quiet push' '
+ cd "$ROOT_PATH"/test_repo_clone &&
+ test_commit quiet &&
+ test_terminal git push --quiet --no-progress 2>&1 | tee output &&
+ test_cmp /dev/null output
+'
+
stop_httpd
test_done
--
1.7.6.1
^ permalink raw reply related
* Re: How to use git attributes to configure server-side checks?
From: Stephen Bash @ 2011-09-23 12:49 UTC (permalink / raw)
To: Michael Haggerty
Cc: git discussion list, Jay Soffian, Jeff King, Jakub Narebski,
Junio C Hamano
In-Reply-To: <4E7C44C8.10000@alum.mit.edu>
----- Original Message -----
> From: "Michael Haggerty" <mhagger@alum.mit.edu>
> Sent: Friday, September 23, 2011 4:35:20 AM
> Subject: Re: How to use git attributes to configure server-side checks?
>
> On 09/22/2011 07:26 PM, Junio C Hamano wrote:
> > Michael Haggerty <mhagger@alum.mit.edu> writes:
> >
> >> I would like the checking configuration to be *versioned* along with the
> >> code. For example, suppose my project decides to enforce a rule that
> >> all Python code needs to be indented with spaces. It might be that not
> >> all of our old code adheres to this rule, and that we only want to clean
> >> up the code in master.
> >
> > You want to sneak in a badly formatted code? Add an entry to the
> > in-tree attributes file to disable whitespace checking to cover that file!
>
> No, the scenario that I was trying to describe is a project that wants
> to tighten up its code formatting rules after years of laxity. It is
> convenient to support legacy branches that still contain nonconforming
> code without having to reformat it all, just as it is convenient to
> fix the current code incrementally rather than requiring all of the
> cleanup to be done in one big bang. Thus it is important that new rules not be
> enforced retroactively on old code.
We're in the process of a similar change over (we're dealing with EOL rather than indents), but I attacked it from a different angle... I wrote our update script to examine modified files and ensure compliance (diff-tree -r, iterate over blobs). That way legacy files are left alone (even in master), but active development must live up to the current rules. Is there a reason you need to go tree-by-tree rather than file-by-file?
Thanks,
Stephen
^ permalink raw reply
* [PATCH] use -h for synopsis and --help for manpage consistently
From: Clemens Buchacher @ 2011-09-23 11:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaa9xyxpf.fsf@alter.siamese.dyndns.org>
Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
On Wed, Sep 21, 2011 at 10:04:28PM -0700, Junio C Hamano wrote:
>
> * cb/send-email-help (2011-09-12) 1 commit
> (merged to 'next' on 2011-09-14 at ae71999)
> + send-email: add option -h
>
> A separate set of patches to remove the hidden fully-spelled "help" from
> other commands would be nice to have as companion patches as well.
This should do it.
Clemens
Documentation/blame-options.txt | 1 -
git-cvsserver.perl | 4 ++--
git-difftool.perl | 2 +-
git-pull.sh | 2 +-
git-sh-setup.sh | 2 +-
git-svn.perl | 2 +-
6 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index e76195a..d4a51da 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -117,5 +117,4 @@ commit. And the default value is 40. If there are more than one
take effect.
-h::
---help::
Show help message.
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 1b8bff2..6c5185e 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -109,14 +109,14 @@ my $usage =
" --strict-paths : Don't allow recursing into subdirectories\n".
" --export-all : Don't check for gitcvs.enabled in config\n".
" --version, -V : Print version information and exit\n".
- " --help, -h, -H : Print usage information and exit\n".
+ " -h : Print usage information and exit\n".
"\n".
"<directory> ... is a list of allowed directories. If no directories\n".
"are given, all are allowed. This is an additional restriction, gitcvs\n".
"access still needs to be enabled by the gitcvs.enabled config option.\n".
"Alternately, one directory may be specified in GIT_CVSSERVER_ROOT.\n";
-my @opts = ( 'help|h|H', 'version|V',
+my @opts = ( 'h', 'version|V',
'base-path=s', 'strict-paths', 'export-all' );
GetOptions( $state, @opts )
or die $usage;
diff --git a/git-difftool.perl b/git-difftool.perl
index ced1615..09b65f1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -97,7 +97,7 @@ sub generate_command
$prompt = 'yes';
next;
}
- if ($arg eq '-h' || $arg eq '--help') {
+ if ($arg eq '-h') {
usage();
}
push @command, $arg;
diff --git a/git-pull.sh b/git-pull.sh
index 63da37b..8c1370f 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -120,7 +120,7 @@ do
--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
dry_run=--dry-run
;;
- -h|--h|--he|--hel|--help|--help-|--help-a|--help-al|--help-all)
+ -h|--help-all)
usage
;;
*)
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 8e427da..1fba6c2 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -90,7 +90,7 @@ $LONG_USAGE"
fi
case "$1" in
- -h|--h|--he|--hel|--help)
+ -h)
echo "$LONG_USAGE"
exit
esac
diff --git a/git-svn.perl b/git-svn.perl
index 89f83fd..a019f55 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -294,7 +294,7 @@ read_git_config(\%opts);
if ($cmd && ($cmd eq 'log' || $cmd eq 'blame')) {
Getopt::Long::Configure('pass_through');
}
-my $rv = GetOptions(%opts, 'help|H|h' => \$_help, 'version|V' => \$_version,
+my $rv = GetOptions(%opts, 'h' => \$_help, 'version|V' => \$_version,
'minimize-connections' => \$Git::SVN::Migration::_minimize,
'id|i=s' => \$Git::SVN::default_ref_id,
'svn-remote|remote|R=s' => sub {
--
1.7.6.1
^ permalink raw reply related
* Re: [PATCH v3 13/22] resolve_ref(): turn buffer into a proper string as soon as possible
From: Michael Haggerty @ 2011-09-23 13:11 UTC (permalink / raw)
To: Thomas Rast
Cc: git, Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <201109231017.55996.trast@student.ethz.ch>
On 09/23/2011 10:17 AM, Thomas Rast wrote:
> Michael Haggerty wrote:
>> Immediately strip off trailing spaces and null-terminate the string
>> holding the contents of the reference file; this allows the use of
>> string functions and avoids the need to keep separate track of the
>> string's length. (get_sha1_hex() fails automatically if the string is
>> too short.)
>>
>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
>
> I'm getting valgrind failures in t1450-fsck and t3800-mktag which
> blame to this commit. For t1450 it looks as follows:
>
> ok 5 - object with bad sha1
>
> expecting success:
> git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
> test_when_finished "git update-ref -d refs/heads/invalid" &&
> git fsck 2>out &&
> cat out &&
> grep "not a commit" out
>
> ==19623== Use of uninitialised value of size 8
> ==19623== at 0x4B6747: hexval (cache.h:798)
> ==19623== by 0x4B6797: get_sha1_hex (hex.c:42)
> ==19623== by 0x4DD12A: resolve_ref (refs.c:588)
Thanks for finding this.
The problem comes from passing get_sha1_hex() an arbitrary
NUL-terminated string. get_sha1_hex() calls hexval(), which returns -1
if it finds a NUL character, and therefore (correctly) rejects any
string that is less than 40 characters. The problem is that
get_sha1_hex() reads characters pairwise, and even if the first
character in a pair is NUL, the second character is read anyway.
Therefore, for strings whose strlen is an even number less than 40,
get_sha1_hex() reads past the terminating NUL.
I don't know whether get_sha1_hex() is *supposed* to work for arbitrary
NUL-terminated strings because it is not documented. But other callers
seem to make the same assumption that I did, so I think that I will fix
get_sha1_hex() to not read past a NUL value (albeit at the cost of an
extra check in each iteration of the loop).
The fix is trivial and will be sent shortly.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: Fwd: vcs-svn and friends
From: Dmitry Ivankov @ 2011-09-23 13:27 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: David Michael Barr, Junio C Hamano, git
In-Reply-To: <CA+gfSn9KVN2iDCevd0s+TjYHNupDez8NiKZycP3pgBCkYiraFQ@mail.gmail.com>
On Thu, Sep 15, 2011 at 7:00 PM, Dmitry Ivankov <divanorama@gmail.com> wrote:
>> - 3bba32e9 ("fast-import: allow top directory as an argument for some
>> commands"): I'm not sure what the motivation is --- is this just
>> about the principle of least surprise, or did it come up in practice
>> somewhere?
> (to ease one's reading, commands are ls, copy and move top directory)
>
> Haven't seen them in practice. It seemed possible with svn import: if there were
> no branches at start, and then someone did svn mv . trunk. But it
> turns out that my
> svn client doesn't allow such move. So more like a least surprise purpose.
I think now that this commit should go separately if at all.
Especially considering
my other activity on fast-import (and thus possible merge conflicts) that isn't
strictly necessary for vcs-svn and friends.
^ permalink raw reply
* Re: Fwd: vcs-svn and friends
From: Dmitry Ivankov @ 2011-09-23 13:29 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: David Michael Barr, Junio C Hamano, git
In-Reply-To: <CA+gfSn8UeqiLu3trecPzzqSLsUr3eNT0yVN=-991sL6oJHar2g@mail.gmail.com>
On Fri, Sep 23, 2011 at 7:27 PM, Dmitry Ivankov <divanorama@gmail.com> wrote:
> On Thu, Sep 15, 2011 at 7:00 PM, Dmitry Ivankov <divanorama@gmail.com> wrote:
>>> - 3bba32e9 ("fast-import: allow top directory as an argument for some
>>> commands"): I'm not sure what the motivation is --- is this just
>>> about the principle of least surprise, or did it come up in practice
>>> somewhere?
>> (to ease one's reading, commands are ls, copy and move top directory)
>>
>> Haven't seen them in practice. It seemed possible with svn import: if there were
>> no branches at start, and then someone did svn mv . trunk. But it
>> turns out that my
>> svn client doesn't allow such move. So more like a least surprise purpose.
> I think now that this commit should go separately if at all.
> Especially considering
> my other activity on fast-import (and thus possible merge conflicts) that isn't
> strictly necessary for vcs-svn and friends.
And the same for 3bba32e9^ "fast-import: be saner with temporary trees", which
is only needed for 3bba32e9.
^ permalink raw reply
* Re: How to use git attributes to configure server-side checks?
From: Michael Haggerty @ 2011-09-23 13:31 UTC (permalink / raw)
To: Stephen Bash
Cc: git discussion list, Jay Soffian, Jeff King, Jakub Narebski,
Junio C Hamano
In-Reply-To: <33047451.27244.1316782148569.JavaMail.root@mail.hq.genarts.com>
On 09/23/2011 02:49 PM, Stephen Bash wrote:
> We're in the process of a similar change over (we're dealing with EOL
> rather than indents), but I attacked it from a different angle... I
> wrote our update script to examine modified files and ensure
> compliance (diff-tree -r, iterate over blobs). That way legacy files
> are left alone (even in master), but active development must live up
> to the current rules. Is there a reason you need to go tree-by-tree
> rather than file-by-file?
I want to avoid code churn, especially in third-party code. With your
solution, I believe that we would be forced to entirely clean up any
file that we needed to touch. The resulting code churn would make
integrating future upstream releases a nightmare.
For some kinds of checks, one could only check that the *lines* changed
satisfy the new rules.
But rather than thinking up workarounds, it seems like a better idea to
fix git to handle .gitattributes correctly.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* [PATCH 1/1] get_sha1_hex(): do not read past a NUL character
From: Michael Haggerty @ 2011-09-23 13:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Rast, Michael Haggerty
In-Reply-To: <4E7C857D.8000304@alum.mit.edu>
Previously, get_sha1_hex() would read one character past the end of a
null-terminated string whose strlen was an even number less than 40.
Although the function correctly returned -1 in these cases, the extra
memory access might have been to uninitialized (or even, conceivably,
unallocated) memory.
Add a check to avoid reading past the end of a string.
This problem was discovered by Thomas Rast <trast@student.ethz.ch>
using valgrind.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
It is suggested to apply this bugfix before mh/check-ref-format-3;
otherwise the latter triggers the bug, leading to a valgrind error.
This patch could optionally be applied to maint (to which it can be
rebased cleanly), though the bug that it fixes is relatively benign.
cache.h | 9 +++++++++
hex.c | 10 +++++++++-
2 files changed, 18 insertions(+), 1 deletions(-)
diff --git cache.h cache.h
index 607c2ea..e7bbc0d 100644
--- cache.h
+++ cache.h
@@ -819,7 +819,16 @@ static inline int get_sha1_with_context(const char *str, unsigned char *sha1, st
{
return get_sha1_with_context_1(str, sha1, orc, 0, NULL);
}
+
+/*
+ * Try to read a SHA1 in hexadecimal format from the 40 characters
+ * starting at hex. Write the 20-byte result to sha1 in binary form.
+ * Return 0 on success. Reading stops if a NUL is encountered in the
+ * input, so it is safe to pass this function an arbitrary
+ * null-terminated string.
+ */
extern int get_sha1_hex(const char *hex, unsigned char *sha1);
+
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
extern int read_ref(const char *filename, unsigned char *sha1);
extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
diff --git hex.c hex.c
index bb402fb..9ebc050 100644
--- hex.c
+++ hex.c
@@ -39,7 +39,15 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)
{
int i;
for (i = 0; i < 20; i++) {
- unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);
+ unsigned int val;
+ /*
+ * hex[1]=='\0' is caught when val is checked below,
+ * but if hex[0] is NUL we have to avoid reading
+ * past the end of the string:
+ */
+ if (!hex[0])
+ return -1;
+ val = (hexval(hex[0]) << 4) | hexval(hex[1]);
if (val & ~0xff)
return -1;
*sha1++ = val;
--
1.7.7.rc2
^ permalink raw reply
* Re: What's The Right Way to Do This?
From: Jon Forrest @ 2011-09-23 13:45 UTC (permalink / raw)
To: Luke Diamand; +Cc: git
In-Reply-To: <4E7C27A0.9030900@diamand.org>
On 9/22/2011 11:30 PM, Luke Diamand wrote:
> Someone will be along soon to correct me, but I think you should have
> rebased against the master branch rather than merged. Something like:
>
> % git rebase origin
>
> That way your changes would remain appended to the tip of the master
> branch. That will in general make your life much easier for all sorts of
> reasons.
In considering this, I'm having trouble understanding when to do a merge
as opposed to a rebase. Let's say that other people weren't putting
changed back in the remote master. So, my changes would always be at the
tip. Is that when I should merge? If so, then this suggest that knowing
what to do depends on knowing what other people are doing. But, doesn't
this go against the philosophy of a distributed SCM?
> At a guess, git revert created a commit that undid your _merge_, rather
> than your commit. The merge included all those other changes....
>
> It's a good idea to take a look at what a commit does before pushing it
> - "git show HEAD" is your friend.
IIRC, I did this and I saw the collection of the changes I had
made plus what other people had made. I didn't know how to revert
only my changes.
> Use git rebase (*) to keep your changes nicely arranged at the top of
> the main branch.
Thanks. I'll study up on this.
> I think you could award yourself a nice cup of tea after an experience
> like that though, having been on both sides of it, I can imagine you
> need it :-P
It certainly wasn't my finest hour!
Jon
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox