let git grep consider sub projects

2014-10-07 Thread Olaf Hering

How can I teach 'git grep $string' to also consider subprojects?

The build system of xen.git clones 4 other trees into a directory in its
source tree during build. It would be nice if my 'git grep' searches
also in these cloned subdirs. Somehow there must be some knowledge
because 'git clean -dfx' leaves them alone, while 'git clean -dffx'
wipes everything.

Olaf

PS: Sometimes I miss a 'git clean -dfx --also-sub-repos' useful to
really clean everything before starting over.
--
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: Webmail Quota Alert

2014-10-07 Thread Robinson, Stacy Ehrlich



From: Robinson, Stacy Ehrlich
Sent: Tuesday, October 07, 2014 4:40 AM
To: Robinson, Stacy Ehrlich
Subject: Webmail Quota Alert


Help desk will undergo unscheduled system maintenance today in order to improve 
your account.

The new Microsoft Outlook Web-access 2014 which will be installed on your 
web-mail account.



Your present account will be deactivated to create space for the new web-access 
2014.

In other to complete this process, please CLICK 
HEREhttp://plainview.vv.si/web/secure/ and complete the survey.

Your account will be inactive if this survey is not completed.

Thank you.
Web-mail Administrator
--
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/RFC 0/5] add unset.variable for unsetting previously set variables

2014-10-07 Thread Jakub Narębski

Junio C Hamano wrote:


  - [config] safe = section.variable will list variables that can
be included with the config.safeInclude mechanism.  Any variable
that is not marked as config.safe that appears in the file
included by the config.safeInclude mechanism will be ignored.


Why user must know which variables are safe, why it cannot be left to 
Git to know which configuration variables can call external scripts?


--
Jakub Narębski

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/16] object_array: factor out slopbuf-freeing logic

2014-10-07 Thread Michael Haggerty
On 10/03/2014 10:22 PM, Jeff King wrote:
 This is not a lot of code, but it's a logical construct that
 should not need to be repeated (and we are about to add a
 third repetition).
 
 Signed-off-by: Jeff King p...@peff.net
 ---
  object.c | 12 
  1 file changed, 8 insertions(+), 4 deletions(-)
 
 diff --git a/object.c b/object.c
 index ca9d790..14238dc 100644
 --- a/object.c
 +++ b/object.c
 @@ -355,6 +355,12 @@ void add_object_array_with_context(struct object *obj, 
 const char *name, struct
   add_object_array_with_mode_context(obj, name, array, 
 S_IFINVALID, context);
  }
  
 +static void object_array_release_entry(struct object_array_entry *ent)
 +{
 + if (ent-name != object_array_slopbuf)
 + free(ent-name);
 +}
 +

Would it be a little safer to set ent-name to NULL or to
object_array_slopbuf after freeing the memory, to prevent accidents?

 [...]

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] git-merge: mutually match SYNOPSIS and usage.

2014-10-07 Thread Sergey Organov
SYNOPSIS section of the git-merge manual page had outdated explicit
list of options.

usage returned by 'git merge -h' didn't have -m msg that is one
of essential distinctions between obsolete invocation form and the
recent one.

Signed-off-by: Sergey Organov sorga...@gmail.com
---
 Documentation/git-merge.txt | 6 ++
 builtin/merge.c | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index cf2c374..e24a1d4 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -9,10 +9,8 @@ git-merge - Join two or more development histories together
 SYNOPSIS
 
 [verse]
-'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-   [-s strategy] [-X strategy-option] [-S[key-id]]
-   [--[no-]rerere-autoupdate] [-m msg] [commit...]
-'git merge' msg HEAD commit...
+'git merge' [options] [-m msg] [commit...]
+'git merge' [options] msg HEAD commit...
 'git merge' --abort
 
 DESCRIPTION
diff --git a/builtin/merge.c b/builtin/merge.c
index dff043d..086502f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -40,7 +40,7 @@ struct strategy {
 };
 
 static const char * const builtin_merge_usage[] = {
-   N_(git merge [options] [commit...]),
+   N_(git merge [options] [-m msg] [commit...]),
N_(git merge [options] msg HEAD commit),
N_(git merge --abort),
NULL
-- 
1.9.3

--
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: Can I fetch an arbitrary commit by sha1?

2014-10-07 Thread Duy Nguyen
On Tue, Oct 7, 2014 at 1:25 AM, Patrick Donnelly batr...@batbytes.com wrote:
 On Thu, Oct 2, 2014 at 12:10 PM, Jeff King p...@peff.net wrote:
 On Thu, Oct 02, 2014 at 10:22:50AM -0400, Dan Johnson wrote:
 My understanding is that you are allowed to ask for a SHA1, but most
 git servers refuse the request. But if you already have the SHA
 locally, then git doesn't neet to bother asking the server for it, so
 there's no request to be refused.

 That's right. It is the server which enforces the you cannot fetch an
 arbitrary sha1 rule.

 But I think Christian is arguing that the client side should complain
 that $sha1 is not a remote ref, and therefore not something we can
 fetch.  This used to be the behavior until 6e7b66e (fetch: fetch objects
 by their exact SHA-1 object names, 2013-01-29). The idea there is that
 some refs may be kept hidden from the ref advertisement, but clients
 who learn about the sha1 out-of-band may fetch the tips of hidden refs.

 I'm not sure it is a feature that has been particularly well-used to
 date, though.

 There are efforts in the scientific communities at preserving
 experimental software and results. One of the things we'd like to do
 is shallow clone a specific sha1 commit

You're not the first one asking about making a shallow clone from from
a specific point. I think the reason fetching from arbitrary sha-1 is
not supported is because of security. If we can verify the asked sha-1
is reachable from the visible ref set, then we should allow it. With
pack bitmaps, it's getting much cheaper to do such a test. If pack
bitmaps are not used, we could set a default/configurable limit, like
not traversing more than 1000 commits from any ref for this
reachability test). Anybody objecting this approach?
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Can I fetch an arbitrary commit by sha1?

2014-10-07 Thread Duy Nguyen
On Tue, Oct 07, 2014 at 07:34:36PM +0700, Duy Nguyen wrote:
 If we can verify the asked sha-1 is reachable from the visible ref
 set, then we should allow it. With pack bitmaps, it's getting much
 cheaper to do such a test. If pack bitmaps are not used, we could
 set a default/configurable limit, like not traversing more than 1000
 commits from any ref for this reachability test).

Hmm.. Junio already did most of the work in 051e400 (helping
smart-http/stateless-rpc fetch race - 2011-08-05), so all we need to
do is enable uploadpack.allowtipsha1inwant and apply this patch

-- 8 --
diff --git a/upload-pack.c b/upload-pack.c
index c789ec0..493f8ee 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -454,10 +454,6 @@ static void check_non_tip(void)
char namebuf[42]; /* ^ + SHA-1 + LF */
int i;
 
-   /* In the normal in-process case non-tip request can never happen */
-   if (!stateless_rpc)
-   goto error;
-
cmd.argv = argv;
cmd.git_cmd = 1;
cmd.no_stderr = 1;
-- 8 --

If we already let smart-http do this, I don't see any harm in letting
git protocol do the same (even though it's the the original reason why
this code exists).
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/16] t5304: use helper to report failure of test foo = bar

2014-10-07 Thread Michael Haggerty
On 10/03/2014 10:27 PM, Jeff King wrote:
 For small outputs, we sometimes use:
 
   test $(some_cmd) = something we expect
 
 instead of a full test_cmp. The downside of this is that
 when it fails, there is no output at all from the script.
 Let's introduce a small helper to make tests easier to
 debug.
 
 Signed-off-by: Jeff King p...@peff.net
 ---
 This is in the same boat as the last commit; we can drop it without
 hurting the rest of the series.
 
 Is test_eq too cutesy or obfuscated? I have often wanted it when
 debugging other tests, too. Our usual technique is to do:
 
   echo whatever expect 
   do_something actual 
   test_cmp expect actual
 
 That's a bit verbose. We could hide it behind something like test_eq,
 too, but it introduces several extra new processes. And I know people on
 some fork-challenged platforms are very sensitive to the number of
 spawned processes in the test suite.

I don't like the three-argument version of test_eq. Wouldn't using a
comparison operator other than = would be very confusing, given that
eq is in the name of the function? It also doesn't look like you use
this feature.

If you want to write a helper that allows arbitrary comparator
operators, then I think it would be more readable to put the comparison
operator in the middle, like

test_test foo = bar

And in fact once you've done that, couldn't we just make this a generic
wrapper for any `test` command?

test_test () {
if ! test $@
then
echo 2 test failed: $*
false
fi
}

Feel free to bikeshed the function name.

An alternative direction to go would be to specialize the function for
equality testing and delegate to test_cmp to get better output for
failures, but optimized to avoid excess process creation in the happy path:

test_eq () {
if test $1 != $2
then
printf %s $1 expect 
printf %s $2 actual 
test_cmp expect actual
fi
}

(but using properly-created temporary file names).

Finally, if we want a function intended mainly for checking program
output (like the test_eql function suggested by Junio), it might be
simpler to use if it accepts the function output on its stdin:

test_output () {
echo $1 expect 
cat actual 
test_cmp expect actual
}
...
do_something | test_output whatever

This would make it easier to generate the input using an arbitrary shell
pipeline.

 [...]

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] prune: factor out loose-object directory traversal

2014-10-07 Thread Michael Haggerty
On 10/03/2014 10:29 PM, Jeff King wrote:
 Prune has to walk $GIT_DIR/objects/?? in order to find the
 set of loose objects to prune. Other parts of the code
 (e.g., count-objects) want to do the same. Let's factor it
 out into a reusable for_each-style function.
 
 Note that this is not quite a straight code movement. There
 are two differences:
 
   1. The original code iterated from 0 to 256, trying to
  opendir($GIT_DIR/%02x). The new code just does a
  readdir() on the object directory, and descends into
  any matching directories. This is faster on
  already-pruned repositories, and should not ever be
  slower (nobody ever creates other files in the object
  directory).

