Re: [PATCH v3 2/3] relative_path should honor DOS and UNC paths

2013-09-18 Thread Jiang Xin
2013/9/18 Junio C Hamano gits...@pobox.com:
 Jiang Xin worldhello@gmail.com writes:

 diff --git a/compat/mingw.h b/compat/mingw.h
 index bd0a88b..06e9f49 100644
 --- a/compat/mingw.h
 +++ b/compat/mingw.h
 @@ -311,6 +311,15 @@ int winansi_fprintf(FILE *stream, const char *format, 
 ...) __attribute__((format

  #define has_dos_drive_prefix(path) (isalpha(*(path))  (path)[1] == ':')
  #define is_dir_sep(c) ((c) == '/' || (c) == '\\')
 +static inline int is_unc_path(const char *path)
 +{
 + if (!is_dir_sep(*path) || !is_dir_sep(*(path+1)) || 
 is_dir_sep(*(path+2)))
 + return 0;

A UNC path must start with two slashes, but not three or more slashes.


 If path[1] == '\0', it would be !is_dir_sep() and we end up
 inspecting past the end of the string?

The funciton is_unc_path will return false (0), if path is
, /, //, ///three/slashes/, or /usr/local.
So the problem is ?

-- 
Jiang Xin
--
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-compat-util: Avoid strcasecmp() being inlined

2013-09-18 Thread Sebastian Schuberth
On Tue, Sep 17, 2013 at 11:46 PM, Junio C Hamano gits...@pobox.com wrote:

 I don't think people on other platforms seeing the ugliness is really
 an issue. After all, the file is called git-*compat*-util.h;

 Well, judging from the way Linus reacted to the patch, I'd have to
 disagree.  After all, that argument leads to the position that
 nothing is needed in compat/, no?

My feeling is that Linus' reaction was more about that this
work-around is even necessary (and MinGW is buggy) rather than
applying it to git-compat-util.h and not elsewhere.

 One ugliness (lack of sane strcasecmp definition whose address can
 be taken) specific to mingw is worked around in compat/mingw.h, and
 another ugliness that some people may use compilers without include_next
 may need help from another configuration in the Makefile to tell it
 where the platform string.h resides.  I am not sure why you see it
 as a problem.

I just don't like that the ugliness is spreading out and requires a
change to config.mak.uname now, too. Also, I regard the change to
config.mak.uname by itself as ugly, mainly because you would have to
set SYSTEM_STRING_H_HEADER to some path, but that path might differ
from system to system, depending on where MinGW is installed on
Windows.

 I do insist to avoid GCC-ism in C files,...

 To that I tend to agree.  Unconditionally killing inlining for any
 mingw compilation in compat/mingw.h may be the simplest (albeit it
 may be less than optimal) solution.

I tried to put the __NO_INLINE__ stuff in compat/mingw.h but failed,
it involved the need to shuffle includes in git-compat-util.h around
because winsock2.h already seems to include string.h, and I did not
find a working include order. So I came up with the following, do you
like that better?

diff --git a/compat/string_no_inline.h b/compat/string_no_inline.h
new file mode 100644
index 000..51eed52
--- /dev/null
+++ b/compat/string_no_inline.h
@@ -0,0 +1,25 @@
+#ifndef STRING_NO_INLINE_H
+#define STRING_NO_INLINE_H
+
+#ifdef __MINGW32__
+#ifdef __NO_INLINE__
+#define __NO_INLINE_ALREADY_DEFINED
+#else
+#define __NO_INLINE__ /* do not inline strcasecmp() */
+#endif
+#endif
+
+#include string.h
+#ifdef HAVE_STRINGS_H
+#include strings.h /* for strcasecmp() */
+#endif
+
+#ifdef __MINGW32__
+#ifdef __NO_INLINE_ALREADY_DEFINED
+#undef __NO_INLINE_ALREADY_DEFINED
+#else
+#undef __NO_INLINE__
+#endif
+#endif
+
+#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index db564b7..348dd55 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,6 +85,8 @@
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1

+#include compat/string_no_inline.h
+
 #ifdef WIN32 /* Both MinGW and MSVC */
 #ifndef _WIN32_WINNT
 #define _WIN32_WINNT 0x0502
@@ -101,10 +103,6 @@
 #include stddef.h
 #include stdlib.h
 #include stdarg.h
-#include string.h
-#ifdef HAVE_STRINGS_H
-#include strings.h /* for strcasecmp() */
-#endif
 #include errno.h
 #include limits.h
 #ifdef NEEDS_SYS_PARAM_H
-- 
1.8.3.mingw.1.dirty

-- 
Sebastian Schuberth
--
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: Locking files / git

2013-09-18 Thread Nicolas Adenis-Lamarre
Thanks a lot for your answer.
That's really good arguments that i was waiting for and that i have
not get until now.

My comprehension now :
- it's not easy to maintain several versions of a binary file in parallel.
So basically, it's not recommanded to have complex workflow for binary files.
In case the project has a low number of binary files, it can be handle
by simple communication,
In case the project has a lot of binary files, a simple workflow with
a centralized workflow is recommanded
- git doesn't hate locks, it's just that it's not the layer to
implement it because git is workflow independant. Locks depend on a
centralized server which is directly linked to the workflow.

I'm not trying to implement a such workflow. I'm just curious, reading
a lot of things about git, and trying to understand what is sometimes
called a limitation of git.
A simple line in the documentation to say that locking should be
handled in the upper layer (and it's done for example in gitolite)
because it's dependant of the workflow could help some people looking
about that point.

Thanks a lot for git.

2013/9/17 Fredrik Gustafsson iv...@iveqy.com:
 On Tue, Sep 17, 2013 at 09:45:26PM +0200, Nicolas Adenis-Lamarre wrote:
 Ooops. It seems that each time somebody says these two words together,
 people hate him, and he is scorned by friends and family.

 For the moment, i want a first feedback, an intermediate between
 locking is bad and ok, but i would prefer in the negativ answer
 something with arguments (Take CVS as an example of what not to do;
 if in doubt, make the exact opposite decision. is one), and in the
 positiv answer, good remarks about problems with my implementation
 that could make it better.

 So working with locks and text-files is generally stupid to do with git
 since you don't use git merging capabilities. Working with binary files
 in git is stupid because git doesn't handle them very well because they
 the deltas can't be calculated very good.

 It seems to me that if you need to do locking one of the above scenarios
 is true for you and you should not use git at all.

 However, there's always the case when you've a mixed project with both
 binary and text-files. In that case I believe Jeff gave an excellent answer.

 But think twice, are you using git in a sane way? Even a small binary
 file will result in a huge git repo if it's updated often and the
 project has a long history.

 --
 Med vänliga hälsningar
 Fredrik Gustafsson

 tel: 0733-608274
 e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] git submodule update should give notice when run without init beforehand

2013-09-18 Thread Tay Ray Chuan
On Tue, Sep 17, 2013 at 1:06 AM, Jens Lehmann jens.lehm...@web.de wrote:

Thanks Jens for having a look!

 Am 15.09.2013 19:38, schrieb Tay Ray Chuan:
 When 'update' is run with no path in a repository with uninitialized
 submodules, the program terminates with no output, and zero status code.
 Be more helpful to users by mentioning this.

 [snip] it would be rather nasty to error out on every submodule
 update.

Just to be sure we're on the right page, with this patch, the 'update'
command still exits with status code zero (non-error), so this patch
doesn't make it error out.

 After the 'autoinit' configuration (which lets upstream hint that
 certain submodules should be initialized on clone) has materialzed we
 might want to enable this error for these specific submodules.

That's cool, I'm looking forward to this. Could you point me to
somewhere detailing this?

But in the meantime, on top of the advice.* config, how about having a
submodule.name.ignoreUninit config to disable the message on a
per-submodule basis?

 But in
 any case the error message should contain a hint on how you can get
 rid of the error in case you know what you are doing ;-).

The message does mention that you can throw in an --init to fix the
problem. This hint is similar to what git-submodule prints when a
path is passed (see region at line 807).

-- 
Cheers,
Ray Chuan
--
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-compat-util: Avoid strcasecmp() being inlined

2013-09-18 Thread Linus Torvalds
On Wed, Sep 18, 2013 at 5:43 AM, Sebastian Schuberth
sschube...@gmail.com wrote:

 My feeling is that Linus' reaction was more about that this
 work-around is even necessary (and MinGW is buggy) rather than
 applying it to git-compat-util.h and not elsewhere.

So I think it's an annoying MinGW bug, but the reason I dislike the
no-inline approach is two-fold:

 - it's *way* too intimate with the bug.

   When you have a bug like this, the *last* thing you want to do is
to make sweet sweet love to it, and get really involved with it.

   You want to say Eww, what a nasty little bug, I don't want to have
anything to do with you.

   And quite frankly, delving into the details of exactly *what* MinGW
does wrong, and defining magic __NO_INLINE__ macros, knowing that that
is the particular incantation that hides the MinGW bug, that's being
too intimate. That's simply a level of detail that *nobody* should
ever have to know.

   The other patch (having just a wrapper function) doesn't have those
kinds of intimacy issues. That patch just says MinGW is buggy and
cannot do this function uninlined, so we wrap it. Notice the lack of
detail, and lack of *interest* in the exact particular pattern of the
bug.

The other reason I'm not a fan of the __NO_INLINE__ approach is even
more straightforward:

 - Why should we disable the inlining of everything in string.h (and
possibly elsewhere too - who the hell knows what __NO_INLINE__ will do
to other header files), when in 99% of all the cases we don't care,
and in fact inlining may well be good and the right thing to do.

So the __NO_INLINE__ games seem to be both too big of a hammer, and
too non-specific, and at the same time it gets really intimate with
MinGW in unhealthy ways.

If you know something is diseased, you keep your distance, you don't
try to embrace it.

 I tried to put the __NO_INLINE__ stuff in compat/mingw.h but failed,
 it involved the need to shuffle includes in git-compat-util.h around
 because winsock2.h already seems to include string.h, and I did not
 find a working include order. So I came up with the following, do you
 like that better?

Ugh, so now that patch is fragile, so we have to complicate it even more.

Really, just make a wrapper function. It doesn't even need to be
conditional on MinGW. Just a single one-liner function, with a comment
above it that says MinGW is broken and doesn't have an out-of-line
copy of strcasecmp(), so we wrap it here.

No unnecessary details about internal workings of a buggy MinGW header
file. No complexity. No subtle issues with include file ordering. Just
a straightforward workaround that is easy to explain.

Linus
--
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 np/pack-v4] index-pack: tighten object type check based on pack version

2013-09-18 Thread Nguyễn Thái Ngọc Duy
In pack version 4, ref-delta technically could be used to compress any
objects including commits and trees (both canonical and v4). But it
does not make sense to do so. It can only lead to performance
degradation. Catch those packers.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 I could now verify that pack-objects does not compress commits nor
 trees using ref-delta in v4. But perhaps these are a bit too strict?
 Maybe downgrade from die() to warning() and still accept the pack?

 builtin/index-pack.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index fbf97f0..e4ecf69 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -788,6 +788,12 @@ static void *unpack_raw_entry(struct object_entry *obj,
case OBJ_BLOB:
case OBJ_TAG:
break;
+
+   /*
+* OBJ_PV4_* are all greater than 7, which is the limit of
+* field type in pack v2. So we do not really need 'if
+* (!packv4) die(wrong type);' here.
+*/
case OBJ_PV4_COMMIT:
obj-real_type = OBJ_COMMIT;
break;
@@ -1288,6 +1294,16 @@ static void resolve_delta(struct object_entry *delta_obj,
hash_sha1_file(result-data, result-size,
   typename(delta_obj-real_type), delta_obj-idx.sha1);
check_against_sha1table(delta_obj-idx.sha1);
+   if (packv4 
+   delta_obj-type == OBJ_REF_DELTA 
+   delta_obj-real_type == OBJ_TREE)
+   bad_object(delta_obj-idx.offset,
+  _(ref-delta on a tree is not supported in pack 
version 4));
+   if (packv4 
+   delta_obj-type == OBJ_REF_DELTA 
+   delta_obj-real_type == OBJ_COMMIT)
+   bad_object(delta_obj-idx.offset,
+  _(ref-delta on a commit is not supported in pack 
version 4));
sha1_object(result-data, NULL, result-size, delta_obj-real_type,
delta_obj-idx.sha1);
counter_lock();
-- 
1.8.2.83.gc99314b

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


[BUG] git clone -q ends with early EOF

2013-09-18 Thread Marek Vasut
Hello,

I am trying to clone a repository and I am getting the following output:

$ git clone -q git://kernel.ubuntu.com/ubuntu/linux.git
fatal: The remote end hung up unexpectedly
fatal: early EOF
fatal: index-pack failed

The fatal: lines usually appear a few minutes after running the clone. Of 
course, the clone does not finish successfully. Interestingly, when I drop the 
'-q' option from the git clone commandline, the clone finishes correctly.

My git version is:

$ git --version
git version 1.8.4.rc3

Is this a known issue?

Thank you!

Best regards,
Marek Vasut
--
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] fetch-pack.c: show correct command name that fails

2013-09-18 Thread Nguyễn Thái Ngọc Duy
When --shallow-file is added to the command line, it has to be
before the subcommand name, the first argument won't be the command
name any more. Stop assuming that and keep track of the command name
explicitly.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 fetch-pack.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 6684348..b259c51 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -688,7 +688,7 @@ static int get_pack(struct fetch_pack_args *args,
const char *argv[22];
char keep_arg[256];
char hdr_arg[256];
-   const char **av;
+   const char **av, *cmd_name;
int do_keep = args-keep_pack;
struct child_process cmd;
int ret;
@@ -735,7 +735,7 @@ static int get_pack(struct fetch_pack_args *args,
if (do_keep) {
if (pack_lockfile)
cmd.out = -1;
-   *av++ = index-pack;
+   *av++ = cmd_name = index-pack;
*av++ = --stdin;
if (!args-quiet  !args-no_progress)
*av++ = -v;
@@ -752,7 +752,7 @@ static int get_pack(struct fetch_pack_args *args,
*av++ = --check-self-contained-and-connected;
}
else {
-   *av++ = unpack-objects;
+   *av++ = cmd_name = unpack-objects;
if (args-quiet || args-no_progress)
*av++ = -q;
args-check_self_contained_and_connected = 0;
@@ -770,7 +770,7 @@ static int get_pack(struct fetch_pack_args *args,
cmd.in = demux.out;
cmd.git_cmd = 1;
if (start_command(cmd))
-   die(fetch-pack: unable to fork off %s, argv[0]);
+   die(fetch-pack: unable to fork off %s, cmd_name);
if (do_keep  pack_lockfile) {
*pack_lockfile = index_pack_lockfile(cmd.out);
close(cmd.out);
@@ -782,7 +782,7 @@ static int get_pack(struct fetch_pack_args *args,
args-check_self_contained_and_connected 
ret == 0;
else
-   die(%s failed, argv[0]);
+   die(%s failed, cmd_name);
if (use_sideband  finish_async(demux))
die(error in sideband demultiplexer);
return 0;
-- 
1.8.2.83.gc99314b

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


Re: [PATCH 1/2] relative_path should honor dos_drive_prefix

2013-09-18 Thread Torsten Bögershausen
On 2013-09-17 21.32, Johannes Sixt wrote:
 Am 17.09.2013 10:24, schrieb Jiang Xin:
 I have checked the behavior of UNC path on Windows (msysGit):

 * I can cd to a UNC path:

 cd //server1/share1/path

 * can cd to other share:

 cd ../../share2/path

 * and can cd to other server's share:

 cd ../../../server2/share/path

 That means relative_path(path1, path2) support UNC paths out of the box.
 We only need to check both path1 and path2 are UNC paths, or both not.
 
 Your tests are flawed. You issued the commands in bash, which (or rather
 MSYS) does everything for you that you need to make it work. But in
 reality it does not, because the system cannot apply .. to //server/share:
 
 $ git ls-remote //srv/public/../repos/his/setups.git
 fatal: '//srv/public/../repos/his/setups.git' does not appear to be a
 git repository
 fatal: Could not read from remote repository.
 
 Please make sure you have the correct access rights
 and the repository exists.
 
 even though the repository (and //srv/public, let me assure) exists:
 
 $ git ls-remote //srv/repos/his/setups.git
 bea489b0611a72c41f133343fdccbd3e2b9f80b5HEAD
 ...
 
 The situation does not change with your latest round (v3).
 
 Please let me suggest not to scratch where there is no itch. ;) Your
 round v2 was good enough.
 
 If you really want to check UNC paths, then you must compare two path
 components after the the double-slash, not just one.
 
 Furthermore, you should audit all code that references
 is_absolute_path(), relative_path(), normalize_path_copy(), and possibly
 a few others whether the functions or call sites need improvement.
 That's worth a separate patch.
 
 -- Hannes

I tend to agree here.
The V2 patch fixed a regression.
This should be one commit on its own:
Documentation/SubmittingPatches:
(1) Make separate commits for logically separate changes.

Fixing a bug is a good thing, thanks for working on this,

The support for UNC paths is a new feature, and this deserves a seperate commit.
/Torsten





--
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 np/pack-v4] index-pack: tighten object type check based on pack version

2013-09-18 Thread Nicolas Pitre
On Wed, 18 Sep 2013, Nguyễn Thái Ngọc Duy wrote:

 In pack version 4, ref-delta technically could be used to compress any
 objects including commits and trees (both canonical and v4). But it
 does not make sense to do so. It can only lead to performance
 degradation. Catch those packers.
 
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  I could now verify that pack-objects does not compress commits nor
  trees using ref-delta in v4. But perhaps these are a bit too strict?
  Maybe downgrade from die() to warning() and still accept the pack?

Even then...  There is a difference between an invalid pack and a 
suboptimal one.  I don't think we should complain when presented with 
suboptimal encoding if it is still valid and there is no problem 
actually processing the data correctly.  You never know when this 
alternative encoding, even if suboptimal, might be handy.

Robustness principle: Be conservative in what you send, be liberal in 
what you accept.


Nicolas


git/path.c - order of accessing ~/.gitconfig

2013-09-18 Thread Madhu

* commit 21cf32279120799a766d22416be7d82d9ecfbd04
|
| Author: Huynh Khoi Nguyen Nguyen huynh-khoi-nguyen.ngu...@ensimag.imag.fr
| Date:   Fri Jun 22 11:03:23 2012 +0200
|
|config: read (but not write) from $XDG_CONFIG_HOME/git/config file
|
|Teach git to read the gitconfig information from a new location,
|$XDG_CONFIG_HOME/git/config; this allows the user to avoid
|cluttering $HOME with many per-application configuration files.
|
|In the order of reading, this file comes between the global
|configuration file (typically $HOME/.gitconfig) and the system wide
|configuration file (typically /etc/gitconfig).


However git/config.c (git_config_early) commit accesses xdg_config
before user_config.  So the comments and documentation are
inconsistent with the code.

[This looks like an intentional bug, I spotted it when commenting out
 the accesses to files under ~/.config.  (protip: chmod 000 ~/.config helps
 identify and blacklist NWO apps and now git started complaining) ---Madhu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/3] relative_path should honor DOS and UNC paths

2013-09-18 Thread Torsten Bögershausen
On 2013-09-18 16.37, Torsten Bögershausen wrote:
 On 2013-09-17 18.12, Junio C Hamano wrote:
 Jiang Xin worldhello@gmail.com writes:

 diff --git a/compat/mingw.h b/compat/mingw.h
 index bd0a88b..06e9f49 100644
 --- a/compat/mingw.h
 +++ b/compat/mingw.h
 @@ -311,6 +311,15 @@ int winansi_fprintf(FILE *stream, const char *format, 
 ...) __attribute__((format
  
  #define has_dos_drive_prefix(path) (isalpha(*(path))  (path)[1] == ':')
  #define is_dir_sep(c) ((c) == '/' || (c) == '\\')
 +static inline int is_unc_path(const char *path)
 +{
 +   if (!is_dir_sep(*path) || !is_dir_sep(*(path+1)) || 
 is_dir_sep(*(path+2)))
 +   return 0;

 If path[1] == '\0', it would be !is_dir_sep() and we end up
 inspecting past the end of the string?
Yes
(If there was a previous mail, it was incomplete, sorry)

I think we want to catch 2 (back)slashes followed by a letter
http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx

#define is_unc_path(path) ((is_dir_sep(path)[0])  is_dir_sep((path)[1])  
isalpha((path[2])))

Then we need 
#define is_relative_path(path)  (((path)[0])  !is_dir_sep((path)[1]))

And may be like this:

static int have_same_root(const char *path1, const char *path2)
{
int is_abs1, is_abs2;

is_abs1 = is_absolute_path(path1);
is_abs2 = is_absolute_path(path2);
if (is_abs1  is_abs2) {
if (is_unc_path(path1)  !is_relative_path(path2))
return 0;
if (!is_relative_path(path1)  is_unc_path(path2))
return 0;
return tolower(path1[0]) == tolower(path2[0]);
} else {
return !is_abs1  !is_abs2;
}
}

Could that work?




--
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/path.c - order of accessing ~/.gitconfig

2013-09-18 Thread Matthieu Moy
Madhu enom...@meer.net writes:

 * commit 21cf32279120799a766d22416be7d82d9ecfbd04
 |
 | Author: Huynh Khoi Nguyen Nguyen huynh-khoi-nguyen.ngu...@ensimag.imag.fr
 | Date:   Fri Jun 22 11:03:23 2012 +0200
 |
 |config: read (but not write) from $XDG_CONFIG_HOME/git/config file
 |
 |Teach git to read the gitconfig information from a new location,
 |$XDG_CONFIG_HOME/git/config; this allows the user to avoid
 |cluttering $HOME with many per-application configuration files.
 |
 |In the order of reading, this file comes between the global
 |configuration file (typically $HOME/.gitconfig) and the system wide
 |configuration file (typically /etc/gitconfig).


 However git/config.c (git_config_early) commit accesses xdg_config
 before user_config.  So the comments and documentation are
 inconsistent with the code.

It seems the commit message is wrong, indeed. But it's too late to fix
it. OTOH, the documentation seems right to me
(Documentation/git-config.txt). It says:

  $(prefix)/etc/gitconfig::
System-wide configuration file.
  
  $XDG_CONFIG_HOME/git/config::
Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
or empty, `$HOME/.config/git/config` will be used. Any single-valued
variable set in this file will be overwritten by whatever is in
`~/.gitconfig`.  It is a good idea not to create this file if
you sometimes use older versions of Git, as support for this
file was added fairly recently.
  
  ~/.gitconfig::
User-specific configuration file. Also called global
configuration file.
  
  $GIT_DIR/config::
Repository specific configuration file.


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


Re: [BUG] git clone -q ends with early EOF

2013-09-18 Thread Junio C Hamano
This sounds suspiciously like 05e95155 (upload-pack: send keepalive
packets during pack computation, 2013-09-08) that is cooking in
'next'.

If you are talking with other people's server side, you are SOL
until the server end gets the patch.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/3] relative_path should honor DOS and UNC paths

2013-09-18 Thread Junio C Hamano
Jiang Xin worldhello@gmail.com writes:

 2013/9/18 Junio C Hamano gits...@pobox.com:
 + if (!is_dir_sep(*path) || !is_dir_sep(*(path+1)) || 
 is_dir_sep(*(path+2)))
 + return 0;
 If path[1] == '\0', it would be !is_dir_sep() and we end up
 inspecting past the end of the string?

 The funciton is_unc_path will return false (0), if path is
 , /, //, ///three/slashes/, or /usr/local.
 So the problem is ?

If path[1] == '\0' (e.g. path=/), !is_dir_sep(path[1]) is true,
not false (as I misread earlier), so we hit an early return and will
not peek path[2].  So no problem.  Sorry for the noise.

But I agree with J6t and Torsten in near-by thread that the simpler
one that does not worry about // should be done as a separate patch
and //, if we decide to do it, should build on top.

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


git clone silently aborts if stdout gets a broken pipe

2013-09-18 Thread Peter Kjellerstedt
One of our Perl scripts that does a git clone suddenly 
started to fail when I upgraded to git 1.8.4 from 1.8.3.1.

The failing Perl code used a construct like this:

Git::command_oneline('clone', $url, $path);

There is no error raised, but the directory specified by 
$path is not created. If I look at the process using strace 
I can see the clone taking place, but then it seems to get 
a broken pipe since the code above only cares about the 
first line from stdout (and with the addition of Checking 
connectivity... git clone now outputs two lines to stdout).

If I change the code to:

my @foo = Git::command('clone', $url, $path);

it works as expected.

I have attached a simple Perl script that shows the problem.
Run it as clone_test.pl git url. With git 1.8.4 it will 
fail for the first two test cases, whereas with older git 
versions it succeeds for all four test cases.

I hope this is enough information for someone to look into 
this regression.

Best regards,
//Peter



clone_test.pl
Description: clone_test.pl


[PATCH] completion: improve untracked directory filtering for filename completion

2013-09-18 Thread SZEDER Gábor
Similar to Bash's default filename completion, our git-aware filename
completion stops at directory boundaries, i.e. it doesn't offer the
full 'path/to/file' at first, but only 'path/'.  To achieve that the
completion script runs 'git ls-files' with specific command line
options to get the list of relevant paths under the current directory,
and then processes each path to strip all but the base directory or
filename (see __git_index_files()).

To offer only modified and untracked files for 'git add' the
completion script runs 'git ls-files --exclude-standard --others
--modified'.  This command lists all non-ignored files in untracked
directories, which leads to a noticeable delay caused by the
processing mentioned above if there are a lot of such files
(__git_index_files() specifies '--exclude-standard' internally):

  $ mkdir untracked-dir
  $ for i in {1..1} ; do untracked-dir/$i ; done
  $ time __git_index_files --others --modified
  untracked-dir

  real  0m0.537s
  user  0m0.452s
  sys   0m0.160s

Eliminate this delay by additionally passing the '--directory
--no-empty-directory' options to 'git ls-files' to show only the
directory name of non-empty untracked directories instead their whole
content:

  $ time __git_index_files --others --modified --directory 
--no-empty-directory
  untracked-dir

  real  0m0.029s
  user  0m0.020s
  sys   0m0.004s

Filename completion for 'git clean' suffers from the same delay, as it
offers untracked files, too.  The fix could be the same, but since it
actually makes sense to 'git clean' empty directories, in this case we
only pass the '--directory' option to 'git ls-files'.

Reported-by: Isaac Levy il...@google.com
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
 contrib/completion/git-completion.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e1b7313072..86f77345fd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -901,7 +901,7 @@ _git_add ()
esac
 
# XXX should we check for --update and --all options ?
-   __git_complete_index_file --others --modified
+   __git_complete_index_file --others --modified --directory 
--no-empty-directory
 }
 
 _git_archive ()
@@ -1063,7 +1063,7 @@ _git_clean ()
esac
 
# XXX should we check for -x option ?
-   __git_complete_index_file --others
+   __git_complete_index_file --others --directory
 }
 
 _git_clone ()
-- 
1.8.4.366.g16e4e67

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


Git pack v4: next step, help required

2013-09-18 Thread Nicolas Pitre

I think the pack v4 format and compatibility code is pretty stable now, 
and probably as optimal as it can reasonably be.

@Junio: might be a good idea to refresh your pu branch again.

Now the biggest problem to solve is the actual tree 
deltification.  I don't have the time to spend on this otherwise very 
interesting problem (IMHO at least) so I'm sending this request for help 
in the hope that more people would be keen to contribute their computing 
skills to solve this challenge.

I'll make a quick description of the pv4 tree encoding first and explain 
the problem to solve afterwards.

A pv4 tree is comprised of two types of records: immediate tree entry 
and tree sequence copy.

1) Immediate Tree Entry

This is made of the following tuple:

path component index,{SHA1 reference index

The path component index refers to the path dictionary table where path 
strings and file modes are uniquely stored.  The SHA1 index refers to 
the sorted SHA1 table as previously found in the pack index but which is 
now part of the pack file itself.  So on average a single tree entry may 
take between 1 to 2 bytes for the path component index and 1 to 3 bytes 
for the SHA1 index.

2) Tree Sequence Copy

This is used to literally copy a contiguous list of tree entries from 
another tree object.  This goes as follows:

tree entry where to start,number of entries to copy
[,SHA1 index of the object to copy from]

So instead of having arbitrary copying of data like in delta objects, we 
refer directly to tree entries in another object.  The big advantage 
here is that the tree walker may directly jump to the copied object 
without having to do any delta patching and caching. The SHA1 index is 
optional if it refers to the same copied object as the previous tree 
sequence copy record in this tree object.  And another possible 
optimization for the tree walker when enumerating objects is to skip the 
copy entry entirely if the object being copied from has already been 
enumerated.

The size of this entry is more variable and is typically between 1 to 2 
bytes for the start index, 1 to 2 bytes for the copy size, and 0 to 3 
bytes for the SHA1 index.

So... what to do with this?

Currently the deltification is achieved using a pretty dumb heuristic 
which is to simply take each tree object in a pack v2 with their 
corresponding base delta object and perform a straight conversion into 
pack v4 tree format i.e. use copy records whenever possible as long as 
they represent a space saving over the corresponding immediate tree 
entries (which is normally the case).

However this is rather sub-optimal for two reasons:

1) This doesn't benefit from the ability to use multiple base objects to 
   copy from and is potentially missing on additional opportunities for 
   copy sequences which are not possible in the selected base object 
   from the pack v2 delta base selection. Pack v4 is already quite 
   smaller than pack v2 yet it might possibly be smaller still.

2) This makes deep delta chains very inefficient at runtime when the 
   pack is subsequently accessed.

Let's consider this example to fully illustrate #2.

Tree A:
entry 0:foo.txt
entry 1-3:  copy from tree B: start=2 count=3

Tree B:
entry 0-5:  copy from tree C: start=2 count=5
entry 6:bar.txt

Tree C:
entry 0:file_a
entry 1:file_b
entry 2:file_c
entry 3:file_D
entry 4:file_E
entry 5:file_F
entry 6:file_G

This is a good example of what typically happens when the above heuristic 
is applied.  And it is not uncommon to see a long indirection chain of 
copy those 2 entries from that other object sometimes reaching 50 
levels deep where the same 2 (or more) entries require up to 50 object 
hops before they can be obtained.

Obviously here the encoding should be optimized to reduce the chaining 
effect simply by referring to the final object directly.  Hence tree A 
could be encoded as follows:

Tree A:
entry 0:foo.txt
entry 1-3:  copy from tree C: start=4 count=3

The on-disk encoding is likely to be the same size but the runtime 
access is greatly optimized.

Now... instead of trying to do reference simplification by factoring out 
the chain effect, I think a whole new approach to tree delta compression 
for pack v4 should be developed which would also address issue #1 above.

For example, we may try to make each canonical tree objects into 
sequences of hashed tree entries in memory where each tree entry would 
be reduced down to a single CRC32 value (or even Adler32 for speed).  
Each tree object would then be represented by some kind of 32-bit 
character string.  Then it is just a matter of applying substring 
matching algorithms to find the best copy sequences without creating 
reference cycles, weighted by the number 

Re: [PATCH] fetch-pack.c: show correct command name that fails

2013-09-18 Thread Jeff King
On Wed, Sep 18, 2013 at 11:16:15AM -0700, Junio C Hamano wrote:

 I wondered if this is something we can have a test for easily.
 Unlike the --upload-pack option, there is no way for the receiving
 end to choose which index-pack (or unpack-objects) to run, but we
 may force a failure by temporarily futzing with PATH to make a dummy
 git-index-pack that always fails to be found or something silly like
 that.

You can feed a bogus pack, which will cause either to fail reliably. I'm
not sure if that counts as easy though. :) Munging PATH is probably
simpler.

   fetch-pack.c | 10 +-
   1 file changed, 5 insertions(+), 5 deletions(-)

Patch looks good to me, too.

-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] fetch-pack.c: show correct command name that fails

2013-09-18 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 When --shallow-file is added to the command line, it has to be
 before the subcommand name, the first argument won't be the command
 name any more. Stop assuming that and keep track of the command name
 explicitly.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---

Thanks; this dates back to v1.8.3.2 but it is at the end of an error
path and only for reporting, so it may not be worth merging it down
to the 1.8.3.x maintenance track.

I wondered if this is something we can have a test for easily.
Unlike the --upload-pack option, there is no way for the receiving
end to choose which index-pack (or unpack-objects) to run, but we
may force a failure by temporarily futzing with PATH to make a dummy
git-index-pack that always fails to be found or something silly like
that.

  fetch-pack.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

 diff --git a/fetch-pack.c b/fetch-pack.c
 index 6684348..b259c51 100644
 --- a/fetch-pack.c
 +++ b/fetch-pack.c
 @@ -688,7 +688,7 @@ static int get_pack(struct fetch_pack_args *args,
   const char *argv[22];
   char keep_arg[256];
   char hdr_arg[256];
 - const char **av;
 + const char **av, *cmd_name;
   int do_keep = args-keep_pack;
   struct child_process cmd;
   int ret;
 @@ -735,7 +735,7 @@ static int get_pack(struct fetch_pack_args *args,
   if (do_keep) {
   if (pack_lockfile)
   cmd.out = -1;
 - *av++ = index-pack;
 + *av++ = cmd_name = index-pack;
   *av++ = --stdin;
   if (!args-quiet  !args-no_progress)
   *av++ = -v;
 @@ -752,7 +752,7 @@ static int get_pack(struct fetch_pack_args *args,
   *av++ = --check-self-contained-and-connected;
   }
   else {
 - *av++ = unpack-objects;
 + *av++ = cmd_name = unpack-objects;
   if (args-quiet || args-no_progress)
   *av++ = -q;
   args-check_self_contained_and_connected = 0;
 @@ -770,7 +770,7 @@ static int get_pack(struct fetch_pack_args *args,
   cmd.in = demux.out;
   cmd.git_cmd = 1;
   if (start_command(cmd))
 - die(fetch-pack: unable to fork off %s, argv[0]);
 + die(fetch-pack: unable to fork off %s, cmd_name);
   if (do_keep  pack_lockfile) {
   *pack_lockfile = index_pack_lockfile(cmd.out);
   close(cmd.out);
 @@ -782,7 +782,7 @@ static int get_pack(struct fetch_pack_args *args,
   args-check_self_contained_and_connected 
   ret == 0;
   else
 - die(%s failed, argv[0]);
 + die(%s failed, cmd_name);
   if (use_sideband  finish_async(demux))
   die(error in sideband demultiplexer);
   return 0;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] git clone -q ends with early EOF

2013-09-18 Thread Jeff King
On Wed, Sep 18, 2013 at 02:44:18PM +0200, Marek Vasut wrote:

 I am trying to clone a repository and I am getting the following output:
 
 $ git clone -q git://kernel.ubuntu.com/ubuntu/linux.git
 fatal: The remote end hung up unexpectedly
 fatal: early EOF
 fatal: index-pack failed
 
 The fatal: lines usually appear a few minutes after running the clone. Of 
 course, the clone does not finish successfully. Interestingly, when I drop 
 the 
 '-q' option from the git clone commandline, the clone finishes correctly.

As Junio mentioned, this does seem like the issue that 05e9515
(upload-pack: send keepalive packets during pack computation,
2013-09-08) tries to fix. The server is quiet for a long period while
preparing the pack, and something in the network stack thinks the
connection has timed out and kills it.

One way you can check this is by running:

  time git clone -q git://kernel.ubuntu.com/ubuntu/linux.git

If it's a network timeout, it will die consistently at some nice round
number. In this case, I just ran that command twice, and it died both
times at just a hair over 2 minutes.  So that is probably what is going
on.  There's no proxy on my end, so presumably there is some front-end
reverse proxying happening on the server end, and it has a 2-minute
timeout.

The keepalive patch is not in any released version yet, but we have been
running it in production at GitHub for a few weeks. You may want to file
a support request with the Ubuntu folks asking to pick up the patch, or
to increase the timeouts on their proxies.

-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: git/path.c - order of accessing ~/.gitconfig

2013-09-18 Thread Madhu
[I posted this via gmane (m3mwnac9ob@leonis4.robolove.meer.net)
 but the posting hasn't appeared, so I'm sending this by email..]

* Matthieu Moy vpq8uyuxjfe@anie.imag.fr :
Wrote on Wed, 18 Sep 2013 17:01:57 +0200:
| * commit 21cf32279120799a766d22416be7d82d9ecfbd04
| |In the order of reading, this file comes between the global
| |configuration file (typically $HOME/.gitconfig) and the system wide
| |configuration file (typically /etc/gitconfig).
|
| However git/config.c (git_config_early) commit accesses xdg_config
| before user_config.  So the comments and documentation are
| inconsistent with the code.
|
| It seems the commit message is wrong, indeed. But it's too late to fix
| it. OTOH, the documentation seems right to me
| (Documentation/git-config.txt). It says:

No, The commit message is also right.  xdg_config indeed comes betweeen
git_etc_gitconfig and user_config, as the commit says, I misunderstood
what the commit was saying because it says this in the reverse order.
Sorry, (nothing to see here) ---Madhu
--
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] build: add default configuration

2013-09-18 Thread David Aguilar
Apologies for top post -- anybody have a recommendation for a better app then 
maildroid?

Will this not conflict with folks that supply their own gitconfig?

I like the idea. Docs?  Also, should this not be done in the C side so that we 
don't waste time reading the config, and also prevent users from overriding 
these?

 

-Original Message-
From: Felipe Contreras felipe.contre...@gmail.com
To: git@vger.kernel.org
Cc: git-us...@googlegroups.com, Bráulio Bhavamitra brauli...@gmail.com, 
Felipe Contreras felipe.contre...@gmail.com
Sent: Tue, 17 Sep 2013 8:23 AM
Subject: [PATCH] build: add default configuration

For now simply add a few common aliases.

  co = checkout
  ci = commit
  rb = rebase
  st = status

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Makefile  | 5 -
 gitconfig | 5 +
 2 files changed, 9 insertions(+), 1 deletion(-)
 create mode 100644 gitconfig

diff --git a/Makefile b/Makefile
index 3588ca1..18081bf 100644
--- a/Makefile
+++ b/Makefile
@@ -1010,7 +1010,7 @@ ifndef sysconfdir
 ifeq ($(prefix),/usr)
 sysconfdir = /etc
 else
-sysconfdir = etc
+sysconfdir = $(prefix)/etc
 endif
 endif
 
@@ -1586,6 +1586,7 @@ template_dir_SQ = $(subst ','\'',$(template_dir))
 htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
 prefix_SQ = $(subst ','\'',$(prefix))
 gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
+sysconfdir_SQ = $(subst ','\'',$(sysconfdir))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
@@ -2340,6 +2341,8 @@ install: all
$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
+   $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(sysconfdir_SQ)'
+   $(INSTALL) -m 644 gitconfig '$(DESTDIR_SQ)$(ETC_GITCONFIG_SQ)'
 ifndef NO_GETTEXT
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
(cd po/build/locale  $(TAR) cf - .) | \
diff --git a/gitconfig b/gitconfig
new file mode 100644
index 000..c45d300
--- /dev/null
+++ b/gitconfig
@@ -0,0 +1,5 @@
+[alias]
+   co = checkout
+   ci = commit
+   rb = rebase
+   st = status
-- 
1.8.4-fc

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


Re: git clone silently aborts if stdout gets a broken pipe

2013-09-18 Thread Jeff King
On Wed, Sep 18, 2013 at 06:52:13PM +0200, Peter Kjellerstedt wrote:

 The failing Perl code used a construct like this:
 
   Git::command_oneline('clone', $url, $path);
 
 There is no error raised, but the directory specified by 
 $path is not created. If I look at the process using strace 
 I can see the clone taking place, but then it seems to get 
 a broken pipe since the code above only cares about the 
 first line from stdout (and with the addition of Checking 
 connectivity... git clone now outputs two lines to stdout).

I think your perl script is somewhat questionable, as it is making
assumptions about the output of git-clone, and you would do better to
accept arbitrary-sized output (or better yet, leave stdout pointing to
the user, so they can see the output, which is meant for them).

That being said, the new messages should almost certainly go to stderr.

-- 8 --
Subject: [PATCH] clone: write checking connectivity to stderr

In commit 0781aa4 (clone: let the user know when
check_everything_connected is run, 2013-05-03), we started
giving the user a progress report during clone. However,
since the actual work happens in a sub-process, we do not
use the usual progress code that counts the objects, but
rather just print a message ourselves.

This message goes to stdout via printf, which is unlike
other progress messages (both the eye candy within clone,
and the checking connectivity progress in other commands).
Let's send it to stderr for consistency.

Signed-off-by: Jeff King p...@peff.net
---
 builtin/clone.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index ca3eb68..3c91844 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -551,12 +551,12 @@ static void update_remote_refs(const struct ref *refs,
 
if (check_connectivity) {
if (0 = option_verbosity)
-   printf(_(Checking connectivity... ));
+   fprintf(stderr, _(Checking connectivity... ));
if (check_everything_connected_with_transport(iterate_ref_map,
  0, rm, 
transport))
die(_(remote did not send all necessary objects));
if (0 = option_verbosity)
-   printf(_(done\n));
+   fprintf(stderr, _(done\n));
}
 
if (refs) {
-- 
1.8.4.rc4.16.g228394f

--
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] build: add default configuration

2013-09-18 Thread Felipe Contreras
On Wed, Sep 18, 2013 at 1:13 PM, David Aguilar dav...@gmail.com wrote:
 Apologies for top post -- anybody have a recommendation for a better app then 
 maildroid?

 Will this not conflict with folks that supply their own gitconfig?

You mean people that provide their own ETC_GITCONFIG? If you mean
distributions, their packaging would override /etc/gitconfig, if you
mean people that have already a /etc/gitconfig, packaging systems
usually save the old one so they can solve the conflict manually (e.g.
/etc/gitconfig.pacsave). So no, it would not conflict. If you mean
people that have ~/.gitconfig, then absolutely not, because that one
takes precedence.

Alternatively, we could have a higher level configuration file (e.g.
/usr/share/git/config), but I think that's overkill.

 I like the idea. Docs?  Also, should this not be done in the C side so that 
 we don't waste time reading the config, and also prevent users from 
 overriding these?

But we want them to be easily readable, and possibly allow
distributions to easily modify them.

-- 
Felipe Contreras
--
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: Locking files / git

2013-09-18 Thread Thomas Koch
On Tuesday, September 17, 2013 09:45:26 PM Nicolas Adenis-Lamarre wrote:
 Ooops. It seems that each time somebody says these two words together,
 people hate him, and he is scorned by friends and family.

See the thread intend-to-edit flag for one implementation idea:

http://git.661346.n2.nabble.com/intend-to-edit-flag-td7591127.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git clone silently aborts if stdout gets a broken pipe

2013-09-18 Thread Jeff King
On Wed, Sep 18, 2013 at 02:45:51PM -0400, Jeff King wrote:

 That being said, the new messages should almost certainly go to stderr.
 
 -- 8 --
 Subject: [PATCH] clone: write checking connectivity to stderr
 
 In commit 0781aa4 (clone: let the user know when
 check_everything_connected is run, 2013-05-03), we started
 giving the user a progress report during clone. However,
 since the actual work happens in a sub-process, we do not
 use the usual progress code that counts the objects, but
 rather just print a message ourselves.
 
 This message goes to stdout via printf, which is unlike
 other progress messages (both the eye candy within clone,
 and the checking connectivity progress in other commands).
 Let's send it to stderr for consistency.

Hrm, this actually breaks t5701, which expects clone 2err to print
nothing to stderr.

What should happen here? The message is emulating the usual progress
messages, which are silent when stderr is redirected. So we could
actually use isatty() in the usual way to suppress them. On the other
hand, the point of that suppression is that the regular progress code
produces long output that is not meant to be seen sequentially (i.e., it
is overwritten in the terminal with \r). But this message does not do
so. So we can just tweak t5701 to be more careful about what it is
looking for.

Also, we should arguably give the Cloning into... message the same
treatment. We have printed that to stdout for a very long time, so there
is a slim chance that somebody actually tries to parse it. But I think
they are wrong to do so; we already changed it once (in 28ba96a), and
these days it is internationalized, anyway.

In May of 2012 I posted this patch, but it got overlooked, and I forgot
about it but carried it in my tree since then. Maybe we should apply it
now (it fixes t5701; the checking connectivity patch can come on top,
or even just be squashed in).

-- 8 --
Subject: [PATCH] clone: send diagnostic messages to stderr

Putting messages like Cloning into.. and done on stdout
is un-Unix and uselessly clutters the stdout channel. Send
them to stderr.

We have to tweak two tests to accommodate this:

  1. t5601 checks for doubled output due to forking, and
 doesn't actually care where the output goes; adjust it
 to check stderr.

  2. t5702 is trying to test whether progress output was
 sent to stderr, but naively does so by checking
 whether stderr produced any output. Instead, have it
 look for %, a token found in progress output but not
 elsewhere (and which lets us avoid hard-coding the
 progress text in the test).

Signed-off-by: Jeff King p...@peff.net
---
 builtin/clone.c  | 6 +++---
 t/t5601-clone.sh | 2 +-
 t/t5702-clone-options.sh | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 3c91844..8723a3a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -379,7 +379,7 @@ static void clone_local(const char *src_repo, const char 
*dest_repo)
}
 
if (0 = option_verbosity)
-   printf(_(done.\n));
+   fprintf(stderr, _(done.\n));
 }
 
 static const char *junk_work_tree;
@@ -849,9 +849,9 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
 
if (0 = option_verbosity) {
if (option_bare)
-   printf(_(Cloning into bare repository '%s'...\n), 
dir);
+   fprintf(stderr, _(Cloning into bare repository 
'%s'...\n), dir);
else
-   printf(_(Cloning into '%s'...\n), dir);
+   fprintf(stderr, _(Cloning into '%s'...\n), dir);
}
init_db(option_template, INIT_DB_QUIET);
write_config(option_config);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 0629149..b3b11e6 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -36,7 +36,7 @@ test_expect_success C_LOCALE_OUTPUT 'output from clone' '
 
 test_expect_success C_LOCALE_OUTPUT 'output from clone' '
rm -fr dst 
-   git clone -n file://$(pwd)/src dst output 
+   git clone -n file://$(pwd)/src dst output 21 
test $(grep Clon output | wc -l) = 1
 '
 
diff --git a/t/t5702-clone-options.sh b/t/t5702-clone-options.sh
index 85cadfa..67e170e 100755
--- a/t/t5702-clone-options.sh
+++ b/t/t5702-clone-options.sh
@@ -22,14 +22,14 @@ test_expect_success 'redirected clone -v' '
 test_expect_success 'redirected clone' '
 
git clone file://$(pwd)/parent clone-redirected out 2err 
-   test_must_be_empty err
+   ! grep % err
 
 '
 test_expect_success 'redirected clone -v' '
 
git clone --progress file://$(pwd)/parent clone-redirected-progress \
out 2err 
-   test -s err
+   grep % err
 
 '
 
-- 
1.8.4.rc4.16.g228394f

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

suspected bug(s) in git-cvsexportcommit.perl

2013-09-18 Thread Loyall, David
Hello.

I don't believe that git-cvsexportcommit.perl is working properly.

First off, invocations of git-apply fail on my system.  git apply works.  
(The aforementioned script uses the former.)

Second, please have a look at this output (specific strings have been replaced 
with 'foo')

hobbes@metalbaby:~/src/foo$ git cvsexportcommit -v 
fecc8b4bb3d91d204f4eb3ebd112f6cec6004819
Applying to CVS commit fecc8b4bb3d91d204f4eb3ebd112f6cec6004819 from parent 
695a544fbdcf7e0614c35d1dab9a3eac0cc57b4c
Checking if patch will apply
cvs status: nothing known about `WebContent/WEB-INF/lib/commons-lang-2.6.jar'
cvs status: nothing known about 
`WebContent/WEB-INF/lib/commons-configuration-1.9.jar'
cvs status: nothing known about `JavaSource/foo/ConfigManager.java'
cvs status: nothing known about `JavaSource/config.xml'
Huh? Status 'Unknown' reported for unexpected file 'no file 
commons-lang-2.6.jar'
Huh? Status 'Unknown' reported for unexpected file 'no file 
commons-configuration-1.9.jar'
Huh? Status 'Unknown' reported for unexpected file 'no file ConfigManager.java'
Huh? Status 'Unknown' reported for unexpected file 'no file config.xml'
Applying
stdin:7: trailing whitespace.
?xml version=1.0 encoding=UTF-8 ?
stdin:8: trailing whitespace.
config
stdin:9: trailing whitespace.
    masterthread
stdin:10: trailing whitespace.
    queuemappings
stdin:11: trailing whitespace.
    mapping threadtype=foo 
service=foo action=foo_test /
error: patch failed: JavaSource/foo/QueueMapping.java:67
error: JavaSource/foo/QueueMapping.java: patch does not apply
Context reduced to (2/2) to apply fragment at 43
cannot patch at /usr/lib/git-core/git-cvsexportcommit line 333.
hobbes@metalbaby:~/src/foo$ 

Even after I altered my copy of the script to invoke 'git apply' in two places, 
I still get the erroneous unknown lines.  My suspicion is that it has 
something to do with the $basename stuff, but, who am I kidding? I don't grok 
perl.

I haven't yet explored the error lines after Applying.

So, what do I want?  I want somebody to test git-cvsexportcommit.perl.  If it 
passes tests, then I'd like help creating a test that it does fail. (Perhaps my 
workspace can help.  Perhaps it is as simple as the presence of a new file in a 
subdirectory.)

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


Re: [PATCH 3/3] git submodule update should give notice when run without init beforehand

2013-09-18 Thread Jens Lehmann
Am 18.09.2013 12:12, schrieb Tay Ray Chuan:
 On Tue, Sep 17, 2013 at 1:06 AM, Jens Lehmann jens.lehm...@web.de wrote:
 
 Thanks Jens for having a look!
 
 Am 15.09.2013 19:38, schrieb Tay Ray Chuan:
 When 'update' is run with no path in a repository with uninitialized
 submodules, the program terminates with no output, and zero status code.
 Be more helpful to users by mentioning this.

 [snip] it would be rather nasty to error out on every submodule
 update.
 
 Just to be sure we're on the right page, with this patch, the 'update'
 command still exits with status code zero (non-error), so this patch
 doesn't make it error out.

Ok, sorry for the confusion. But I still think we should not change
the default to print these messages, but make it an opt-in. And the
commit message (and maybe the documentation too) should talk about
the use cases where this makes sense.

 After the 'autoinit' configuration (which lets upstream hint that
 certain submodules should be initialized on clone) has materialzed we
 might want to enable this error for these specific submodules.
 
 That's cool, I'm looking forward to this. Could you point me to
 somewhere detailing this?

Not yet. I'm still wrestling with the autoupdate series, which is
the logical step before autoinit. autoupdate enables to configure
that initialized submodules are updated on checkout, reset, merge
and all the other work tree manipulating commands. autoinit then
also clones the repos of submodules into .git/modules and inits
them in .git/config, so that autoupdate will automagically populate
them. Unfortunately autoupdate is a rather largish series touching
lots of commands and needing tons of tests ...

 But in the meantime, on top of the advice.* config, how about having a
 submodule.name.ignoreUninit config to disable the message on a
 per-submodule basis?

I'd not add such an option unless users request it because it helps
their not-so-terribly-specific use case.

 But in
 any case the error message should contain a hint on how you can get
 rid of the error in case you know what you are doing ;-).
 
 The message does mention that you can throw in an --init to fix the
 problem. This hint is similar to what git-submodule prints when a
 path is passed (see region at line 807).

But for a lot of users that isn't a solution, as they never want to
init it, they want to ignore it. And if you explicitly ask to update
a special submodule, that message is ok.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git clone silently aborts if stdout gets a broken pipe

2013-09-18 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Wed, Sep 18, 2013 at 02:45:51PM -0400, Jeff King wrote:

 That being said, the new messages should almost certainly go to stderr.
 
 -- 8 --
 Subject: [PATCH] clone: write checking connectivity to stderr
 
 In commit 0781aa4 (clone: let the user know when
 check_everything_connected is run, 2013-05-03), we started
 giving the user a progress report during clone. However,
 since the actual work happens in a sub-process, we do not
 use the usual progress code that counts the objects, but
 rather just print a message ourselves.
 
 This message goes to stdout via printf, which is unlike
 other progress messages (both the eye candy within clone,
 and the checking connectivity progress in other commands).
 Let's send it to stderr for consistency.

 Hrm, this actually breaks t5701, which expects clone 2err to print
 nothing to stderr.

Hmm, where in t5701?  Ah, you meant t5702 and possibly t5601.

 What should happen here? The message is emulating the usual progress
 messages, which are silent when stderr is redirected. So we could
 actually use isatty() in the usual way to suppress them. On the other
 hand, the point of that suppression is that the regular progress code
 produces long output that is not meant to be seen sequentially (i.e., it
 is overwritten in the terminal with \r). But this message does not do
 so. So we can just tweak t5701 to be more careful about what it is
 looking for.

I actually think it is long and not meant to be seen sequentially
is a bad classifier; these new messages are also progress report in
that it reports we are now in this phase.  So if I were to vote, I
would say we should apply the same progress-silencing criteria,
preferrably by not checking isatty() again, but by recording the
decision we have already made when squelching the progress during
the transfer in order to make sure they stay consistent.

 Also, we should arguably give the Cloning into... message the same
 treatment. We have printed that to stdout for a very long time, so there
 is a slim chance that somebody actually tries to parse it. But I think
 they are wrong to do so; we already changed it once (in 28ba96a), and
 these days it is internationalized, anyway.

Good thinking.  Please make it so ;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] git clone -q ends with early EOF

2013-09-18 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 The keepalive patch is not in any released version yet, but we have been
 running it in production at GitHub for a few weeks.

That is good to hear; I'd feel safer to bump the scheduled
graduation date to 'master' for the topic in that case.

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


[PATCH 2/2] clone: treat checking connectivity like other progress

2013-09-18 Thread Jeff King
When stderr does not point to a tty, we typically suppress
we are now in this phase progress reporting (e.g., we ask
the server not to send us counting objects and the like).

The new checking connectivity message is in the same vein,
and should be suppressed. Since clone relies on the
transport code to make the decision, we can simply sneak a
peek at the progress field of the transport struct. That
properly takes into account both the verbosity and progress
options we were given, as well as the result of isatty().

Note that we do not set up that progress flag for a local
clone, as we do not fetch using the transport at all. That's
acceptable here, though, because we also do not perform a
connectivity check in that case.

Signed-off-by: Jeff King p...@peff.net
---
Though the last paragraph explains why this is OK, it feels a bit
fragile. I wonder if we should hoist the call to transport_set_verbosity
outside the !is_local conditional. I do not think it would hurt
anything.

 builtin/clone.c  | 4 ++--
 t/t5702-clone-options.sh | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 8723a3a..7c62298 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -550,12 +550,12 @@ static void update_remote_refs(const struct ref *refs,
const struct ref *rm = mapped_refs;
 
if (check_connectivity) {
-   if (0 = option_verbosity)
+   if (transport-progress)
fprintf(stderr, _(Checking connectivity... ));
if (check_everything_connected_with_transport(iterate_ref_map,
  0, rm, 
transport))
die(_(remote did not send all necessary objects));
-   if (0 = option_verbosity)
+   if (transport-progress)
fprintf(stderr, _(done\n));
}
 
diff --git a/t/t5702-clone-options.sh b/t/t5702-clone-options.sh
index d3dbdfe..9e24ec8 100755
--- a/t/t5702-clone-options.sh
+++ b/t/t5702-clone-options.sh
@@ -22,7 +22,8 @@ test_expect_success 'redirected clone does not show progress' 
'
 test_expect_success 'redirected clone does not show progress' '
 
git clone file://$(pwd)/parent clone-redirected out 2err 
-   ! grep % err
+   ! grep % err 
+   test_i18ngrep ! Checking connectivity err
 
 '
 
-- 
1.8.4.rc4.16.g228394f
--
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 clone silently aborts if stdout gets a broken pipe

2013-09-18 Thread Jeff King
On Wed, Sep 18, 2013 at 12:31:23PM -0700, Junio C Hamano wrote:

  Hrm, this actually breaks t5701, which expects clone 2err to print
  nothing to stderr.
 
 Hmm, where in t5701?  Ah, you meant t5702 and possibly t5601.

Yes, sorry, I meant t5702.

 I actually think it is long and not meant to be seen sequentially
 is a bad classifier; these new messages are also progress report in
 that it reports we are now in this phase.  So if I were to vote, I
 would say we should apply the same progress-silencing criteria,
 preferrably by not checking isatty() again, but by recording the
 decision we have already made when squelching the progress during
 the transfer in order to make sure they stay consistent.

Unfortunately that decision is made in the transport code, not by clone
itself. We can cheat and peek at transport-progress after
initializing the transport. That would require some refactoring, though;
we print Cloning into before setting up the transport. And we do not
even tell the transport about our progress options if we are doing a
local clone.

If we wanted to _just_ suppress Checking connectivity (and not
Cloning into...), that's a bit easier. And I could see an argument
that the former is the only one that falls into the progress report
category.

  Also, we should arguably give the Cloning into... message the same
  treatment. We have printed that to stdout for a very long time, so there
  is a slim chance that somebody actually tries to parse it. But I think
  they are wrong to do so; we already changed it once (in 28ba96a), and
  these days it is internationalized, anyway.
 
 Good thinking.  Please make it so ;-)

OK. I've squashed the use stderr patches into one, and added a patch
on top to correctly check the progress flag.

  [1/2]: clone: send diagnostic messages to stderr
  [2/2]: clone: treat checking connectivity like other progress

-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


[PATCH 3/2] clone: always set transport options

2013-09-18 Thread Jeff King
On Wed, Sep 18, 2013 at 04:06:50PM -0400, Jeff King wrote:

 Note that we do not set up that progress flag for a local
 clone, as we do not fetch using the transport at all. That's
 acceptable here, though, because we also do not perform a
 connectivity check in that case.
 
 Signed-off-by: Jeff King p...@peff.net
 ---
 Though the last paragraph explains why this is OK, it feels a bit
 fragile. I wonder if we should hoist the call to transport_set_verbosity
 outside the !is_local conditional. I do not think it would hurt
 anything.

Actually, I think the option-setting in clone is a little bit broken.
Mostly it is just making fragile assumptions that happen to be true
(e.g., that fetching the ref list will never care about the progress
flag), but there are some options that should be respected in both
cases.

I think we should do this on top.

-- 8 --
Subject: [PATCH] clone: always set transport options

A clone will always create a transport struct, whether we
are cloning locally or using an actual protocol. In the
local case, we only use the transport to get the list of
refs, and then transfer the objects out-of-band.

However, there are many options that we do not bother
setting up in the local case. For the most part, these are
noops, because they only affect the object-fetching stage
(e.g., the --depth option).  However, some options do have a
visible impact. For example, giving the path to upload-pack
via -u does not currently work for a local clone, even
though we need upload-pack to get the ref list.

We can just drop the conditional entirely and set these
options for both local and non-local clones. Rather than
keep track of which options impact the object versus the ref
fetching stage, we can simply let the noops be noops (and
the cost of setting the options in the first place is not
high).

The one exception is that we also check that the transport
provides both a get_refs_list and a fetch method. We
will now be checking the former for both cases (which is
good, since a transport that cannot fetch refs would not
work for a local clone), and we tweak the conditional to
check for a fetch only when we are non-local.

Signed-off-by: Jeff King p...@peff.net
---
The diff is rather unreadable, but using show -b reveals the actual
changes.

 builtin/clone.c| 30 ++
 t/t5701-clone-local.sh |  4 
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 7c62298..7ac677d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -884,27 +884,25 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
remote = remote_get(option_origin);
transport = transport_get(remote, remote-url[0]);
 
-   if (!is_local) {
-   if (!transport-get_refs_list || !transport-fetch)
-   die(_(Don't know how to clone %s), transport-url);
+   if (!transport-get_refs_list || (!is_local  !transport-fetch))
+   die(_(Don't know how to clone %s), transport-url);
 
-   transport_set_option(transport, TRANS_OPT_KEEP, yes);
+   transport_set_option(transport, TRANS_OPT_KEEP, yes);
 
-   if (option_depth)
-   transport_set_option(transport, TRANS_OPT_DEPTH,
-option_depth);
-   if (option_single_branch)
-   transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 
1);
+   if (option_depth)
+   transport_set_option(transport, TRANS_OPT_DEPTH,
+option_depth);
+   if (option_single_branch)
+   transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
 
-   transport_set_verbosity(transport, option_verbosity, 
option_progress);
+   transport_set_verbosity(transport, option_verbosity, option_progress);
 
-   if (option_upload_pack)
-   transport_set_option(transport, TRANS_OPT_UPLOADPACK,
-option_upload_pack);
+   if (option_upload_pack)
+   transport_set_option(transport, TRANS_OPT_UPLOADPACK,
+option_upload_pack);
 
-   if (transport-smart_options  !option_depth)
-   
transport-smart_options-check_self_contained_and_connected = 1;
-   }
+   if (transport-smart_options  !option_depth)
+   transport-smart_options-check_self_contained_and_connected = 
1;
 
refs = transport_get_remote_refs(transport);
 
diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index 7ff6e0e..c490368 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -134,4 +134,8 @@ test_expect_success 'cloning a local path with --no-local 
does not hardlink' '
! repo_is_hardlinked force-nonlocal
 '
 
+test_expect_success 'cloning locally respects -u for fetching refs' '
+   test_must_fail git clone 

Re: [BUG] git clone -q ends with early EOF

2013-09-18 Thread Marek Vasut
Hi,

 Jeff King p...@peff.net writes:
  The keepalive patch is not in any released version yet, but we have been
  running it in production at GitHub for a few weeks.
 
 That is good to hear; I'd feel safer to bump the scheduled
 graduation date to 'master' for the topic in that case.
 
 Like tomorrow ;-)

Thanks for explaining guys, I will try to poke the canonical guys with 
reference 
to this thread.

Best regards,
Marek Vasut
--
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: Locking files / git

2013-09-18 Thread Sitaram Chamarty
On 09/18/2013 01:15 AM, Nicolas Adenis-Lamarre wrote:
 Ooops. It seems that each time somebody says these two words together,
 people hate him, and he is scorned by friends and family.
 
 However,
 - gitolite implement it (but gitolite is not git).

No.  It pretends to implement it, for people who absolutely must have
something and are willing to play by the rules.

Quoting from the doc [1], When git is used in a truly distributed
fashion, locking is impossible.

I wrote it as a sort of bandaid, and that is all it is.  Implement is
too kind a word.

regards

sitaram
--
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] shortlog: ignore commits with missing authors

2013-09-18 Thread Jeff King
From: Jeff King p...@peff.net

Most of git's traversals are robust against minor breakages
in commit data. For example, git log will still output an
entry for a commit that has a broken encoding or missing
author, and will not abort the whole operation.

Shortlog, on the other hand, will die as soon as it sees a
commit without an author, meaning that a repository with
a broken commit cannot get any shortlog output at all.

Let's downgrade this fatal error to a warning, and continue
the operation.

We simply ignore the commit and do not count it in the total
(since we do not have any author under which to file it).
Alternatively, we could output some kind of empty record
to collect these bogus commits. It is probably not worth it,
though; we have already warned to stderr, so the user is
aware that such bogosities exist, and any placeholder we
came up with would either be syntactically invalid, or would
potentially conflict with real data.

Signed-off-by: Jeff King p...@peff.net
---
The real-world breakage I saw that caused this was not a missing author
line, but rather a commit that was in proper utf8, but whose encoding
header said utf16. We can tweak the test to reproduce the exact case
by making the sed line:

  sed /^\$/iencoding utf16 commit.tmp broken.tmp

But I think that may be less portable, as systems with iconv that does
not understand utf16 might leave the text intact. Simply removing the
author line, as the test below does, has the same effect.

 builtin/shortlog.c  |  6 --
 t/t4201-shortlog.sh | 16 
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index ae73d17..c226f76 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -127,9 +127,11 @@ void shortlog_add_commit(struct shortlog *log, struct 
commit *commit)
author = buffer + 7;
buffer = eol;
}
-   if (!author)
-   die(_(Missing author: %s),
+   if (!author) {
+   warning(_(Missing author: %s),
sha1_to_hex(commit-object.sha1));
+   return;
+   }
if (log-user_format) {
struct pretty_print_context ctx = {0};
ctx.fmt = CMIT_FMT_USERFORMAT;
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 5493500..4286699 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -172,4 +172,20 @@ test_cmp expect out'
git shortlog HEAD~2..  out 
 test_cmp expect out'
 
+test_expect_success 'shortlog ignores commits with missing authors' '
+   git commit --allow-empty -m normal 
+   git commit --allow-empty -m soon-to-be-broken 
+   git cat-file commit HEAD commit.tmp 
+   sed /^author/d commit.tmp broken.tmp 
+   commit=$(git hash-object -w -t commit --stdin broken.tmp) 
+   git update-ref HEAD $commit 
+   cat expect -\EOF 
+   A U Thor (1):
+ normal
+
+   EOF
+   git shortlog HEAD~2.. actual 
+   test_cmp expect actual
+'
+
 test_done
-- 
1.8.4.rc4.16.g228394f
--
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: Locking files / git

2013-09-18 Thread Sitaram Chamarty
On 09/18/2013 03:42 PM, Nicolas Adenis-Lamarre wrote:
 Thanks a lot for your answer.
 That's really good arguments that i was waiting for and that i have
 not get until now.
 
 My comprehension now :
 - it's not easy to maintain several versions of a binary file in parallel.
 So basically, it's not recommanded to have complex workflow for binary files.
 In case the project has a low number of binary files, it can be handle
 by simple communication,

Yes.  Since you mentioned gitolite in your original post, I assume you
read this caution also in the doc [1]:

Of course, locking by itself is not quite enough. You may still get
into merge situations if you make changes in branches. For best
results you should actually keep all the binary files in their own
branch, separate from the ones containing source code.

The point is that locking and distribution don't go together at all.
The **core** of distributed VCS is the old coding on an airplane
story.  What if someone locks a file after I am in the air, and I manage
to get in a good 4 hours of solid work?

CVCSs can also get into this situation, but to a lesser extent, I think.
At least you won't be able to commit!

[1]: http://gitolite.com/gitolite/locking.html

 In case the project has a lot of binary files, a simple workflow with
 a centralized workflow is recommanded
 - git doesn't hate locks, it's just that it's not the layer to
 implement it because git is workflow independant. Locks depend on a
 centralized server which is directly linked to the workflow.
 
 I'm not trying to implement a such workflow. I'm just curious, reading
 a lot of things about git, and trying to understand what is sometimes
 called a limitation of git.

It's not a limitation of git.  It's a fundamental conflict between the
idea of distributed and what locking necessitates.

 A simple line in the documentation to say that locking should be
 handled in the upper layer (and it's done for example in gitolite)
 because it's dependant of the workflow could help some people looking
 about that point.

For people who don't realise how important the D in DVCS is, and
assume some sort of a central server will always exist, this simple
line won't do.  You'd have to explain all of that.

And for people who do understand it, it's not necessary :-)

 Thanks a lot for git.
 
 2013/9/17 Fredrik Gustafsson iv...@iveqy.com:
 On Tue, Sep 17, 2013 at 09:45:26PM +0200, Nicolas Adenis-Lamarre wrote:
 Ooops. It seems that each time somebody says these two words together,
 people hate him, and he is scorned by friends and family.

 For the moment, i want a first feedback, an intermediate between
 locking is bad and ok, but i would prefer in the negativ answer
 something with arguments (Take CVS as an example of what not to do;
 if in doubt, make the exact opposite decision. is one), and in the
 positiv answer, good remarks about problems with my implementation
 that could make it better.

 So working with locks and text-files is generally stupid to do with git
 since you don't use git merging capabilities. Working with binary files
 in git is stupid because git doesn't handle them very well because they
 the deltas can't be calculated very good.

 It seems to me that if you need to do locking one of the above scenarios
 is true for you and you should not use git at all.

 However, there's always the case when you've a mixed project with both
 binary and text-files. In that case I believe Jeff gave an excellent answer.

 But think twice, are you using git in a sane way? Even a small binary
 file will result in a huge git repo if it's updated often and the
 project has a long history.

 --
 Med vänliga hälsningar
 Fredrik Gustafsson

 tel: 0733-608274
 e-post: iv...@iveqy.com
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
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 pack v4: next step, help required

2013-09-18 Thread Duy Nguyen
On Thu, Sep 19, 2013 at 12:40 AM, Nicolas Pitre n...@fluxnic.net wrote:

 I think the pack v4 format and compatibility code is pretty stable now,
 and probably as optimal as it can reasonably be.

 @Junio: might be a good idea to refresh your pu branch again.

 Now the biggest problem to solve is the actual tree
 deltification.  I don't have the time to spend on this otherwise very
 interesting problem (IMHO at least) so I'm sending this request for help
 in the hope that more people would be keen to contribute their computing
 skills to solve this challenge.

Just so we're clear as I'm involved a bit in packv4 series. I plan to
make the test suite fully pass, then add v4 support to git protocol.
After that I may look into adding multi-base tree support to
index-pack/unpack-objects, and only then either look into this
challenge or continue to add v4-aware tree walker to git. In short,
you you are intereted in Nico's challenge, go ahead, it will not
overlap with my work, at least in the next one or two months.
-- 
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] build: add default configuration

2013-09-18 Thread David Aguilar
On Wed, Sep 18, 2013 at 1:13 PM, David Aguilar dav...@gmail.com wrote:

 Will this not conflict with folks that supply their own gitconfig?

 You mean people that provide their own ETC_GITCONFIG? If you mean
distributions, their packaging would override /etc/gitconfig, if you
mean people that have already a /etc/gitconfig, packaging systems
usually save the old one so they can solve the conflict manually (e.g.
/etc/gitconfig.pacsave). So no, it would not conflict.

Yuck. Yes, that one. I package my own /etc/gitconfig (as we have long 
advertised as the way to do it) and asking users to manually fix up thousands 
of machines is a bad idea. 

Yes, thousands.  We're much past 30,000 cores at the moment. 

 I like the idea. Docs?  Also, should this not be done in the C side so that 
 we don't waste time reading the config, and also prevent users from 
 overriding these?

 But we want them to be easily readable, and possibly allow
distributions to easily modify them.

In that case I take it back -- I dont like that approach.  We want consistency, 
not divergence. This encourages the former. 
-- 
David
--
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