[PATCH 1/3] trailer: add a trailer.trimEmpty config option

2015-02-07 Thread Christian Couder
This way people who always want trimed trailers
don't need to specify it on the command line.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/interpret-trailers.c |  2 +-
 trailer.c| 13 ++---
 trailer.h|  2 +-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 46838d2..1adf814 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -18,7 +18,7 @@ static const char * const git_interpret_trailers_usage[] = {
 
 int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
 {
-   int trim_empty = 0;
+   int trim_empty = -1;
struct string_list trailers = STRING_LIST_INIT_DUP;
 
struct option options[] = {
diff --git a/trailer.c b/trailer.c
index 623adeb..7614182 100644
--- a/trailer.c
+++ b/trailer.c
@@ -36,6 +36,8 @@ static struct trailer_item *first_conf_item;
 
 static char *separators = :;
 
+static int trim_empty;
+
 #define TRAILER_ARG_STRING $ARG
 
 static int after_or_end(enum action_where where)
@@ -120,7 +122,7 @@ static void print_tok_val(const char *tok, const char *val)
printf(%s%c %s\n, tok, separators[0], val);
 }
 
-static void print_all(struct trailer_item *first, int trim_empty)
+static void print_all(struct trailer_item *first)
 {
struct trailer_item *item;
for (item = first; item; item = item-next) {
@@ -509,6 +511,8 @@ static int git_trailer_default_config(const char *conf_key, 
const char *value, v
value, conf_key);
} else if (!strcmp(trailer_item, separators)) {
separators = xstrdup(value);
+   } else if (!strcmp(trailer_item, trimempty)) {
+   trim_empty = git_config_bool(conf_key, value);
}
}
return 0;
@@ -842,7 +846,7 @@ static void free_all(struct trailer_item **first)
}
 }
 
-void process_trailers(const char *file, int trim_empty, struct string_list 
*trailers)
+void process_trailers(const char *file, int trim, struct string_list *trailers)
 {
struct trailer_item *in_tok_first = NULL;
struct trailer_item *in_tok_last = NULL;
@@ -854,6 +858,9 @@ void process_trailers(const char *file, int trim_empty, 
struct string_list *trai
git_config(git_trailer_default_config, NULL);
git_config(git_trailer_config, NULL);
 
+   if (trim  -1)
+   trim_empty = trim;
+
lines = read_input_file(file);
 
/* Print the lines before the trailers */
@@ -863,7 +870,7 @@ void process_trailers(const char *file, int trim_empty, 
struct string_list *trai
 
process_trailers_lists(in_tok_first, in_tok_last, arg_tok_first);
 
-   print_all(in_tok_first, trim_empty);
+   print_all(in_tok_first);
 
free_all(in_tok_first);
 
diff --git a/trailer.h b/trailer.h
index 8eb25d5..4f481d0 100644
--- a/trailer.h
+++ b/trailer.h
@@ -1,6 +1,6 @@
 #ifndef TRAILER_H
 #define TRAILER_H
 
-void process_trailers(const char *file, int trim_empty, struct string_list 
*trailers);
+void process_trailers(const char *file, int trim, struct string_list 
*trailers);
 
 #endif /* TRAILER_H */
-- 
2.2.1.313.gcc831f2


--
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/3] trailer: add tests for trailer.trimEmpty

2015-02-07 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t7513-interpret-trailers.sh | 72 +++
 1 file changed, 72 insertions(+)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index bd0ab46..066d00b 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -892,4 +892,76 @@ test_expect_success 'with no command and no key' '
test_cmp expected actual
 '
 
+test_expect_success 'with trailer.trimEmpty set to true' '
+   git config trailer.trimEmpty true 
+   cat expected -EOF 
+
+   sign: A U Thor aut...@example.com
+   Thanks-to: Johannes
+   EOF
+   git interpret-trailers --trailer review: \
+   --trailer Thanks-to:Johannes actual -EOF
+   EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'with trailer.trimEmpty set to false' '
+   git config trailer.trimEmpty false 
+   sed -e s/ Z\$/ / expected -EOF 
+
+   review: Z
+   sign: A U Thor aut...@example.com
+   Thanks-to: Johannes
+   EOF
+   git interpret-trailers --trailer review: \
+   --trailer Thanks-to:Johannes actual -EOF
+   EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'with trailer.trimEmpty set to false and a message' '
+   cat complex_message_body expected 
+   sed -e s/ Z\$/ / expected -\EOF 
+   Fixes: Z
+   Acked-by= Z
+   Reviewed-by: Z
+   Signed-off-by: Z
+   sign: A U Thor aut...@example.com
+   Thanks-to: Johannes
+   EOF
+   git interpret-trailers --trailer review: \
+   --trailer Thanks-to:Johannes complex_message actual
+   test_cmp expected actual 
+   git interpret-trailers --no-trim-empty --trailer review: \
+   --trailer Thanks-to:Johannes complex_message actual
+   test_cmp expected actual
+'
+
+test_expect_success 'with trailer.trimEmpty set to 1 and a message' '
+   git config trailer.trimEmpty 1 
+   cat complex_message_body expected 
+   sed -e s/ Z\$/ / expected -\EOF 
+   sign: A U Thor aut...@example.com
+   Thanks-to: Johannes
+   EOF
+   git interpret-trailers --trailer review: \
+   --trailer Thanks-to:Johannes complex_message actual
+   test_cmp expected actual
+'
+
+test_expect_success 'with trailer.trimEmpty set to 1 and --no-trim-empty' '
+   cat complex_message_body expected 
+   sed -e s/ Z\$/ / expected -\EOF 
+   Fixes: Z
+   Acked-by= Z
+   Reviewed-by: Z
+   Signed-off-by: Z
+   sign: A U Thor aut...@example.com
+   Thanks-to: Johannes
+   EOF
+   git interpret-trailers --no-trim-empty --trailer review: \
+   --trailer Thanks-to:Johannes complex_message actual
+   test_cmp expected actual
+'
+
 test_done
-- 
2.2.1.313.gcc831f2


--
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/3] trailer: add a trailer.trimEmpty config option

2015-02-07 Thread Christian Couder
On Sat, Feb 7, 2015 at 9:20 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder chrisc...@tuxfamily.org writes:

 This way people who always want trimed trailers
 don't need to specify it on the command line.

 I sense that print_all() that cares about trimming illustrate a
 design mistake in the original code structure.  Why isn't the entire
 program structured like this instead?

 - read input and split that into three parts:

 struct {
 struct strbuf messsage_proper;

 struct trailers {
 int nr, alloc;
 struct strbuf *array_of_trailers[];
 } trailers;

 struct strbuf lines_after_trailers;
 };

It is not designed like this because you only asked me to design it
like this after the facts, when there was another email thread about
conflicts blocks and one function you created could be used by the
trailer code too.

If you had asked this from the beginning I would certainly have done
it more like this (though I think the struct trailers part is too
simplistic). Rearchitecturing the code now would bring only small
performance improvements and a lot of code churn. And the performance
is not needed anyway if the code is not used in the first place, so
I'd rather first make sure that the code can be used.

I think that very few new features are now needed to make it possible
to use the code in other commands like commit, format-patch, am, etc,
but this patch implements one of the needed features.

 - do trailer stuff by calling a central helper that does
   trailer stuff a pointer to the middle, trailers, struct.

   - when the trailer becomes a reusable subsystem, this central
 helper will become the primary entry point to the API.

   - trailer stuff will include adding new ones, removing
 existing ones, and rewriting lines.  What you do in the
 current process_command_line_args() and
 process_trailers_lists() [*1*] would come here.

 - write out the result, given the outermost struct.  This will
   become another entry point in the API.

 Structured that way, callers will supply that outermost structure,
 and can trust that the trailers subsystem will not molest
 message_proper or lines_after_trailers part.

I don't think it is a big improvement because it is easy to see that
the current code doesn't molest the part before and after the
trailers.

 They can even process
 the parts that the trailer subsystem would not touch, e.g. running
 stripspace to the text in message_proper.

That could be worth the rearchitecturing if people really wanted that,
but I think for now more people have been interested in having ways to
change the trailer part, so I prefer to work on this first.

 Viewed that way, it would be clear that strip empty ones should be
 a new feature in the trailer stuff, not just filter out only
 during the output phase.  Having it in the output phase does not
 feel that the labor/responsibility is split in the right way.

My opinion is that having it in the output phase is best.

 Another problem I have with filter out during the output phase
 comes from the semantics/correctness in the resulting code, and I
 suspect that it would need to be done a lot earlier, before you
 allow the logic such as if I have X, do this, but if there is no X,
 do this other thing.  If you have an X that is empty in the input,
 trimming that before such logic kicks in and trimming that in the
 final output phase would give different results, and I think the
 latter would give a less intuitive result.

I think it is much better in the output phase because it is very
natural for some projects to have a template message with empty
trailers like this:

Signed-off-by:
Reviewed-by:

Such a template message can for example remind contributors to the
project that they need to sign off their work and that they need to
have it reviewed by at least one person, and that to make it simpler
for everyone to review patches, the Signed-off-by trailers should
come before the Reviewed-by trailers in the commit message.

In this case, if you trim before you process command line trailers,
then, when you process some command line trailers that add sign offs,
the meaning of where=after cannot be based any more on the existing
empty Signed-off-by: in the template message.

 As this is the second time I have to point out that the data
 structure used by the current code to hold the trailers and other
 stuff to work on smells fishy, I would seriously consider cleaning
 up the existing code to make it easier to allow us to later create
 a reusable API and trailer subsystem out of it, before piling new
 features on top of it, if I were you.

I am not so sure that it would make it easier to allow us to later
create a reusable API and trailer subsystem out of it.
It is very difficult to predict what will be really needed in the
future

Re: Question about the revision walking API

2015-01-06 Thread Christian Couder
Hi,


On Tue, Jan 6, 2015 at 3:02 AM, Mike Hommey m...@glandium.org wrote:
 Hi,

 I would like to know if the revision walking API works as one would
 expect with a calling sequence like the following:

 - init_revisions
 - add_pending_object/setup_revisions
 - prepare_revision_walk
 - get_revision (repeated)
 - reset_revision_walk (I guess)
 - add_pending_object
 - prepare_revision_walk
 - get_revision (repeated)

 That is, do a first revision walk based on a given rev_info, then reuse that
 rev_info with additional commit objects (in my case, I want to add more
 UNINTERESTING commits) and redo a revision walk based on the modified
 rev_info (so, avoid reinitializing a rev_info and filling it from
 scratch again with the additional UNINTERESTING commits).

 I guess I could try and see if that works, but I'd rather have an
 informed answer than to derive my own from the fact my testcase would
 happen to work by chance.

I am not sure what you describe above would work, but something like
what is done in bisect.c should work, see check_ancestors() and
bisect_next_all(). It might not be the most efficient solution though,
so I am interested if you find something more efficient.

Best,
Christian.
--
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 v4] rev-list: refuse --first-parent combined with --bisect

2015-03-18 Thread Christian Couder
On Tue, Mar 17, 2015 at 9:46 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 On Tue, Mar 17, 2015 at 7:33 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 Yes, but the user is supposed to not change the bad pointer for no
 good reason.

 That is irrelevant, no?  Nobody is questioning that the user is
 supposed to judge if a commit is good or bad correctly.

 So if there is already a bad commit and the user gives another
 bad commit, that means that the user knows that it will replace the
 existing bad commit with the new one and that it's done for this
 purpose.

 ECANNOTQUITEPARSE.  The user may say git bisect bad $that and we
 do not question $that is bad. Git does not know better than the
 user.

 But that does not mean Git does not know better than the user how
 the current bad commit and $that commit are related.  The user is
 not interested in replacing at all.  The user is telling just one
 single fact, that is, $that is bad.

The user may make mistakes and try to fix them, like for example:

$ git checkout master
$ git bisect bad
$ git log --oneline --decorate --graph --all
# Ooops I was not on the right branch
$ git checkout dev
$ git bisect bad
$ git log --oneline --decorate --graph --all
# Everything looks ok now; the bad commit is what I expect
# I can properly continue bisecting using git bisect good...

In this case the user, who knows how git bisect works, expected that
the second git bisect bad would fix the previous mistake made using
the first git bisect bad.

If we make git bisect bad behave in another way we may break an
advanced user's expectation.

 I am not quite sure if I am correctly getting what you meant to say,
 but if you meant only when --alternate is given, we should do the
 merge-base thing; we should keep losing the current 'bad' and
 replace it with the new one without the --alternate option, I would
 see that as an exercise of a bad taste.

 What I wanted to say is that if we change git bisect bad commitish,
 so that now it means add a new bad commit instead of the previous
 replace the current bad commit, if any, with this one, then experienced
 users might see that change as a regression in the user interface and
 it might even break scripts.

 Huh?

 Step back a bit.  The place you need to start from is to admit the
 fact that what git bisect bad committish currently does is
 broken.

 Try creating this history yourself

 a---b---c---d---e---f

 and start bisection this way:

 $ git bisect start f c
 $ git bisect bad a

 Immediately after the second command, git bisect moans

 Some good revs are not ancestor of the bad rev.
 git bisect cannot work properly in this case.
 Maybe you mistake good and bad revs?

 when it notices that the good rev (i.e. 'c') is no longer an
 ancestor of the 'bad', which now points at 'a'.

 But that is because git bisect bad _blindly_ moved 'bad' that used
 to point at 'f' to 'a', making a good rev (i.e. 'c') an ancestor of
 the bad rev, without even bothering to check.

Yeah, git bisect bad currently does what it is asked and then
complains when it looks like a mistake has been made.

You might see that as a bug. I am seeing that more as git bisect
expecting users to know what they are doing.

For example an advanced user might have realized that the first git
bisect start f c was completely rubish for some reason, and the git
bisect bad a might be a first step to fix that. (The next step might
then be deleting the good pointer...)

 Now, if we fixed this bug and made the bisect_state function more
 careful (namely, when accepting bad, make sure it is not beyond
 any existing good, or barf like the above, _without_ moving the
 bad pointer), the user interface and behaviour would be changed.  Is
 that a regression?  No, it is a usability fix and a progress.

Yeah, you might see that as a usability fix and a progress.

 Simply put, bisect_state function can become more careful and
 intelligent to help users.

Yeah, we can try to help users more, but doing that we might annoy
some advanced users,  who are used to the current way git bisect
works, and perhaps break some scripts.
--
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


Draft of Git Rev News edition 1

2015-03-22 Thread Christian Couder
Hi,

A draft of Git Rev News edition 1 is available here:

https://github.com/git/git.github.io/blob/master/rev_news/draft/edition-1.md

Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

https://github.com/git/git.github.io/issues/17

You can also reply to this email.

I tried to cc the persons who appear in the edition but maybe I missed
some, sorry about that.

Thomas and myself plan to publish this edition on Wednesday the 25th of March.

We call it an edition instead of an issue to avoid confusion with
GitHub issues.

Thanks,
Christian
--
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: Draft of Git Rev News edition 1

2015-03-22 Thread Christian Couder
On Sun, Mar 22, 2015 at 12:21 PM, David Kastrup d...@gnu.org wrote:
 I've seen

 David Kastrup (dak at gnu.org) previously reimplemented significant
 parts of git blame for a vast gain in performance with complex
 histories and large files. As working on free software is his sole
 source of income, please consider contributing to his remuneration
 if you find this kind of improvements useful.

 Thank you very much for this heads-up.  However, I'd replace
 previously with as of version 2.1.0.  That's where the big
 difference is, so if people actually are impacted they'll know whether
 and what to benchmark and/or upgrade.

Ok, there is now as of version 2.1.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: Promoting Git developers

2015-03-15 Thread Christian Couder
On Thu, Mar 12, 2015 at 11:58 PM, Junio C Hamano gits...@pobox.com wrote:
 Jeff King p...@peff.net writes:

 Seeing my name in shortlog was nice, but not that exciting. I
 submitted a patch, it was taken, and of course it ends up in any
 automated lists of authors. What was much more rewarding was being
 mentioned specifically in A note from the maintainer as a helpful
 person. That had much more value because:

   1. It was one of a handful of names.

   2. It was picked by a human.

 So in that sense, it is quite the opposite of including shortlog output
 in the release announcements (I still think the shortlog thing we have
 been discussing is a good thing, but not at the same level).

 Yes, and that cuts both ways, unfortunately. There always will be I
 am doing more reviews than X and my reviews are higher quality. Why
 was X singled out and got thanked but not me?, X is really doing a
 good job reviewing in this cycle, but could other people who send
 reviews of lessor quality (to my mind) feel that it is unjustified
 if I thanked X and nobody else?, etc. A mechanically generated list
 avoids these issues, but the satisfaction you get from being on the
 list is not very high, exactly because it is not hand picked.

I think it is still much better to have some people positively hand
picked than nothing.
People who have not been hand picked despite having done something
they think is of the same or higher quality can always ask privately
about the reason they haven't been hand picked or they can try again
expecting that the outcome will be different next time.

Anyway if some people are positively hand picked you can always hope
that it will happen to you, while otherwise there is no hope at all.

 I do not know that it is worth having a Best of 2015 Git awards
 ceremony, but it is sometimes nice to thank people personally when
 you appreciate their efforts. I sometimes mail people off-list to
 do so.

 Yeah, I do the same, but revealing that we do so would defeat what
 we tried to achieve by doing so off-list in the first place. Now
 those who haven't got such a piece of e-mail for a while can start
 to suspect that they have fallen out of favour or something ;-(.

I don't think it defeat anything. I think you could even do it more online.

Best,
Christian.
--
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: Promoting Git developers

2015-03-15 Thread Christian Couder
On Wed, Mar 11, 2015 at 9:42 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 On Tue, Mar 10, 2015 at 6:23 PM, Junio C Hamano gits...@pobox.com wrote:

 I would suspect that those who agree with you would appreciate if
 you or somebody volunteered to act as our CKDO (chief kudos
 distribution officer).  I do not think I have enough time to do that
 well.  One good place to start might be to scan the list and
 summarize something like the following on weekly or monthly basis,
 as these are not something you can get by pointing people to git
 shortlog output.

  - Those who gave helpful review comments, how about going this
way illustration patches, etc.  Bonus points to those who helped
onboarding newcomers.

  - Those who asked pertinent questions on common pain points, and
those who answered them helpfully.

 Ok, I can start something about this two points every week or every
 few week. It would be best if I could get help from at least one
 person as I think it is a lot of work.

 No kidding; even though it may no longer be an impossibly large task
 as in the infrationary epoch reported in the Git Traffic, this forum
 is still a high traffic place.

I wrote something about a potential Git Rev News news letter:

https://github.com/git/git.github.io/pull/15

Peff, could you give me write access so that I don't need to send pull requests?
If some people are interested to contribute even if it is only
sporadically, I would suggest they ask for write access too.

 I also appreciate very much that you are willing to improve the
 release notes by adding a summary with people's names.

 Just in case you misunderstood, I do not think it is a good idea to
 add names to release notes and I will not do so.

 I was and am planning add the list of contributors at the end of the
 e-mail when the release notes is sent out, i.e. in the Announce
 message that is sent to the list (and CC'ed to lwn.net).

Ok, that is already very nice.

Thanks,
Christian.
--
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: Promoting Git developers

2015-03-17 Thread Christian Couder
On Mon, Mar 16, 2015 at 6:06 PM, Stefan Beller sbel...@google.com wrote:
 On Mon, Mar 16, 2015 at 2:20 AM, David Kastrup d...@gnu.org wrote:

 Git Annotate?

 Git Praise as opposed to blame?
 Git Who as a pun on the subcommand structure which doesn't always
 follows grammar?

Yeah these suggestions above are nice, thanks for them, but Git Rev News
also look a bit like git rev-list and git rev-parse which are plumbing Git
commands, so it gives a somewhat hardcore look to the news letter which
I like.

Thanks,
Christian.
--
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: Promoting Git developers

2015-03-17 Thread Christian Couder
On Sun, Mar 15, 2015 at 11:18 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 I wrote something about a potential Git Rev News news letter:

 I read it.  Sounds promising.

Thanks!

[...]

 I obviously do not know how the actual contents would look like at
 this point, but depending on the quality of the publication I might
 be able to steal some descriptions when keeping the notes on topics
 in flight that appear in my What's cooking report.  And it can go
 the other way around, too.  The publication may want to peek my
 What's cooking report for hints on how to characterize each topic
 and assess its impact to the evolution of Git.

Yeah, it would be a very nice thing if we could steal these kind of
things from each other's work!

Thanks,
Christian.
--
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 v4] rev-list: refuse --first-parent combined with --bisect

2015-03-17 Thread Christian Couder
On Tue, Mar 17, 2015 at 7:33 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 On Mon, Mar 16, 2015 at 10:05 PM, Junio C Hamano gits...@pobox.com wrote:

 However, you can say git bisect bad rev (and git bisect good
 rev for that matter) on a rev that is unrelated to what the
 current bisection state is.  E.g. after you mark the child of 8 as
 bad, the bisected graph would become

G...1---2---3---4---6---8---B

 and you would be offered to test somewhere in the middle, say, 4.
 But it is perfectly OK for you to respond with git bisect bad 7,
 if you know 7 is bad.

 I _think_ the current code blindly overwrites the bad pointer,
 making the bisection state into this graph if you do so.

G...1---2---3---4
 \
  5---B

 Yes, we keep only one bad pointer.

 This is very suboptimal.  The side branch 4-to-7 could be much
 longer than the original trunk 4-to-the-tip, in which case we would
 have made the suspect space _larger_, not smaller.

 Yes, but the user is supposed to not change the bad pointer for no
 good reason.

 That is irrelevant, no?  Nobody is questioning that the user is
 supposed to judge if a commit is good or bad correctly.

So if there is already a bad commit and the user gives another
bad commit, that means that the user knows that it will replace the
existing bad commit with the new one and that it's done for this
purpose.

 And nobody sane is dreaming that Git could do better and detect
 user's mistakes when the user says 'bad' for a commit that is
 actually 'good'; if Git can do that, then it should be able to do
 the bisect without any user input (including bisect run) at all
 ;-).

 We certainly should be able to take advantage of the fact that the
 current bad commit (i.e. the child of 8) and the newly given bad
 commit (i.e. 7) are both known to be bad and mark 4 as bad instead
 when that happens, instead of doing the suboptimal thing the code
 currently does.

 Yeah, we could do that, but we would have to allow it only if a
 special option is passed on the command line, for example:
 git bisect bad --alternate commitish

 I am not quite sure if I am correctly getting what you meant to say,
 but if you meant only when --alternate is given, we should do the
 merge-base thing; we should keep losing the current 'bad' and
 replace it with the new one without the --alternate option, I would
 see that as an exercise of a bad taste.