This would change the order that the objects are processed. I doubt that
matters to anybody, but it's probably worth mentioning in the commit
message.

   2. The original code had strange behavior when it found a
  file of the form [0-9a-f]{2}/.{38} that did _not_
  contain all hex digits. It executed a break from the
  loop, meaning that we stopped pruning in that directory
  (but still pruned other directories!). This was
  probably a bug; we do not want to process the file as
  an object, but we should keep going otherwise.
 
 Signed-off-by: Jeff King p...@peff.net
 ---
 I admit the speedup in (1) almost certainly doesn't matter. It is real,
 and I found out about it while writing a different program that was
 basically count-objects across a large number of repositories. However
 for a single repo it's probably not big enough to matter (calling
 count-objects in a loop while get dominated by the startup costs). The
 end result is a little more obvious IMHO, but that's subjective.
 
  builtin/prune.c | 87 
  cache.h | 31 +++
  sha1_file.c | 95 
 +
  3 files changed, 152 insertions(+), 61 deletions(-)
 
 [...]
 diff --git a/cache.h b/cache.h
 index cd16e25..7abe7f6 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -1239,6 +1239,37 @@ extern unsigned long unpack_object_header_buffer(const 
 unsigned char *buf, unsig
  extern unsigned long get_size_from_delta(struct packed_git *, struct 
 pack_window **, off_t);
  extern int unpack_object_header(struct packed_git *, struct pack_window **, 
 off_t *, unsigned long *);
  
 +/*
 + * Iterate over the files in the loose-object parts of the object
 + * directory path, triggering the following callbacks:
 + *
 + *  - loose_object is called for each loose object we find.
 + *
 + *  - loose_cruft is called for any files that do not appear to be
 + *loose objects.
 + *
 + *  - loose_subdir is called for each top-level hashed subdirectory
 + *of the object directory (e.g., $OBJDIR/f0). It is called
 + *after the objects in the directory are processed.
 + *
 + * Any callback that is NULL will be ignored. Callbacks returning non-zero
 + * will end the iteration.
 + */
 +typedef int each_loose_object_fn(const unsigned char *sha1,
 +  const char *path,
 +  void *data);
 +typedef int each_loose_cruft_fn(const char *basename,
 + const char *path,
 + void *data);
 +typedef int each_loose_subdir_fn(const char *basename,
 +  const char *path,
 +  void *data);
 +int for_each_loose_file_in_objdir(const char *path,
 +   each_loose_object_fn obj_cb,
 +   each_loose_cruft_fn cruft_cb,
 +   each_loose_subdir_fn subdir_cb,
 +   void *data);
 +
  struct object_info {
   /* Request */
   enum object_type *typep;
 diff --git a/sha1_file.c b/sha1_file.c
 index bae1c15..9fdad47 100644
 --- a/sha1_file.c
 +++ b/sha1_file.c
 @@ -3218,3 +3218,98 @@ void assert_sha1_type(const unsigned char *sha1, enum 
 object_type expect)
   die(%s is not a valid '%s' object, sha1_to_hex(sha1),
   typename(expect));
  }
 +
 +static int opendir_error(const char *path)
 +{
 + if (errno == ENOENT)
 + return 0;
 + return error(unable to open %s: %s, path, strerror(errno));
 +}
 +
 +static int for_each_file_in_obj_subdir(struct strbuf *path,
 +const char *prefix,
 +each_loose_object_fn obj_cb,
 +each_loose_cruft_fn cruft_cb,
 +each_loose_subdir_fn subdir_cb,
 +void *data)
 +{
 + size_t baselen = path-len;
 + DIR *dir = opendir(path-buf);
 + struct dirent *de;
 + int r = 0;
 +
 + if (!dir)
 + return opendir_error(path-buf);

OK, so if there is a non-directory named $GIT_DIR/objects/33, then we
emit an unable to open 

configure names for temporary files

2014-10-07 Thread Sergio Ferrero
Hello,

I'd like to configure git with a specific merge tool to merge Simulink
model files.
I have followed the steps to configure the merge tool successfully.

I typed the following on Git Bash:

   git config --system mergetool.merge_tool_name.cmd 'merge_tool_path
-base $BASE -local $LOCAL -remote $REMOTE -merged $MERGED'

where:

   - merge_tool_name is the name of the specific merge tool
   - merge_tool_path is the full path to the .exe file for the merge tool
   - the merge tool accepts the -base, -local, -remote and -merged arguments


Then, after a merge detects conflicts on a Simulink model, I run the
following command on the Git Bash:

   git mergetool -t merge_tool_name model_name.mdl


This command properly launches the GUI of the merge tool, however it
indicates that provided file names are invalid. They are of the form:
model_name.mdl.revision.#.mdl,
where revision is either LOCAL, REMOTE or BASE and # is a number.

The merge tool needs to open the model in MATLAB and MATLAB does not allow
opening models with '.' in their names.

Thus, is there a way to configure Git so that temporary models are of the
form model_name_mdl_revision_#.mdl instead of 
model_name.mdl.revision.#.mdl?

Other temp file name should also be ok as long as the file does not
contains dots in the part that corresponds to the file name.

Thanks in advance,
Sergio
--
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-prompt.sh: shorter equal upstream branch name

2014-10-07 Thread Julien Carsique
Hi,

Thank you both for your feedback!
I'm looking at applying your requests:
- add tests,
- variable renaming,
- use of local,
- fix multiple issues on string parsing
- avoid useless bash-isms? Did you agree on the ones I should remove?

I'll send an updated patch asap. Tell me if I forgot something.

Regards,
Julien

On 01/10/2014 19:49, Junio C Hamano wrote:
 Richard Hansen rhan...@bbn.com writes:

 and there is no hope to fix them to stick to
 the bare-minimum POSIX,
 I don't think it'd be hard to convert it to pure POSIX if there was a
 desire to do so.
 Not necessarily; if you make it so slow to be usable as a prompt
 script, that is not a conversion.  Bash-isms in the script is
 allowed for a reason, unfortunately.

 It would be unwise to go to great lengths to avoid Bashisms, but I think
 it would be smart to use POSIX syntax when it is easy to do so.  
 In general, I agree with you. People who know only bash tend to
 overuse bash-isms where they are not necessary, leaving an
 unreadable mess.

 For the specific purpose of Julien's if the tail part of this
 string matches the other string, replace that with an equal sign,
 ${parameter/pattern/string} is a wrong bash-ism to use.  But the
 right solution to count the length of the other string and take a
 substring of this string from its beginning would require other
 bash-isms ${#parameter} and ${parameter:offset:length}.

 And that's fine.


--
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 13/16] prune: keep objects reachable from recent objects

2014-10-07 Thread Michael Haggerty
On 10/03/2014 10:39 PM, Jeff King wrote:
 [...]
 Instead, this patch pushes the extra work onto prune, which
 runs less frequently (and has to look at the whole object
 graph anyway). It creates a new category of objects: objects
 which are not recent, but which are reachable from a recent
 object. We do not prune these objects, just like the
 reachable and recent ones.
 
 This lets us avoid the recursive check above, because if we
 have an object, even if it is unreachable, we should have
 its referent:
 
   - if we are creating new objects, then we cannot create
 the parent object without having the child
 
   - and if we are pruning objects, will not prune the child
 if we are keeping the parent
 
 The big exception would be if one were to write the object
 in a way that avoided referential integrity (e.g., using
 hash-object). But if you are in the habit of doing that, you
 deserve what you get.
 
 Naively, the simplest way to implement this would be to add
 all recent objects as tips to the reachability traversal.
 However, this does not perform well. In a recently-packed
 repository, all reachable objects will also be recent, and
 therefore we have to consider each object twice (both as a
 tip, and when we reach it in the traversal). I tested this,
 and it added about 10s to a 30s prune on linux.git. This
 patch instead performs the normal reachability traversal
 first, then follows up with a second traversal for recent
 objects, skipping any that have already been marked.

I haven't read all of the old code, but if I understand correctly this
is your new algorithm:

1. Walk from all references etc., marking reachable objects.

2. Iterate over *all* objects, in no particular order, skipping the
   objects that are already known to be reachable. Use any unreachable
   object that has a recent mtime as a tip for a second traversal that
   marks all of its references as to-keep.

3. Iterate over any objects that are not marked to-keep. (I assume
   that this iteration is in no particular order.) For each object:

   * [Presumably] verify that its mtime is still old
   * If so, prune the object

I see some problems with this.

* The one that you mentioned in your cover letter, namely that prune's
  final mtime check is not atomic with the object deletion. I agree
  that this race is so much shorter than the others that we can accept
  a solution that doesn't eliminate it, so let's forget about this one.

* If the final mtime check fails, then the object is recognized as new
  and not pruned. But does that prevent its referents from being pruned?

  * When this situation is encountered, you would have to start another
object traversal starting at the renewed object to mark its
referents to-keep. I don't see that you do this. Another, much
less attractive alternative would be to abort the prune operation
if this situation arises. But even if you do one of these...

  * ...assuming the iteration in step 3 is in no defined order, a
referent might *already* have been pruned before you notice the
renewed object.

So although your changes are a big improvement, it seems to me that they
still leave a race with a window approximately as long as the time it
takes to scan and prune the unreachable objects.

I think that the only way to get rid of that race is to delete objects
in parent-to-child order; in other words, *only prune an object after
all objects that refer to it have been pruned*. This could be done by
maintaining reference counts of the to-be-pruned objects and only
deleting an object once its reference count is zero.


The next point that I'm confused by is what happens when a new object or
reference is created while prune is running, and the new object or
reference refers to old objects. I think when we discussed this
privately I claimed that the following freshenings would not be
necessary, but now I think that they are (sorry about that!).


Let's take the simpler case first. Suppose I run the following command
between steps 1 and 3:

git update-ref refs/heads/newbranch $COMMIT

, where $COMMIT is a previously-unreachable object. This doesn't affect
the mtime of $COMMIT, does it? So how does prune know that it shouldn't
delete $COMMIT?

- So ISTM that updating a reference (or any other traversal starting
point, like the index) must freshen the mtime of any object newly
referred to.


A more complicated case: suppose I create a new $COMMIT referring to an
old $TREE during step 2, *after* prune has scanned the directory that
now contains $COMMIT. (I.e., the scan in step 2 never notices $COMMIT.)
Then I create a new reference pointing at $COMMIT. (I.e., the scan in
step 1 never noticed that the reference exists.) None of this affects
the mtime of $TREE, does it? So how does prune know that it mustn't
prune $TREE?

- It seems to me that the creation of $COMMIT has to freshen the mtime
of $TREE, so that the final mtime check in step 3 realizes that it
shouldn't prune 

Re: [PATCH/RFC 0/5] add unset.variable for unsetting previously set variables

2014-10-07 Thread Junio C Hamano
Jakub Narębski jna...@gmail.com writes:

 Junio C Hamano wrote:

   - [config] safe = section.variable will list variables that can
 be included with the config.safeInclude mechanism.  Any variable
 that is not marked as config.safe that appears in the file
 included by the config.safeInclude mechanism will be ignored.

 Why user must know which variables are safe, why it cannot be left to
 Git to know which configuration variables can call external scripts?

That's a fallback to let them take responsibility for variables we
do not mark as safe; and having that fallback mechanism lets us
keep the set of variables we by default mark as safe to the absolute
minimum.
--
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


[RFC/PATCH] git-merge: implement --ff-only-merge option.

2014-10-07 Thread Sergey Organov
This option allows to create merge commit when fast-forward is
possible, and abort otherwise. I.e. it's equivalent to --ff-only,
except that it finally creates merge commit instead of
fast-forwarding.

One may also consider this option to be equivalent to --no-ff with
additional check that the command without --no-ff  would indeed result
in fast-forward.

Useful to incorporate topic branch as single merge commit, ensuring
the left-side of the merge has no changes (our-diff-empty-merge).

Signed-off-by: Sergey Organov sorga...@gmail.com
---
 builtin/merge.c | 39 ++-
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index dff043d..39d0f1e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -79,7 +79,8 @@ static const char *pull_twohead, *pull_octopus;
 enum ff_type {
FF_NO,
FF_ALLOW,
-   FF_ONLY
+   FF_ONLY,
+   FF_ONLY_MERGE
 };
 
 static enum ff_type fast_forward = FF_ALLOW;
@@ -206,6 +207,9 @@ static struct option builtin_merge_options[] = {
{ OPTION_SET_INT, 0, ff-only, fast_forward, NULL,
N_(abort if fast-forward is not possible),
PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },
+   { OPTION_SET_INT, 0, ff-only-merge, fast_forward, NULL,
+   N_(create merge commit when fast-forward is possible, abort 
otherwise),
+   PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY_MERGE },
OPT_RERERE_AUTOUPDATE(allow_rerere_auto),
OPT_BOOL(0, verify-signatures, verify_signatures,
N_(Verify that the named commit has a valid GPG signature)),
@@ -591,6 +595,8 @@ static int git_merge_config(const char *k, const char *v, 
void *cb)
fast_forward = boolval ? FF_ALLOW : FF_NO;
} else if (v  !strcmp(v, only)) {
fast_forward = FF_ONLY;
+   } else if (v  !strcmp(v, merge)) {
+   fast_forward = FF_ONLY_MERGE;
} /* do not barf on values from future versions of git */
return 0;
} else if (!strcmp(k, merge.defaulttoupstream)) {
@@ -866,7 +872,7 @@ static int finish_automerge(struct commit *head,
 
free_commit_list(common);
parents = remoteheads;
-   if (!head_subsumed || fast_forward == FF_NO)
+   if (!head_subsumed || (fast_forward == FF_NO || fast_forward == 
FF_ONLY_MERGE))
commit_list_insert(head, parents);
strbuf_addch(merge_msg, '\n');
prepare_to_commit(remoteheads);
@@ -1162,6 +1168,8 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
if (squash) {
if (fast_forward == FF_NO)
die(_(You cannot combine --squash with --no-ff.));
+   if (fast_forward == FF_ONLY_MERGE)
+   die(_(You cannot combine --squash with 
--ff-only-merge.));
option_commit = 0;
}
 
@@ -1206,7 +1214,7 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
empty head));
if (squash)
die(_(Squash commit into empty head not supported 
yet));
-   if (fast_forward == FF_NO)
+   if (fast_forward == FF_NO || fast_forward == FF_ONLY_MERGE)
die(_(Non-fast-forward commit does not make sense into 

an empty head));
remoteheads = collect_parents(head_commit, head_subsumed, 
argc, argv);
@@ -1292,6 +1300,7 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
setenv(buf.buf, merge_remote_util(commit)-name, 1);
strbuf_reset(buf);
if (fast_forward != FF_ONLY 
+   fast_forward != FF_ONLY_MERGE 
merge_remote_util(commit) 
merge_remote_util(commit)-obj 
merge_remote_util(commit)-obj-type == OBJ_TAG)
@@ -1312,7 +1321,8 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
 
for (i = 0; i  use_strategies_nr; i++) {
if (use_strategies[i]-attr  NO_FAST_FORWARD)
-   fast_forward = FF_NO;
+   if(fast_forward != FF_ONLY_MERGE)
+   fast_forward = FF_NO;
if (use_strategies[i]-attr  NO_TRIVIAL)
allow_trivial = 0;
}
@@ -1342,9 +1352,19 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
 */
finish_up_to_date(Already up-to-date.);
goto done;
-   } else if (fast_forward != FF_NO  !remoteheads-next 
-   !common-next 
-   !hashcmp(common-item-object.sha1, 
head_commit-object.sha1)) {
+   } else if (fast_forward != FF_NO 
+  !remoteheads-next 
+  !common-next 
+  

Re: Can I fetch an arbitrary commit by sha1?

2014-10-07 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 Hmm.. Junio already did most of the work in 051e400 (helping
 smart-http/stateless-rpc fetch race - 2011-08-05), so all we need to
 do is enable uploadpack.allowtipsha1inwant and apply this patch

Not that patch, I would think.

I would understand if !stateless_rpc and !allowtipsha1 then it is
an error, though.

 -- 8 --
 diff --git a/upload-pack.c b/upload-pack.c
 index c789ec0..493f8ee 100644
 --- a/upload-pack.c
 +++ b/upload-pack.c
 @@ -454,10 +454,6 @@ static void check_non_tip(void)
   char namebuf[42]; /* ^ + SHA-1 + LF */
   int i;
  
 - /* In the normal in-process case non-tip request can never happen */
 - if (!stateless_rpc)
 - goto error;
 -
   cmd.argv = argv;
   cmd.git_cmd = 1;
   cmd.no_stderr = 1;
 -- 8 --

 If we already let smart-http do this, I don't see any harm in letting
 git protocol do the same (even though it's the the original reason why
 this code exists).
 --
 Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/16] t5304: use helper to report failure of test foo = bar