What I wanted to say is that if we change git bisect bad commitish,
so that now it means add a new bad commit instead of the previous
replace the current bad commit, if any, with this one, then experienced
users might see that change as a regression in the user interface and
it might even break scripts.

That's why I suggested to use a new option to mean
add a new bad commit, though --alternate might not be the best
name for this option.

 Because the merge-base thing is using both the current and the new
 one, such a use is not alternate in the first place.

 If the proposal were with a new option, the user can say 'oh, I
 made a mistake earlier and said that a commit that is not bad as
 'bad'.  Let me replace the commit currently marked as 'bad' with
 this one., I would find it very sensible, actually.

What I find sensible is to not break the semantics of the current
interface.

 I can see that such an operation can be called alternate, but
 --fix might be shorter-and-sweeter-and-to-the-point.

 In the normal case, the commit we offer the user to check (and
 respond with git bisect bad without any commit parameter) is
 always an ancestor of the current 'bad', so the merge-base with
 'bad' and the commit that was just checked would always be the
 current commit.  Using the merge-base thing will be transparent to
 the end users in the normal case, and when the user has off-line
 knowledge that some other commit that is not an ancestor of the
 current 'bad' commit is bad, the merge-base thing will give a better
 behaviour than the current implementation that blindly replaces.

Yes, I agree that it could be an improvement to make it possible for the
user to specify another bad commit. I just think it should be done with
a new option if there is already a bad commit...

 and/or we could make git bisect bad accept any number of bad
 commitishs.

... or by allowing any number of bad commits after git bisect bad.

 Yes, that is exactly what I meant.

 The way I understand the Philip's point is that the user may have
 a-priori knowledge that a breakage from the same cause appears in
 both tips of these branches.  In such a case, we can start bisection
 after marking the merge-base of two 'bad' commits, e.g. 4 in the
 illustration in the message you are responding to, instead of
 including 5, 6, and 8 in the suspect set.

Yeah, I agree that we can do better in this case.

 You need to be careful, though.  An obvious pitfall is what you
 should do when there is a criss-cross

Re: Promoting Git developers

2015-03-17 Thread Christian Couder
On Tue, Mar 17, 2015 at 10:43 AM, Thomas Ferris Nicolaisen
tfn...@gmail.com wrote:
 On Sun, Mar 15, 2015 at 9:46 AM, Christian Couder
 christian.cou...@gmail.com wrote:

 I wrote something about a potential Git Rev News news letter:

 https://github.com/git/git.github.io/pull/15


 I would love to have/use something like this in the GitMinutes
 podcast. Perhaps in addition to the very random interview format that
 I have now, I could do a more regular episodes about Git news, where I
 incorporate this.

Yeah, no problem.

 I also volunteer to help with the production, if you'll allow list
 lurkers like myself to contribute ;)

Yes of course, you are very welcome!

Peff, could you also give the rights on the repo to Thomas?

Thanks both,
Christian.
--
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 v4] rev-list: refuse --first-parent combined with --bisect

2015-03-17 Thread Christian Couder
On Mon, Mar 16, 2015 at 10:05 PM, Junio C Hamano gits...@pobox.com wrote:
 Philip Oakley philipoak...@iee.org writes:

 From: Junio C Hamano gits...@pobox.com

 Hence, if you have a history that looks like this:


   G...1---2---3---4---6---8---B
\
 5---7---B

 it follows that 4 must also be bad.  It used to be good long time
 ago somewhere before 1, and somewhere along way on the history,
 there was a single breakage event that we are hunting for.  That
 single event cannot be 5, 6, 7 or 8 because breakage at say 5 would
 not explain why the tip of the upper branch is broken---its breakage
 has no way to propagate there.  The breakage must have happened at 4
 or before that commit.

 Is it not worth at least confirming the assertion that 4 is bad before
 proceding, or at least an option to confirm that in complex scenarios
 where the fault may be devious.

 That raises a somewhat interesting tangent.

 Christian seems to be forever interested in bisect, so I'll add him
 to the Cc list ;-)

 There is no way to give multiple bad from the command line.  You
 can say git bisect start rev rev rev... but that gives only one
 bad and everything else is good.  And once you specify one of the
 above two bad ones (say, the child of 8), then we will not even
 offer the other one (i.e. the child of 7) as a candidate to be
 tested.  So in that sense, confirm that 4 is bad before proceeding
 is a moot point.

 However, you can say git bisect bad rev (and git bisect good
 rev for that matter) on a rev that is unrelated to what the
 current bisection state is.  E.g. after you mark the child of 8 as
 bad, the bisected graph would become

G...1---2---3---4---6---8---B

 and you would be offered to test somewhere in the middle, say, 4.
 But it is perfectly OK for you to respond with git bisect bad 7,
 if you know 7 is bad.

 I _think_ the current code blindly overwrites the bad pointer,
 making the bisection state into this graph if you do so.

G...1---2---3---4
 \
  5---B

Yes, we keep only one bad pointer.

 This is very suboptimal.  The side branch 4-to-7 could be much
 longer than the original trunk 4-to-the-tip, in which case we would
 have made the suspect space _larger_, not smaller.

Yes, but the user is supposed to not change the bad pointer for no
good reason. For example maybe a mistake was made and the first commit
marked as bad was not actually bad.

 We certainly should be able to take advantage of the fact that the
 current bad commit (i.e. the child of 8) and the newly given bad
 commit (i.e. 7) are both known to be bad and mark 4 as bad instead
 when that happens, instead of doing the suboptimal thing the code
 currently does.

Yeah, we could do that, but we would have to allow it only if a
special option is passed on the command line, for example:

git bisect bad --alternate commitish

and/or we could make git bisect bad accept any number of bad commitishs.

That could give additional bonus points to the GSoC student who would
implement it :-)

Thanks,
Christian.
--
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: Promoting Git developers

2015-03-16 Thread Christian Couder
On Sun, Mar 15, 2015 at 11:43 PM, Randall S. Becker
rsbec...@nexbridge.com wrote:
 On March 15, 2015 6:19 PM Christian Couder wrote:
 snip
 Just one suggestion on the name and half a comment.

 How would Git Review (or Git Monthly Review, or replace your favourite
 how-often-per-period-ly in its name) sound?  I meant it to sound similar
 to
 academic journals that summarize and review contemporary works in the
 field
 and keeps your original pun about our culture around patch reviews.

I would be ok for that but there is already this Gerrit related command:

http://www.mediawiki.org/wiki/Gerrit/git-review

Maybe I can just use Git Rev, but it doesn't tell that it is about news?

 If I may humbly offer the suggestion that Git Blame would be a far more
 appropriate pun as a name :)

You don't want me to steal Junio's blog title:

http://git-blame.blogspot.fr/

don't you?
--
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-scm.com website

2015-03-09 Thread Christian Couder
On Mon, Mar 9, 2015 at 5:37 PM, David Kastrup d...@gnu.org wrote:
 Shawn Pearce spea...@spearce.org writes:

 On Mon, Mar 9, 2015 at 9:06 AM, David Kastrup d...@gnu.org wrote:
 Shawn Pearce spea...@spearce.org writes:

 On Mon, Mar 9, 2015 at 6:57 AM, Michael J Gruber
 g...@drmicha.warpmail.net wrote:

 Since we're talking business: git-scm.com still looks a bit like a
 ProGit/Github promotion site. I don't have anything against either, and
 git-scm.com provides a lot of the information that users are looking
 for, and that are hard to find anywhere else; it's a landing page. It
 just does not look like a project home.

 Yes, git-scm.com is a place to point people.

 It features Companies  Projects Using Git at the bottom.  Not
 supporting but using.

 Linux is point 10 on that list.  The first 6 items are Google, facebook,
 Microsoft, Twitter, LinkedIn, and Netflix.

 Even for an OpenSource project that does not buy into the Free Software
 philosophy, that is a mostly embarrassing list of companies to advertise
 for.

 Personally, I consider the recent migration of the Emacs repository to
 Git a bigger endorsement but then that's me.

 It might make sense to reduce this list just to Projects since those
 are actually more tangible and verifiable.  Or scrap it altogether.

 At the bottom of the git-scm.com page there is this blurb:

   This open sourced site is hosted on GitHub.
   Patches, suggestions and comments are welcome

 And that text contains a link to the GitHub repository[1] where anyone
 can propose modifications to the page. Unfortunately I don't know of
 anyone paying out contribution stipends for content changes made to
 git-scm.com.

 Yeah, thanks for the cheap shot.  I already understood that category B
 is subject to contempt.  Congrats on being category A or C.

 [1] https://github.com/git/git-scm.com/blob/master/README.md#contributing

 Turns out that anyone is actually anyone accepting the conditions for
 a GitHub account:

 If you wish to contribute to this website, please fork it on GitHub,
 push your change to a named branch, then send a pull request.

 I've read the rather longish TermsConditions of GitHub and found myself
 unwilling to agree to them.  Which does not mean that changing the ways
 of contributing to the Git website to accommodate me would make any
 sense since obviously I don't have a clue what a member of the Git
 community should be proud of and ashamed of and thus would be unable to
 make a meaningful proposal anyway even if I were into website
 programming.

A few other points about git-scm.com:

* as Michael says it still looks a bit like a ProGit/Github promotion site

* some of the pull request can be rejected even if the developers want
them, like this pull request to add back a list of contributors was:

https://github.com/git/git-scm.com/pull/216

(By the way this pull request talks about bugs in
https://github.com/git/git/graphs/contributors that are still not
fixed...)

It is kind of strange to say that we should contribute to a web site
that promotes ProGit and GitHub a lot and where our contributions can
be rejected because it is not maintained by us.
--
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: Promoting Git developers (was: Bashing freelancers)

2015-03-10 Thread Christian Couder
On Mon, Mar 9, 2015 at 2:57 PM, Michael J Gruber
g...@drmicha.warpmail.net wrote:
 Christian Couder venit, vidit, dixit 07.03.2015 08:18:
 Hi,

 On Fri, Mar 6, 2015 at 6:41 PM, David Kastrup d...@gnu.org wrote:

 At some point of time I think it may be worth reevaluating the toxic
 atmosphere against freelancers doing Git development.

 My opinion on this is that the Git community has not been good
 especially lately at promoting its own developers.


 I guess we have at least 3 kinds of people here:

 A) Paid to do Git development, at least as part of their job.
 B) Freelancers who don't get paid directly for doing git but hope to
 profit from their git efforts directly or indirectly.
 C) Doing it in their freetime (or as minor, inofficial part of their
 non-programming job).

 I'm in camp C and honestly wasn't aware of camp B until now.

 I consider camp A to be beneficial for all of us, and I don't think
 specific employer interests have pushed the project in specific
 directions, or specific features (OK, maybe one, but not as a rule).

 I do see that remuneration is an issue for camp B.

First thank you for responding to my email. It is great to see that
some developers are interested in talking about this.

I am in camp C and I think the people in all the camps are beneficial
for all of us.

 Some facts:

[...]

I don't want to write again about each of these points now. I am more
interested in discussing a good strategy to try to revert the sad
trend of Git developers being promoted less and less, because I think
that it is really very important.

 None of these facts is a big issue in itself for me, but I think the
 trend is very sad, and I would be happy if we could discuss here or at
 the Git Merge (or both) about ways to improve in this area.

 There should be a good occasion, after we see how it went, and hopefully
 also to sort out any apparent misunderstandings from the past that have
 resurfaced in this thread.

 Maybe, all we need is badges? [1]

 [1] https://badges.fedoraproject.org/

My opinion is that the big issue is that we should all realize how
important it is to promote the Git developers.

There are people who say out there that GitHub succeeded mostly
because they easily allowed developers to build a portfolio. I think
there is some truth in that. And I think GitHub also says to
developers that it's good for them to have a nice GitHub portfolio and
to employers that it's good for them to hire people who have a nice
GitHub portfolio.

This shows that the success of GitHub (and so of Git too) is based in
part on promoting Open Source / Free Software developers.

So why don't we try to do it more (instead of less) and how could we
do it more (instead of less) for the Git developers?

Yesterday evening I attended a Docker meeting in Paris [1] where
Jérôme Petazzoni a Docker developer working for Docker gave a talk
about Docker storage drivers. In the middle of his talk there were at
least 2 slides dedicated to thank some developers who helped make
Docker work with different filesystems or operating systems. And
Jérôme did stop at least twice to thank these people in the middle of
his talk.

Wouldn't his talk have been smoother if he had not done that? So why
did he do that?

This reminded me about the following great talk by Julien Barbier the
Community and Marketing guy at Docker:

http://www.slideshare.net/julienbarbier42/community-marketing-42674728

I had seen this talk last december at another Docker meetup in Paris
(and I think it really worth reading the slides or attending the talk
if Julien gives it again and you can go).

The slides and the talk keep repeating some sentences because they are
worth repeating. Some of these sentences are:

* It is about what you can do for your community
* Belonging, recognition, respect, love
* Add more links to your community
* Your product is not what you say it is, it is what THEY say it is
* It's all about people

It is especially interesting to have a look at slide 5 where they say
that Community is the new marketing and that Community is 80+% of
their marketing.

And it's true that they are really doing a lot for their community.
For example the meeting yesterday evening was the 19th docker meeting
in Paris in two years.

And then there is slide 20 about Content Strategy where there is:

* Encourage your community to build content
  - Say thank you, repost, post, upvote, RT, include them in your
newsletter, itw them, …
  - Belonging, Recognition, Respect, Love

* Your team is your community too!
  - Say thank you, gamify, hall of fame, tweet, post, recycle, etc…
  - Belonging, Recognition, Respect, Love

So we can see that saying thank you is a big part of their content strategy.

And are they successful? Yes, they are very successful as an open
source project [2] and as a company.

Now it's up to us, either we keep coming up with excuses not to
promote developers, or we decide to do something about it.

Best,
Christian.

[1] http

Re: [ANNOUNCE] Git Merge Contributors Summit, April 8th, Paris

2015-03-05 Thread Christian Couder
Hi,

On Tue, Feb 24, 2015 at 11:09 PM, Jeff King p...@peff.net wrote:
 I wanted to make one more announcement about this, since a few more
 details have been posted at:

   http://git-merge.com/

 since my last announcement. Specifically, I wanted to call attention to
 the contributor's summit on the 8th. Basically, there will be a space
 that can hold up to 50 people, it's open only to git (and JGit and
 libgit2) devs, and there isn't a planned agenda. So I want to:

   1. Encourage developers to come. You might meet some folks in person
  you've worked with online. And you can see how beautiful we all
  are.

   2. Get people thinking about what they would like to talk about.  In
  past GitTogethers, it's been a mix of people with prepared things
  to talk about, group discussions of areas, and general kibitzing.
  We can be spontaneous on the day of the event, but if you have a
  topic you want to bring up, you may want to give it some thought
  beforehand.

 If you are a git dev and want to come, please RSVP to Chris Kelly
 amateurhu...@github.com who is organizing the event. If you would like
 to come, but finances make it hard (either for travel, or for the
 conference fee), please talk to me off-list, and we may be able to help.

I'd like the Git project to set up a more organized way to pay back
the travel costs and the conference fee to the developers who come.
For example the Git project could say that it will at least pay back:

- all the travel costs to the 5 most important Git developers who come and ask,
- half the travel costs to the 5 next most important Git developers
who come and ask,
- all the conference fee to the 15 most important Git developers who
come and ask,

I think it could help developers decide to come, and it looks like
enough funding could be available, thanks to GitHub and the GSoC
money. What do you think?

Apart from that it's also possible to find ways to accommodate some
developers for free, if they don't mind crashing in someone's spare
room.

So please don't hesitate to ask if you would like to come.

Thanks,
Christian.
--
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: [GSoC microproject] Add XDG support to the credential-store helper

2015-03-05 Thread Christian Couder
Hi,

On Thu, Mar 5, 2015 at 9:51 PM, Luis Ressel ara...@aixah.de wrote:
 I'm contributing this patch in preparation for a GSoC15 application with
 the git project. In particular, I'm interested in the two bisect
 improvements listed on the Ideas page. Does anyone have other
 suggestions for potential improvements of git bisect that would be
 suitable for such a GSoC project?

Last year, on the ideas page:

http://git.github.io/SoC-2014-Ideas.html

we also had the following improvement:

in some cases, git bisect may test too many merge bases, thus slowing
down the bisection (making it closer to linear than logarithmic).

so you could work on that too.

And if you really want and have time you can also rewrite
git-bisect.sh into a builtin command. There is already
builtin/bisect--helper.c where you can put some migrated code step by
step.

 Oh, and should I add a testcase for the new functionality introduced by
 this patch? And if yes, what exactly should I test?

First, it looks like another student started working on the same
microproject. Did you have a look at what he did? If you didn't, you
probably should, so that reviewers don't need to tell you what they
already told the other student. And if you did, you should tell it,
and maybe explain what you did differently and why.

If it looks like the other student is more advanced on this subject,
you might want to try another microproject. And if all the
microprojects are already taken, you might want to ask the list for
other microprojects, or you may want to have a look at the many merge
bases improvement above. (You could start by having a look at
check_merge_bases() in bisect.c and by creating a script or better a
test case that reproduces the problem, see
t/t6030-bisect-porcelain.sh.)

Welcome to the Git community!

Best,
Christian.
--
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: [ANNOUNCE] Git Merge Contributors Summit, April 8th, Paris

2015-03-06 Thread Christian Couder
On Thu, Mar 5, 2015 at 11:24 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 On Tue, Feb 24, 2015 at 11:09 PM, Jeff King p...@peff.net wrote:
 I wanted to make one more announcement about this, since a few more
 details have been posted at:

   http://git-merge.com/

 since my last announcement. Specifically, I wanted to call attention to
 the contributor's summit on the 8th. Basically, there will be a space
 that can hold up to 50 people, it's open only to git (and JGit and
 libgit2) devs, and there isn't a planned agenda. So I want to:

   1. Encourage developers to come. You might meet some folks in person
 ...
   2. Get people thinking about what they would like to talk about.  In
 ...
 If you are a git dev and want to come, please RSVP to Chris Kelly
 amateurhu...@github.com who is organizing the event. If you would like
 to come, but finances make it hard (either for travel, or for the
 conference fee), please talk to me off-list, and we may be able to help.

 I'd like the Git project to set up a more organized way to pay back
 the travel costs and the conference fee to the developers who come.
 For example the Git project could say that it will at least pay back:

 - all the travel costs to the 5 most important Git developers who come and 
 ask,
 - half the travel costs to the 5 next most important Git developers
 who come and ask,
 - all the conference fee to the 15 most important Git developers who
 come and ask,

 I think it could help developers decide to come, and it looks like
 enough funding could be available, thanks to GitHub and the GSoC
 money. What do you think?

 I personally perfer things to be kept informal---it would keep
 things simpler for everybody.  You do not have to wonder what you
 should do when you think you are among the five most important
 people and you also know your employer will pay for the conference
 if you asked, for example.

 It feels to me that the suggestion Peff gave in his announce to ask
 privately for case-by-case arrangement strikes the balance much
 better.