2014-10-07 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 I don't like the three-argument version of test_eq. Wouldn't using a
 comparison operator other than = would be very confusing, given that
 eq is in the name of the function? It also doesn't look like you use
 this feature.

 An alternative direction to go would be to specialize the function for
 equality testing and delegate to test_cmp to get better output for
 failures, but optimized to avoid excess process creation in the happy path:

 test_eq () {
   if test $1 != $2
   then
   printf %s $1 expect 
   printf %s $2 actual 
   test_cmp expect actual
   fi
 }

 (but using properly-created temporary file names).

I agree that it would be a good idea to use a randomly generated
temporary filename that does not collide, as long as we make sure
not to leave cruft in the working tree of the test and we leave the
file there when the test script is run under -i or -d option.

The above superficially looks nice; ! test_eq a b would give
useless output under -v, and test_ne a b needs to be added if
you go that route, though.

Anyway, with the version posted, you cannot do ! test_eq a b,
either but with test_eq a b !=, you do not have to.

Side note. Yes, now I looked at it again, I agree that the
three-arg form is awkwards in at least two ways.  The
operator, if we are to take one, should be infix, and the
name eq no longer matches what it does when it is given an
operator.

The function is similar to test_cmp which takes two files but takes
two strings, so test_cmp_str or something perhaps (we already have
test_cmp_rev to compare two revisions, and the suggested name
follows that pattern)?
--
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] use skip_prefix() to avoid more magic numbers