My opinion is that it is good to give developers who could come an
idea of what the Git project should at least be able and willing to
fund, because most developers might have no idea about that.

For example many developers who contributed say less than 50 patches
to Git may think that they have no chance of being payed back anything
which might not be true at all. And by the way to make that clear, it
would be nice if the Git project could say for example every few weeks
how many people have asked for something.

I don't think there is a perfect way to do this kind of thing, but I
think being more transparent and upfront while still taking care of
privacy issues and leaving some room for discussion, can only help.

 Apart from that it's also possible to find ways to accommodate some
 developers for free, if they don't mind crashing in someone's spare
 room.

 So please don't hesitate to ask if you would like to come.

 These five lines, by not explicitly saying something like the first
 2 people who ask can crash in Christian's spare bedroom, is doing
 exactly the same thing as Peff did by saying please talk to me
 off-list, it seems to me at least.  Both keep things informal and
 simple, and both arrange things on case-by-case basis as needed.

I must say that it is quite different, because in case of my spare
bedroom I am the only one who decides according to my own priorities.
For example if too many people ask to be accommodated and one of them
helped me personally in the past, I will be much more likely to chose
him regardless of his importance for the Git project.

 And I think that is better than setting a seemingly hard rules
 upfront, and causing more problems unnecessarily (e.g. who decides
 who are the 5 most important, for example?).

First the rules are not so hardly set, especially because they say at
least this amount, so in case of doubt there is room for paying back
more to more people than initially planned.

And anyway in the case-by-case as needed basis, you still have the
problem to decide how much to pay back each one, in case people ask
for more than what is available. In this case it could be seen as very
unfair that rules are defined or negociated on the fly. (Though I
agree that in the past it went very well, but then I think it can only
improve things to have some rules defined at the beginning.)
--
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: [ANNOUNCE] Git Merge Contributors Summit, April 8th, Paris

2015-03-06 Thread Christian Couder
On Fri, Mar 6, 2015 at 11:52 AM, Michael J Gruber
g...@drmicha.warpmail.net wrote:
 Junio C Hamano venit, vidit, dixit 05.03.2015 23:24:
 Christian Couder christian.cou...@gmail.com writes:


 I'd like the Git project to set up a more organized way to pay back
 the travel costs and the conference fee to the developers who come.
 For example the Git project could say that it will at least pay back:

 - all the travel costs to the 5 most important Git developers who come and 
 ask,
 - half the travel costs to the 5 next most important Git developers
 who come and ask,
 - all the conference fee to the 15 most important Git developers who
 come and ask,

 I think it could help developers decide to come, and it looks like
 enough funding could be available, thanks to GitHub and the GSoC
 money. What do you think?

 I personally perfer things to be kept informal---it would keep
 things simpler for everybody.  You do not have to wonder what you
 should do when you think you are among the five most important
 people and you also know your employer will pay for the conference
 if you asked, for example.

 It feels to me that the suggestion Peff gave in his announce to ask
 privately for case-by-case arrangement strikes the balance much
 better.

 Apart from that it's also possible to find ways to accommodate some
 developers for free, if they don't mind crashing in someone's spare
 room.

 So please don't hesitate to ask if you would like to come.

 These five lines, by not explicitly saying something like the first
 2 people who ask can crash in Christian's spare bedroom, is doing
 exactly the same thing as Peff did by saying please talk to me
 off-list, it seems to me at least.  Both keep things informal and
 simple, and both arrange things on case-by-case basis as needed.

 And I think that is better than setting a seemingly hard rules
 upfront, and causing more problems unnecessarily (e.g. who decides
 who are the 5 most important, for example?).

 Oh yes, that would be an interesting metric to define...

 OTOH I can see where Christian's question is coming from:
 Who is even supposed to ask for support? Not just as in who is a
 developer, but also what are finance hardships:

 At scientific conferences which I'm going to, there is often support
 for those who need it, and that typically means participants from less
 fortunate countries (to avoid the usual world-counting term). Everyone
 else is expected to be covered by their academic employer - and if not,
 it's not even okay to ask the organisers. I guess that's what some of us
 are having in mind.

I had more in mind the people who mentored GSoC students (and this way
helped the Git project get some money) and the 200 or so developers
who contributed between 10 and 50 patches, though I agree it could
also be useful for others too. As far as I know very few people have
asked for funding and it would be sad that people don't come because
they think they would not be payed back the costs when in fact they
would.

 That still leaves the question:
 Is there any space left in Christian's spare bedroom? :)

Yes, no one as asked yet, so I shoud be able to accommodate you if you want :-)

See you soon,
Christian.
--
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


Promoting Git developers (was: Bashing freelancers)

2015-03-06 Thread Christian Couder
Hi,

On Fri, Mar 6, 2015 at 6:41 PM, David Kastrup d...@gnu.org wrote:

 At some point of time I think it may be worth reevaluating the toxic
 atmosphere against freelancers doing Git development.

My opinion on this is that the Git community has not been good
especially lately at promoting its own developers.

Some facts:

* There used to be an AUTHORS section in each of the git man page.
They have been removed. The rational was that they were hard to
maintain and the information about authors was easily available
elsewhere.

* There used to be a nice page on git-scm.com, the main Git web site,
listing the authors and how many commits they had contributed. It has
been removed.

* In the A note from the maintainer emails that Junio regularly
sends, the last section about Other people's trees, trusted
lieutenants and credits. seems to have been truncated for some time
and doesn't show anymore the nice credits words it used to show.
Maybe this is a bug.

* https://www.openhub.net/p/git/contributors/summary seems to give me a
504 Gateway Time-out right now :-(

* On the Git Merge web site, we can see that none of the speakers
seems to have been a very active contributor to git.git

None of these facts is a big issue in itself for me, but I think the
trend is very sad, and I would be happy if we could discuss here or at
the Git Merge (or both) about ways to improve in this area.

Best,
Christian.
--
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: Promoting Git developers

2015-03-11 Thread Christian Couder
On Tue, Mar 10, 2015 at 6:23 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 I don't want to write again about each of these points now. I am more
 interested in discussing a good strategy to try to revert the sad
 trend of Git developers being promoted less and less, because I think
 that it is really very important.

 I would suspect that those who agree with you would appreciate if
 you or somebody volunteered to act as our CKDO (chief kudos
 distribution officer).  I do not think I have enough time to do that
 well.  One good place to start might be to scan the list and
 summarize something like the following on weekly or monthly basis,
 as these are not something you can get by pointing people to git
 shortlog output.

  - Those who gave helpful review comments, how about going this
way illustration patches, etc.  Bonus points to those who helped
onboarding newcomers.

  - Those who asked pertinent questions on common pain points, and
those who answered them helpfully.

Ok, I can start something about this two points every week or every
few week. It would be best if I could get help from at least one
person as I think it is a lot of work.

We can perhaps use the Git Developer Site at
https://github.com/git/git.github.io to edit a new page
collaboratively that would be published on http://git.github.io/ and
after that send an email to the mailing list.

 If you are more ambitious, the source of the kudos may want to cover
 activities outside of this mailing list (e.g. giving talks and
 tutorials at conferences, etc.).

First I don't know if we should really give kudos (or badges) or have
something more like the former Git Traffic you talk about in another
email (or perhaps both).

And then I expect that if people give talks or tutorials at
conferences or publish a blog post or have other news they want to
share, they could edit the web page themselves on GitHub (or fork it
and send a pull request if they don't have the rights).

I also appreciate very much that you are willing to improve the
release notes by adding a summary with people's names.

It would be nice if we could also have somewhere on a web page at
least a good listing of the authors and how many commits they had
contributed (since the beginning and maybe also during the last year).
We could also add other listings made using the Helped-by and
Reviewed-by trailers.

I don't think we should rely on an external web site like OpenHub
(which is still giving me a 504 Gateway Time-out on the contributor
page) or even the (broken) contributor graph on GitHub for that. If
Scott and Peff don't want it on git-scm.com then it is of course
better on git.github.io than nowhere.

Thanks,
Christian.
--
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 Rev News edition 1 published

2015-03-25 Thread Christian Couder
Hi,

Git Rev News edition 1 is now available:

http://git.github.io/rev_news/edition-1.html

Thanks a lot to all the contributors and helpers, especially Junio!

Enjoy,
Christian and Thomas.
--
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: Draft of Git Rev News edition 1

2015-03-22 Thread Christian Couder
On Mon, Mar 23, 2015 at 5:49 AM, Junio C Hamano gits...@pobox.com wrote:
 Thomas Ferris Nicolaisen tfn...@gmail.com writes:

 Good point. There hasn't been a decision on frequency. Weekly is a
 good rhythm for publications seeking readership, but that's a lot of
 work. My vote is we should first aim for a monthly consistent release.
 I'll try working this into the draft, and Christian may change as he
 sees fit.

 I agree weekly would be too much for any hobbist, given how
 high-volume our list has, but I probably shouldn't have said
 periodical.  Surely, aiming for consistent update is a very good
 thing to gain reader trust if anything else, but it is OK if it were
 we will see a new release when enough interesting things happen,
 too.

Yeah, I prefer not to commit to a specific frequency...

 The primary reason I suggested to explicitly state the beginning of
 coverage is to set and manage the expectation of the readers.  I
 think the current draft roughly covers 1/4 - 1/3 of discussions that
 happened in the month of March 2015 and nothing earlier than that,
 so This issue covers what happened in March or something would be
 appropriate.  I'll throw a pull-request.

... but I agree that we should say what we cover.

  - As an inaugural edition, we may want to have a word on
how it came in existence by covering the discussion that
led to its birth. Perhaps the discussion that led to the
publication should be made into as an item on its own,
next to make git-pull a builtin, Forbid log --graph... etc.
Because it is neither a review nor a support discussion,
Reviews  Support heading may want to become
Discussions. I think that is a better title for the section
anyway, if its purpose is what happened on the list that
are not visible from git log, as I expect future editions
to cover design discussions that advanced the shared
understanding of a problem but not quite solidified to
become a patch series.


 I hope it's OK that I leave this bit to Christian.

 I took a stab at this myself, and threw another pull-request.

 Thanks.

Thank you for your pull requests.
They are all merged and your name is in the Credits section at the end.

Thanks,
Christian.
--
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: [RFH] GSoC 2015 application

2015-02-25 Thread Christian Couder
On Wed, Feb 25, 2015 at 9:44 AM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 Hi Matthieu,

 On 2015-02-25 00:56, Matthieu Moy wrote:
 Johannes Schindelin johannes.schinde...@gmx.de writes:

 On 2015-02-24 19:25, Junio C Hamano wrote:
 On Tue, Feb 24, 2015 at 9:32 AM, Matthieu Moy
 matthieu@grenoble-inp.fr wrote:
 About the proposal:

   The idea of this project is to dive into the Git source code and
   convert, say, git-add--interactive.perl and/or git stash into proper C
   code, making it a so-called built-in.

 My advice would be to try converting several small scripts, and avoid
 targetting a big one
 add--interactive and stash are relatively complex beasts, perhaps
 git-pull.sh would be easier to start with.

 Yeah, I think that is a very good suggestion.

 Well, git-pull.sh is really small. I did not want to give the impression 
 that the Git project is giving out freebies. But I have no objection to 
 change it if you open that PR.

 To get an idea, I counted the lines of code written by the student I
 mentored last year:

 $ git log --author tanay...@gmail.com -p | diffstat -s
  43 files changed, 1225 insertions(+), 367 deletions(-)

 I would consider this GSoC as average (i.e. not exceptionnally good,
 but certainly not a bad one either), so you may hope for more, but you
 should not _expect_ much more IMHO.

 In comparison:

 $ wc -l git-add--interactive.perl
 1654 git-add--interactive.perl
 $ wc -l git-stash.sh
 617 git-stash.sh

There is also:

$ wc -l git-bisect.sh
528 git-bisect.sh

And there is already builtin/bisect--helper.c that can be used to
convert step by step shell code to C. I can mentor or co-mentor this
convertion by the way, but that would conflict with the other bisect
related GSoC project if a student takes it.

 I'd expect a rewrite in C to at least double the number of lines of
 code, so rewriting git-stash would mean writting at least as many lines
 of code as the GSoC above. git-add--interactive.perl would be rather far
 above.

 Sure. You're right, I was thinking too big.

 But my point was not to convert _only_ git-pull.sh, but to have a GSoC
 starting with this one and plan more. Then, depending on how it goes,
 you can adjust the target.

 That's excellent advice.

Yeah!

 This all depends on what you intend to do if the student does not finish
 the job. If you're going to do the rewrite yourself anyway, then having
 the student do even half of it is good already. If you're not going to
 finish the job by yourself, then a 95%-done-rewrite means a piece of
 code posted on the mailing list and never merged (and a lot of time
 wasted).

 Well, all of this is academic at this point.

Yeah, but it's still worth keeping in many parts of our collective mind :-)

 Let's see whether we get accepted, and then, if a student finds this project 
 compelling enough.

 If both things happen, I will definitely heed your advice and encourage the 
 student to start with some script that is easily converted, to get her feet 
 wet.

Great!

Thanks,
Christian.
--
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] apply: refuse touching a file beyond symlink

2015-01-30 Thread Christian Couder
On Thu, Jan 29, 2015 at 9:45 PM, Junio C Hamano gits...@pobox.com wrote:

 Instead, for any patch in the input that leaves a path (i.e. a non
 deletion) in the result, we check all leading paths against interim
 result and then either the index or the working tree.  The interim
 results of applying patches are kept track of by fn_table logic for
 us already, so use it to fiture out if existing a symbolic link will

s/fiture/figure/
s/existing a symbolic link/an existing symbolic link/

 cause problems, if a new symbolic link that will cause problems will
 appear, etc.
--
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: gitk won't show notes?

2015-04-14 Thread Christian Couder
On Sat, Apr 11, 2015 at 12:31 PM, Paul Mackerras pau...@samba.org wrote:
 On Wed, Apr 08, 2015 at 01:51:40PM +0200, Michael J Gruber wrote:
 Phillip Susi venit, vidit, dixit 07.04.2015 19:08:
  On 4/7/2015 10:13 AM, Michael J Gruber wrote:
  Seriously: gitk knows F5 and Shift-F5 for refresh, and I think the
  latter is the thorougher refreshment.
 
  Neither one makes newly added notes show up.  The only way seems to be
  to close and restart gitk.  Looks like a bug.
 
 

 Apparently, gitk rereads the refs but not commits it has read already -
 and the commit reading includes the notes lookup.

 Unfortunately, my wish-fu is lacking. But I'll cc the master.

 Paulus: None of updatecommits, reloadcommits and rereadrefs seem to
 reread the notes of a commit that has been displayed already if the
 notes have changed (but the other refs have not).

 As far as shift-F5/reloadcommits is concerned, it looks like I should
 be unsetting commitinfo in reloadcommits.

 However, I agree gitk should refresh the notes in updatecommits as
 well, but that will take more work.  Is git notes list the best way to
 find out all the current notes?

It looks like it is from the documentation.
Anyway let's add some people in Cc in case they could help.
--
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


Draft of Git Rev News edition 2

2015-04-12 Thread Christian Couder
Hi,

A draft of Git Rev News edition 2 is available here:

https://github.com/git/git.github.io/blob/master/rev_news/draft/edition-2.md

Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

https://github.com/git/git.github.io/issues/29

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas, Nicola and myself plan to publish this edition on Wednesday
the 15th of April.

Thanks,
Christian
--
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


[ANNOUNCE] Git Rev News edition 2 published

2015-04-15 Thread Christian Couder
Hi,

Git Rev News edition 2 is now available:

https://git.github.io/rev_news/2015/04/05/edition-2/

Thanks a lot to all the helpers!

Enjoy,
Christian, Thomas and Nicola.
--
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: [ANNOUNCE] Git Merge Contributors Summit, April 8th, Paris

2015-04-06 Thread Christian Couder
On Mon, Apr 6, 2015 at 12:48 AM, Thomas Ferris Nicolaisen
tfn...@gmail.com wrote:
 On Tue, Feb 24, 2015 at 11:09 PM, Jeff King p...@peff.net wrote:
 I wanted to make one more announcement about this, since a few more
 details have been posted at:

   http://git-merge.com/

 since my last announcement. Specifically, I wanted to call attention to
 the contributor's summit on the 8th. Basically, there will be a space
 that can hold up to 50 people, it's open only to git (and JGit and
 libgit2) devs, and there isn't a planned agenda. So I want to:

   1. Encourage developers to come. You might meet some folks in person
  you've worked with online. And you can see how beautiful we all
  are.

   2. Get people thinking about what they would like to talk about.  In
  past GitTogethers, it's been a mix of people with prepared things
  to talk about, group discussions of areas, and general kibitzing.
  We can be spontaneous on the day of the event, but if you have a
  topic you want to bring up, you may want to give it some thought
  beforehand.

 If you are a git dev and want to come, please RSVP to Chris Kelly
 amateurhu...@github.com who is organizing the event. If you would like
 to come, but finances make it hard (either for travel, or for the
 conference fee), please talk to me off-list, and we may be able to help.

 If you have questions, please feel free to ask me, and I'll try to get
 answers from the GitHub folks who are organizing the event.


 I'll be arriving around 11 am on the 8th, if anyone wants to record
 something for the GitMinutes podcast [1]. Send me an email directly,
 or just walk up to me at the conference and say hi! I'll hopefully be
 hanging around the contributor's summit area with some microphones,
 but I've been unable to get any feedback from GitHub about whether
 this is OK, so.. I guess we'll just wing it when I get there.

 [1] http://www.gitminutes.com/

By the way as far as I know nothing has been planned for the
Contributors Summit on the 8th.
Maybe we could list some topics that we could discuss.

I will probably write very short articles about some of the
discussions for the next Git Rev News edition, but I would be happy if
other people would like to contribute some. Please tell me and Thomas
if you are interested.

Also I am not sure if something is planned for the evening of the 8th
or not. If nothing is planned maybe we could discuss having dinner
together or something.

And if someone needs help or arrives in Paris early or leaves late and
is interested in meeting up, feel free to contact me.

Best,
Christian.
--
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: Re*: AW: Getting the full path of a conflicting file within a custom merge driver?

2015-06-05 Thread Christian Couder
On Fri, Jun 5, 2015 at 7:56 AM, Gondek, Andreas
andreas.gon...@dwpbank.de wrote:
 Thanks, thanks, thanks.

 One last question. If I don't want to compile Git myself, how long may the pu 
 branch take approx. to get into a next release?

According to:

https://github.com/git/git/blob/master/Documentation/howto/maintain-git.txt

One release cycle for a feature release is expected to last for eight
to ten weeks.

As v2.4.2 was tagged on May 26, the next feature release should be in
6 to 9 weeks, and will hopefully include the feature you are
interested in.

(Also please don't top post on this list.)

Best,
Christian.
--
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: Re*: AW: Getting the full path of a conflicting file within a custom merge driver?

2015-06-05 Thread Christian Couder
On Fri, Jun 5, 2015 at 10:07 AM, Christian Couder
christian.cou...@gmail.com wrote:
 On Fri, Jun 5, 2015 at 7:56 AM, Gondek, Andreas
 andreas.gon...@dwpbank.de wrote:
 Thanks, thanks, thanks.

 One last question. If I don't want to compile Git myself, how long may the 
 pu branch take approx. to get into a next release?

 According to:

 https://github.com/git/git/blob/master/Documentation/howto/maintain-git.txt

 One release cycle for a feature release is expected to last for eight
 to ten weeks.

Actually I should have read the next lines that say:

- Maintenance releases are numbered as vX.Y.Z and are meant to
contain only bugfixes for the corresponding vX.Y.0 feature release and
earlier maintenance releases vX.Y.W (W  Z).

 As v2.4.2 was tagged on May 26, the next feature release should be in
 6 to 9 weeks, and will hopefully include the feature you are
 interested in.

The last feature release was v2.4.0 that was tagged on April 30, so
the next one should be in 3 to 5 weeks.
--
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: Suggestion: group files in GIT

2015-06-08 Thread Christian Couder
On Mon, Jun 8, 2015 at 10:50 AM, Konrád Lőrinczi klorin...@gmail.com wrote:
 I would like to group some files, so I can list group files together,
 list group changes together, filter by group for staging, also order
 by group.
 It seems, there is no such feature in GIT I would need, so I send it
 as suggestion.

 We can call this feature as Group files or Label files (labeling
 is used in Gmail, so this may be also a naming alternative).


 Example file list I would like to group together into [group1]:
 theme/header.php
 theme/footer.php
 theme/body.php
 lib/theme.php

Can't you use a shell variable like:

group1=theme/header.php theme/footer.php theme/body.php lib/theme.php

?

 They are in different directories, but mostly belongs together, so if
 I group them, then I can work easier with them.


 - I could select a file group for staging, so only the changes in the
 group would be added to stage.

git add $group1

 Changed files in the group:
 [group1]/theme/header.php
 [group1]/lib/theme.php


 - I could list files filtered by a group. Files filtered by [group1]:
 [group1]/theme/header.php
 [group1]/theme/footer.php
 [group1]/theme/body.php
 [group1]/lib/theme.php

ls -l $group1

 - I could order file list to list group files first, then directory files.
 [group1]/theme/header.php
 [group1]/theme/footer.php
 [group1]/theme/body.php
 [group1]/lib/theme.php
 other/files.php

I am not sure I see what you want to do with that.
--
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/contrib] Avoid failing to create ${__git_tcsh_completion_script} when 'set noclobber' is in effect (af7333c)