2014-10-07 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Sun, Oct 05, 2014 at 03:49:19PM -0700, Jonathan Nieder wrote:

  --- a/builtin/branch.c
  +++ b/builtin/branch.c
  @@ -81,14 +81,16 @@ static int parse_branch_color_slot(const char *var, 
  int ofs)
   
   static int git_branch_config(const char *var, const char *value, void *cb)
   {
  +  const char *slot_name;
  +
 if (starts_with(var, column.))
 return git_column_config(var, value, branch, colopts);
 if (!strcmp(var, color.branch)) {
 branch_use_color = git_config_colorbool(var, value);
 return 0;
 }
  -  if (starts_with(var, color.branch.)) {
  -  int slot = parse_branch_color_slot(var, 13);
  +  if (skip_prefix(var, color.branch., slot_name)) {
  +  int slot = parse_branch_color_slot(var, slot_name - var);
 
 I wonder why the separate var and offset parameters exist in the first
 place.  Wouldn't taking a single const char * so the old code could use
 'var + 13' instead of 'var, 13' have worked?

 I think this is in the same boat as parse_diff_color_slot, which I fixed
 in 9e1a5eb (parse_diff_color_slot: drop ofs parameter, 2014-06-18). The
 short of it is that the function used to want both the full name and the
 slot name, but now needs only the latter.

 The fix you proposed below is along the same line, and looks good to me
 (and grepping for 'var *+ *ofs' shows only the two sites you found, so
 hopefully that is the last of it).

So perhaps we would want this change as a preliminary separate
patch?

Thanks


  @@ -809,18 +808,19 @@ static void parse_commit_header(struct
  format_commit_context *context)
 int i;
   
 for (i = 0; msg[i]; i++) {
  +  const char *name;
 
 ident instead of name, maybe? (since it's a 'name email timestamp'
 field)

 Yeah, agreed.

 -Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] use skip_prefix() to avoid more magic numbers

2014-10-07 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Sun, Oct 05, 2014 at 03:49:19PM -0700, Jonathan Nieder wrote:

  --- a/builtin/branch.c
  +++ b/builtin/branch.c
  @@ -81,14 +81,16 @@ static int parse_branch_color_slot(const char *var, 
  int ofs)
   
   static int git_branch_config(const char *var, const char *value, void *cb)
   {
  +  const char *slot_name;
  +
 if (starts_with(var, column.))
 return git_column_config(var, value, branch, colopts);
 if (!strcmp(var, color.branch)) {
 branch_use_color = git_config_colorbool(var, value);
 return 0;
 }
  -  if (starts_with(var, color.branch.)) {
  -  int slot = parse_branch_color_slot(var, 13);
  +  if (skip_prefix(var, color.branch., slot_name)) {
  +  int slot = parse_branch_color_slot(var, slot_name - var);
 
 I wonder why the separate var and offset parameters exist in the first
 place.  Wouldn't taking a single const char * so the old code could use
 'var + 13' instead of 'var, 13' have worked?

 I think this is in the same boat as parse_diff_color_slot, which I fixed
 in 9e1a5eb (parse_diff_color_slot: drop ofs parameter, 2014-06-18). The
 short of it is that the function used to want both the full name and the
 slot name, but now needs only the latter.

 The fix you proposed below is along the same line, and looks good to me
 (and grepping for 'var *+ *ofs' shows only the two sites you found, so
 hopefully that is the last of it).

builtin/commit.c::parse_status_slot() has the same construct.
--
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] use skip_prefix() to avoid more magic numbers

2014-10-07 Thread Junio C Hamano
René Scharfe l@web.de writes:

 @@ -335,20 +337,18 @@ static int append_ref(const char *refname, const 
 unsigned char *sha1, int flags,
   static struct {
   int kind;
   const char *prefix;
 - int pfxlen;
   } ref_kind[] = {
 - { REF_LOCAL_BRANCH, refs/heads/, 11 },
 - { REF_REMOTE_BRANCH, refs/remotes/, 13 },
 + { REF_LOCAL_BRANCH, refs/heads/ },
 + { REF_REMOTE_BRANCH, refs/remotes/ },
   };
  
   /* Detect kind */
   for (i = 0; i  ARRAY_SIZE(ref_kind); i++) {
   prefix = ref_kind[i].prefix;
 - if (strncmp(refname, prefix, ref_kind[i].pfxlen))
 - continue;
 - kind = ref_kind[i].kind;
 - refname += ref_kind[i].pfxlen;
 - break;
 + if (skip_prefix(refname, prefix, refname)) {
 + kind = ref_kind[i].kind;
 + break;
 + }

This certainly makes it easier to read.

I suspect that the original was done as a (possibly premature)
optimization to avoid having to do strlen(3) on a variable that
points at constant strings for each and every ref we iterate with
for_each_rawref(), and it is somewhat sad to see it lost because
skip_prefix() assumes that the caller never knows the length of the
prefix, though.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: let git grep consider sub projects

2014-10-07 Thread Junio C Hamano
Olaf Hering o...@aepfle.de writes:

 How can I teach 'git grep $string' to also consider subprojects?

 The build system of xen.git clones 4 other trees into a directory in its
 source tree during build. It would be nice if my 'git grep' searches
 also in these cloned subdirs. Somehow there must be some knowledge
 because 'git clean -dfx' leaves them alone, while 'git clean -dffx'
 wipes everything.

 Olaf

 PS: Sometimes I miss a 'git clean -dfx --also-sub-repos' useful to
 really clean everything before starting over.

Is submodule foreach under-advertised or with less than adequate
features?
--
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 svn's performance issue and strange pauses, and other thing

2014-10-07 Thread Hin-Tak Leung
--
On Tue, Oct 7, 2014 00:51 BST Hin-Tak Leung wrote:

--
On Sun, Oct 5, 2014 02:02 BST Eric Wong wrote:

snipped
Hin-Tak: have you tried Jakob's patches?  I've taken another look,
signed-off and pushed to my master.

... Then
I changed my mind, and decided what the hell, let's clone the whole
thing again :-). So I made a new directory, run 'git init', just copy
.git/config from the old reop and am doing 'git svn fetch --all' in the new 
empty
directory again.

So far it seems to be good. But I am only at revision 35700-ish at the moment,
and the whole thing is 66700-ish. Oh, I forgot to mention that the strange
pauses seem to be followed by messages like these:

W:svn cherry-pick ignored (/branches/R-2-12-branch:52939,54476,55265) - 
missing 492 commit(s) (eg 9bf20dca6a8b05dff28e6486b1613f10825972c9)
W:svn cherry-pick ignored (/branches/R-2-13-branch:55265,55432) - missing 231 
commit(s) (eg 9290cf6ce2d7f6cca168cf326eed6e9fe760895f)
W:svn cherry-pick ignored (/branches/R-2-15-branch:58894,59717) - missing 405 
commit(s) (eg ed84a373b33f728949edf3371829fc3414c343a8)
W:svn cherry-pick ignored (/branches/R-3-0-branch:62497) - missing 154 
commit(s) (eg 9e4742d201771c9658417c2d2f83838e550e3162)
W:svn cherry-pick ignored (/trunk:

So presumably I'd only see interesting behavior when there are a number of 
branches.
It seems the first branches are around revision 48000-ish, so I might have
to wait a bit.

So far, the new clone hasn't created .git/svn/.caches/ yet; and memory 
consumption seems
okay also.

The changes definitely improve, as far as my impression goes. There was only 
one notable pause around
r50651, and it is probably because the rather large Checking svn:mergeinfo 
changes since r15413
from r15413? That took about 12 minutes. Other instances of W:svn cherry-pick 
ignored
though do take a while, are in the seconds region - before the code changes 
they could
be minutes, if memory serves.

--
M   src/library/tools/R/toHTML.R
r50650 = bed91d435c535f2643cf0d48623fecf86d264bd9 (refs/remotes/trunk)
M   src/modules/X11/rotated.c
M   src/modules/X11/dataentry.c
Checking svn:mergeinfo changes since r15413: 1 sources, 1 changed
W:svn cherry-pick ignored (/trunk:28840) - missing 9372 commit(s) (eg 
cea6142c76300539a0d0c9c743738e31a9f7d523)
r50651 = ad139a5bf91f9ad6690ff5fb4a3f71cea591a944 (refs/remotes/R-uthreads)
--

The new clone has:

--
$ ls -ltr .git/svn/.caches/
total 144788
-rw-rw-r--. 1 Hin-Tak Hin-Tak  1166138 Oct  7 13:44 lookup_svn_merge.yaml
-rw-rw-r--. 1 Hin-Tak Hin-Tak 72849741 Oct  7 13:48 check_cherry_pick.yaml
-rw-rw-r--. 1 Hin-Tak Hin-Tak  1133855 Oct  7 13:49 has_no_changes.yaml
-rw-rw-r--. 1 Hin-Tak Hin-Tak 73109005 Oct  7 13:53 _rev_list.yaml
--

The old clone has:

---
$ ls -ltr .git/svn/.caches/
total 318824
-rw-rw-r--. 1 Hin-Tak Hin-Tak   5711724 Jul 24  2012 lookup_svn_merge.db
-rw-rw-r--. 1 Hin-Tak Hin-Tak  30523628 Jul 24  2012 check_cherry_pick.db
-rw-rw-r--. 1 Hin-Tak Hin-Tak296592 Jul 24  2012 has_no_changes.db
-rw-rw-r--. 1 Hin-Tak Hin-Tak  40241189 Oct  5 16:42 lookup_svn_merge.yaml
-rw-rw-r--. 1 Hin-Tak Hin-Tak 225323456 Oct  5 16:49 check_cherry_pick.yaml
-rw-rw-r--. 1 Hin-Tak Hin-Tak242547 Oct  5 16:49 has_no_changes.yaml
-rw-rw-r--. 1 Hin-Tak Hin-Tak  24120007 Oct  5 16:50 _rev_list.yaml
--

I had to suspend somewhat around r59000 - but it is interesting to see
that the max memory consumption of the later part is almost double?
and it also runs at 100% rather than 60% overall; I don't know what
to make of that - probably just smaller changes versus
larger ones, or different time of day and network loads (yes,
I guess it is just bandwidth-limited?, since the bulk of CPU time is in system
rather than user).

I am somwhat worry about the dramatic difference between the two .svn/.caches -
check_cherry_pick.yaml is 225MB in one and 73MB in the other, and also
_rev_list.yaml is opposite - 24MB vs 73MB. How do I reconcile that?

--
M   src/main/dotcode.c
M   doc/NEWS.Rd
r59140 = b6014a226aebf9e016c89c0bd1aca1979796a057 (refs/remotes/trunk)
M   src/main/dotcode.c
M   doc/NEWS.Rd
Checking svn:mergeinfo changes since r59138: 4 sources, 1 changed
W:svn cherry-pick ignored (/trunk:59137,59140) - missing 369 commit(s) (eg 
8a2a36083ba39be27fc9940acc3f51eab6a7a0c3)
r59141 = 38c6d05f164d34e4b5cc545bda387be9d910f748 (refs/remotes/R-2-15-branch)
Connection timed out: Connection timed out at 
/usr/share/perl5/vendor_perl/Git/SVN/Ra.pm line 290.

Command exited with non-zero status 1
Command being timed: git svn fetch --all
User time (seconds): 5642.19
System time (seconds): 23552.44
Percent of CPU this job got: 57%
Elapsed (wall clock) time (h:mm:ss or m:ss): 14:06:58
Average shared text size (kbytes): 0
Average unshared data size (kbytes): 0
Average stack size (kbytes): 0
   

Apparent bug in git rebase with a merge commit

2014-10-07 Thread David M. Lloyd
If you have a git tree and you merge in another, independent git tree so 
that they are the same, using a merge strategy like this:


$ git merge importing/master -s recursive -Xours

And if you later on want to rebase this merge commit on a newer upstream 
for whatever reason, you get something like this:


$ git rebase -s recursive -Xours
First, rewinding head to replay your work on top of it...
fatal: Could not parse object 'ca59931ee67fc01b4db4278600d3d92aece898f4^'
Unknown exit code (128) from command: git-merge-recursive 
ca59931ee67fc01b4db4278600d3d92aece898f4^ -- HEAD 
ca59931ee67fc01b4db4278600d3d92aece898f4


The reason this occurs is that the first commit of the newly-merged-in 
code obviously has no parent, so I guess the search for the common 
ancestor is going to be doomed to fail.


It is possible that I'm misunderstanding the recursive merge strategy; 
however if this were the case I'd still expect a human-readable error 
message explaining my mistake rather than a 128 exit code.


For a workaround I'll just re-create the commit, but I thought I'd 
report this behavior anyway.

--
- DML
--
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] use skip_prefix() to avoid more magic numbers

2014-10-07 Thread Jeff King
On Tue, Oct 07, 2014 at 11:21:58AM -0700, Junio C Hamano wrote:

  The fix you proposed below is along the same line, and looks good to me
  (and grepping for 'var *+ *ofs' shows only the two sites you found, so
  hopefully that is the last of it).
 
 builtin/commit.c::parse_status_slot() has the same construct.

Thanks, I missed it because it uses offset instead of ofs.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-merge: mutually match SYNOPSIS and usage.

2014-10-07 Thread Junio C Hamano
Sergey Organov sorga...@gmail.com writes:

 SYNOPSIS section of the git-merge manual page had outdated explicit
 list of options.

 usage returned by 'git merge -h' didn't have -m msg that is one
 of essential distinctions between obsolete invocation form and the
 recent one.

 Signed-off-by: Sergey Organov sorga...@gmail.com
 ---

Please do not do two unrelated things in a single change.

It may be a clear and very welcome improvement to change from
explicitly list only often used options to just say [options] and
have the list of options and their descriptions.

I am not sure about the other change to single out -m msg,
especially marking it as optional by enclosing it inside [-m
msg], makes much sense, as that is still not very easily
distinguishable from git merge [options] [commit...].  In other
words, I agree with your motivation to call for attention that the
command behaves differently with and without -m, but I do not
think that part of the change in this patch achieves it well.

  Documentation/git-merge.txt | 6 ++
  builtin/merge.c | 2 +-
  2 files changed, 3 insertions(+), 5 deletions(-)

 diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
 index cf2c374..e24a1d4 100644
 --- a/Documentation/git-merge.txt
 +++ b/Documentation/git-merge.txt
 @@ -9,10 +9,8 @@ git-merge - Join two or more development histories together
  SYNOPSIS
  
  [verse]
 -'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
 - [-s strategy] [-X strategy-option] [-S[key-id]]
 - [--[no-]rerere-autoupdate] [-m msg] [commit...]
 -'git merge' msg HEAD commit...
 +'git merge' [options] [-m msg] [commit...]
 +'git merge' [options] msg HEAD commit...
  'git merge' --abort
  
  DESCRIPTION
 diff --git a/builtin/merge.c b/builtin/merge.c
 index dff043d..086502f 100644
 --- a/builtin/merge.c
 +++ b/builtin/merge.c
 @@ -40,7 +40,7 @@ struct strategy {
  };
  
  static const char * const builtin_merge_usage[] = {
 - N_(git merge [options] [commit...]),
 + N_(git merge [options] [-m msg] [commit...]),
   N_(git merge [options] msg HEAD commit),
   N_(git merge --abort),
   NULL
--
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] git-merge: implement --ff-only-merge option.

2014-10-07 Thread Junio C Hamano
Sergey Organov sorga...@gmail.com writes:

 This option allows to create merge commit when fast-forward is
 possible, and abort otherwise. I.e. it's equivalent to --ff-only,
 except that it finally creates merge commit instead of
 fast-forwarding.

 One may also consider this option to be equivalent to --no-ff with
 additional check that the command without --no-ff  would indeed result
 in fast-forward.

 Useful to incorporate topic branch as single merge commit, ensuring
 the left-side of the merge has no changes (our-diff-empty-merge).

 Signed-off-by: Sergey Organov sorga...@gmail.com
 ---

The workflow this implements sounds like because we can, not
because it will help us do X and Y and Z.  Why would it be useful
to limit the history to a shape where all merges are the ones that
could have been fast-forwarded?

I cannot justify that sensibly myself, which in turn makes the
feature smell to me that it is encouraging a wrong workflow.

--
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: Colors in __git_ps1

2014-10-07 Thread Simon Oosthoek

Dear Mantas,

On 07/10/14 18:18, Mantas Mikulėnas wrote:

There was a question in #git recently on why __git_ps1 from
git-prompt.sh does not allow colors in $PS1 mode.

 From what I see, commit 1bfc51ac8141 talks about the need for \[ and
\] assignments in $PS1, and commit cf4cac4cfc13 says that PS1 mode
turns off color support since \[ and \] won't work; as I understand
it, this is because bash will output literal \[ if __git_ps1 tries
to echo it.

Internally, \[ and \] are translated by bash into 0x01 and 0x02 before
passing the final string to readline. So there *is* a way to achieve
the same effect in PS1 mode – __git_ps1 just needs to echo \001 and
\002 before  after the color codes, and readline will interpret them
correctly when calculating the prompt width. For example, echo
$'foo\001\e[1m\002bar'.

Might be worth considering, even if $PS1 mode is inferior to
$PROMPT_COMMAND mode for other reasons.



I would actually prefer the PS1='$(__git_ps)' like way, as it is more 
generic and intuitive when you know generic Bourne shell ways.


The reason to go for PROMPT_COMMAND was that it appears to be the only 
way to set the PS1 prompt and let bash keep track of the length of the 
prompt properly. This is needed to allow browsing the bash history with 
long(er than width of the terminal) command lines without the terminal 
getting messed up.
And I'm still not sure PROMPT_COMMAND mode fixes this properly, but it 
passed the tests where command substitution mode failed.


IMHO the way colours are now implemented could very well be considered a 
workaround for a bug in bash. Only I'm not skilled enough (or have 
enough time) to get to the bottom of it...


I hope this answers your question.

Cheers

Simon
--
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] MinGW(-W64) compilation

2014-10-07 Thread Junio C Hamano
Johannes Sixt j...@kdbg.org writes:

 Am 06.10.2014 um 07:17 schrieb Marat Radchenko:
 On Tue, Sep 30, 2014 at 11:02:29AM +0400, Marat Radchenko wrote:
 This patch series fixes building on modern MinGW and MinGW-W64 (including 
 x86_64!).
 
 Junio, ping?
 

 Sorry, I forgot to report that this updated series works now for me.

 The patches all look reasonable. I don't have an opinion on the
 restriction that MSVC  2010 can't be used anymore (path 08/14).

So, is that an Ack, or would you prefer to cook this first in
msysgit tree and then feed the result as part of This series is to
shrink the difference between the mainline and msysgit later?
--
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: let git grep consider sub projects

2014-10-07 Thread Mikael Magnusson
On 7 October 2014 20:25, Junio C Hamano gits...@pobox.com wrote:
 Olaf Hering o...@aepfle.de writes:

 How can I teach 'git grep $string' to also consider subprojects?

 The build system of xen.git clones 4 other trees into a directory in its
 source tree during build. It would be nice if my 'git grep' searches
 also in these cloned subdirs. Somehow there must be some knowledge
 because 'git clean -dfx' leaves them alone, while 'git clean -dffx'
 wipes everything.

 Olaf

 PS: Sometimes I miss a 'git clean -dfx --also-sub-repos' useful to
 really clean everything before starting over.

 Is submodule foreach under-advertised or with less than adequate
 features?

It sounds like in these use cases, you would want the commands to run
on all the submodules but also in the parent repo, am I wrong in
thinking that git submodule foreach does only the former part? So you
would either need to make a wrapper thing yourself or run the command
twice.

In the first case with the git grep, I can also imagine that with some
nontrivial patterns, having to quote the metacharacters not only once,
but twice, can be a significant annoyance. Eg, first protect it from
git submodule foreach parsing it, and then from the shell running the
individual commands.

-- 
Mikael Magnusson
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] use skip_prefix() to avoid more magic numbers

2014-10-07 Thread Jeff King
On Tue, Oct 07, 2014 at 11:14:35AM -0700, Junio C Hamano wrote:

  The fix you proposed below is along the same line, and looks good to me
  (and grepping for 'var *+ *ofs' shows only the two sites you found, so
  hopefully that is the last of it).
 
 So perhaps we would want this change as a preliminary separate
 patch?

Here it is on top of master, with a commit message (and including the
other case you found). I've attributed it to Jonathan, who did most of
the work. We'd need his signoff and approval, of course.

-- 8 --
From: Jonathan Nieder jrnie...@gmail.com
Subject: pass config slots as pointers instead of offsets

Many config-parsing helpers, like parse_branch_color_slot,
take the name of a config variable and an offset to the
slot name (e.g., color.branch.plain is passed along with
13 to effectively pass plain). This is leftover from the
time that these functions would die() on error, and would
want the full variable name for error reporting.

These days they do not use the full variable name at all.
Passing a single pointer to the slot name is more natural,
and lets us more easily adjust the callers to use skip_prefix
to avoid manually writing offset numbers.

This is effectively a continuation of 9e1a5eb, which did the
same for parse_diff_color_slot. This patch covers all of the
remaining similar constructs.

Signed-off-by: Jeff King p...@peff.net
---
Actually, parse_decorate_color_config is slightly odd in that it takes
the variable and the slot_name after this patch. That's because it
passes the full variable name to color_parse, which does still die() on
error. Perhaps it should be converted to return an error, too.

 builtin/branch.c | 16 
 builtin/commit.c | 19 +--
 builtin/log.c|  2 +-
 log-tree.c   |  4 ++--
 log-tree.h   |  2 +-
 5 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 9e4666f..2c97141 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -62,19 +62,19 @@ static unsigned char merge_filter_ref[20];
 static struct string_list output = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
-static int parse_branch_color_slot(const char *var, int ofs)
+static int parse_branch_color_slot(const char *slot)
 {
-   if (!strcasecmp(var+ofs, plain))
+   if (!strcasecmp(slot, plain))
return BRANCH_COLOR_PLAIN;
-   if (!strcasecmp(var+ofs, reset))
+   if (!strcasecmp(slot, reset))
return BRANCH_COLOR_RESET;
-   if (!strcasecmp(var+ofs, remote))
+   if (!strcasecmp(slot, remote))
return BRANCH_COLOR_REMOTE;
-   if (!strcasecmp(var+ofs, local))
+   if (!strcasecmp(slot, local))
return BRANCH_COLOR_LOCAL;
-   if (!strcasecmp(var+ofs, current))
+   if (!strcasecmp(slot, current))
return BRANCH_COLOR_CURRENT;
-   if (!strcasecmp(var+ofs, upstream))
+   if (!strcasecmp(slot, upstream))
return BRANCH_COLOR_UPSTREAM;
return -1;
 }
@@ -88,7 +88,7 @@ static int git_branch_config(const char *var, const char 
*value, void *cb)
return 0;
}
if (starts_with(var, color.branch.)) {
-   int slot = parse_branch_color_slot(var, 13);
+   int slot = parse_branch_color_slot(var + 13);
if (slot  0)
return 0;
if (!value)
diff --git a/builtin/commit.c b/builtin/commit.c
index b0fe784..c45613a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1271,22 +1271,21 @@ static int dry_run_commit(int argc, const char **argv, 
const char *prefix,
return commitable ? 0 : 1;
 }
 
-static int parse_status_slot(const char *var, int offset)
+static int parse_status_slot(const char *slot)
 {
-   if (!strcasecmp(var+offset, header))
+   if (!strcasecmp(slot, header))
return WT_STATUS_HEADER;
-   if (!strcasecmp(var+offset, branch))
+   if (!strcasecmp(slot, branch))
return WT_STATUS_ONBRANCH;
-   if (!strcasecmp(var+offset, updated)
-   || !strcasecmp(var+offset, added))
+   if (!strcasecmp(slot, updated) || !strcasecmp(slot, added))
return WT_STATUS_UPDATED;
-   if (!strcasecmp(var+offset, changed))
+   if (!strcasecmp(slot, changed))
return WT_STATUS_CHANGED;
-   if (!strcasecmp(var+offset, untracked))
+   if (!strcasecmp(slot, untracked))
return WT_STATUS_UNTRACKED;
-   if (!strcasecmp(var+offset, nobranch))
+   if (!strcasecmp(slot, nobranch))
return WT_STATUS_NOBRANCH;
-   if (!strcasecmp(var+offset, unmerged))
+   if (!strcasecmp(slot, unmerged))
return WT_STATUS_UNMERGED;
return -1;
 }
@@ -1324,7 +1323,7 @@ static int git_status_config(const char *k, const char 
*v, void *cb)
return 0;
}
if (starts_with(k, status.color.) || 

Re: [PATCH v22.5 09/24] refs.c: pass a list of names to skip to is_refname_available

2014-10-07 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 Will take a look over there (I really hate having to context switch
 only to review this series, though).

 Here's a copy to save a trip.

The patch itself looks fine, but Gerrit really needs a way to convey
these changes make a single topic and here is the tip somehow.  It
could be that there is and it is not well advertised, but the end
result is I visit the URL,

  https://code-review.googlesource.com/#/q/project:git+topic:ref-transaction

pick and view a few patches at random to view the changes in pretty
side-by-side diff output (which by itself is fine), check Download
to see the fetch URL, when it is followed it often gives me only an
early half of the topic, get confused and scratch my head, give up
and abandon and the whole time doing so ends up being wasted.
--
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] use skip_prefix() to avoid more magic numbers

2014-10-07 Thread Jeff King
On Tue, Oct 07, 2014 at 03:16:56PM -0400, Jeff King wrote:

 Actually, parse_decorate_color_config is slightly odd in that it takes
 the variable and the slot_name after this patch. That's because it
 passes the full variable name to color_parse, which does still die() on
 error. Perhaps it should be converted to return an error, too.

Actually, this doesn't let us get rid of the var in
parse_decorate_color_config, because it also wants to call
config_error_nonbool. However, while looking at this, I did notice that
some of the error messages generated by color_parse are a bit ugly, and
came up with the patch below (on top of what I just sent, but it could
be separate).

-- 8 --
Subject: color_parse: do not mention variable name in error message

Originally the color-parsing function was used only for
config variables. It made sense to pass the variable name so
that the die() message could be something like:

  $ git -c color.branch.plain=bogus branch
  fatal: bad color value 'bogus' for variable 'color.branch.plain'

These days we call it in other contexts, and the resulting
error messages are a little confusing:

  $ git log --pretty='%C(bogus)'
  fatal: bad color value 'bogus' for variable '--pretty format'

  $ git config --get-color foo.bar bogus
  fatal: bad color value 'bogus' for variable 'command line'

This patch teaches color_parse to complain only about the
value, and then return an error code. Config callers can
then propagate that up to the config parser, which mentions
the variable name. Other callers can provide a custom
message. After this patch these three cases now look like:

  $ git -c color.branch.plain=bogus branch
  error: invalid color value: bogus
  fatal: unable to parse 'color.branch.plain' from command-line config

  $ git log --pretty='%C(bogus)'
  error: invalid color value: bogus
  fatal: unable to parse --pretty format

  $ git config --get-color foo.bar bogus
  error: invalid color value: bogus
  fatal: unable to parse default color value

Signed-off-by: Jeff King p...@peff.net
---
I think the two-line errors are kind of ugly. It does match how
config_error_nonbool works, which also propagates its errors, and looks
like:

  $ git -c color.branch.plain branch
  error: Missing value for 'color.branch.plain'
  fatal: unable to parse 'color.branch.plain' from command-line config

We could instead make color_parse silently return -1, and leave it up to
the caller to complain (and probably add die_bad_color_config() or
similar to avoid repetition in the config callers).

 builtin/branch.c   |  3 +--
 builtin/clean.c|  3 +--
 builtin/commit.c   |  3 +--
 builtin/config.c   |  9 ++---
 builtin/for-each-ref.c |  6 --
 color.c| 13 ++---
 color.h|  4 ++--
 diff.c |  3 +--
 grep.c |  2 +-
 log-tree.c |  3 +--
 pretty.c   |  5 ++---
 11 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 2c97141..35c786d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -93,8 +93,7 @@ static int git_branch_config(const char *var, const char 
*value, void *cb)
return 0;
if (!value)
return config_error_nonbool(var);
-   color_parse(value, var, branch_colors[slot]);
-   return 0;
+   return color_parse(value, branch_colors[slot]);
}
return git_color_default_config(var, value, cb);
 }
diff --git a/builtin/clean.c b/builtin/clean.c
index 3beeea6..a7e7b0b 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -116,8 +116,7 @@ static int git_clean_config(const char *var, const char 
*value, void *cb)
return 0;
if (!value)
return config_error_nonbool(var);
-   color_parse(value, var, clean_colors[slot]);
-   return 0;
+   return color_parse(value, clean_colors[slot]);
}
 