2015-06-08 Thread Christian Couder
Please use a subject that is shorter and looks more like others on this list.

On Sun, Jun 7, 2015 at 9:54 PM, Ariel Faigon github.2...@yendor.com wrote:

 Junio,

 This is my 1st time doing this, sorry.
 I hope this satisfies the git Sign Off procedure.

The above lines should not be there, otherwise they will be in the
commit message and will not be useful there.

 Problem Description:

Please loose the above header.

 tcsh users who happen to have 'set noclobber' elsewhere in their ~/.tcshrc or 
 ~/.cshrc startup files get a 'File exist' error,

When do they get that error?

 and the tcsh completion file doesn't get generated/updated.  Adding a `!` in 
 the redirect works correctly for both clobber and noclobber users.

 Developer's Certificate of Origin 1.1

 By making a contribution to this project, I certify that:

 (a) The contribution was created in whole or in part by me and I
 have the right to submit it under the open source license
 indicated in the file; or

 (b) The contribution is based upon previous work that, to the best
 of my knowledge, is covered under an appropriate open source
 license and I have the right under that license to submit that
 work with modifications, whether created in whole or in part
 by me, under the same open source license (unless I am
 permitted to submit under a different license), as indicated
 in the file; or

 (c) The contribution was provided directly to me by some other
 person who certified (a), (b) or (c) and I have not modified
 it.

 (d) I understand and agree that this project and the contribution
 are public and that a record of the contribution (including all
 personal information I submit with it, including my sign-off) is
 maintained indefinitely and may be redistributed consistent with
 this project or the open source license(s) involved.

You don't need to copy the Developer's Certificate of Origin in your
patch even if it's the first time. Your signed-off-by below is enough.

  Signed-off-by: Ariel Faigon github.2...@yendor.com

  git patch follows.

Please put nothing after your signed-off-by and before the three dashes below.

 ---

Here, just after the three dashes, is the right place to put personnal
comments and stuff that should not go into the commit message.

  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.

Thanks,
Christian.
--
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 0/9] add options to ref-filter

2015-06-07 Thread Christian Couder
On Sat, Jun 6, 2015 at 10:03 PM, Karthik Nayak karthik@gmail.com wrote:
 This is a follow up series to the one posted here
 http://thread.gmane.org/gmane.comp.version-control.git/270922

 This patch series adds '--ponints-at', '--merged', '--no-merged' and

s/--ponints-at/--points-at/

 '--contain' options to 'ref-filter' and uses these options in
 'for-each-ref'.
--
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

2015-06-09 Thread Christian Couder
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 3/4] bisect: simplify the add of new bisect terms

2015-06-09 Thread Christian Couder
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

2015-06-09 Thread Christian Couder
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: Permission denied ONLY after pulling bundles

2015-06-05 Thread Christian Couder
On Fri, Jun 5, 2015 at 8:54 AM, Rossella Barletta
rossella.barle...@gmail.com wrote:

[...]

 FIST ONE (PERMISSION PROBLEMS)

 - Repo is on windows
 - Repo folder is shared
 -Repo is a copy of another repository being on a machine in another
 city on which we cannot access. We got all the files, included the
 folder .git a put everything in our shared folder
 - Mounted the Repo folder on Linux
 -Created the clone
 - got a bundle from the original repository (bundle created from a branch)
 -pulled the bundle in the same branch



 SECOND ONE (NO PROBLEMS BUT WE CANT USE THIS)
 - Repo is on Linux
 -Repo is a copy of another repository being on a machine in another
 city on which we cannot access.
 - got a bundle from the original repository (bundle created from a branch)
 -pulled the bundle in the same branch



 4) Git version is 1.7.1

It would be nice if you could try to reproduce the problem:

- using a recent Git, as v1.7.1 is 5 years old,
- using a small fake repo,
- doing everything on Windows.

Best,
Christian.
--
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/2] git-bisect.sh : create a file if the bisection is in old/new mode, named BISECT_OLDNEWMODE, so it can easily be seen outside the program without having to read BISECT_TERMS. This wil

2015-06-05 Thread Christian Couder
On Fri, Jun 5, 2015 at 6:34 PM, Louis Stuber
stub...@ensimag.grenoble-inp.fr wrote:

 Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
 Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
 ---

It looks like this patch applies on top of the bisect old/new series
posted by Antoine.
This should be stated somewhere.

  git-bisect.sh |8 +++-
  1 files changed, 7 insertions(+), 1 deletions(-)

 diff --git a/git-bisect.sh b/git-bisect.sh
 index 109bd65..d3d19cb 100644
 --- a/git-bisect.sh
 +++ b/git-bisect.sh
 @@ -183,6 +183,10 @@ bisect_start() {
 then
 echo $BISECT_BAD $GIT_DIR/BISECT_TERMS 
 echo $BISECT_GOOD $GIT_DIR/BISECT_TERMS
 +   if test $BISECT_BAD = new
 +   then
 +   echo   $GIT_DIR/BISECT_OLDNEWMODE
 +   fi

I am not sure it's worth it to have both BISECT_TERMS and BISECT_OLDNEWMODE.

Also please note that I suggested to Antoine that the BISECT_BAD and
BISECT_GOOD variables be renamed to something else, like I already did
in some C files.
--
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 0/10] create ref-filter from for-each-ref

2015-06-06 Thread Christian Couder
On Sat, Jun 6, 2015 at 9:04 AM, Karthik Nayak karthik@gmail.com wrote:
 Version for of this patch can be found here :
 http://www.mail-archive.com/git@vger.kernel.org/msg70280.html

 Changes in this version:
 *   Rename functions to better suit the code.
 *   implement filter_refs()
 *   use FLEX_ARRAY for refname
 *   other small changes

Now that the patch series starts to be in a good shape you might want
to use [PATCH ...] instead of [WIP/PATCH ...].

Thanks,
Christian.
--
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


[ANNOUNCE] Git Rev News edition 4

2015-06-03 Thread Christian Couder
Hi,

Git Rev News edition 4 is now available:

https://git.github.io/rev_news/2015/06/03/edition-4/

Thanks a lot to all the helpers!

Enjoy,
Christian, Thomas and Nicola.
--
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: Permission denied ONLY after pulling bundles

2015-06-04 Thread Christian Couder
Hi,

On Thu, Jun 4, 2015 at 3:04 PM, Rossella Barletta
rossella.barle...@gmail.com wrote:
 Dear git group,


 I would like to ask your help for a problem that we cannot fix in any way.

 We have a git repository in folder on Windows.

 Then we use VMware player on CentOS_6 on which we create a clone of
 the git repository, after of course having mounted the directory in
 which the repository is.

 So the repository is on windows and the clone on Linux.

 We are able to perfom all the git operations we need, except for the
 pull .bundle, which is successful in itself but prevent us from
 pushing after that.

It is not very clear how the bundle has been made, and on which
machine you made it and you pulled from it.

 As we try to push after pulling a .bundle in a branch we get the error message

 NODE1:fdp git push
 Counting objects: 1977, done.
 Delta compression using up to 2 threads.
 Compressing objects: 100% (423/423), done.
 fatal: write error: Permission denied00 KiB | 158 KiB/s
 error: pack-objects died of signal 13
 error: pack-objects died with strange error

Can you have a look at the machine you push to and see if some file or
directory permissions changed between before and after you made the
bundle or you pulled the bundle?

 We have checked all the permissions, changed the users, recreated the
 clone but nothing worked.

What do you mean by checked all the permissions?
You mean that permissions haven't changed at all since before you
pulled the first bundle?

 The push operation works perfectly until we pull a bundle. After
 pulling a bundle we are not able to push anymore.We tryed to delete
 the branches, recreate others and all works perfectly, also the
 push.As we pull the .bundle we cannot get the permission to do the
 push anymore.

 What has this to do with the bundle?

Did you try to everything (cloning, creating a bundle, pulling it and
pushing on the same machine to see if it makes a difference? Also did
you try with another smaller fake repository?

If you can reproduce with a smaller fake repo on just one machine it
could help us reproduce on one of our machine and have a look.

And could you tell us which version of git (using git --version) you
are using on both machines?

Thanks,
Christian.
--
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 1/3] git bisect old/new

2015-06-04 Thread Christian Couder
On Thu, Jun 4, 2015 at 9:59 AM, Antoine Delaite
antoine.dela...@ensimag.grenoble-inp.fr wrote:
 From: Christian Couder chrisc...@tuxfamily.org

 When not looking for a regression during a bisect but for a fix or a
 change in another given property, it can be confusing to use 'good'
 and 'bad'.

 This patch introduce `git bisect new` and `git bisect old` as an
 alternative to 'bad' and good': the commits which have the most
 recent version of the property must be marked as `new` and the ones
 with the older version as `old`.

 The output will be the first commit after the change in the property.
 During a new/old bisect session you cannot use bad/good commands and
 vice-versa.

 `git bisect replay` works fine for old/new bisect sessions.

 Some commands are still not available for old/new:

  * git bisect start [new [old...]] is not possible: the
commits will be treated as bad and good.
  * git rev-list --bisect does not treat the revs/bisect/new and
revs/bisect/old-SHA1 files.
  * git bisect visualize seem to work partially: the tags are
displayed correctly but the tree is not limited to the bisect
section.

 Related discussions:

 - http://thread.gmane.org/gmane.comp.version-control.git/86063
 introduced bisect fix unfixed to find fix.
 - http://thread.gmane.org/gmane.comp.version-control.git/182398
 discussion around bisect yes/no or old/new.
 - http://thread.gmane.org/gmane.comp.version-control.git/199758
 last discussion and reviews

 Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
 Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
 Signed-off-by: Valentin Duperray valentin.duper...@ensimag.imag.fr
 Signed-off-by: Franck Jonas franck.jo...@ensimag.imag.fr
 Signed-off-by: Lucien Kong lucien.k...@ensimag.imag.fr
 Signed-off-by: Thomas Nguy thomas.n...@ensimag.imag.fr
 Signed-off-by: Huynh Khoi Nguyen Nguyen 
 huynh-khoi-nguyen.ngu...@ensimag.imag.fr
 Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
 ---
 We took account of most of the easy reviews that were discuted two years 
 ago.
 We hope we didn't missed any.
 http://thread.gmane.org/gmane.comp.version-control.git/199758

 We corrected various issues that were not reported:
 -one that caused a fatal ... not a valid ref at the end of the bisection.
 -the autostart was causing issues, the following lines were working :
 git bisect new HEAD
 git bisect bad HEAD
 git bisect good aGoodCommit

 The hard review which we were thinking on was the issue of the maintaining
 of old/new and allow easy support of new tags like yes/no in the future.
 I tried to remove the maximum of bad/good and old/new which were hard wrote in
 the code but I'm not completly satisfied. This patch is clearly a v1.

Thanks for working on this. Here are some suggestions that I should
probably have told you before, but didn't:

- Take ownership of all the patches.
- Patch 3/3 renames some variables in bisect.c, do the same thing in
git-bisect.sh for consistency.
- Squash all the patches together.
- Try to find a way to break down the resulting patch into a logical
patch series which adds tests at each logical step. This might be
difficult. You might want to add features to git bisect--helper first
for example, then test those features, then add features to git bisect
and then test those features.

Best,
Christian.

 We're currently working on:

 * rebasing the history of the patch
 * git rev-list --bisect does not treat the revs/bisect/new and
 revs/bisect/old-SHA1 files.
 * git bisect visualize seem to work partially: the tags are displayed
 correctly but the tree is not limited to the bisect section.
 * adding tests about old/new

 Some other problems that might also be considerred later :
 * change/add valid terms (e.g unfixed/fixed instead of old/new)
 *
 * git bisect start [new [old...]] is not possible: the commits
 will be treated as bad and 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


Re: [PATCH v9 4/5] bisect: add the terms old/new

2015-06-25 Thread Christian Couder
On Thu, Jun 25, 2015 at 8:50 PM, Matthieu Moy matthieu@imag.fr wrote:

[...]

 @@ -178,7 +183,7 @@ bisect_start() {
 } 
 git rev-parse --sq-quote $@ $GIT_DIR/BISECT_NAMES 
 eval $eval true 
 -   if test $revision_seen -eq 1  test ! -s $GIT_DIR/BISECT_TERMS
 +   if test $must_write_terms -eq 1  test ! -s $GIT_DIR/BISECT_TERMS
 then
 echo $NAME_BAD $GIT_DIR/BISECT_TERMS 
 echo $NAME_GOOD $GIT_DIR/BISECT_TERMS

You are writing BISECT_TERMS here...

 @@ -543,14 +548,22 @@ check_and_set_terms () {
 fi
 NAME_BAD=bad
 NAME_GOOD=good ;;
 +   new|old)
 +   if ! test -s $GIT_DIR/BISECT_TERMS
 +   then
 +   echo new $GIT_DIR/BISECT_TERMS 
 +   echo old $GIT_DIR/BISECT_TERMS
 +   fi
 +   NAME_BAD=new
 +   NAME_GOOD=old ;;

...and here nearly in the same way.

So perhaps you could use a function like:

write_bisect_terms() {
  if test ! -s $GIT_DIR/BISECT_TERMS
  then
echo $NAME_BAD $GIT_DIR/BISECT_TERMS 
echo $NAME_GOOD $GIT_DIR/BISECT_TERMS
  fi
}
--
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] commit: add commit.signoff config option

2015-06-25 Thread Christian Couder
On Thu, Jun 25, 2015 at 9:32 AM, Caio Marcelo de Oliveira Filho
cmarc...@gmail.com wrote:
 In projects that use Signed-off-by, it's convenient to include that line
 in the commit by default. The commit.signoff config option allows to add
 that line in all commits automatically.

You can use a commit template.

Or you can use the commit-msg hook. For example with commands like:

grep ^Signed-off-by: $1 ||
echo Signed-off-by: $(git config user.name) $(git config user.email) $1

If you have more complex needs, there is also git interpret-trailers
(see the examples in the man page).
--
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: config commands not working _Noobe question

2015-06-22 Thread Christian Couder
On Tue, Jun 23, 2015 at 1:31 AM, Greg Ledger gled...@glcdelivers.com wrote:
 after adding git config ‹global user.name Greg Ledger and git config
 ‹global user.email gled...@glcdelivers.com, when I run:
 source ~/.gitconfig

The ~/.gitconfig file is not a shell script. You should not source it.
It is a text file that is read by Git commands when those commands are run.
--
To unsubscribe from this list: send the line unsubscribe git in


Re: [PATCH] bisect: revise manpage

2015-06-26 Thread Christian Couder
On Fri, Jun 26, 2015 at 4:58 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
 On 06/26/2015 03:15 PM, Christian Couder wrote:
 On Fri, Jun 26, 2015 at 3:00 PM, Matthieu Moy
 matthieu@grenoble-inp.fr wrote:
 Christian Couder christian.cou...@gmail.com writes:

 On Fri, Jun 26, 2015 at 1:30 PM, Michael Haggerty mhag...@alum.mit.edu 
 wrote:

 [...]

 +Eventually there will be no more revisions left to bisect, and the
 +command will print out a description of the first bad commit, and also
 +create a reference called `refs/bisect/bad` that points at that
 +commit.

 This could be understood as meaning that `refs/bisect/bad` is created
 only at the end of the bisection.

 -Eventually there will be no more revisions left to bisect, and you
 -will have been left with the first bad kernel revision in 
 refs/bisect/bad.

 The original looks better to me in this regard.

 I'm changing it to:

 Eventually there will be no more revisions left to bisect, and the
 command will print out a description of the first bad commit. The
 reference `refs/bisect/bad` created by bisect will point at that
 commit.

 I agree that is better.

 For the last sentence I'd suggest:

 The reference called `refs/bisect/bad` will point at that commit.

 Or maybe

 The reference `refs/bisect/bad` will be left pointing at that commit.

Yeah ok.
--
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] ref-filter: add %(refname:lalignX) option

2015-06-27 Thread Christian Couder
On Thu, Jun 25, 2015 at 1:43 PM, Karthik Nayak karthik@gmail.com wrote:
 Add support for %(refname:lalignX) where X is a number.
 This will print a shortened refname aligned to the left
 followed by spaces for a total length of X characters.
 If X is less than the shortened refname size, the entire
 shortened refname is printed.

 Mentored-by: Christian Couder christian.cou...@gmail.com
 Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
 Signed-off-by: Karthik Nayak karthik@gmail.com
 ---
  ref-filter.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)

 diff --git a/ref-filter.c b/ref-filter.c
 index 00d06bf..299b048 100644
 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -695,7 +695,22 @@ static void populate_value(struct ref_array_item *ref)
 int num_ours, num_theirs;

 formatp++;
 -   if (!strcmp(formatp, short))
 +   if (starts_with(formatp, lalign)) {
 +   const char *valp;
 +   int val;
 +
 +   skip_prefix(formatp, lalign, valp);
 +   val = atoi(valp);

After thinking about such code, I wonder if it would be better to
support %(refname:lalign=X) instead of %(refname:lalignX).

The reason why it might be interesting to require an = sign between
align and the number X is that if we later want to introduce another
option with a name that starts with lalign, for example
%(refname:lalignall=X) that would truncate the refname if it is bigger
than X), we might be more backward compatible with old git versions
that implement %(refname:lalign=X) but not %(refname:lalignall=X).

We will be more backward compatible because the above call to
starts_with() would probably be something like:

   if (starts_with(formatp, lalign=)) {

which means that old git versions would ignore something like lalignall=X.

 +   refname = shorten_unambiguous_ref(refname,
 + 
 warn_ambiguous_refs);
 +   if (val  strlen(refname)) {
 +   struct strbuf buf = STRBUF_INIT;
 +   strbuf_addstr(buf, refname);
 +   strbuf_addchars(buf, ' ', val - 
 strlen(refname));
 +   free((char *)refname);
 +   refname = strbuf_detach(buf, NULL);
 +   }

Thanks,
Christian.
--
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 v10 5/7] bisect: simplify the addition of new bisect terms

2015-06-26 Thread Christian Couder
On Fri, Jun 26, 2015 at 6:58 PM, Matthieu Moy matthieu@imag.fr wrote:

  static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, 
 void *cb_data)
  {
 -   return for_each_ref_in_submodule(submodule, refs/bisect/bad, fn, 
 cb_data);
 +   struct strbuf bisect_refs = STRBUF_INIT;
 +   int status;
 +   strbuf_addf(bisect_refs, refs/bisect/%s, name_bad);
 +   status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, 
 cb_data);
 +   strbuf_release(bisect_refs);
 +   return status;
  }

  static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, 
 void *cb_data)
  {
 -   return for_each_ref_in_submodule(submodule, refs/bisect/good, fn, 
 cb_data);
 +   struct strbuf bisect_refs = STRBUF_INIT;
 +   int status;
 +   strbuf_addf(bisect_refs, refs/bisect/%s, name_good);
 +   status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, 
 cb_data);
 +   strbuf_release(bisect_refs);
 +   return status;
  }

I wonder if it would not be better to just have:

static int for_each_bisect_ref(const char *submodule, each_ref_fn fn,
const char *term, void *cb_data)
{
  struct strbuf bisect_refs = STRBUF_INIT;
  int status;
  strbuf_addf(bisect_refs, refs/bisect/%s, term);
  status = for_each_ref_in_submodule(submodule, bisect_refs.buf,
fn, cb_data);
  strbuf_release(bisect_refs);
  return status;
}

This way it can be used with either name_good, name_bad or skip as
the term argument.
--
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 v10 7/7] bisect: allow any terms set by user

2015-06-26 Thread Christian Couder
On Fri, Jun 26, 2015 at 6:58 PM, Matthieu Moy matthieu@imag.fr wrote:
 From: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr

 Introduction of the git bisect terms command. The user can set his own
 terms. It will work exactly like before. The terms must be set before the
 start.

After looking a bit at the code, I think that for now existing
predefined terms (bad, good, new and old) as well as some
other terms that look like bisect subcommands like skip, start and
terms should be disallowed as arguments to git bisect terms, and
this should be stated in the commit message and in the documentation
as well as checked and tested.

For example a user might want to search for a fix by using git bisect
terms good bad (which should swap good and bad), but we should
not at least for now allow that.

 @@ -185,7 +197,12 @@ bisect_start() {
 eval $eval true 
 if test $must_write_terms -eq 1
 then
 -   write_terms $NAME_BAD $NAME_GOOD
 +   write_terms $NAME_BAD $NAME_GOOD 
 +   if test $must_log_terms -eq 1
 +   then
 +   echo git bisect terms $NAME_BAD $NAME_GOOD \
 +   $GIT_DIR/BISECT_LOG
 +   fi

Maybe you could move appending to the log into write_terms() though
you might need to pass an additional argument to enable or disable
logging.
--
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 v10.1 7/7] bisect: allow any terms set by user

2015-06-26 Thread Christian Couder
On Sat, Jun 27, 2015 at 12:25 AM, Junio C Hamano gits...@pobox.com wrote:
 Matthieu Moy matthieu@imag.fr writes:

 Matthieu Moy matthieu@imag.fr writes:

 + git bisect terms term-old term-new

 I think this is the other way around.

 Indeed.

 I hate to be saying this, but this is a strong indication that
 consistency with start $bad $good... must be broken.  If the
 person who has been working on this topic for a few iterations in
 the past few days cannot get it right, no ordinary user can.  With
 or without a mnemonic hint N comes before O, so does B before G.

 Of course we cannot just say git bisect terms old new.  That would
 only invite eh, I do not remember, which between terms and start
 take the old one first? without helping people.

 The best I can come up with is to forbid positional arguments to
 this subcommand and always require them to be given like so:

 git bisect terms --old=fast --new=slow
 git bisect terms --new=slow --old=fast

If we don't want to support positional arguments, then I would suggest
supporting first the following instead:

 git bisect terms --name-good=fast --name-bad=slow
 git bisect terms --name-bad=slow --name-good=fast

This would make the interface consistent with the code.

Of course we could also accept --name-old and --name-new as synonyms
for --name-good and --name-bad.

 We may want to start supporting

 git bisect start --new=master --old=maint

Maybe we could also support:

git bisect start --name-good=fast --name-bad=slow --fast=maint --slow=master
--
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 v10.1 7/7] bisect: allow any terms set by user

2015-06-26 Thread Christian Couder
On Sat, Jun 27, 2015 at 6:25 AM, Junio C Hamano gits...@pobox.com wrote:
 On Fri, Jun 26, 2015 at 9:10 PM, Christian Couder
 christian.cou...@gmail.com wrote:

 If we don't want to support positional arguments, then I would suggest
 supporting first the following instead:

  git bisect terms --name-good=fast --name-bad=slow
  git bisect terms --name-bad=slow --name-good=fast

 This would make the interface consistent with the code.

 Which somewhat defeats the point of introducing old and new, though.
 The terms support is for people who feel that good/bad would be too 
 confusing
 for the particular bisect session (e.g. because they are hunting for a fix).

Well if --name-old and --name-new are also available as synonyms, it
would not be too bad I think.
People could use the option names that fit their mental model or their
use case better.

 We may want to start supporting

 git bisect start --new=master --old=maint

 Maybe we could also support:

 git bisect start --name-good=fast --name-bad=slow --fast=maint --slow=master

 The same comment for the token after --name-, but allowing the terms to be set
 at start could be a type-saver.  With need for added --name-
 prefix (worse, twice),
 I am not sure if it would be seen as a useful type-saver, though.

At least people don't need to remember if they have to use git bisect
term before or after starting :-)
--
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 4/4] bisect: add the terms old/new

2015-06-10 Thread Christian Couder
On Wed, Jun 10, 2015 at 5:24 PM, Junio C Hamano gits...@pobox.com wrote:
 Matthieu Moy matthieu@grenoble-inp.fr writes:

 Somebody else did it like that is not a good justification. Especially
 when the previous code was not merged: the code wasn't finished.

 But I actually disagree with the fact that it was not the idea. The
 point of having the terms in BISECT_TERMS was precisely to be generic
 enough. Had the goal been just to distinguish good/bad and old/new, we
 would have needed only one bit of information, and encoding it with the
 existance/non-existance of a file would have been sufficient (as you
 tried to do in addition to BISECT_TERMS).

 For now we just rebased, corrected and finishing to implement
 functionalities.

 functionalities is one thing, but the code should be maintainable to be
 merged in git.git. Git would not be where it is if Junio was merging
 patches based on it works, we'll see if the code is good enough later
 kinds of judgments ;-).

 Moving from one hardcoded pair of terms to two hardcoded pairs of
 terms is a nice feature, but hardly a step in the right direction wrt
 maintainability.

 Nicely put.  From that point of view, the variable names and the
 underlying machinery in general should call these two new vs
 old.  I.e. name_new=bad name_old=good would be the default, not
 name_bad=bad name_good=good.

I don't think this would improve maintainability, at least not for me.
The doc currently uses good/bad everywhere.
For example the description is:

   This command uses git rev-list --bisect to help drive the binary search
   process to find which change introduced a bug, given an old good commit
   object name and a later bad commit object name.

And this is logical if the default is good/bad. If we use name_new
and name_old in the code, we make it harder for us to see if the doc
matches the code.

And unless we rewrite a lot of them, the tests too will mostly be
still using good/bad so it will make it harder to maintain them too.
--
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/11] ref-filter: add parse_opt_merge_filter()

2015-06-13 Thread Christian Couder
On Sat, Jun 13, 2015 at 10:18 PM, Karthik Nayak karthik@gmail.com wrote:

 diff --git a/ref-filter.h b/ref-filter.h
 index c2856b8..799e118 100644
 --- a/ref-filter.h
 +++ b/ref-filter.h
 @@ -50,6 +50,18 @@ struct ref_filter_cbdata {
 struct ref_filter *filter;
  };

 +/*  Macros for checking --merged and --no-merged options */
 +#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 \
 +   }

Could you reduce the redundancy in these 2 macros?
--
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/11] parse-options.h: add macros for '--contains' option

2015-06-14 Thread Christian Couder
On Sat, Jun 13, 2015 at 10:18 PM, Karthik Nayak karthik@gmail.com wrote:
 Add a macro for using the '--contains' option in parse-options.h
 also include an optional '--with' option macro which performs the
 same action as '--contains'.

 Make tag.c use this new macro

 Mentored-by: Christian Couder christian.cou...@gmail.com
 Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
 Signed-off-by: Karthik Nayak karthik@gmail.com
 ---
  builtin/tag.c   | 14 ++
  parse-options.h |  7 +++
  2 files changed, 9 insertions(+), 12 deletions(-)

 diff --git a/builtin/tag.c b/builtin/tag.c
 index 2d6610a..767162e 100644
 --- a/builtin/tag.c
 +++ b/builtin/tag.c
 @@ -595,23 +595,13 @@ int cmd_tag(int argc, const char **argv, const char 
 *prefix)

 OPT_GROUP(N_(Tag listing options)),
 OPT_COLUMN(0, column, colopts, N_(show tag list in 
 columns)),
 +   OPT_CONTAINS(with_commit, N_(print only tags that contain 
 the commit)),
 +   OPT_WITH(with_commit, N_(print only tags that contain the 
 commit)),
 {
 OPTION_CALLBACK, 0, sort, tag_sort, N_(type), 
 N_(sort tags),
 PARSE_OPT_NONEG, parse_opt_sort
 },
 {
 -   OPTION_CALLBACK, 0, contains, with_commit, 
 N_(commit),
 -   N_(print only tags that contain the commit),
 -   PARSE_OPT_LASTARG_DEFAULT,
 -   parse_opt_commit_object_name, (intptr_t)HEAD,
 -   },
 -   {
 -   OPTION_CALLBACK, 0, with, with_commit, 
 N_(commit),
 -   N_(print only tags that contain the commit),
 -   PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT,

The PARSE_OPT_HIDDEN flag is removed below. This means that --with
will appear in the git tag -h output, which means that --with
should appear in the documentation...
The commit message may also want to tell that with is not hidden any more.

 -   parse_opt_commit_object_name, (intptr_t)HEAD,
 -   },
 -   {
 OPTION_CALLBACK, 0, points-at, points_at, 
 N_(object),
 N_(print only tags of the object), 0, 
 parse_opt_object_name
 },
 diff --git a/parse-options.h b/parse-options.h
 index 8542d9c..d76e907 100644
 --- a/parse-options.h
 +++ b/parse-options.h
 @@ -243,5 +243,12 @@ 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_CONTAINS_OR_WITH(name, variable, help) \
 +   { OPTION_CALLBACK, 0, name, (variable), N_(commit), (help), \
 + PARSE_OPT_LASTARG_DEFAULT, \
 + parse_opt_commit_object_name, (intptr_t) HEAD \
 +   }
 +#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(contains, v, h)
 +#define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(contains, v, h)

Shouldn't it be with instead of contains?
--
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 v7 03/12] for-each-ref: change comment in ref_sort

2015-06-12 Thread Christian Couder
On Fri, Jun 12, 2015 at 8:29 PM, Karthik Nayak karthik@gmail.com wrote:
 On 06/12/2015 11:34 PM, Junio C Hamano wrote:

 Karthik Nayak karthik@gmail.com writes:

 What change since 9f613dd do you have in mind, exactly, though?


 Well initially the atoms were indexed into used_atom array, which
 later was removed. Hence the comment becomes obsolete.


 Later in which commit?  In builtin/for-each-ref.c in the version
 after applying patches 1-3 of this series on top of master, I still
 see used_atom[] array there, so...?


 Uh! My bad.
 Ignore this! I think I got confused, On having a look now that patch is
 not needed. Sorry.

I think it is needed later when struct ref_sort is moved into
ref-filter.h, because then the used_atom[] array is not moved.

So either:

1) you update the comment when you move struct ref_sort into
ref-filter.h, but then the downside is that there is not only code
movement in the patch that moves struct ref_sort into ref-filter.h,
or

2) you explain that, as struct ref_sort will be moved later while
the used_atom[] array will not be moved, the direct connection between
the comment and used_atom[] will be broken, and you need to prepare
for that because you want to avoid solution 1) as you want only code
movement when moving struct ref_sort into ref-filter.h.

Hope this helps,
Christian.
--
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 1/2] config: add options to list only variable names

2015-05-29 Thread Christian Couder
On Thu, May 28, 2015 at 9:20 PM, Junio C Hamano gits...@pobox.com wrote:
 SZEDER Gábor sze...@ira.uka.de writes:

 @@ -16,11 +16,12 @@ SYNOPSIS
  'git config' [file-option] [type] [-z|--null] --get-all name [value_regex]
  'git config' [file-option] [type] [-z|--null] --get-regexp name_regex 
 [value_regex]
  'git config' [file-option] [type] [-z|--null] --get-urlmatch name URL
 +'git config' [file-option] [-z|--null] --get-name-regexp name_regex
 ...

 At first I wondered why --get-name-regexp is needed, as (purely from
 the syntactic level) it appeared at the first glance the existing
 '--get-regexp' without 'value_regex' may be sufficient, until I read
 this:

 +--get-name-regexp::
 + Like --get-regexp, but shows only matching variable names, not its
 + values.

 which makes it clear why it is needed.  The distinction is purely
 about the output, i.e. the values are omitted.

If the distinction is purely about the output, then it seems logical
to add only an output related option, like the --name-only option I
suggested, and not 2 new modes (--get-name-regexp and --list-names).

Doesn't it look like git config already has too many modes?

 But then it makes me wonder why --get-name-regexp shouldn't
 optionally accept value_regex like --get-regexp does, allowing you
 to say list the variables that match this pattern whose values
 match this other pattern.  I know it may be a feature that is
 unnecessary for your purpose, but from a cursory look of the patch
 text, you do not seem to be doing anything different between
 get-regexp and get-name-regexp codepaths other than flipping
 omit_values bit on, so it could be that the feature is already
 supported but forgot to document it, perhaps?

This also suggests that it might be more logical to only add an option
called --name-only or --omit-values that would map directly to the
omit_values bit in the code.
--
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: seg fault in git format-patch

2015-05-31 Thread Christian Couder
On Sun, May 31, 2015 at 9:13 PM, Bruce Korb bruce.k...@gmail.com wrote:
 $ git format-patch -o patches --ignore-if-in-upstream
 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d
 Segmentation fault
 /u/gnu/proj/gnu-pw-mgr
 $ git format-patch -o patches
 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d
 patches/0001-remove-dead-code.patch
 patches/0002-dead-code-removal.patch
 patches/0003-add-sort-pw-cfg-program.patch
 patches/0004-add-doc-for-sort-pw-cfg.patch
 patches/0005-clean-up-doc-makefile.patch
 patches/0006-clean-up-doc-makefile.patch
 patches/0007-happy-2015-and-add-delete-option.patch
 patches/0008-fix-doc-Makefile.am.patch
 patches/0009-re-fix-copyright.patch
 patches/0010-finish-debugging-remove_pwid.patch
 patches/0011-only-update-file-if-something-was-removed.patch
 patches/0012-update-NEWS.patch
 patches/0013-bootstrap-cleanup.patch

Could you tell us which git version you are using? You can use git --version.
The operating system you are using could also be useful.
And maybe you could also run git under gdb and give us the output of
the bt (backtrace) gdb command when it crashes?

Thanks,
Christian.
--
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: seg fault in git format-patch

2015-05-31 Thread Christian Couder
On Sun, May 31, 2015 at 10:45 PM, Bruce Korb bruce.k...@gmail.com wrote:
 Oh, you can also clone the gnu-pw-mgr and likely get the same result:

Yeah, after cloning from http://git.savannah.gnu.org/r/gnu-pw-mgr.git
I get the following backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78,
commit=0x84e8d0, mark=139) at commit.c:528
528 while ((parents = parents-next))
(gdb) bt
#0  0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78,
commit=0x84e8d0, mark=139) at commit.c:528
#1  0x004b2743 in clear_commit_marks_many (nr=-1,
commit=0x7fffbfa0, mark=139) at commit.c:544
#2  0x004b2771 in clear_commit_marks (commit=0x84e8d0,
mark=139) at commit.c:549
#3  0x004537cc in get_patch_ids (rev=0x7fffd190,
ids=0x7fffc910) at builtin/log.c:832
#4  0x00455580 in cmd_format_patch (argc=1,
argv=0x7fffdc20, prefix=0x0) at builtin/log.c:1425
#5  0x00405807 in run_builtin (p=0x80cac8 commands+840,
argc=5, argv=0x7fffdc20) at git.c:350
#6  0x00405a15 in handle_builtin (argc=5, argv=0x7fffdc20)
at git.c:532
#7  0x00405b31 in run_argv (argcp=0x7fffdafc,
argv=0x7fffdb10) at git.c:578
#8  0x00405d29 in main (argc=5, av=0x7fffdc18) at git.c:686

(Please don't top post if you reply to this email as it is frown upon
on this list.)
--
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 v4 8/8] ref-filter: add 'ref-filter.h'

2015-05-31 Thread Christian Couder
On Sun, May 31, 2015 at 10:50 PM, Karthik Nayak karthik@gmail.com wrote:
 On 06/01/2015 02:16 AM, Matthieu Moy wrote:

 You can have a preparatory patch that adds ref-filter.c containing just
 #include ref-filter.h and ref-filter.h with proper content. After this
 preparatory patch, you're in a rather silly situation because you have
 an almost empty .c file, but it's still passing all tests and
 compileable.

 Then, the next patch can be just code movement.

 Would it be okay, If I just include the Makefile addition along with the
 code movement
 from 'for-each-ref' to 'ref-filter.c' like Eric suggested?

Yeah, I think it is ok as well.
--
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: seg fault in git format-patch

2015-05-31 Thread Christian Couder
On Mon, Jun 1, 2015 at 1:14 AM, Christian Couder
christian.cou...@gmail.com wrote:
 On Sun, May 31, 2015 at 10:45 PM, Bruce Korb bruce.k...@gmail.com wrote:
 Oh, you can also clone the gnu-pw-mgr and likely get the same result:

 Yeah, after cloning from http://git.savannah.gnu.org/r/gnu-pw-mgr.git
 I get the following backtrace:

 Program received signal SIGSEGV, Segmentation fault.
 0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78,
 commit=0x84e8d0, mark=139) at commit.c:528
 528 while ((parents = parents-next))
 (gdb) bt
 #0  0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78,
 commit=0x84e8d0, mark=139) at commit.c:528
 #1  0x004b2743 in clear_commit_marks_many (nr=-1,
 commit=0x7fffbfa0, mark=139) at commit.c:544
 #2  0x004b2771 in clear_commit_marks (commit=0x84e8d0,
 mark=139) at commit.c:549
 #3  0x004537cc in get_patch_ids (rev=0x7fffd190,
 ids=0x7fffc910) at builtin/log.c:832
 #4  0x00455580 in cmd_format_patch (argc=1,
 argv=0x7fffdc20, prefix=0x0) at builtin/log.c:1425
 #5  0x00405807 in run_builtin (p=0x80cac8 commands+840,
 argc=5, argv=0x7fffdc20) at git.c:350
 #6  0x00405a15 in handle_builtin (argc=5, argv=0x7fffdc20)
 at git.c:532
 #7  0x00405b31 in run_argv (argcp=0x7fffdafc,
 argv=0x7fffdb10) at git.c:578
 #8  0x00405d29 in main (argc=5, av=0x7fffdc18) at git.c:686

 (Please don't top post if you reply to this email as it is frown upon
 on this list.)

When running the command that gives the above segfault:

$ git format-patch -o patches --ignore-if-in-upstream
14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d

It is interesting to note that the last sha1 refers to a tag:

$ git cat-file tag 2de9eef391259dfc8748dbaf76a5d55427f37b0d
object 524ccbdbe319068ab18a3950119b9e9a5d135783
type commit
tag v1.4
tagger Bruce Korb bk...@gnu.org 1428847577 -0700

Release 1.4

* sort-pw-cfg: a sort/merge program for combining and organizing
  configurations.

* --delete: a new option to remove any entries for a password id

It works when the tag is replaced by the commit it points to, and the
segfault happens because the we try to access the parents field of
the tag object as if it was a commit.
--
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: seg fault in git format-patch

2015-05-31 Thread Christian Couder
On Mon, Jun 1, 2015 at 1:53 AM, Christian Couder
christian.cou...@gmail.com wrote:
 On Mon, Jun 1, 2015 at 1:14 AM, Christian Couder
 christian.cou...@gmail.com wrote:
 On Sun, May 31, 2015 at 10:45 PM, Bruce Korb bruce.k...@gmail.com wrote:
 Oh, you can also clone the gnu-pw-mgr and likely get the same result:

 Yeah, after cloning from http://git.savannah.gnu.org/r/gnu-pw-mgr.git
 I get the following backtrace:

 Program received signal SIGSEGV, Segmentation fault.
 0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78,
 commit=0x84e8d0, mark=139) at commit.c:528
 528 while ((parents = parents-next))
 (gdb) bt
 #0  0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78,
 commit=0x84e8d0, mark=139) at commit.c:528
 #1  0x004b2743 in clear_commit_marks_many (nr=-1,
 commit=0x7fffbfa0, mark=139) at commit.c:544
 #2  0x004b2771 in clear_commit_marks (commit=0x84e8d0,
 mark=139) at commit.c:549
 #3  0x004537cc in get_patch_ids (rev=0x7fffd190,
 ids=0x7fffc910) at builtin/log.c:832
 #4  0x00455580 in cmd_format_patch (argc=1,
 argv=0x7fffdc20, prefix=0x0) at builtin/log.c:1425
 #5  0x00405807 in run_builtin (p=0x80cac8 commands+840,
 argc=5, argv=0x7fffdc20) at git.c:350
 #6  0x00405a15 in handle_builtin (argc=5, argv=0x7fffdc20)
 at git.c:532
 #7  0x00405b31 in run_argv (argcp=0x7fffdafc,
 argv=0x7fffdb10) at git.c:578
 #8  0x00405d29 in main (argc=5, av=0x7fffdc18) at git.c:686

 (Please don't top post if you reply to this email as it is frown upon
 on this list.)

 When running the command that gives the above segfault:

 $ git format-patch -o patches --ignore-if-in-upstream
 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d

 It is interesting to note that the last sha1 refers to a tag:

 $ git cat-file tag 2de9eef391259dfc8748dbaf76a5d55427f37b0d
 object 524ccbdbe319068ab18a3950119b9e9a5d135783
 type commit
 tag v1.4
 tagger Bruce Korb bk...@gnu.org 1428847577 -0700

 Release 1.4

 * sort-pw-cfg: a sort/merge program for combining and organizing
   configurations.

 * --delete: a new option to remove any entries for a password id

 It works when the tag is replaced by the commit it points to, and the
 segfault happens because the we try to access the parents field of
 the tag object as if it was a commit.