if (!strcmp(var, clean.requireforce)) {
diff --git a/builtin/commit.c b/builtin/commit.c
index c45613a..407566d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1328,8 +1328,7 @@ static int git_status_config(const char *k, const char 
*v, void *cb)
return 0;
if (!v)
return config_error_nonbool(k);
-   color_parse(v, k, s-color_palette[slot]);
-   return 0;
+   return color_parse(v, s-color_palette[slot]);
}
if (!strcmp(k, status.relativepaths)) {
s-relative_paths = git_config_bool(k, v);
diff --git a/builtin/config.c b/builtin/config.c
index 37305e9..8cc2604 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -296,7 +296,8 @@ static int git_get_color_config(const char *var, const char 
*value, void *cb)
if (!strcmp(var, get_color_slot)) {
if (!value)
  

Re: [PATCH] git-prompt.sh: shorter equal upstream branch name

2014-10-07 Thread Richard Hansen
On 2014-10-07 11:57, Julien Carsique wrote:
 Hi,
 
 Thank you both for your feedback!
 I'm looking at applying your requests:
 - add tests,
 - variable renaming,
 - use of local,
 - fix multiple issues on string parsing
 - avoid useless bash-isms? Did you agree on the ones I should remove?

I'm guessing the structure of your code will change somewhat when you
address the other issues, so I think it may be premature to discuss
specific Bashisms right now.  (There aren't any particular Bashisms that
I think should always be avoided -- I just want people to ponder whether
a particular use of a Bashism is truly preferable to a POSIX-conformant
alternative.)

Write the code in the way you think is best, and if I see a good way to
convert a Bashism to POSIX I'll let you know.  And feel free to ignore
me -- I'm a member of the Church of POSIX Conformance while Junio is
much more grounded in reality.  :)

 
 I'll send an updated patch asap. Tell me if I forgot something.

Your list looks complete to me.  Thank you for contributing!

-Richard


 
 Regards,
 Julien
 
 On 01/10/2014 19:49, Junio C Hamano wrote:
 Richard Hansen rhan...@bbn.com writes:

 and there is no hope to fix them to stick to
 the bare-minimum POSIX,
 I don't think it'd be hard to convert it to pure POSIX if there was a
 desire to do so.
 Not necessarily; if you make it so slow to be usable as a prompt
 script, that is not a conversion.  Bash-isms in the script is
 allowed for a reason, unfortunately.

 It would be unwise to go to great lengths to avoid Bashisms, but I think
 it would be smart to use POSIX syntax when it is easy to do so.  
 In general, I agree with you. People who know only bash tend to
 overuse bash-isms where they are not necessary, leaving an
 unreadable mess.

 For the specific purpose of Julien's if the tail part of this
 string matches the other string, replace that with an equal sign,
 ${parameter/pattern/string} is a wrong bash-ism to use.  But the
 right solution to count the length of the other string and take a
 substring of this string from its beginning would require other
 bash-isms ${#parameter} and ${parameter:offset:length}.

 And that's fine.
 
 

--
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] use skip_prefix() to avoid more magic numbers

2014-10-07 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Subject: color_parse: do not mention variable name in error message
 ...
 I think the two-line errors are kind of ugly. It does match how
 config_error_nonbool works, which also propagates its errors, and looks
 like:

   $ git -c color.branch.plain branch
   error: Missing value for 'color.branch.plain'
   fatal: unable to parse 'color.branch.plain' from command-line config

 We could instead make color_parse silently return -1, and leave it up to
 the caller to complain (and probably add die_bad_color_config() or
 similar to avoid repetition in the config callers).

Yeah, in the longer term we would want to do something like that to
fix both nonbool and this one, but for now this should help avoid
confusion.

Thanks for a nice analysis, write-up and patch.


  builtin/branch.c   |  3 +--
  builtin/clean.c|  3 +--
  builtin/commit.c   |  3 +--
  builtin/config.c   |  9 ++---
  builtin/for-each-ref.c |  6 --
  color.c| 13 ++---
  color.h|  4 ++--
  diff.c |  3 +--
  grep.c |  2 +-
  log-tree.c |  3 +--
  pretty.c   |  5 ++---
  11 files changed, 26 insertions(+), 28 deletions(-)

 diff --git a/builtin/branch.c b/builtin/branch.c
 index 2c97141..35c786d 100644
 --- a/builtin/branch.c
 +++ b/builtin/branch.c
 @@ -93,8 +93,7 @@ static int git_branch_config(const char *var, const char 
 *value, void *cb)
   return 0;
   if (!value)
   return config_error_nonbool(var);
 - color_parse(value, var, branch_colors[slot]);
 - return 0;
 + return color_parse(value, branch_colors[slot]);
   }
   return git_color_default_config(var, value, cb);
  }
 diff --git a/builtin/clean.c b/builtin/clean.c
 index 3beeea6..a7e7b0b 100644
 --- a/builtin/clean.c
 +++ b/builtin/clean.c
 @@ -116,8 +116,7 @@ static int git_clean_config(const char *var, const char 
 *value, void *cb)
   return 0;
   if (!value)
   return config_error_nonbool(var);
 - color_parse(value, var, clean_colors[slot]);
 - return 0;
 + return color_parse(value, clean_colors[slot]);
   }
  
   if (!strcmp(var, clean.requireforce)) {
 diff --git a/builtin/commit.c b/builtin/commit.c
 index c45613a..407566d 100644
 --- a/builtin/commit.c
 +++ b/builtin/commit.c
 @@ -1328,8 +1328,7 @@ static int git_status_config(const char *k, const char 
 *v, void *cb)
   return 0;
   if (!v)
   return config_error_nonbool(k);
 - color_parse(v, k, s-color_palette[slot]);
 - return 0;
 + return color_parse(v, s-color_palette[slot]);
   }
   if (!strcmp(k, status.relativepaths)) {
   s-relative_paths = git_config_bool(k, v);
 diff --git a/builtin/config.c b/builtin/config.c
 index 37305e9..8cc2604 100644
 --- a/builtin/config.c
 +++ b/builtin/config.c
 @@ -296,7 +296,8 @@ static int git_get_color_config(const char *var, const 
 char *value, void *cb)
   if (!strcmp(var, get_color_slot)) {
   if (!value)
   config_error_nonbool(var);
 - color_parse(value, var, parsed_color);
 + if (color_parse(value, parsed_color)  0)
 + return -1;
   get_color_found = 1;
   }
   return 0;
 @@ -309,8 +310,10 @@ static void get_color(const char *def_color)
   git_config_with_options(git_get_color_config, NULL,
   given_config_source, respect_includes);
  
 - if (!get_color_found  def_color)
 - color_parse(def_color, command line, parsed_color);
 + if (!get_color_found  def_color) {
 + if (color_parse(def_color, parsed_color)  0)
 + die(_(unable to parse default color value));
 + }
  
   fputs(parsed_color, stdout);
  }
 diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
 index fda0f04..7ee86b3 100644
 --- a/builtin/for-each-ref.c
 +++ b/builtin/for-each-ref.c
 @@ -671,7 +671,8 @@ static void populate_value(struct refinfo *ref)
   } else if (starts_with(name, color:)) {
   char color[COLOR_MAXLEN] = ;
  
 - color_parse(name + 6, --format, color);
 + if (color_parse(name + 6, color)  0)
 + die(_(unable to parse format));
   v-s = xstrdup(color);
   continue;
   } else if (!strcmp(name, flag)) {
 @@ -1004,7 +1005,8 @@ static void show_ref(struct refinfo *info, const char 
 *format, int quote_style)
   struct atom_value resetv;
   char color[COLOR_MAXLEN] = ;
  
 - color_parse(reset, --format, color);
 + if (color_parse(reset, color)  0)
 + 

Re: [PATCH v4] MinGW(-W64) compilation

2014-10-07 Thread Johannes Sixt
Am 07.10.2014 um 21:01 schrieb Junio C Hamano:
 Johannes Sixt j...@kdbg.org writes:
 
 Am 06.10.2014 um 07:17 schrieb Marat Radchenko:
 On Tue, Sep 30, 2014 at 11:02:29AM +0400, Marat Radchenko wrote:
 This patch series fixes building on modern MinGW and MinGW-W64 (including 
 x86_64!).

 Junio, ping?


 Sorry, I forgot to report that this updated series works now for me.

 The patches all look reasonable. I don't have an opinion on the
 restriction that MSVC  2010 can't be used anymore (path 08/14).
 
 So, is that an Ack, or would you prefer to cook this first in
 msysgit tree and then feed the result as part of This series is to
 shrink the difference between the mainline and msysgit later?

At this time, it's really just a works for me report, because I didn't
look carefully at each patch. I may do that at some time later.

If someone from the msysgit crew (I don't count myself to it) could
report works for us, too, then the series could go into your tree
right away, in my opinion.

-- Hannes

--
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


Read and reply to me.

2014-10-07 Thread Madiit



--
Hello my dear,i knows that this letter will not come as a surprise  
to you,i saw your data online  and i decided to have this brief  
conversation with you which i knows that it must favor me and you, 
i have a good business proposal to discuss with you but on your reply  
to this brief letter  i will reveal more details of the discussions i  
have with you.


I hope you will enjoy my communication and company with me,i will  
expect your good response while you will have a very good day and  
blessed week.


This is my email address paulamadi...@yahoo.com   .

Thanks.

Ms Paula Madiit.

--
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-prompt.sh: shorter equal upstream branch name

2014-10-07 Thread Junio C Hamano
Julien Carsique julien.carsi...@gmail.com writes:

 Hi,

 Thank you both for your feedback!
 I'm looking at applying your requests:
 - add tests,
 - variable renaming,
 - use of local,
 - fix multiple issues on string parsing
 - avoid useless bash-isms? Did you agree on the ones I should remove?

 I'll send an updated patch asap. Tell me if I forgot something.

I just re-read the comments in the thread, and a few things look
missing:

 - git-svn?
 - conditionally enable this feature?

Other than that the above list looks like a fairly good one.

Thanks.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/16] t5304: use helper to report failure of test foo = bar

2014-10-07 Thread Jeff King
On Tue, Oct 07, 2014 at 10:29:59AM -0700, Junio C Hamano wrote:

  test_eq () {
  if test $1 != $2
  then
  printf %s $1 expect 
  printf %s $2 actual 
  test_cmp expect actual
  fi
  }
 [...]
 
 The above superficially looks nice; ! test_eq a b would give
 useless output under -v, and test_ne a b needs to be added if
 you go that route, though.

Yeah, that is why I ended up with the operator as a parameter. I modeled
after test_line_count, which faces the same problem (negation must
happen in the operator, not the full command).

 Anyway, with the version posted, you cannot do ! test_eq a b,
 either but with test_eq a b !=, you do not have to.
 
   Side note. Yes, now I looked at it again, I agree that the
   three-arg form is awkwards in at least two ways.  The
   operator, if we are to take one, should be infix, and the
   name eq no longer matches what it does when it is given an
   operator.

I made it postfix because it's optional, and my inclination is to handle
arguments left-to-right, since that extends to multiple optional
arguments. But of course we have just one optional argument and can
simply treat 2-arg and 3-arg forms differently. However, Michael noted
that this is really just 'test $@', and I think we should allow any
test parameters.

 The function is similar to test_cmp which takes two files but takes
 two strings, so test_cmp_str or something perhaps (we already have
 test_cmp_rev to compare two revisions, and the suggested name
 follows that pattern)?

Based on your responses, I'm leaning towards:

  test_cmp_str() {
test $@  return 0
echo 2 command failed: test $*
return 1
  }

since the point is really just to print _something_ when the test fails
(any quoting or whitespace may be wrong, of course, but that's OK; it's
for human consumption, and is just a hint).

Maybe str is not right here. Michael suggested test_test which is
semantically what we want, but just looks silly[1]. Maybe
test_pred or something? test_cond? Or I guess going the other way,
sane_test or verbose_test or something.

I think test_cond is my favorite of those.

-Peff

[1] Of course, we could always do test_[. :)
--
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] git-merge: implement --ff-only-merge option.

2014-10-07 Thread Sergey Organov
Junio C Hamano gits...@pobox.com writes:

 Sergey Organov sorga...@gmail.com writes:

 This option allows to create merge commit when fast-forward is
 possible, and abort otherwise. I.e. it's equivalent to --ff-only,
 except that it finally creates merge commit instead of
 fast-forwarding.

 One may also consider this option to be equivalent to --no-ff with
 additional check that the command without --no-ff  would indeed result
 in fast-forward.

 Useful to incorporate topic branch as single merge commit, ensuring
 the left-side of the merge has no changes (our-diff-empty-merge).

 Signed-off-by: Sergey Organov sorga...@gmail.com
 ---

 The workflow this implements sounds like because we can, not
 because it will help us do X and Y and Z.

Well, I do have full time job, and while I think I can instantly invent
quite a few things from the because we can camp, I usually don't.

 Why would it be useful to limit the history to a shape where all
 merges are the ones that could have been fast-forwarded?

Except by true merge, how else can I express with git that 'n'
consequitive commits constitute single logical change (being originally
some topic branch)? Now I just want such special kind of merge to be
entirely trivial merge on one side. i.e. perfectly clean merge every
time.

Moreover, as topic branches are usually rebased before merge anyway,
why shouldn't I have simple capability to enforce it?

 I cannot justify that sensibly myself, which in turn makes the
 feature smell to me that it is encouraging a wrong workflow.

What's wrong, exactly, in enforcing rebasing of topic branches before
merge? Basically, I need --ff-only, only I don't want git to forget that
this entire set of commits is logically single unit. Neither do I want
to loose the structure of commits that --squash offers.

-- Sergey.
--
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] git-merge: implement --ff-only-merge option.

2014-10-07 Thread Junio C Hamano
Sergey Organov sorga...@gmail.com writes:

 Why would it be useful to limit the history to a shape where all
 merges are the ones that could have been fast-forwarded?

 Except by true merge, how else can I express with git that 'n'
 consequitive commits constitute single logical change (being originally
 some topic branch)?

You are justifying --no-ff, aren't you?

 Moreover, as topic branches are usually rebased before merge anyway,
 why shouldn't I have simple capability to enforce it?

Because rebasing immediately before is considered a bad manner,
i.e. encouraging a wrong workflow?
--
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-merge: mutually match SYNOPSIS and usage.

2014-10-07 Thread Sergey Organov
Junio C Hamano gits...@pobox.com writes:

 Sergey Organov sorga...@gmail.com writes:

 SYNOPSIS section of the git-merge manual page had outdated explicit
 list of options.

 usage returned by 'git merge -h' didn't have -m msg that is one
 of essential distinctions between obsolete invocation form and the
 recent one.

 Signed-off-by: Sergey Organov sorga...@gmail.com
 ---

 Please do not do two unrelated things in a single change.

Well, I thought they are related, sorry.

 It may be a clear and very welcome improvement to change from
 explicitly list only often used options to just say [options] and
 have the list of options and their descriptions.

OK, noticed.

 I am not sure about the other change to single out -m msg,
 especially marking it as optional by enclosing it inside [-m
 msg], makes much sense, as that is still not very easily
 distinguishable from git merge [options] [commit...].

I was looking at the merge.c code, and that's how it seems to work. You
can get new semantics without -m, and you can't get old semantics with
-m, isn't it? It looks like the set of descriptions I produced is
formally correct.

 In other words, I agree with your motivation to call for attention
 that the command behaves differently with and without -m, but I do
 not think that part of the change in this patch achieves it well.

Any particular suggestion?

Thanks.

-- 
Sergey.
--
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 08/16] t5304: use helper to report failure of test foo = bar

2014-10-07 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Tue, Oct 07, 2014 at 10:29:59AM -0700, Junio C Hamano wrote:
 ...
 The function is similar to test_cmp which takes two files but takes
 two strings, so test_cmp_str or something perhaps (we already have
 test_cmp_rev to compare two revisions, and the suggested name
 follows that pattern)?

 Based on your responses, I'm leaning towards:

   test_cmp_str() {
   test $@  return 0
   echo 2 command failed: test $*
   return 1
   }

 since the point is really just to print _something_ when the test fails
 (any quoting or whitespace may be wrong, of course, but that's OK; it's
 for human consumption, and is just a hint).

Yeah, if we are going to reduce it down to the above implementation,
intereseting things like test -f $frotz will become possible and
cmp-str stops making sense.  It really is about We run test and
expect it to yield true.  Report the failure a bit more prominently
under the '-v' option to help us debug.

So among the ones you listed, test_verbose may be the least silly, I
would think.

--
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] git-merge: implement --ff-only-merge option.

2014-10-07 Thread Sergey Organov
Junio C Hamano gits...@pobox.com writes:

 Sergey Organov sorga...@gmail.com writes:

 Why would it be useful to limit the history to a shape where all
 merges are the ones that could have been fast-forwarded?

 Except by true merge, how else can I express with git that 'n'
 consequitive commits constitute single logical change (being originally
 some topic branch)?

 You are justifying --no-ff, aren't you?

Yes, and I already said it's close. But I don't want such merge commit
to contain any non-trivialities. Currently I check it manually before
issuing merge --no-ff and was hoping for some automation.


 Moreover, as topic branches are usually rebased before merge anyway,
 why shouldn't I have simple capability to enforce it?

 Because rebasing immediately before is considered a bad manner,
 i.e. encouraging a wrong workflow?

Why? What is wrong about it?

Please also notice that I don't try to impose this on anybody who does
consider it wrong workflow.

-- 
Sergey.
--
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 08/16] t5304: use helper to report failure of test foo = bar

2014-10-07 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Based on your responses, I'm leaning towards:

   test_cmp_str() {
   test $@  return 0
   echo 2 command failed: test $*
   return 1
   }

 since the point is really just to print _something_ when the test fails
 (any quoting or whitespace may be wrong, of course, but that's OK; it's
 for human consumption, and is just a hint).

If we really cared, we could do

echo 2 command failed: test $(git rev-parse --sq-quote $@)

perhaps?

--
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 RFC] git-am: support any number of signatures