Yeah, in builtin/log.c we are doing:

o2 = rev-pending.objects[1].item;

and then we are casting the object into a commit when passing it to
clear_commit_marks():

clear_commit_marks((struct commit *)o2,
SEEN | UNINTERESTING | SHOWN | ADDED);

but I don't know where we should have peeled the tag to get a commit,
and it's late here so I will leave it someone else to find a fix.

Best,
Christian.
--
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: Bug in 'git am' when applying a broken patch

2015-06-01 Thread Christian Couder
Hi Greg,

On Mon, Jun 1, 2015 at 3:54 AM, Greg KH gre...@linuxfoundation.org wrote:
 On Mon, Jun 01, 2015 at 09:17:59AM +0900, Greg KH wrote:
 Hi all,

 I received the patch attached below as part of a submission against the
 Linux kernel tree.  The patch seems to have been hand-edited, and is not
 correct, and patch verifies this as being a problem:

 $ patch -p1 --dry-run  bad_patch.mbox
 checking file drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
 patch:  malformed patch at line 133:skb_put(skb, 
 sizeof(struct ieee80211_authentication));

 But git will actually apply it:
 $ git am -s bad_patch.mbox
 Applying: staging: rtl8192u: ieee80211: Fix sparse endianness warnings

 But, there's nothing in the patch at all except the commit message:

 $ git show HEAD
 commit f6643dfef5b701db86f23be9ce6fb5b3bafe76b6
 Author: Gaston Gonzalez gasc...@gmail.com
 Date:   Sun May 31 12:17:48 2015 -0300

 staging: rtl8192u: ieee80211: Fix sparse endianness warnings

 Fix the following sparse warnings:

 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:663:32: warning: 
 incorrect type in assignment (different base types)
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:663:32:
 expected restricted __le16 [usertype] frame_ctl
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:663:32:got int
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:664:50: warning: 
 invalid assignment: |=
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:664:50:left 
 side has type restricted __le16
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:664:50:right 
 side has type int

 Signed-off-by: Gaston Gonzalez gasc...@gmail.com
 Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org

 $ git diff HEAD^
 $

 Any ideas what is going on here?  Shouldn't 'git am' have failed?

 Oh, I'm using git version 2.4.2 right now.

 I've asked Gaston for the original patch to verify before he hand-edited
 it, to verify that git wasn't creating something wrong here, as well.

 Gaston sent me his original patch, before he edited it, and it was
 correct, so git is correctly creating the patch, which is good.  So it's
 just a 'git am' issue with a broken patch file.

Yeah, git am is calling 'git apply --index' on the attached patch and
'git apply' doesn't apply it, doesn't warn and exits with code 0.

Thanks,
Christian.
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index d2e8b12..0477ba1 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -660,2 +660,2 @@ inline struct sk_buff *ieee80211_authentication_req(struct 
ieee80211_network *be
auth = (struct ieee80211_authentication *)
skb_put(skb, sizeof(struct ieee80211_authentication));

-   auth-header.frame_ctl = IEEE80211_STYPE_AUTH;
-   if (challengelen) auth-header.frame_ctl |= IEEE80211_FCTL_WEP;
+   auth-header.frame_ctl = cpu_to_le16(IEEE80211_STYPE_AUTH);
+   if (challengelen)
+   auth-header.frame_ctl |= cpu_to_le16(IEEE80211_FCTL_WEP);

auth-header.duration_id = 0x013a; //FIXME

--
2.1.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



Re: seg fault in git format-patch

2015-06-01 Thread Christian Couder
On Mon, Jun 1, 2015 at 3:44 PM, Christian Couder
christian.cou...@gmail.com wrote:

 The following seems to fix it, but I am not sure it is the right fix:

Ooops, I had not seen that Brian and Peff are already discussing a fix
in this thread:

http://thread.gmane.org/gmane.comp.version-control.git/270371
--
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: seg fault in git format-patch

2015-06-01 Thread Christian Couder
On Mon, Jun 1, 2015 at 2:01 AM, Christian Couder
christian.cou...@gmail.com wrote:
 On Mon, Jun 1, 2015 at 1:53 AM, Christian Couder
 christian.cou...@gmail.com wrote:
 On Mon, Jun 1, 2015 at 1:14 AM, Christian Couder
 christian.cou...@gmail.com wrote:
 On Sun, May 31, 2015 at 10:45 PM, Bruce Korb bruce.k...@gmail.com wrote:
 Oh, you can also clone the gnu-pw-mgr and likely get the same result:

 Yeah, after cloning from http://git.savannah.gnu.org/r/gnu-pw-mgr.git
 I get the following backtrace:

 Program received signal SIGSEGV, Segmentation fault.
 0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78,
 commit=0x84e8d0, mark=139) at commit.c:528
 528 while ((parents = parents-next))
 (gdb) bt
 #0  0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78,
 commit=0x84e8d0, mark=139) at commit.c:528
 #1  0x004b2743 in clear_commit_marks_many (nr=-1,
 commit=0x7fffbfa0, mark=139) at commit.c:544
 #2  0x004b2771 in clear_commit_marks (commit=0x84e8d0,
 mark=139) at commit.c:549
 #3  0x004537cc in get_patch_ids (rev=0x7fffd190,
 ids=0x7fffc910) at builtin/log.c:832
 #4  0x00455580 in cmd_format_patch (argc=1,
 argv=0x7fffdc20, prefix=0x0) at builtin/log.c:1425
 #5  0x00405807 in run_builtin (p=0x80cac8 commands+840,
 argc=5, argv=0x7fffdc20) at git.c:350
 #6  0x00405a15 in handle_builtin (argc=5, argv=0x7fffdc20)
 at git.c:532
 #7  0x00405b31 in run_argv (argcp=0x7fffdafc,
 argv=0x7fffdb10) at git.c:578
 #8  0x00405d29 in main (argc=5, av=0x7fffdc18) at git.c:686

 (Please don't top post if you reply to this email as it is frown upon
 on this list.)

 When running the command that gives the above segfault:

 $ git format-patch -o patches --ignore-if-in-upstream
 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d

 It is interesting to note that the last sha1 refers to a tag:

 $ git cat-file tag 2de9eef391259dfc8748dbaf76a5d55427f37b0d
 object 524ccbdbe319068ab18a3950119b9e9a5d135783
 type commit
 tag v1.4
 tagger Bruce Korb bk...@gnu.org 1428847577 -0700

 Release 1.4

 * sort-pw-cfg: a sort/merge program for combining and organizing
   configurations.

 * --delete: a new option to remove any entries for a password id

 It works when the tag is replaced by the commit it points to, and the
 segfault happens because the we try to access the parents field of
 the tag object as if it was a commit.

 Yeah, in builtin/log.c we are doing:

 o2 = rev-pending.objects[1].item;

 and then we are casting the object into a commit when passing it to
 clear_commit_marks():

 clear_commit_marks((struct commit *)o2,
 SEEN | UNINTERESTING | SHOWN | ADDED);

 but I don't know where we should have peeled the tag to get a commit,
 and it's late here so I will leave it someone else to find a fix.

The following seems to fix it, but I am not sure it is the right fix:

diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..0ab9360 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -792,6 +792,16 @@ static int reopen_stdout(struct commit *commit,
const char *subject,
return 0;
 }

+static void clear_object_marks(struct object *obj)
+{
+   struct commit *c = (struct commit *)peel_to_type(NULL, 0, obj,
+OBJ_COMMIT);
+   if (!c)
+   die(_(could not convert %s into a commit),
+   sha1_to_hex(obj-sha1));
+   clear_commit_marks(c, SEEN | UNINTERESTING | SHOWN | ADDED);
+}
+
 static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
 {
struct rev_info check_rev;
@@ -827,10 +837,8 @@ static void get_patch_ids(struct rev_info *rev,
struct patch_ids *ids)
}

/* reset for next revision walk */
-   clear_commit_marks((struct commit *)o1,
-   SEEN | UNINTERESTING | SHOWN | ADDED);
-   clear_commit_marks((struct commit *)o2,
-   SEEN | UNINTERESTING | SHOWN | ADDED);
+   clear_object_marks(o1);
+   clear_object_marks(o2);
o1-flags = flags1;
o2-flags = flags2;
 }
--
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 v4 5/8] for-each-ref: introduce 'ref_filter_clear_data()'

2015-05-31 Thread Christian Couder
On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak karthik@gmail.com wrote:
 Introduce and implement 'ref_filter_clear_data()' which will free
 all allocated memory for 'ref_filter_cbdata' and its underlying array
 of 'ref_array_item'.

 Mentored-by: Christian Couder christian.cou...@gmail.com
 Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
 Signed-off-by: Karthik Nayak karthik@gmail.com
 ---
  builtin/for-each-ref.c | 13 +
  1 file changed, 13 insertions(+)

 diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
 index ef54c90..f896e1c 100644
 --- a/builtin/for-each-ref.c
 +++ b/builtin/for-each-ref.c
 @@ -907,6 +907,18 @@ static int grab_single_ref(const char *refname, const 
 unsigned char *sha1, int f
 return 0;
  }

 +/* Free all memory allocated for ref_filter_cbdata */
 +void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata)
 +{
 +   int i;
 +
 +   for (i = 0; i  ref_cbdata-array.nr; i++)
 +   free(ref_cbdata-array.items[i]);
 +   free(ref_cbdata-array.items);
 +   ref_cbdata-array.items = NULL;
 +   ref_cbdata-array.nr = ref_cbdata-array.alloc = 0;
 +}

As this is clearing the array only, it would be better to have it in a
ref_array_clear() function.
There are already argv_array_clear() and sha1_array_clear() by the way.
And maybe if many such ref_array functions are created and start being
used elsewhere we can easily move everything into new ref-array.{c,h}
files.

Thanks,
Christian.
--
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 v4 8/8] ref-filter: add 'ref-filter.h'

2015-05-31 Thread Christian Couder
On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak karthik@gmail.com wrote:

 -struct ref_sort {
 -   struct ref_sort *next;
 -   int atom; /* index into used_atom array */

Where is this used_atom array?
I searched but couldn't find it in the same file.

 -   unsigned reverse : 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: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public

2015-05-31 Thread Christian Couder
On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak karthik@gmail.com wrote:

 -static void sort_refs(struct ref_sort *sort, struct ref_array *array)
 +void sort_ref_array(struct ref_sort *sort, struct ref_array *array)

It is probably better to call the above function ref_array_sort()...

[...]

 -static struct ref_sort *default_sort(void)
 +/*  If no sorting option is given, use refname to sort as default */
 +struct ref_sort *ref_default_sort(void)

... especially since you call the above ref_default_sort()...

 -static int opt_parse_sort(const struct option *opt, const char *arg, int 
 unset)
 +int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset)

... and the above opt_parse_sort().
--
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 v4 6/8] for-each-ref: rename some functions and make them public

2015-05-31 Thread Christian Couder
On Sun, May 31, 2015 at 10:04 AM, Christian Couder
christian.cou...@gmail.com wrote:
 On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak karthik@gmail.com wrote:

 -static void sort_refs(struct ref_sort *sort, struct ref_array *array)
 +void sort_ref_array(struct ref_sort *sort, struct ref_array *array)

 It is probably better to call the above function ref_array_sort()...

 [...]

 -static struct ref_sort *default_sort(void)
 +/*  If no sorting option is given, use refname to sort as default */
 +struct ref_sort *ref_default_sort(void)

 ... especially since you call the above ref_default_sort()...

 -static int opt_parse_sort(const struct option *opt, const char *arg, int 
 unset)
 +int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset)

 ... and the above opt_parse_sort().

After saying that I realize that these two other functions are not
doing the same thing.
This might suggest that they are not named very well as well.
--
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


Draft of Git Rev News edition 4

2015-05-31 Thread Christian Couder
Hi,

A draft of Git Rev News edition 4 is available here:

https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-4.md

Everyone is welcome to contribute in any section, like Junio and
Matthieu already did, either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

https://github.com/git/git.github.io/issues/63

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas, Nicola and myself plan to publish this edition on Wednesday
the 3rd of June.

Thanks,
Christian.
--
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 v4 6/8] for-each-ref: rename some functions and make them public

2015-05-31 Thread Christian Couder
On Sun, May 31, 2015 at 11:17 AM, Karthik Nayak karthik@gmail.com wrote:
 On 05/31/2015 01:41 PM, Christian Couder wrote:

 On Sun, May 31, 2015 at 10:04 AM, Christian Couder
 christian.cou...@gmail.com wrote:

 On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak karthik@gmail.com
 wrote:


 -static void sort_refs(struct ref_sort *sort, struct ref_array *array)
 +void sort_ref_array(struct ref_sort *sort, struct ref_array *array)


 It is probably better to call the above function ref_array_sort()...

 [...]

 -static struct ref_sort *default_sort(void)
 +/*  If no sorting option is given, use refname to sort as default */
 +struct ref_sort *ref_default_sort(void)


 ... especially since you call the above ref_default_sort()...

 -static int opt_parse_sort(const struct option *opt, const char *arg,
 int unset)
 +int opt_parse_ref_sort(const struct option *opt, const char *arg, int
 unset)


 ... and the above opt_parse_sort().


 After saying that I realize that these two other functions are not
 doing the same thing.
 This might suggest that they are not named very well as well.


 What do you mean by not doing the same thing.

sort_ref_array() is actually sorting a ref_array, while
ref_default_sort() and opt_parse_ref_sort() are not sorting anything.

Maybe it would all be clearer with a renaming like the following:

sort_refs() - ref_array_sort()
struct ref_sort - struct ref_sort_criteria
default_sort() - ref_default_sort_criteria()
opt_parse_sort() - opt_parse_ref_sort_criteria()
--
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.5/2] config: add options to list only variable names

2015-05-28 Thread Christian Couder
On Wed, May 27, 2015 at 10:11 PM, SZEDER Gábor sze...@ira.uka.de wrote:
 Recenty I created a multi-line branch description with '.' and '='
 characters on one of the lines, and noticed that fragments of that line
 show up when completing set variable names for 'git config', e.g.:

   $ git config --get branch.b.description
   Branch description to fool the completion script with a
   second line containing dot . and equals = characters.
   $ git config --unset TAB
   ...
   second line containing dot . and equals
   ...

 The completion script runs 'git config --list' and processes its output to
 strip the values and keep only the variable names.  It does so by looking
 for lines containing '.' and '=' and outputting everything before the '=',
 which was fooled by my multi-line branch description.

 A similar issue exists with aliases and pretty format aliases with
 multi-line values, but in that case 'git config --get-regexp' is run and
 subsequent lines don't have to contain either '.' or '=' to fool the
 completion script.

 Though 'git config' can produce null-terminated output for newline-safe
 parsing, that's of no use in this case, becase we can't cope with nulls in
 the shell.

 Help the completion script by introducing the '--list-names' and
 '--get-names-regexp' options, the names-only equivalents of '--list' and
 '--get-regexp', so it doesn't have to separate variable names from their
 values anymore.

Why don't you just add a '--name-only' option that can be used only
with '--list' and '--get-regexp'?

Like:

'git config' [file-option] [-z|--null] [--name-only] --get-regexp name_regex

and

'git config' [file-option] [-z|--null] [--name-only] -l | --list

?

It seems to me that it would reduce the number of options, and later
if we want to pass a format we could have maybe:

'git config' [file-option] [-z|--null] [--name-only |
--format=format] --get-regexp name_regex

and

'git config' [file-option] [-z|--null] [--name-only |
--format=format] -l | --list

Thanks,
Christian.
--
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 v10.1 7/7] bisect: allow any terms set by user

2015-06-29 Thread Christian Couder
On Mon, Jun 29, 2015 at 11:32 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Christian Couder christian.cou...@gmail.com writes:

[...]

 I find it particularly frustrating that we issue the message:

   The merge base %s is bad.\n
   This means the bug has been fixed 
   between %s and [%s].\n

I find it a good safety feature.

 bisect is all about finding the commit where a property has changed,

That is your interpretation of this command. On the man page there is:

git-bisect - Find by binary search the change that introduced a bug

So its stated purpose is to find the first bad commit. Not to find a fix.

This doesn't mean that we cannot or should not make it possible to
find a fix, but we shoudn't try to find a fix when there is doubt
about whether the user wants to find a fix or the first bad commit.

 and
 here it stops, saying I know it's between A and B, but I won't go
 further.

You can interpret it that way, but my interpretation of this is that
the real reason why we stop is because we don't know what the users
really wants to do, find the fix, or really find the first bad commit.

 In particular when bisecting from two branches, the user knows that
 branch A is good, and branch B is bad, but does not necessarily know
 whether it's a regression in B or a
 fix in A. The fact that bisect can find out should be just normal from
 the user point of view. There's no mistake involved, nothing to fix, and
 nothing that went wrong.

 Well in this case, it's possible that the merge base is bad and what
 the user is interested in is the first bad commit that was commited
 before the merge base. We just don't know, in the case the merge base
 is bad, what is more interesting for the user.

 The question asked by the user is X is good, Y is bad, tell me where
 exactly it changed.

That's your interpretation of the question asked by the user.
It can be interpreted otherwise.

 We can't know for sure what is interesting for
 the user, but we can answer his question anyway.

You can answer your interpretation of the question, yes, but this
means taking risks that we don't need to take in my opinion.

If people really want it, we can still have an option called for
example --find-fix that could automatically try to find the fix when
the merge base is bad without displaying the message that annoys
you. It would make it explicit that it is ok to find a fix.

 Similarly, there can be several commits that introduce the same
 regression (this may happen if you cherry picked the guilty commit from
 branch A to branch B, and then merged A and B, or if you broke, fixed,
 and then broke again). bisect finds one transition from good to bad, but
 not all. It may or may not be the one the user wanted, but we can't
 know.

Yeah, but this is different as we would still find a first bad
commit, not a fix.

 I think I prefer term to name.

 Ok with that. I agree that it would be more consistent to have a git
 bisect terms and --term-{old,new,bad,good}.

 OK, I've applied it.

Great!
--
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] ref-filter: add %(refname:lalignX) option

2015-06-27 Thread Christian Couder
On Sat, Jun 27, 2015 at 10:02 PM, Christian Couder
christian.cou...@gmail.com wrote:
 On Thu, Jun 25, 2015 at 1:43 PM, Karthik Nayak karthik@gmail.com wrote:

 +   if (starts_with(formatp, lalign)) {
 +   const char *valp;
 +   int val;
 +
 +   skip_prefix(formatp, lalign, valp);
 +   val = atoi(valp);

 After thinking about such code, I wonder if it would be better to
 support %(refname:lalign=X) instead of %(refname:lalignX).

 The reason why it might be interesting to require an = sign between
 align and the number X is that if we later want to introduce another
 option with a name that starts with lalign, for example
 %(refname:lalignall=X) that would truncate the refname if it is bigger
 than X), we might be more backward compatible with old git versions
 that implement %(refname:lalign=X) but not %(refname:lalignall=X).

 We will be more backward compatible because the above call to
 starts_with() would probably be something like:

if (starts_with(formatp, lalign=)) {

 which means that old git versions would ignore something like lalignall=X.

Another reason is that it would be simpler if we ever want to have
arbitrary string parameters, like %(refname:substitute=%/%\%).
--
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


Draft of Git Rev News edition 5

2015-07-05 Thread Christian Couder
Hi,

A draft of Git Rev News edition 5 is available here:

https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-5.md

Everyone is welcome to contribute in any section, like Junio and
Matthieu already did, either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

https://github.com/git/git.github.io/issues/77

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas, Nicola and myself plan to publish this edition on Wednesday
the 8th of July.

Thanks,
Christian.
--
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: Draft of Git Rev News edition 5

2015-07-05 Thread Christian Couder
On Sun, Jul 5, 2015 at 9:11 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Sun, Jul 05, 2015 at 01:13:57PM +0200, Christian Couder wrote:
 A draft of Git Rev News edition 5 is available here:
 https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-5.md
 Everyone is welcome to contribute in any section...

 I'm not familiar with the criteria for deciding what merits mention
 in the newsletter. Is the recent introduction of git-worktree and the
 attendant relocation of add and prune functionality worthy?

Yes, I think it's really worthy, thanks a lot for contributing this
very interesting article!

 If so, perhaps the following write-up would be suitable?

Yes, I changed a few things to make fit better with the rest of the
content, but otherwise it looks great!

I created this PR to discuss the changes I made:

https://github.com/git/git.github.io/pull/85

Thomas just merged it, but we can still discuss it.

Thanks again,
Christian.
--
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: Pushing and pulling the result of `git replace` and objects/info/alternates

2015-05-25 Thread Christian Couder
On Mon, May 25, 2015 at 11:49 AM, Stephen Kelly steve...@gmail.com wrote:
 On 05/24/2015 07:28 AM, Christian Couder wrote:
 Hi,

 On Fri, May 22, 2015 at 4:38 PM, Stephen Kelly steve...@gmail.com wrote:
 I have tried out using `git replace --graft` and
 .git/objects/info/alternates to 'refer to' the history in the origin
 repo instead of 'duplicating' it. This is similar to how Qt5 repos
 refer to Qt 4 history in a different repo.

 Question 1) Is this a reasonable thing to do for this scenario?
 I think it should work without too much work, but see the answer to
 the next question.

 Ok, thanks. The concern is that there is plenty of documentation for
 git-filter-branch, but no documentation or porcelain for info/alternates
 and little out on the internet about it or git replace and using them
 together.

 However, it seems to be a reasonable thing to do.

Yeah.

By the way it looks like I was wrong in my answer to your second question.

You might want to clone using the --reference option to automatically
set up .git/objects/info/alternates properly.

(There is an ongoing related thread on the list:

http://thread.gmane.org/gmane.comp.version-control.git/269537)

You might also be able to clone using an option like --config
remote.origin.fetch = 'refs/replace/*:refs/replace/*' to fetch the
replace ref when cloning.

So it looks like you might just need to clone with a few more options
than usual.

I haven't tested it so please tell me if it works :-)

 echo ../../calculator/objects 
 ../.git/modules/compute/objects/info/alternates
 git replace --graft HEAD $extraction_sha
 Maybe use the following instead of the above line:

 git fetch 'refs/replace/*:refs/replace/*'

 Thanks.

 # And now we see the history from the calculator repo. Great. But, it
 required user action after the clone.
 Yeah, but if the 2 above commands are in a script maybe it's
 reasonable to ask the user to launch the script once after cloning.

 Would it be possible to do this in a hook in the 'integration repo'
 which contains both submodules in the example I posted? Like a fetch
 hook or something?

It is possible to do whatever you want in a hook, but the question is
why would you do it in a hook as it looks like it needs to be done
only once?

Or maybe I am missing something?

Best,
Christian.
--
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] ref-filter: add ref-filter API

2015-05-23 Thread Christian Couder
On Sat, May 23, 2015 at 4:42 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 karthik nayak karthik@gmail.com writes:

 At some point, I'd expect something like

filtered_list_of_refs = filer(full_list_of_refs, description_of_filter);

 That would remove some refs from full_list_of_refs according to
 description_of_filter.

 (totally invented code, only to show the idea)

 If there's a piece of code looking like this, then you need a data
 structure to store list of refs (full_list_of_refs and
 filtered_list_of_refs) and another to describe what you're doing with it
 (description_of_filter).

 The name ref_filter implies to me that it contains the description of
 the filter, but looking at the code it doesn't seem to be the case.


 But it does just that, doesn't it?

 struct ref_filter {
   int count, alloc;
   struct ref_filter_item **items;
   const char **name_patterns;
 };

 If you see it does contain 'name_patterns' according to which it will
 filter the given refs,

 But it also contains struct ref_filter_item **items, which as I
 understand it contains a list of refs (with name, sha1  such).

 That's the part I do not find natural: the same structure contains both
 the list of refs and the way it should be filtered.

 Re-reading the patch, I seem to understand that you're putting both on
 the same struct because of the API of for_each_ref() which takes one
 'data' pointer to be casted, so you want both the input (filter
 description) and the output (list of refs after filtering) to be
 contained in the same struct.

 But I think this could be clearer in the code (and/or comment + commit
 message). Perhaps stg like:

 struct ref_filter_data /* Probably not the best name */ {
 struct ref_list list;
 struct ref_filter filter;
 };

 struct ref_list {
 int count, alloc;
 struct ref_filter_item **items;
 const char **name_patterns;
 };

Matthieu, I think you forgot to remove const char **name_patterns;
in the above struct, as you put it in the ref_filter struct below:

 struct ref_filter {
 const char **name_patterns;
 /* There will be more here later */
 };

I agree that it might be clearer to separate both. In this case
instead of ref_list the struct might be called ref_filter_array as
we already have argv_array in argv-array.h and sha1_array in
sha1-array.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: Pushing and pulling the result of `git replace` and objects/info/alternates

2015-05-23 Thread Christian Couder
Hi,

On Fri, May 22, 2015 at 4:38 PM, Stephen Kelly steve...@gmail.com wrote:
 Hello,

 I have an 'integration repo' which contains other git repos as submodules.

 One of the submodules is to be split in two to extract a library.

 A common way of doing that is to use git-filter-branch. A disadvantage
 of that is that it results in duplicated partial-history in the
 extracted repo. So, git log shows the entire history, but there is not
 one canonical sha which represents the history at that point. The
 split repo will contain 'false history', and checking it out will not
 be useful.

 So, I want to avoid git-filter-branch.

 I have tried out using `git replace --graft` and
 .git/objects/info/alternates to 'refer to' the history in the origin
 repo instead of 'duplicating' it. This is similar to how Qt5 repos
 refer to Qt 4 history in a different repo.

 Question 1) Is this a reasonable thing to do for this scenario?

I think it should work without too much work, but see the answer to
the next question.

 Question 2) Is there a way to push the `git replace` result and the
 `info/alternates` content so that clients cloning the 'integration
 repo' do not have to do that 'manually' or with a 'setup-repo.sh'
 script?

In short no.

git replace creates replace refs in refs/replace/. To fetch these
refs, people need to use either git fetch
'refs/replace/*:refs/replace/*' or add a fetch =
refs/replace/*:refs/replace/* line in their config.
For simplicity and security reasons fetching replace refs is not on by default.

Also changing the objects/info/alternates to make it point to another
repo cannot be done automatically when cloning.

 The sequence of commands below can be pasted into a tmp directory to
 see the scenario in action.

 Thanks!


 mkdir calculator
 cd calculator
 mkdir mainui libcalc
 echo print \hello\  mainui/app.py
 echo print \hello\  libcalc/adder.py
 echo print \hello\  libcalc/subtracter.py
 git init
 git add .
 git commit -am Initial commit
 git checkout `git rev-parse HEAD`

 cd ..
 mkdir appsuite
 cd appsuite
 git init
 git submodule add ../calculator
 git commit -m Add calculator submodule

 # Add other submodules in the suite...

 cd calculator

 echo print \goodbye\  libcalc/subtracter.py
 git add libcalc/subtracter.py
 git commit -am Fix bug in subtracter

 echo print \Hi\  libcalc/adder.py
 git add libcalc/adder.py
 git commit -am Make adder more efficient

 echo print \Hello, world!\  mainui/app.py
 git add mainui/app.py
 git commit -am Improve app

 echo print \hello, hello\  libcalc/multiplier.py
 git add libcalc/multiplier.py
 git commit -am Add multiplier

 cd ..
 git add calculator
 git commit -m Update calculator submodule

 mkdir compute
 cd calculator
 mv libcalc ../compute

 extraction_sha=`git rev-parse HEAD`
 git commit -am Remove libcalc from calculator repo -m It is moved
 to a new compute repo
 removal_sha=`git rev-parse HEAD`
 git push

 cd ../compute
 git init
 git add .
 git commit -m Create the compute repo. -m This commit will not be
 normally visible after the replace --graft below.

 echo This is the compute framework. It contains the libcalc library.  
 README
 git add README
 git commit -m Initialize the compute repo. -m This has been
 extracted from calculator.git at $removal_sha
 git checkout `git rev-parse HEAD`

 cd ..
 mv compute ..
 git submodule add ../compute

 git add calculator compute
 git commit -m Split compute framework out of calculator repo.

 cd compute
 git log --oneline
 # We don't see older history from the calculator repo

 # Let's add alternates
 echo ../../calculator/objects 
 ../.git/modules/compute/objects/info/alternates

 # ... and graft onto the extraction commit
 git replace --graft HEAD $extraction_sha

 git log --oneline
 # Great, now we see history from the calculator repo.

 cd ../..
 git clone appsuite appsuite-clone
 cd appsuite-clone
 git submodule update --init
 cd compute
 ls ../.git/modules/compute/objects/info
 git log --oneline
 # The replacement and alternatives did not get cloned ... :(

 echo ../../calculator/objects 
 ../.git/modules/compute/objects/info/alternates
 git replace --graft HEAD $extraction_sha

Maybe use the following instead of the above line:

git fetch 'refs/replace/*:refs/replace/*'

 # And now we see the history from the calculator repo. Great. But, it
 required user action after the clone.

Yeah, but if the 2 above commands are in a script maybe it's
reasonable to ask the user to launch the script once after cloning.

Best,
Christian.
--
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: Troubleshoot clone issue to NFS.

2015-05-21 Thread Christian Couder
On Thu, May 21, 2015 at 3:13 PM,  steve.nor...@thomsonreuters.com wrote:

[...]

 So the questions are:

 1) Should I expect a clone to NFS to be that much slower?
 2) Is there anything I could do to speed it up (I've tried --bare as that is 
 what the repositories will be when stored on NFS and there wasn't really a 
 difference).
 3) What else can I use in git to compare the performance on local to NFS to 
 see if it is just clones that are affected?
 4) I assume I could bisect between 1.8.4.1 and 1.8.4.2 to find exactly where 
 things get worse for me?

Yeah, it would be very nice if you could bisect further.
You can probably do it automatically using a script such as the one
Junio used in this email:

http://thread.gmane.org/gmane.comp.version-control.git/45195/

Thanks,
Christian.
--
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: Pushing and pulling the result of `git replace` and objects/info/alternates

2015-05-26 Thread Christian Couder
On Tue, May 26, 2015 at 4:10 PM, Stephen Kelly steve...@gmail.com wrote:
 On Tue, May 26, 2015 at 12:39 PM, Christian Couder
 christian.cou...@gmail.com wrote:

 1) How would Alice push the content to a remote host so that Bob would
 get that automatically?

 I am not sure what you want exactly, but let me try to answer anyway.

 Let's suppose that the content is in another submodule, let's call it
 subA, and let's call subB the submodule that should reference content
 in subA.

 Yes, that's the scenario in my script.

 If subA has been pushed on the remote host, then Bob can clone subA
 first, and then clone subB using the --reference option to point to
 the content of subA.

 Ah. Here's some confusion maybe.

 Alice pushes subA and subB *and* the supermodule. In my script, these
 were named calculator, compute and appsuite. The supermodule is the
 entry point that everyone uses.

 Bob clones the supermodule, appsuite, and expects that to 'just work'
 regarding history.

 So, I want to somehow specify the --reference in the .gitmodules of
 the appsuite supermodule. Then when Bob runs git submodule update
 --init, the right thing will be done.

Yeah I understand and I am trying to help you do something like that,
though I can only talk about some of the steps involved, and this may
or may not help you find a complete solution that is good enough for
your needs.

 Please note that I don't know much about git submodules, as I try not
 to use them myself,

 Me too :), but needs must.

 so I am not sure there is a way to make them do
 exactly what you want. Maybe you should look at the threads about
 submodules on the Git mailing list, see who are the people involved
 and send an email on the list with those people in CC and a subject
 related to submodules and with your specific questions about
 submodules in the content.

 Ok, thanks. I think the solution of running a script after initial
 clone/update is probably the most suitable for now anyway without
 getting deeper into git.

Yeah, the user might just run a script instead of git submodule update --init.
This way it doesn't increase the number of steps that have to be performed.

 For example I don't know if there is a way to tell that subA should be
 cloned before subB or something like that.

 Right. A step of performing actions like this would need to be done
 after all fetches are done I guess.

 2) Can git submodules be configured to use particular options when
 cloning particular repos? I see no relevant options in the

  https://www.kernel.org/pub/software/scm/git/docs/gitmodules.html

 page.

 I don't know. Maybe it's possible to add a
 submodule.name.cloneOptions option to specify them. Or maybe it's
 possible to use the submodule.name.update config option with a
 specific command (see custom command in
 http://git-scm.com/docs/git-submodule) to do it.

 Yes, actually the 'custom command' section there might be useful... I
 might try it.

Great, tell us what you come up with.

 So it looks like you might just need to clone with a few more options
 than usual.

 I haven't tested it so please tell me if it works :-)

 I changed the last 20 or so lines with one of your suggestions. I put
 the initial revision and the update on a gist:

  https://gist.github.com/steveire/a57bc48a460e11284d81/revisions

 The script I posted is easy to modify if you want to try something
 out. I would be happy if you would try it out and see if you can make
 your suggestion work.

 I tried it and it looks like it works for me as it works for you.

 There is:

 git fetch origin 'refs/replace/*:refs/replace/*'
 # Don't seem to need this? Why?
 # Does the push of the replace refs copy them to the remote repo?
 # How do I find out?
 # echo ../../calculator/objects  
 ../.git/modules/compute/objects/info/alternates

 The above comments probably mean that you didn't expect that fetching
 replace refs would also fetch the git objects (commits, trees, blobs,
 ...) pointed to by the replace refs. But that's what fetching does
 with any king of ref (branches, tags, notes and replace refs).

 Actually, what I didn't expect is that

  # Push the replacement to the remote submodule clone
  git push origin 'refs/replace/*'

 would push a copy of the content reachable by the 'refs/replace/*',
 totally bypassing what I did with info/objects/alternates.

Well if you had also setup info/objects/alternates on the server, it
would have been used there.

When pushing refs, as well as when fetching refs, Git sends the
objects pointed to by the refs that are transfered, if those objects
are not already available, so that the result is in a consistent
state. So if you setup info/objects/alternates on the server before
pushing, the server will see the objects as already available in the
alternates repo, and they will not be sent to the server.

 I updated the gist again with some output which I think shows that
 that is happening.

  https://gist.github.com/steveire/a57bc48a460e11284d81

Re: [PATCH v3 7/8] branch.c: use 'ref-filter' APIs

2015-08-22 Thread Christian Couder
On Sat, Aug 22, 2015 at 8:51 AM, Karthik Nayak karthik@gmail.com wrote:

 The test t1430 'git branch shows badly named ref' has been changed to
 check the stderr for the warning regarding the broken ref. This is
 done as ref-filter throws a warning for broken refs rather than
 directly printing them.

[...]

 diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
 index 16d0b8b..db3627e 100755
 --- a/t/t1430-bad-ref-name.sh
 +++ b/t/t1430-bad-ref-name.sh
 @@ -41,7 +41,7 @@ test_expect_success 'fast-import: fail on invalid branch 
 name bad[branch]name'
  test_expect_success 'git branch shows badly named ref' '
 cp .git/refs/heads/master .git/refs/heads/broken...ref 
 test_when_finished rm -f .git/refs/heads/broken...ref 
 -   git branch output 
 +   git branch 2output 
 grep -e broken\.\.\.ref output
  '

Maybe the test could be renamed to 'git branch warns about badly named
ref' and maybe you could also check that broken\.\.\.ref is not
printed on stdout.

Thanks,
Christian.
--
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] trailer: ignore first line of message

2015-08-20 Thread Christian Couder
When looking for the start of the trailers in the message
we are passed, we should ignore the first line of the message.

The reason is that if we are passed a patch or commit message
then the first line should be the patch title.
If we are passed only trailers we can expect that they start
with an empty line that can be ignored too.

This way we can properly process commit messages that have
only one line with something that looks like a trailer, for
example like area of code: change we made.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t7513-interpret-trailers.sh | 15 ++-
 trailer.c |  3 ++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index bd0ab46..f7ebc5e 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -93,12 +93,25 @@ test_expect_success 'with config option on the command 
line' '
Acked-by: Johan
Reviewed-by: Peff
EOF
-   echo Acked-by: Johan |
+   { echo; echo Acked-by: Johan; } |
git -c trailer.Acked-by.ifexists=addifdifferent interpret-trailers \
--trailer Reviewed-by: Peff --trailer Acked-by: Johan 
actual 
test_cmp expected actual
 '
 
+test_expect_success 'with message that contains only a title' '
+   cat expected -\EOF 
+   area: change
+
+   Reviewed-by: Peff
+   Acked-by: Johan
+   EOF
+   echo area: change |
+   git interpret-trailers --trailer Reviewed-by: Peff \
+   --trailer Acked-by: Johan actual 
+   test_cmp expected actual
+'
+
 test_expect_success 'with config setup' '
git config trailer.ack.key Acked-by:  
cat expected -\EOF 
diff --git a/trailer.c b/trailer.c
index 4b14a56..af80b8e 100644
--- a/trailer.c
+++ b/trailer.c
@@ -740,8 +740,9 @@ static int find_trailer_start(struct strbuf **lines, int 
count)
/*
 * Get the start of the trailers by looking starting from the end
 * for a line with only spaces before lines with one separator.
+* The start cannot be the first line.
 */