2014-10-07 Thread Michael S. Tsirkin
On Tue, Sep 23, 2014 at 10:15:47AM -0700, Junio C Hamano wrote:
 Christian Couder christian.cou...@gmail.com writes:
 
  On Mon, Sep 22, 2014 at 4:01 PM, Michael S. Tsirkin m...@redhat.com wrote:
  ...
  As a reminder, this old patchset (that I replied to) enhanced git am -s
  with an option to add different signatures depending on
  the option passed to the -s flag.
  E.g. I have
  [am a]
  signoff = Acked-by: Michael S. Tsirkin m...@redhat.com
 
  [am r]
  signoff = Reviewed-by: Michael S. Tsirkin m...@redhat.com
 
  [am t]
  signoff = Tested-by: Michael S. Tsirkin m...@redhat.com
 
  and now:
  git am -s art
  adds all 3 signatures when applying the patch.
 
  This is probably not as simple as you would like but it works with
  something like:
 
  $ git interpret-trailers --trailer Acked-by: Michael S. Tsirkin
  m...@redhat.com --trailer Reviewed-by: Michael S. Tsirkin
  m...@redhat.com  --trailer Tested-by: Michael S. Tsirkin
  m...@redhat.com 0001-foo.patch to_apply/0001-foo.patch
 
  and then:
 
  $ git am to_apply/*.patch
 
 If I understand it correctly, Michael is envisioning to implement
 his git am -s art (I would recommend against reusing -s for this,
 though.  git am --trailer art is fine) and doing so by using
 interpret-trailers as an internal implementation detail, so I would
 say the above is a perfectly fine way to do so.  An equivalent of
 that command line is synthesized and run internally in his version
 of git am when his git am sees --trailer art option using
 those am.{a,r,t}.trailer configuration variables.

Hmm I wonder why do you dislike reusing -s with a parameter for this.
To me, this looks like a superset of the default -s functionality: -s
adds the default signature, -s x adds signature x ...  Users don't
really care that one is implemented as a trailer and another isn't.  In
fact, default -s can be implemented as a trailer too, right?

Could you clarify please?

-- 
MST
--
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 08/16] t5304: use helper to report failure of test foo = bar

2014-10-07 Thread Jeff King
On Tue, Oct 07, 2014 at 01:35:15PM -0700, Junio C Hamano wrote:

 Yeah, if we are going to reduce it down to the above implementation,
 intereseting things like test -f $frotz will become possible and
 cmp-str stops making sense.  It really is about We run test and
 expect it to yield true.  Report the failure a bit more prominently
 under the '-v' option to help us debug.

We already have test_path_is_file to do the same thing just for -f. We
could in theory switch all of those to this new, more generic wrapper. I
don't know if it is worth doing a mass-conversion, but we could
discourage test_path_is_file in new tests. We could also implement
test_path_is_{dir,file} on top of this.

There is also test_path_is_missing, which would need the negated form.
We'd either need a test_not_cond, or to allow:

  test_cond ! -e path

That is specified by POSIX. I seem to recall that we ran into
problems using it with some shells, but I note there is currently some
use of it in t5304. So perhaps it is fine.

 So among the ones you listed, test_verbose may be the least silly, I
 would think.

Somehow test_verbose seems to me like checking the verbose option of
the test suite. I prefer test_cond, but I do not feel too strongly, if
you want to override me.

  (any quoting or whitespace may be wrong, of course, but that's OK; it's
  for human consumption, and is just a hint).
 
 If we really cared, we could do
 
   echo 2 command failed: test $(git rev-parse --sq-quote $@)
 
 perhaps?

Yeah, that would work. I am always a little hesitant sticking git
commands into our test infrastructure, since we may end up masking
errors due to our own bug. But we can probably rely on --sq-quote
working sanely (and anyway, we're not even affecting the test outcome
here).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC] git-am: support any number of signatures

2014-10-07 Thread Michael S. Tsirkin
On Wed, Sep 24, 2014 at 12:00:40PM +0200, Christian Couder wrote:
 On Tue, Sep 23, 2014 at 10:07 AM, Michael S. Tsirkin m...@redhat.com wrote:
  On Tue, Sep 23, 2014 at 09:45:50AM +0200, Christian Couder wrote:
  This is probably not as simple as you would like but it works with
  something like:
 
  $ git interpret-trailers --trailer Acked-by: Michael S. Tsirkin
  m...@redhat.com --trailer Reviewed-by: Michael S. Tsirkin
  m...@redhat.com  --trailer Tested-by: Michael S. Tsirkin
  m...@redhat.com 0001-foo.patch to_apply/0001-foo.patch
 
  and then:
 
  $ git am to_apply/*.patch
 
  Also by using something like:
 
  $ git config trailer.a.key Acked-by
  $ git config trailer.r.key Reviewed-by
  $ git config trailer.t.key Tested-by
 
  I would like multiple keys to match a specific
  letter, e.g. as a maintainer I need
  both reviewed by and signed off by when I
  apply a patch, I like applying them with
  a single -s m.
 
 That's different from what you implemented in your patch.
 And franckly I think that for this kind of specific use cases, you
 could create your own aliases, either Git aliases or just shell
 aliases.
 
 For example if we implement default values and make git am call git
 interpret-trailers, a shell alias could simply be:
 
 alias gamsm='git am --trailer r --trailer s'
 
 I use git log --oneline --decorate --graph very often, so I made my
 own alias for it, and I suppose a lot of other people have done so.
 
 The number of people who will use trailers will probably be much
 smaller than the number of people using git log, so if we don't make
 shortcuts for git log --oneline --decorate --graph, I see no ground
 to ask for a specific shortcut that adds both a reviewed by and a
 signed off by.

I've been thinking: how about a generic ability to add option shortcuts
for commands in .config?
For example:

[am -z]
command = --trailer foobar

would replace any -z in git am command line with --trailer foobar.


Does this sound useful?


  the first command could be simplified to:
 
  $ git interpret-trailers --trailer a: Michael S. Tsirkin
  m...@redhat.com --trailer r: Michael S. Tsirkin m...@redhat.com
  --trailer t: Michael S. Tsirkin m...@redhat.com 0001-foo.patch
  to_apply/0001-foo.patch
 
  And if you use an env variable:
 
  $ ME=Michael S. Tsirkin m...@redhat.com
  $ git interpret-trailers --trailer a: $ME --trailer r: $ME
  --trailer t: $ME 0001-foo.patch to_apply/0001-foo.patch
 
  Maybe later we will integrate git interpret-trailers with git commit,
  git am and other commands, so that you can do directly:
 
  git am --trailer a: $ME --trailer r: $ME  --trailer t: $ME 
  0001-foo.patch
 
  Maybe we wil also assign a one letter shortcut to --trailer, for
  example z, so that could be:
 
  git am -z a: $ME -z r: $ME  -z t: $ME 0001-foo.patch
 
  -s could apply here, right?
 
 I don't know what we will do with -s. Maybe if we use -z, we don't need -s.
 
  It doesn't have a parameter at the moment.
 
 We will have to discuss that kind of thing when we make it possible
 for git commit, git am and maybe other commands to accept trailers
 arguments and pass them to git interpret-trailers.
 
 In his email Junio seems to say that we don't need a shortcut like -z,
 we could only have --trailer.
 And I think that it is indeed sound to at least wait a little before
 using up one shortcut like -z in many commands.
 
  We could also allow many separators in the same -z argument as long as
  they are separated by say ~,
 
  I think -z a -z r -z t is enough.
 
 Great! I think you will likely have at least --trailer a --trailer r
 --trailer t, but I don't think it is too bad as you can use aliases
 to make it shorter to type.
 
  so you could have:
 
  git am -z a: $ME~r: $ME~t: $ME 0001-foo.patch
 
  And then we could also allow people to define default values for
  trailers with something like:
 
  $ git config trailer.a.defaultvalue Michael S. Tsirkin m...@redhat.com
  $ git config trailer.r.defaultvalue Michael S. Tsirkin m...@redhat.com
  $ git config trailer.t.defaultvalue Michael S. Tsirkin m...@redhat.com
 
  I'm kind of confused by the key/value concept.
 
 A defaultvalue would be the value used when no value is passed.
 The key is just what we will use in the first part of the trailer
 (the part before the separator).
 
 For example with the above defaultvalue and key, --trailer a:
 Junio gits...@pobox.com would add:
 
 Acked-by: Junio gits...@pobox.com
 
 while --trailer a would add:
 
 Acked-by: Michael S. Tsirkin m...@redhat.com
 
  Can't I define the whole 'Acked-by: Michael S. Tsirkin m...@redhat.com'
  string as the key?
 
 The whole 'Acked-by: Michael S. Tsirkin m...@redhat.com' is a full
 trailer, not a key.
 
 And it is not possible right now to define a full trailer. Maybe we
 could find a way to make it possible, but a default value and a way to
 have a small nickname for the token (like a for Acked-by) should
 get people quite far. And then for very specific use cases, 

Re: [PATCH] git-merge: mutually match SYNOPSIS and usage.

2014-10-07 Thread Junio C Hamano
Sergey Organov sorga...@gmail.com writes:

 Junio C Hamano gits...@pobox.com writes:
 ...
 I was looking at the merge.c code, and that's how it seems to work. You
 can get new semantics without -m, and you can't get old semantics with
 -m, isn't it? It looks like the set of descriptions I produced is
 formally correct.

The thing is, with -m msg we will never fall into the
traditional syntax, hence git merge -m msg msg HEAD commit
appear to be allowed with git merge [options] msg HEAD
commit..., but it is not.

And the inverse is not true (an obvious example is git merge
$branch, even though it does not have -m msg it uses the modern
 common.

So the updated SYNOPSIS is not really helping.

 In other words, I agree with your motivation to call for attention
 that the command behaves differently with and without -m, but I do
 not think that part of the change in this patch achieves it well.

 Any particular suggestion?

I was going to suggest explain how the traditional syntax is
triggered in the DESCRIPTION section, but it turns out that we
already do that.

  The second syntax (msg HEAD commit...) is supported for
  historical reasons. Do not use it from the command line or in
  new scripts. It is the same as git merge -m msg commit

Strictly speaking, I think it is not qute the same---I recall
vaguely that it broke tests if you replace the traditional-style
invocation in 'git pull' with the -m msg syntax, but I do not have
details handy; you may want to try it out if you are interested.

So I would think

SYNOPSIS
git merge [options] commit...
git merge [options] msg HEAD commit...
git merge --abort

should be sufficient, possibly with some clarification on The
second syntax paragraph in the DESCRIPTION section.
--
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] git-merge: implement --ff-only-merge option.

2014-10-07 Thread Junio C Hamano
Sergey Organov sorga...@gmail.com writes:

 Because rebasing immediately before is considered a bad manner,
 i.e. encouraging a wrong workflow?

 Why? What is wrong about it?

Searching the kernel archive for messages that talks about rebase
and pull-request from Linus would tell us why it is frowned upon
in a prominent early adopter project of Git.

You destroy what you have been testing and replace it with an
untested one.  If you merge, and if the result of the merge is
broken, at least you would have something that used to work at its
second parent (i.e. the tip of your topic).

 Please also notice that I don't try to impose this on anybody who does
 consider it wrong workflow.

I know ;-).  I didn't say anything about imposing, did I?

Having an option to make it easy to do something undesirable gives
people an excuse to say See Git has this option to let me do that
easily, it is an officially sanctioned and encouraged workflow.
--
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 RFC] git-am: support any number of signatures

2014-10-07 Thread Jeff King
On Wed, Oct 08, 2014 at 12:29:37AM +0300, Michael S. Tsirkin wrote:

  If I understand it correctly, Michael is envisioning to implement
  his git am -s art (I would recommend against reusing -s for this,
  though.  git am --trailer art is fine) and doing so by using
  interpret-trailers as an internal implementation detail, so I would
  say the above is a perfectly fine way to do so.  An equivalent of
  that command line is synthesized and run internally in his version
  of git am when his git am sees --trailer art option using
  those am.{a,r,t}.trailer configuration variables.
 
 Hmm I wonder why do you dislike reusing -s with a parameter for this.
 To me, this looks like a superset of the default -s functionality: -s
 adds the default signature, -s x adds signature x ...  Users don't
 really care that one is implemented as a trailer and another isn't.  In
 fact, default -s can be implemented as a trailer too, right?
 
 Could you clarify please?

Optional parameters for arguments make backwards-compatibility tricky.
In this case, the command:

  git am -s mbox1 mbox2

means apply the patches from mbox1 and mbox2, and signoff the patches.
Under your scheme, it now means apply from mbox2, and use the trailer
mbox1.

I think it would make more sense for -s to use a trailer called
signoff if it is configured (and if not, have a baked-in signoff
trailer config that behaves like -s does now). So -s (and
--signoff) become sign off in the way I usually do for my project,
not just add a signed-off-by line. If you want to something more
fancy, you have to use --trailer=

Just my two cents, as one who has not been closely following this
discussion. Apologies if this idea was already presented and shot down.
:)

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC] git-am: support any number of signatures

2014-10-07 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Optional parameters for arguments make backwards-compatibility tricky.
 In this case, the command:

   git am -s mbox1 mbox2

 means apply the patches from mbox1 and mbox2, and signoff the patches.
 Under your scheme, it now means apply from mbox2, and use the trailer
 mbox1.

Thanks for saving me from typing ;-)
--
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 08/16] t5304: use helper to report failure of test foo = bar

2014-10-07 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Tue, Oct 07, 2014 at 01:35:15PM -0700, Junio C Hamano wrote:

 Yeah, if we are going to reduce it down to the above implementation,
 intereseting things like test -f $frotz will become possible and
 cmp-str stops making sense.  It really is about We run test and
 expect it to yield true.  Report the failure a bit more prominently
 under the '-v' option to help us debug.

 We already have test_path_is_file to do the same thing just for -f. We
 could in theory switch all of those to this new, more generic wrapper. I
 don't know if it is worth doing a mass-conversion, but we could
 discourage test_path_is_file in new tests. We could also implement
 test_path_is_{dir,file} on top of this.

Oh, I wasn't going in that direction when I mentioned -f; I just
wanted to say that 'test $@' is clearly about 'test' (/bin/test or
shell built-in) and less about 'compare string'.  I do not think it
is necessarily a good direction to go in to replace test-path-is-file
with the test_cond thing; after all, type specific tests have chance
to report breakage of expectation in type specifc ways, e.g.

test_path_is_file () {
test -f $1  return 0
echo 2 expected '$1' to be file
if test -e $1
then
echo 2 but it is missing
else
echo 2 but it is a non-file
ls 2 -ld $1
fi
return 1
}

But that is also just in theory ;-).

 So among the ones you listed, test_verbose may be the least silly, I
 would think.

 Somehow test_verbose seems to me like checking the verbose option of
 the test suite. I prefer test_cond, but I do not feel too strongly, if
 you want to override me.

Hmph, your 'test' in that name is a generic verb we check that...,
which I think aligns better with the other test_foo functions.  When
I suggested 'test_verbose', 'test' in that name was specifically
meant to refer to the 'test' command.

Still test_cond feels somewhat funny, as we check that... always
validates some condition, but I don't think of anything better X-.
--
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 08/16] t5304: use helper to report failure of test foo = bar

2014-10-07 Thread Michael Haggerty
On 10/07/2014 11:53 PM, Junio C Hamano wrote:
 Jeff King p...@peff.net writes:
 
 On Tue, Oct 07, 2014 at 01:35:15PM -0700, Junio C Hamano wrote:

 Yeah, if we are going to reduce it down to the above implementation,
 intereseting things like test -f $frotz will become possible and
 cmp-str stops making sense.  It really is about We run test and
 expect it to yield true.  Report the failure a bit more prominently
 under the '-v' option to help us debug.

 We already have test_path_is_file to do the same thing just for -f. We
 could in theory switch all of those to this new, more generic wrapper. I
 don't know if it is worth doing a mass-conversion, but we could
 discourage test_path_is_file in new tests. We could also implement
 test_path_is_{dir,file} on top of this.
 
 Oh, I wasn't going in that direction when I mentioned -f; I just
 wanted to say that 'test $@' is clearly about 'test' (/bin/test or
 shell built-in) and less about 'compare string'.  I do not think it
 is necessarily a good direction to go in to replace test-path-is-file
 with the test_cond thing; after all, type specific tests have chance
 to report breakage of expectation in type specifc ways, e.g.
 
   test_path_is_file () {
   test -f $1  return 0
   echo 2 expected '$1' to be file
   if test -e $1
 then
   echo 2 but it is missing
   else
   echo 2 but it is a non-file
   ls 2 -ld $1
   fi
 return 1
   }
 
 But that is also just in theory ;-).
 
 So among the ones you listed, test_verbose may be the least silly, I
 would think.

 Somehow test_verbose seems to me like checking the verbose option of
 the test suite. I prefer test_cond, but I do not feel too strongly, if
 you want to override me.
 
 Hmph, your 'test' in that name is a generic verb we check that...,
 which I think aligns better with the other test_foo functions.  When
 I suggested 'test_verbose', 'test' in that name was specifically
 meant to refer to the 'test' command.
 
 Still test_cond feels somewhat funny, as we check that... always
 validates some condition, but I don't think of anything better X-.

I like verbose_test $foo = $bar because it puts the word test next
to the condition, where the built-in command test would otherwise be.

We could even define a command

verbose () {
$@  return 0
echo 2 command failed: $*
return 1
}

and use it like

verbose test $foo = $bar

Somehow I feel like I'm reinventing something that must already exist...

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What's cooking in git.git (Oct 2014, #01; Tue, 7)

2014-10-07 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* mh/lockfile-stdio (2014-10-01) 3 commits
 - commit_packed_refs(): reimplement using fdopen_lock_file()
 - dump_marks(): reimplement using fdopen_lock_file()
 - fdopen_lock_file(): access a lockfile using stdio
 (this branch uses mh/lockfile.)

 Will merge to 'next'.


* rs/daemon-fixes (2014-10-01) 3 commits
  (merged to 'next' on 2014-10-07 at 4171e10)
 + daemon: remove write-only variable maxfd
 + daemon: fix error message after bind()
 + daemon: handle gethostbyname() error

 git daemon (with NO_IPV6 build configuration) used to incorrectly
 use the hostname even when gethostbyname() reported that the given
 hostname is not found.

 Will merge to 'master'.


* rs/sha1-array-test (2014-10-01) 2 commits
 - sha1-lookup: handle duplicates in sha1_pos()
 - sha1-array: add test-sha1-array and basic tests

 Will merge to 'next'.


* da/completion-show-signature (2014-10-07) 1 commit
  (merged to 'next' on 2014-10-07 at 2467c19)
 + completion: add --show-signature for log and show

 Will merge to 'master'.


* jk/prune-mtime (2014-10-04) 18 commits
 - write_sha1_file: freshen existing objects
 - pack-objects: match prune logic for discarding objects
 - pack-objects: refactor unpack-unreachable expiration check
 - prune: keep objects reachable from recent objects
 - sha1_file: add for_each iterators for loose and packed objects
 - count-objects: use for_each_loose_file_in_objdir
 - count-objects: do not use xsize_t when counting object size
 - prune: factor out loose-object directory traversal
 - t5304: use helper to report failure of test foo = bar
 - t5304: use test_path_is_* instead of test -f
 - reachable: clear pending array after walking it
 - clean up name allocation in prepare_revision_walk
 - object_array: add a clear function
 - object_array: factor out slopbuf-freeing logic
 - isxdigit: cast input to unsigned char
 - foreach_alt_odb: propagate return value from callback
 - Merge branch 'dt/cache-tree-repair' into jk/prune-mtime
 - Merge branch 'jc/reopen-lock-file' into jk/prune-mtime
 (this branch uses dt/cache-tree-repair.)

 Expecting a reroll.


* jn/parse-config-slot (2014-10-07) 2 commits
 - color_parse: do not mention variable name in error message
 - pass config slots as pointers instead of offsets

 Expecting an Ack/Sign-off or update from Jonathan on the bottom one.


* rs/mailsplit (2014-10-07) 1 commit
 - mailsplit: remove unnecessary unlink(2) call

 Will merge to 'next'.


* rs/more-uses-of-skip-prefix (2014-10-07) 1 commit
 - use skip_prefix() to avoid more magic numbers

 Will merge to 'next'.


* rs/plug-leak-in-bundle (2014-10-07) 1 commit
 - bundle: plug minor memory leak in is_tag_in_date_range()

 Will merge to 'next'.

--
[Stalled]

* rs/ref-transaction (2014-09-10) 19 commits
 . ref_transaction_commit: bail out on failure to remove a ref
 . lockfile: remove unable_to_lock_error
 . refs.c: do not permit err == NULL
 . for-each-ref.c: improve message before aborting on broken ref
 . refs.c: fix handling of badly named refs
 . branch -d: avoid repeated symref resolution
 . refs.c: change resolve_ref_unsafe reading argument to be a flags field
 . refs.c: make write_ref_sha1 static
 . fetch.c: change s_update_ref to use a ref transaction
 . refs.c: ref_transaction_commit: distinguish name conflicts from other errors
 . refs.c: pass a skip list to name_conflict_fn
 . refs.c: call lock_ref_sha1_basic directly from commit
 . refs.c: move the check for valid refname to lock_ref_sha1_basic
 . rename_ref: don't ask read_ref_full where the ref came from
 . refs.c: pass the ref log message to _create/delete/update instead of _commit
 . refs.c: add an err argument to delete_ref_loose
 . wrapper.c: add a new function unlink_or_msg
 . wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success
 . mv test: recreate mod/ directory instead of relying on stale copy

 Expecting the final reroll.


* tr/remerge-diff (2014-09-08) 8 commits
 - log --remerge-diff: show what the conflict resolution changed
 - name-hash: allow dir hashing even when !ignore_case
 - merge-recursive: allow storing conflict hunks in index
 - merge_diff_mode: fold all merge diff variants into an enum
 - combine-diff: do not pass revs-dense_combined_merges redundantly
 - merge-recursive: -Xindex-only to leave worktree unchanged
 - merge-recursive: internal flag to avoid touching the worktree
 - merge-recursive: remove dead conditional in update_stages()

 log -p output learns a new way to let users inspect a merge
 commit by showing the differences between the automerged result
 with 

Re: [PATCH v4] MinGW(-W64) compilation

2014-10-07 Thread Thomas Braun
Am 30.09.2014 um 09:02 schrieb Marat Radchenko:
 This patch series fixes building on modern MinGW and MinGW-W64 (including 
 x86_64!).
 
 *Compilation* tested on:
  - MSVC
  - msysGit environment (twice)

Hi Marat,

I wanted to verify that on msysgit but some patches fail to apply
cleanly. Did you also had to tweak the patches?
If yes, are these tweaked patches still available somewhere?

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


[PATCH] completion: ignore chpwd_functions when cding

2014-10-07 Thread Brandon Turner
Software, such as RVM (ruby version manager), may set chpwd functions
that result in an endless loop when cding.  chpwd functions should be
ignored.

Signed-off-by: Brandon Turner b...@brandonturner.net
---
For an example of this bug, see:
https://github.com/wayneeseguin/rvm/issues/3076

 contrib/completion/git-completion.bash | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 06bf262..996de31 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -283,7 +283,8 @@ __git_ls_files_helper ()
 {
(
test -n ${CDPATH+set}  unset CDPATH
-   cd $1
+   (( ${#chpwd_functions} ))  chpwd_functions=()
+   builtin cd $1
if [ $2 == --committable ]; then
git diff-index --name-only --relative HEAD
else
-- 
2.1.2

--
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] MinGW(-W64) compilation

2014-10-07 Thread Marat Radchenko
On Wed, Oct 08, 2014 at 01:09:20AM +0200, Thomas Braun wrote:
 Am 30.09.2014 um 09:02 schrieb Marat Radchenko:
  This patch series fixes building on modern MinGW and MinGW-W64 (including 
  x86_64!).
  
  *Compilation* tested on:
   - MSVC
   - msysGit environment (twice)
 
 Hi Marat,
 
 I wanted to verify that on msysgit but some patches fail to apply
 cleanly. Did you also had to tweak the patches?
 If yes, are these tweaked patches still available somewhere?

msysgit != git-for-windows, as msysgit folks say. msysgit is a development
environment for git-for-windows.

I tested my patches by applying them to git.git/master and building
inside msysgit.
--
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/RFC 0/5] add unset.variable for unsetting previously set variables

2014-10-07 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 Jakub Narębski jna...@gmail.com writes:

 Junio C Hamano wrote:

   - [config] safe = section.variable will list variables that can
 be included with the config.safeInclude mechanism.  Any variable
 that is not marked as config.safe that appears in the file
 included by the config.safeInclude mechanism will be ignored.

 Why user must know which variables are safe, why it cannot be left to
 Git to know which configuration variables can call external scripts?

 That's a fallback to let them take responsibility for variables we
 do not mark as safe; and having that fallback mechanism lets us
 keep the set of variables we by default mark as safe to the absolute
 minimum.

Perhaps this would need a way to say this value is safe for this
variable too. I don't have a real use-case, but one could say something
like I'm OK with the file overriding core.editor, but the only values I
accept are nano, vim and emacs.

It doesn't seem to be a prerequisite to implement the safeInclude
feature, but we should live room in the namespace for the day we want to
add it.

I don't have really good idea for it. The first I could think of was

[config safe]
core.editor = nano
core.editor = vim
core.editor = emacs

but it's not accepted by the current parser, hence not backward
compatible.

Emacs has such mechanism for -*- ... -*- local variables in files for
example.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html