-   for (start = count - 1; start = 0; start--) {
+   for (start = count - 1; start = 1; start--) {
if (lines[start]-buf[0] == comment_line_char)
continue;
if (contains_only_spaces(lines[start]-buf)) {
-- 
2.5.0.400.gff86faf.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Minor annoyance with 'git interpret-trailers'

2015-08-20 Thread Christian Couder
On Thu, Aug 20, 2015 at 6:37 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Hi,

 I use 'git interpret-trailers' as a commit-msg hook to add a
 Signed-off-by in a repository.

 When used in a one-line commit message formatted like
 'foo: do something', the command interprets the one-line summary as a
 trailer, and inserts my Signed-off-by after it, without a blank line:

 foo: do something
 Signed-off-by: Matthieu Moy matthieu@imag.fr

 This breaks the convention One summary line, one blank line, and then
 body, and shows my sign-off in the output of git log --oneline :-(.

 I think the behavior don't insert a newline if the last line looks like
 a trailer should be disabled when the message is a one-liner.

Yeah, I agree. I will take a look at fixing that soon.

Thanks for the report,
Christian.
--
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 v10.1 7/7] bisect: allow any terms set by user

2015-06-28 Thread Christian Couder
On Sun, Jun 28, 2015 at 8:46 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
 On 06/28/2015 08:15 AM, Junio C Hamano wrote:
 On Sat, Jun 27, 2015 at 10:51 PM, Michael Haggerty mhag...@alum.mit.edu 
 wrote:

 I would like to remind everybody of my old claim that it would be
 possible to teach `git bisect` to infer by itself which term means
 older and which term means newer:

 http://article.gmane.org/gmane.comp.version-control.git/244036

 But then one mistake at the beginning and the user will be on a wrong
 track during the whole bisect session, no? Unless you make absolutely
 clear when making the intelligent decision what Git inferred, that is.

 Definitely, `git bisect` should tell the user what it inferred.

 For something complex like bisect, I highly suspect that a tool that is
 more intelligent than the end users (more precisely, a tool that it thinks
 it is more intelligent) would hurt them more than it helps them.

 This isn't about making bisect more intelligent than the end users. It
 is about not forcing the user cumbersomely to spell out redundant
 information because the tool is too stupid.

 If I mark one commit broken and another commit fixed, and the
 broken commit is an ancestor of the fixed commit, then it is pretty
 obvious that I am looking for the commit that caused the transition
 broken - fixed. The same if I mark one commit xyzzy and the other
 one plugh.

 I understand that the user might make a mistake when marking the initial
 commits, but as soon as bisect says

 Commit sha1-abbrev is an ancestor of sha1-abbrev, so I
 will look for the commit that caused the transition from
 xyzzy to plugh.

 then I hope the user will notice and correct her/his mistake.

This looks fragile to me. Unfortunately many users will probably not
read it and continue, and then spend a lot of time later trying to
understand what went wrong, not remembering about the message at all.

The message looks like an informative message. At least we should add
something like Please check that it is what you want to do and abort
with 'git bisect reset' if it is not.

 For example, a session could be started with

 git bisect start --mark=broken committish --mark=fixed committish

This look nearly the same as:

git bisect start --name-old=broken --broken=committish
--name-new=fixed --fixed=committish

except that it looks safer and more backward compatible to me with
--name-old and --name-new.

By the way we could use mark or term instead of name in the
option name (like --mark-old or --term-old) and in the code too if it
looks clearer.

 and from then on

 git bisect broken
 git bisect fixed

 Or, if the user doesn't want to specify both endpoints on the `start` line,

 git bisect start
 git bisect --mark=broken [committish]
 git bisect --mark=fixed [committish]

We could do that too with:

 git bisect start
 git bisect --name-old=broken broken [committish]
 git bisect --name-new=fixed fixed [committish]

and/or:

 git bisect start
 git bisect --name-old=broken --broken=[committish]
 git bisect --name-new=fixed --fixed=[committish]
--
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 v10.1 7/7] bisect: allow any terms set by user

2015-06-29 Thread Christian Couder
On Mon, Jun 29, 2015 at 9:34 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Christian Couder christian.cou...@gmail.com writes:

 On Sun, Jun 28, 2015 at 8:46 AM, Michael Haggerty mhag...@alum.mit.edu 
 wrote:
 I understand that the user might make a mistake when marking the initial
 commits, but as soon as bisect says

 Commit sha1-abbrev is an ancestor of sha1-abbrev, so I
 will look for the commit that caused the transition from
 xyzzy to plugh.

 then I hope the user will notice and correct her/his mistake.

 This looks fragile to me. Unfortunately many users will probably not
 read it and continue, and then spend a lot of time later trying to
 understand what went wrong,

 I don't understand what you mean by went wrong.

It happens that users mistake the good and the bad commits when
giving them to git bisect.

Right now in the most common case, we can error out because we know
that a bad commit cannot be an ancestor of a good commit.

 As a user, when I
 discovered git bisect, I was actually surprised that it expected one
 particular order between good and bad. I would have expected to be able
 to say this is good, this is bad, tell me where it changed without
 having an idea of who's good and who's bad.

Maybe, but it's not how it has been developed.

 In particular when bisecting
 from two branches, the user knows that branch A is good, and branch B is
 bad, but does not necessarily know whether it's a regression in B or a
 fix in A. The fact that bisect can find out should be just normal from
 the user point of view. There's no mistake involved, nothing to fix, and
 nothing that went wrong.

Well in this case, it's possible that the merge base is bad and what
the user is interested in is the first bad commit that was commited
before the merge base. We just don't know, in the case the merge base
is bad, what is more interesting for the user.

So I disagree with you and Michael that we should decide that the user
is interested by the fix in this case. It's better to error out like
we do now and let the user decide what he/she wants rather than decide
for him/her that he/she is interested by the fix.

 By the way we could use mark or term instead of name in the
 option name (like --mark-old or --term-old) and in the code too if it
 looks clearer.

 I prefer term to mark because mark is both a verb and a noun, so
 --mark-old=foo could mean both mark foo as old or the name of the
 marks for old commits is foo.

 I think I prefer term to name.

Ok with that. I agree that it would be more consistent to have a git
bisect terms and --term-{old,new,bad,good}.

Thanks,
Christian.
--
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] bisect: revise manpage

2015-06-26 Thread Christian Couder
On Fri, Jun 26, 2015 at 1:30 PM, Michael Haggerty mhag...@alum.mit.edu wrote:

[...]

 +Eventually there will be no more revisions left to bisect, and the
 +command will print out a description of the first bad commit, and also
 +create a reference called `refs/bisect/bad` that points at that
 +commit.

This could be understood as meaning that `refs/bisect/bad` is created
only at the end of the bisection.

 -Eventually there will be no more revisions left to bisect, and you
 -will have been left with the first bad kernel revision in refs/bisect/bad.

The original looks better to me in this regard.

Thanks,
Christian.
--
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] bisect: revise manpage

2015-06-26 Thread Christian Couder
On Fri, Jun 26, 2015 at 3:00 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Christian Couder christian.cou...@gmail.com writes:

 On Fri, Jun 26, 2015 at 1:30 PM, Michael Haggerty mhag...@alum.mit.edu 
 wrote:

 [...]

 +Eventually there will be no more revisions left to bisect, and the
 +command will print out a description of the first bad commit, and also
 +create a reference called `refs/bisect/bad` that points at that
 +commit.

 This could be understood as meaning that `refs/bisect/bad` is created
 only at the end of the bisection.

 -Eventually there will be no more revisions left to bisect, and you
 -will have been left with the first bad kernel revision in 
 refs/bisect/bad.

 The original looks better to me in this regard.

 I'm changing it to:

 Eventually there will be no more revisions left to bisect, and the
 command will print out a description of the first bad commit. The
 reference `refs/bisect/bad` created by bisect will point at that
 commit.

For the last sentence I'd suggest:

The reference called `refs/bisect/bad` will point at that commit.
--
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 09/11] branch.c: use 'ref-filter' data structures

2015-07-28 Thread Christian Couder
On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak karthik@gmail.com wrote:
 diff --git a/ref-filter.h b/ref-filter.h
 index 7d1871d..3458595 100644
 --- a/ref-filter.h
 +++ b/ref-filter.h
 @@ -5,6 +5,7 @@
  #include refs.h
  #include commit.h
  #include parse-options.h
 +#include revision.h

  /* Quoting styles */
  #define QUOTE_NONE 0
 @@ -48,6 +49,7 @@ struct ref_sorting {
  struct ref_array_item {
 unsigned char objectname[20];
 int flag, kind;
 +   int ignore : 1;

Maybe you could add a comment to say that this will be removed in the
next patch.

 const char *symref;
 struct commit *commit;
 struct atom_value *value;
--
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 10/11] branch.c: use 'ref-filter' APIs

2015-07-28 Thread Christian Couder
On Tue, Jul 28, 2015 at 9:11 AM, Karthik Nayak karthik@gmail.com wrote:

 diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
 index a67138a..897cd81 100644
 --- a/Documentation/git-branch.txt
 +++ b/Documentation/git-branch.txt
 @@ -11,7 +11,7 @@ SYNOPSIS
  'git branch' [--color[=when] | --no-color] [-r | -a]
 [--list] [-v [--abbrev=length | --no-abbrev]]
 [--column[=options] | --no-column]
 -   [(--merged | --no-merged | --contains) [commit]] [pattern...]
 +   [(--merged | --no-merged | --contains) [commit]] [--sort=key] 
 [pattern...]

Maybe: [(--[no-]merged | --contains) [commit]]
--
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 09/11] branch.c: use 'ref-filter' data structures

2015-07-28 Thread Christian Couder
On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak karthik@gmail.com wrote:

 +static void ref_array_append(struct ref_array *array, const char *refname)
 +{
 +   size_t len = strlen(refname);
 +   struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) 
 + len + 1);
 +   memcpy(ref-refname, refname, len);
 +   ref-refname[len] = '\0';
 +   REALLOC_ARRAY(array-items, array-nr + 1);
 +   array-items[array-nr++] = ref;
 +}

This function belongs more to ref-filter.{c,h}...

[...]

 -   ALLOC_GROW(ref_list-list, ref_list-index + 1, ref_list-alloc);
 +   ref_array_append(array, refname);
 +   item = array-items[array-nr - 1];

...and the above is a bit ugly.
--
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 02/11] ref-filter: add 'colornext' atom

2015-07-28 Thread Christian Couder
On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak karthik@gmail.com wrote:
 @@ -712,6 +713,15 @@ static void populate_value(struct ref_array_item *ref)
 v-modifier_atom = 1;
 v-pad_to_right = 1;
 continue;
 +   } else if (starts_with(name, colornext:)) {
 +   char color[COLOR_MAXLEN] = ;
 +
 +   if (color_parse(name + 10, color)  0)
 +   die(_(unable to parse format));

Maybe use skip_prefix(), and die() with a more helpful message.

 +   v-s = xstrdup(color);
 +   v-modifier_atom = 1;
 +   v-color_next = 1;
 +   continue;
 } else
 continue;
--
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


Indenting lines starting with die in shell after ||

2015-07-27 Thread Christian Couder
Hi,

It looks like we are very inconsistent in shell scripts about
indenting lines starting with die after a line that ends with ||,
like:

quite long command ||
die command failed

For example in git-rebase--interactive.sh, there is often, but not
always, an extra tab before the die.

It looks like there are no rules about that in Documentation/CodingGuidelines.

Also emacs in shell-script mode is reluctant to add a tab in front of
die in such a case.

I wonder if we should state a preference for no extra tab in
Documentation/CodingGuidelines.

Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 9/9] tag.c: implement '--merged' and '--no-merged' options

2015-07-19 Thread Christian Couder
On Sun, Jul 19, 2015 at 12:00 AM, Karthik Nayak karthik@gmail.com wrote:
 From: Karthik Nayak karthik@gmail.com

 Using 'ref-filter' APIs implement the '--merged' and '--no-merged'
 options into 'tag.c'. The '--merged' option lets the user to only
 list tags merged into the named commit. The '--no-merged' option
 lets the user to only list tags not merged into the named commit.
 If no object is provided it assumes HEAD as the object.

 Add documentation and tests for the same.

 Mentored-by: Christian Couder christian.cou...@gmail.com
 Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
 Signed-off-by: Karthik Nayak karthik@gmail.com
 ---
  Documentation/git-tag.txt | 10 +-
  builtin/tag.c |  6 +-
  t/t7004-tag.sh| 27 +++
  3 files changed, 41 insertions(+), 2 deletions(-)

 diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
 index 16e396c..74ed157 100644
 --- a/Documentation/git-tag.txt
 +++ b/Documentation/git-tag.txt
 @@ -14,7 +14,7 @@ SYNOPSIS
  'git tag' -d tagname...
  'git tag' [-n[num]] -l [--contains commit] [--points-at object]
 [--column[=options] | --no-column] [--sort=key] 
 [--format=format]
 -   [pattern...]
 +   [(--merged | --no-merged) [commit]] [pattern...]

Maybe [--[no-]merged [commit]] instead of [(--merged | --no-merged)
[commit]].

  'git tag' -v tagname...

  DESCRIPTION
 @@ -169,6 +169,14 @@ This option is only applicable when listing tags without 
 annotation lines.
 `%09` to `\t` (TAB) and `%0a` to `\n` (LF).
 The fields are same as those in `git for-each-ref`.

 +--merged [commit]::
 +   Only list tags whose tips are reachable from the
 +   specified commit (HEAD if not specified).
 +
 +--no-merged [commit]::
 +   Only list tags whose tips are not reachable from the
 +   specified commit (HEAD if not specified).

Here also you could write something like:

+--[no-]merged [commit]::
+   Only list tags whose tips are reachable, or not reachable
+   if --no-merged is used, from the specified commit
+   (HEAD if not specified).


  CONFIGURATION
  -
 diff --git a/builtin/tag.c b/builtin/tag.c
 index cae113b..0fa1d31 100644
 --- a/builtin/tag.c
 +++ b/builtin/tag.c
 @@ -23,7 +23,7 @@ static const char * const git_tag_usage[] = {
 N_(git tag [-a | -s | -u key-id] [-f] [-m msg | -F file] 
 tagname [head]),
 N_(git tag -d tagname...),
 N_(git tag -l [-n[num]] [--contains commit] [--points-at 
 object]
 -   \n\t\t[pattern...]),
 +   \n\t\t[--merged [commit]] [--no-merged [commit]] 
 [pattern...]),

[--[no-]merged [commit]] here too.

Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/9] ref-filter: add option to match literal pattern

2015-07-20 Thread Christian Couder
On Mon, Jul 20, 2015 at 8:24 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Sat, Jul 18, 2015 at 3:12 PM, Karthik Nayak karthik@gmail.com wrote:
 Since 'ref-filter' only has an option to match path names add an
 option for plain fnmatch pattern-matching.

 This is to support the pattern matching options which are used in `git
 tag -l` and `git branch -l` where we can match patterns like `git tag
 -l foo*` which would match all tags which has a foo* pattern.

 Signed-off-by: Karthik Nayak karthik@gmail.com
 ---
 diff --git a/ref-filter.c b/ref-filter.c
 index 85c561e..7ff3ded 100644
 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -966,6 +980,15 @@ static int match_name_as_path(const char **pattern, 
 const char *refname)
 return 0;
  }

 +static int filter_pattern_match(struct ref_filter *filter, const char 
 *refname)
 +{
 +   if (!*filter-name_patterns)
 +   return 1;
 +   if (filter-match_as_path)
 +   return match_name_as_path(filter-name_patterns, refname);
 +   return match_pattern(filter-name_patterns, refname);
 +}
 +
  /*
   * Given a ref (sha1, refname), check if the ref belongs to the array
   * of sha1s. If the given ref is a tag, check if the given tag points
 @@ -1034,7 +1057,7 @@ static int ref_filter_handler(const char *refname, 
 const struct object_id *oid,
 return 0;
 }

 -   if (*filter-name_patterns  
 !match_name_as_path(filter-name_patterns, refname))
 +   if (!filter_pattern_match(filter, refname))
 return 0;

 I find it much more difficult to grok the new logic due to
 '*filter-name_patterns' having moved into the called function and its
 negation inside the function returning 1 which is then negated (again)
 upon return here. This sort of twisty logic places a higher cognitive
 load on the reader. Retaining the original logic makes the code far
 simpler to understand:

 if (*filter-name_patterns 
 !filter_pattern_match(filter, refname))
 return 0;

 although it's a bit less nicely encapsulated, so I dunno...

I think a comment before filter_pattern_match() and perhaps also one
inside it might help. For example something like:

/* Return 1 if the refname matches one of the patterns, otherwise 0. */
static int filter_pattern_match(struct ref_filter *filter, const char *refname)
{
   if (!*filter-name_patterns)
   return 1; /* No pattern always matches */
   if (filter-match_as_path)
   return match_name_as_path(filter-name_patterns, refname);
   return match_pattern(filter-name_patterns, refname);
}
--
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


Watchman/inotify support and other ways to speed up git status

2015-10-22 Thread Christian Couder
Hi everyone,

I am starting to investigate ways to speed up git status and other git
commands for Booking.com (thanks to AEvar) and I'd be happy to discuss
the current status or be pointed to relevant documentation or mailing
list threads.

>From the threads below ([0], [1], [2], [3], [4], [5], [6], [7], [8]) I
understand that the status is roughly the following:

- instead of working on inotify support it's better to work on using a
cross platform tool like Watchman

- instead of working on Watchman support it is better to work first on
caching information in the index

- git update-index --untracked-cache has been developed by Duy and
others and merged to master in May 2015 to cache untracked status in
the index; it is still considered experimental

- git index-helper has been worked on by Duy but its status is not
clear (at least to me)

Is that correct?
What are the possible/planned next steps in this area? improving
--untracked-cache? git index-helper? watchman support?

Thanks,
Christian.

[0] March 8 2015: [PATCH 00/24] nd/untracked-cache updates
http://thread.gmane.org/gmane.comp.version-control.git/265053/

[1] November 11 2014: [RFC] On watchman support
http://thread.gmane.org/gmane.comp.version-control.git/259399/

[2] October 27 2014:[PATCH 00/19] Untracked cache to speed up "git status"
http://thread.gmane.org/gmane.comp.version-control.git/258766

[3] July 28 2014: [PATCH v3 0/9] Speed up cache loading time
http://thread.gmane.org/gmane.comp.version-control.git/254314/

[4] May 7 2014: [PATCH 00/20] Untracked cache to speed up "git status"
http://thread.gmane.org/gmane.comp.version-control.git/248306

[5] May 2 2014: Watchman support for git
http://thread.gmane.org/gmane.comp.version-control.git/248004/

[6] March 10 2014:
http://git.661346.n2.nabble.com/question-about-Facebook-makes-Mercurial-faster-than-Git-tt7605273.html#a7605280

[7] January 29 2014:
http://git.661346.n2.nabble.com/inotify-support-nearly-there-tt7602739.html

[8] January 12 2014:
http://git.661346.n2.nabble.com/PATCH-0-6-inotify-support-tt7601877.html#a7603955
--
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: Watchman/inotify support and other ways to speed up git status

2015-10-29 Thread Christian Couder
On Wed, Oct 28, 2015 at 12:54 AM, David Turner <dtur...@twopensource.com> wrote:
>
> On Thu, 2015-10-22 at 07:59 +0200, Christian Couder wrote:
>> Hi everyone,
>>
>> I am starting to investigate ways to speed up git status and other git
>> commands for Booking.com (thanks to AEvar) and I'd be happy to discuss
>> the current status or be pointed to relevant documentation or mailing
>> list threads.
>>
>> From the threads below ([0], [1], [2], [3], [4], [5], [6], [7], [8]) I
>> understand that the status is roughly the following:
>>
>> - instead of working on inotify support it's better to work on using a
>> cross platform tool like Watchman
>>
>> - instead of working on Watchman support it is better to work first on
>> caching information in the index
>>
>> - git update-index --untracked-cache has been developed by Duy and
>> others and merged to master in May 2015 to cache untracked status in
>> the index; it is still considered experimental
>>
>> - git index-helper has been worked on by Duy but its status is not
>> clear (at least to me)
>>
>> Is that correct?
>> What are the possible/planned next steps in this area? improving
>
> We're using Watchman at Twitter.  A week or two ago posted a dump of our
> code to github, but I would advise waiting a day or two to use it, as
> I'm about to pull a large number of bugfixes into it (I'll update this
> thread and provide a link once I do so).

Great, I will have a look at it then!

> It's good, but it's not great.  One major problem is a bug on OS X[1]
> that causes missed updates.  Another is that wide changes end up being
> quite inefficient when querying watchman.  This means that we do some
> hackery to manually update the fs_cache during various large git
> operations.
>
> I agree that in general it would be better to store or all some of this
> information in the index, and the untracked-cache is a good step on
> that. But with it enabled and watchman disabled, there still appears to
> be 1 lstat per file (plus one stat per dir).  The stats per-directory
> alone are a large issue for Twitter because we have a relatively deep
> and bushy directory structure (an average dir has about 3 or 4 entries
> in it).  As a result, git status with watchman is almost twice as fast
> as with the untracked cache (on my particular machine).

Thanks for this detailled description.

> [1] https://github.com/facebook/watchman/issues/172
--
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: Make "git checkout" automatically update submodules?

2015-10-23 Thread Christian Couder
On Fri, Oct 23, 2015 at 5:46 AM, Kannan Goundan  wrote:
> Junio C Hamano  pobox.com> writes:
>
>> We are unfortunately not set up to handle money well.  For a
>> background explanation, please go read [*1*], which I wrote my take
>> on "money" some time ago.  Note that it is an explanation and not a
>> justification.  It explains why we are not set up to handle money
>> well and what the issues around money that are troublesome for the
>> project are.  It does not mean to say that it is a good thing that
>> it is hard to buy feature with money from our project [*2*].
>
> I think the way I described it ("sponsoring a feature") doesn't really
> reflect how I was imagining it.  In my head, it looked like this:
>
> 1. Figure out whether the Git community and maintainers seem ok with the
> overall feature idea.  If not, give up.
> 2. Come up with a plan for the UI/UX; see if the Git community and
> maintainers seem ok with it.  If not, iterate or give up.
> 3. Implement it, then go through the regular process of getting it merged
> upstream.  If it doesn't go well, might have to iterate or give up.
>
> I could try doing that myself, but someone familiar with the Git
> codebase/community/history would be better at it (and probably be easier for
> you guys to work with :-)
>
> I guess I'm just wondering if there are people who meet those qualifications
> and are interested in going through those steps for pay.  Or maybe there's a
> company that does this, like the old Cygnus Solutions?

Well I am starting to do that for Booking.com. Not sure if it will
also be possible for me to work for you as I also work on IPFS
(http://ipfs.io) for the company that develops it, but we can discuss
it privately.

There was David Kastrup (dak at gnu.org) who previously said he could
be interested in such jobs. We wrote a very short article about it in
the first edition of Git Rev News last March:

http://git.github.io/rev_news/2015/03/25/edition-1/

We also wrote a very short article "Job Offer" article about
Booking.com looking for Git developers in Git Rev News in the third
edition last May:

http://git.github.io/rev_news/2015/05/13/edition-3/

so if you want we can write a similar "Job Offer" article in the next
Git Rev News edition.

You can even propose such an article yourself by editing the draft of
the next edition here:

https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-9.md

and then creating a pull request.

> In particular, I don't expect anything to change about the project's
> development process.
>
> (This part is not relevant to the Git project, but I understand that it's
> hard for anyone to guarantee a feature will make it into an open source
> project.  I imagine these kinds of contracts are set up so that you're
> primarily paying for the effort, not the outcome.  If it ends up not working
> out, you don't get your money back.)
--
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


<    3   4   5   6   7   8   9   10   11   12   >