[PATCH 0/5] submodule config lookup API

[Heiko Voigt]

I have been holding back this series during the RC phase but now I think
it is ready for another round. The most important changes:

  * The API is using a singleton now. No need to pass in the cache
object anymore.

  * Local configuration can be looked up by passing in the null_sha1

  * We use the API for existing lookup of submodule values

One open question:

  * Since behind the scenes there is a global cache filled with the
values: Do we need to free it explicitely? Or is it ok to let it be
dealt with on exit?

The last iteration was here:

http://article.gmane.org/gmane.comp.version-control.git/243818

Heiko Voigt (5):
  hashmap: add enum for hashmap free_entries option
  implement submodule config cache for lookup of submodule names
  extract functions for submodule config set and lookup
  use new config API for worktree configurations of submodules
  do not die on error of parsing fetchrecursesubmodules option

 .gitignore   |   1 +
 Documentation/technical/api-hashmap.txt  |   2 +-
 Documentation/technical/api-submodule-config.txt |  63 
 Makefile |   2 +
 builtin/checkout.c   |   1 +
 builtin/fetch.c  |   1 +
 diff.c   |   1 +
 diffcore-rename.c|   2 +-
 hashmap.c|   2 +-
 hashmap.h|   8 +-
 name-hash.c  |   4 +-
 submodule-config.c   | 435 +++
 submodule-config.h   |  29 ++
 submodule.c  | 122 ++-
 submodule.h  |   4 +-
 t/t7410-submodule-config.sh  | 141 
 test-hashmap.c   |   6 +-
 test-submodule-config.c  |  74 
 18 files changed, 791 insertions(+), 107 deletions(-)
 create mode 100644 Documentation/technical/api-submodule-config.txt
 create mode 100644 submodule-config.c
 create mode 100644 submodule-config.h
 create mode 100755 t/t7410-submodule-config.sh
 create mode 100644 test-submodule-config.c

-- 
2.0.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: Git autocorrect bug

[Øystein Walle]

David Turner dturner at twopensource.com writes:

 
 $ cd [some existing git repo]
 $ git git foo
 WARNING: You called a Git command named 'git', which does not exist.
 Continuing under the assumption that you meant 'init'
 in 0.1 seconds automatically...
 fatal: internal error: work tree has already been set
 Current worktree: /home/dturner/git
 New worktree: /home/dturner/git/foo
 
 (I am extremely unlikely to fix this bug myself, since it only arises in
 very rare circumstances).
 
 

You have help.autocorrect enabled. Then this is expected behaviour (it
doesn't seem out of place to me at least)

Running `git config --unset help.autocorrect` should disable it. It
may be that you enabled it for only one repo by accident.

Øsse


--
To unsubscribe from this list: send the line 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 1/5] hashmap: add enum for hashmap free_entries option

[Heiko Voigt]

This allows a reader to immediately know which options can be used and
what this parameter is about.

Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
 Documentation/technical/api-hashmap.txt | 2 +-
 diffcore-rename.c   | 2 +-
 hashmap.c   | 2 +-
 hashmap.h   | 8 +++-
 name-hash.c | 4 ++--
 test-hashmap.c  | 6 +++---
 6 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/Documentation/technical/api-hashmap.txt 
b/Documentation/technical/api-hashmap.txt
index b977ae8..b04bb40 100644
--- a/Documentation/technical/api-hashmap.txt
+++ b/Documentation/technical/api-hashmap.txt
@@ -187,7 +187,7 @@ void long2double_init(void)
 
 void long2double_free(void)
 {
-   hashmap_free(map, 1);
+   hashmap_free(map, HASHMAP_FREE_ENTRIES);
 }
 
 static struct long2double *find_entry(long key)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 749a35d..f30239a 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -335,7 +335,7 @@ static int find_exact_renames(struct diff_options *options)
renames += find_identical_files(file_table, i, options);
 
/* Free the hash data structure and entries */
-   hashmap_free(file_table, 1);
+   hashmap_free(file_table, HASHMAP_FREE_ENTRIES);
 
return renames;
 }
diff --git a/hashmap.c b/hashmap.c
index d1b8056..9a3555a 100644
--- a/hashmap.c
+++ b/hashmap.c
@@ -135,7 +135,7 @@ void hashmap_init(struct hashmap *map, hashmap_cmp_fn 
equals_function,
alloc_table(map, size);
 }
 
-void hashmap_free(struct hashmap *map, int free_entries)
+void hashmap_free(struct hashmap *map, enum hashmap_free_options free_entries)
 {
if (!map || !map-table)
return;
diff --git a/hashmap.h b/hashmap.h
index a816ad4..6c558df 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -1,6 +1,11 @@
 #ifndef HASHMAP_H
 #define HASHMAP_H
 
+enum hashmap_free_options {
+   HASHMAP_NO_FREE_ENTRIES = 0,
+   HASHMAP_FREE_ENTRIES = 1,
+};
+
 /*
  * Generic implementation of hash-based key-value mappings.
  * See Documentation/technical/api-hashmap.txt.
@@ -39,7 +44,8 @@ struct hashmap_iter {
 
 extern void hashmap_init(struct hashmap *map, hashmap_cmp_fn equals_function,
size_t initial_size);
-extern void hashmap_free(struct hashmap *map, int free_entries);
+extern void hashmap_free(struct hashmap *map,
+enum hashmap_free_options free_entries);
 
 /* hashmap_entry functions */
 
diff --git a/name-hash.c b/name-hash.c
index 97444d0..be7c4ae 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -233,6 +233,6 @@ void free_name_hash(struct index_state *istate)
return;
istate-name_hash_initialized = 0;
 
-   hashmap_free(istate-name_hash, 0);
-   hashmap_free(istate-dir_hash, 1);
+   hashmap_free(istate-name_hash, HASHMAP_NO_FREE_ENTRIES);
+   hashmap_free(istate-dir_hash, HASHMAP_FREE_ENTRIES);
 }
diff --git a/test-hashmap.c b/test-hashmap.c
index f5183fb..ac8d6a2 100644
--- a/test-hashmap.c
+++ b/test-hashmap.c
@@ -100,7 +100,7 @@ static void perf_hashmap(unsigned int method, unsigned int 
rounds)
hashmap_add(map, entries[i]);
}
 
-   hashmap_free(map, 0);
+   hashmap_free(map, HASHMAP_NO_FREE_ENTRIES);
}
} else {
/* test map lookups */
@@ -121,7 +121,7 @@ static void perf_hashmap(unsigned int method, unsigned int 
rounds)
}
}
 
-   hashmap_free(map, 0);
+   hashmap_free(map, HASHMAP_NO_FREE_ENTRIES);
}
 }
 
@@ -250,6 +250,6 @@ int main(int argc, char *argv[])
}
}
 
-   hashmap_free(map, 1);
+   hashmap_free(map, HASHMAP_FREE_ENTRIES);
return 0;
 }
-- 
2.0.0

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

[PATCH 2/5] implement submodule config cache for lookup of submodule names

[Heiko Voigt]

This submodule configuration cache allows us to lazily read .gitmodules
configurations by commit into a runtime cache which can then be used to
easily lookup values from it. Currently only the values for path or name
are stored but it can be extended for any value needed.

It is expected that .gitmodules files do not change often between
commits. Thats why we lookup the .gitmodules sha1 from a commit and then
either lookup an already parsed configuration or parse and cache an
unknown one for each sha1. The cache is lazily build on demand for each
requested commit.

This cache can be used for all purposes which need knowledge about
submodule configurations. Example use cases are:

 * Recursive submodule checkout needs lookup a submodule name from its
   path when a submodule first appears. This needs be done before this
   configuration exists in the worktree.

 * The implementation of submodule support for 'git archive' needs to
   lookup the submodule name to generate the archive when given a
   revision that is not checked out.

 * 'git fetch' when given the --recurse-submodules=on-demand option (or
   configuration) needs to lookup submodule names by path from the
   database rather than reading from the worktree. For new submodule it
   needs to lookup the name from its path to allow cloning new
   submodules into the .git folder so they can be checked out without
   any network interaction when the user does a checkout of that
   revision.

Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
 .gitignore   |   1 +
 Documentation/technical/api-submodule-config.txt |  46 +++
 Makefile |   2 +
 submodule-config.c   | 396 +++
 submodule-config.h   |  27 ++
 submodule.c  |   1 +
 submodule.h  |   1 +
 t/t7410-submodule-config.sh  |  73 +
 test-submodule-config.c  |  64 
 9 files changed, 611 insertions(+)
 create mode 100644 Documentation/technical/api-submodule-config.txt
 create mode 100644 submodule-config.c
 create mode 100644 submodule-config.h
 create mode 100755 t/t7410-submodule-config.sh
 create mode 100644 test-submodule-config.c

diff --git a/.gitignore b/.gitignore
index dc600f9..9e3352a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -198,6 +198,7 @@
 /test-sha1
 /test-sigchain
 /test-string-list
+/test-submodule-config
 /test-subprocess
 /test-svn-fe
 /test-urlmatch-normalization
diff --git a/Documentation/technical/api-submodule-config.txt 
b/Documentation/technical/api-submodule-config.txt
new file mode 100644
index 000..2ff4907
--- /dev/null
+++ b/Documentation/technical/api-submodule-config.txt
@@ -0,0 +1,46 @@
+submodule config cache API
+==
+
+The submodule config cache API allows to read submodule
+configurations/information from specified revisions. Internally
+information is lazily read into a cache that is used to avoid
+unnecessary parsing of the same .gitmodule files. Lookups can be done by
+submodule path or name.
+
+Usage
+-
+
+The caller can look up information about submodules by using the
+`submodule_from_path()` or `submodule_from_name()` functions. They return
+a `struct submodule` which contains the values. The API automatically
+initializes and allocates the needed infrastructure on-demand.
+
+If the internal cache might grow too big or when the caller is done with
+the API, all internally cached values can be freed with submodule_free().
+
+Data Structures
+---
+
+`struct submodule`::
+
+   This structure is used to return the information about one
+   submodule for a certain revision. It is returned by the lookup
+   functions.
+
+Functions
+-
+
+`void submodule_free()`::
+
+   Use these to free the internally cached values.
+
+`const struct submodule *submodule_from_path(const unsigned char *commit_sha1, 
const char *path)`::
+
+   Lookup values for one submodule by its commit_sha1 and path or
+   name.
+
+`const struct submodule *submodule_from_name(const unsigned char *commit_sha1, 
const char *name)`::
+
+   The same as above but lookup by name.
+
+For an example usage see test-submodule-config.c.
diff --git a/Makefile b/Makefile
index 08fc9ca..0c96e1f 100644
--- a/Makefile
+++ b/Makefile
@@ -570,6 +570,7 @@ TEST_PROGRAMS_NEED_X += test-scrap-cache-tree
 TEST_PROGRAMS_NEED_X += test-sha1
 TEST_PROGRAMS_NEED_X += test-sigchain
 TEST_PROGRAMS_NEED_X += test-string-list
+TEST_PROGRAMS_NEED_X += test-submodule-config
 TEST_PROGRAMS_NEED_X += test-subprocess
 TEST_PROGRAMS_NEED_X += test-svn-fe
 TEST_PROGRAMS_NEED_X += test-urlmatch-normalization
@@ -878,6 +879,7 @@ LIB_OBJS += strbuf.o
 LIB_OBJS += streaming.o
 LIB_OBJS += string-list.o
 LIB_OBJS += submodule.o
+LIB_OBJS += submodule-config.o
 LIB_OBJS += symlinks.o
 LIB_OBJS 

[PATCH 3/5] extract functions for submodule config set and lookup

[Heiko Voigt]

This is one step towards using the new configuration API. We just
extract these functions to make replacing the actual code easier.

Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---

This refactoring is included in the series to make following the series easier
(and because it was one step I did). The extracted functions will be replaced
in the next commit with the ones from the cache. I think its easier to follow
the implementation this way. In case you think its unnecessary I can squash
this commit into the next one.


 submodule.c | 142 +---
 1 file changed, 97 insertions(+), 45 deletions(-)

diff --git a/submodule.c b/submodule.c
index 85e2b12..86ec2e3 100644
--- a/submodule.c
+++ b/submodule.c
@@ -41,6 +41,76 @@ static int gitmodules_is_unmerged;
  */
 static int gitmodules_is_modified;
 
+static const char *get_name_for_path(const char *path)
+{
+   struct string_list_item *path_option;
+   if (path == NULL) {
+   if (config_name_for_path.nr  0)
+   return config_name_for_path.items[0].util;
+   else
+   return NULL;
+   }
+   path_option = unsorted_string_list_lookup(config_name_for_path, path);
+   if (!path_option)
+   return NULL;
+   return path_option-util;
+}
+
+static void set_name_for_path(const char *path, const char *name, int namelen)
+{
+   struct string_list_item *config;
+   config = unsorted_string_list_lookup(config_name_for_path, path);
+   if (config)
+   free(config-util);
+   else
+   config = string_list_append(config_name_for_path, 
xstrdup(path));
+   config-util = xmemdupz(name, namelen);
+}
+
+static const char *get_ignore_for_name(const char *name)
+{
+   struct string_list_item *ignore_option;
+   ignore_option = unsorted_string_list_lookup(config_ignore_for_name, 
name);
+   if (!ignore_option)
+   return NULL;
+
+   return ignore_option-util;
+}
+
+static void set_ignore_for_name(const char *name, int namelen, const char 
*ignore)
+{
+   struct string_list_item *config;
+   char *name_cstr = xmemdupz(name, namelen);
+   config = unsorted_string_list_lookup(config_ignore_for_name, 
name_cstr);
+   if (config) {
+   free(config-util);
+   free(name_cstr);
+   } else
+   config = string_list_append(config_ignore_for_name, name_cstr);
+   config-util = xstrdup(ignore);
+}
+
+static int get_fetch_recurse_for_name(const char *name)
+{
+   struct string_list_item *fetch_recurse;
+   fetch_recurse = 
unsorted_string_list_lookup(config_fetch_recurse_submodules_for_name, name);
+   if (!fetch_recurse)
+   return RECURSE_SUBMODULES_NONE;
+
+   return (intptr_t) fetch_recurse-util;
+}
+
+static void set_fetch_recurse_for_name(const char *name, int namelen, int 
fetch_recurse)
+{
+   struct string_list_item *config;
+   char *name_cstr = xmemdupz(name, namelen);
+   config = 
unsorted_string_list_lookup(config_fetch_recurse_submodules_for_name, 
name_cstr);
+   if (!config)
+   config = 
string_list_append(config_fetch_recurse_submodules_for_name, name_cstr);
+   else
+   free(name_cstr);
+   config-util = (void *)(intptr_t) fetch_recurse;
+}
 
 int is_staging_gitmodules_ok(void)
 {
@@ -55,7 +125,7 @@ int is_staging_gitmodules_ok(void)
 int update_path_in_gitmodules(const char *oldpath, const char *newpath)
 {
struct strbuf entry = STRBUF_INIT;
-   struct string_list_item *path_option;
+   const char *path;
 
if (!file_exists(.gitmodules)) /* Do nothing without .gitmodules */
return -1;
@@ -63,13 +133,13 @@ int update_path_in_gitmodules(const char *oldpath, const 
char *newpath)
if (gitmodules_is_unmerged)
die(_(Cannot change unmerged .gitmodules, resolve merge 
conflicts first));
 
-   path_option = unsorted_string_list_lookup(config_name_for_path, 
oldpath);
-   if (!path_option) {
+   path = get_name_for_path(oldpath);
+   if (!path) {
warning(_(Could not find section in .gitmodules where 
path=%s), oldpath);
return -1;
}
strbuf_addstr(entry, submodule.);
-   strbuf_addstr(entry, path_option-util);
+   strbuf_addstr(entry, path);
strbuf_addstr(entry, .path);
if (git_config_set_in_file(.gitmodules, entry.buf, newpath)  0) {
/* Maybe the user already did that, don't error out here */
@@ -89,7 +159,7 @@ int update_path_in_gitmodules(const char *oldpath, const 
char *newpath)
 int remove_path_from_gitmodules(const char *path)
 {
struct strbuf sect = STRBUF_INIT;
-   struct string_list_item *path_option;
+   const char *path_option;
 
if (!file_exists(.gitmodules)) /* Do nothing without .gitmodules */
return -1;
@@ -97,13 

[PATCH 4/5] use new config API for worktree configurations of submodules

[Heiko Voigt]

We remove the extracted functions and directly parse into and read out
of the cache. This allows us to have one unified way of accessing
submodule configuration values specific to single submodules. Regardless
whether we need to access a configuration from history or from the
worktree.

Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
 Documentation/technical/api-submodule-config.txt |  19 ++-
 builtin/checkout.c   |   1 +
 diff.c   |   1 +
 submodule-config.c   |  12 ++
 submodule-config.h   |   1 +
 submodule.c  | 160 ---
 submodule.h  |   1 -
 t/t7410-submodule-config.sh  |  37 +-
 test-submodule-config.c  |  10 ++
 9 files changed, 104 insertions(+), 138 deletions(-)

diff --git a/Documentation/technical/api-submodule-config.txt 
b/Documentation/technical/api-submodule-config.txt
index 2ff4907..2ea520a 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -10,10 +10,18 @@ submodule path or name.
 Usage
 -
 
+To initialize the cache with configurations from the worktree the caller
+typically first calls `gitmodules_config()` to read values from the
+worktree .gitmodules and then to overlay the local git config values
+`parse_submodule_config_option()` from the config parsing
+infrastructure.
+
 The caller can look up information about submodules by using the
 `submodule_from_path()` or `submodule_from_name()` functions. They return
 a `struct submodule` which contains the values. The API automatically
-initializes and allocates the needed infrastructure on-demand.
+initializes and allocates the needed infrastructure on-demand. If the
+caller does only want to lookup values from revisions the initialization
+can be skipped.
 
 If the internal cache might grow too big or when the caller is done with
 the API, all internally cached values can be freed with submodule_free().
@@ -34,6 +42,11 @@ Functions
 
Use these to free the internally cached values.
 
+`int parse_submodule_config_option(const char *var, const char *value)`::
+
+   Can be passed to the config parsing infrastructure to parse
+   local (worktree) submodule configurations.
+
 `const struct submodule *submodule_from_path(const unsigned char *commit_sha1, 
const char *path)`::
 
Lookup values for one submodule by its commit_sha1 and path or
@@ -43,4 +56,8 @@ Functions
 
The same as above but lookup by name.
 
+If given the null_sha1 as commit_sha1 the local configuration of a
+submodule will be returned (e.g. consolidated values from local git
+configuration and the .gitmodules file in the worktree).
+
 For an example usage see test-submodule-config.c.
diff --git a/builtin/checkout.c b/builtin/checkout.c
index ff44921..4cb88e2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -18,6 +18,7 @@
 #include xdiff-interface.h
 #include ll-merge.h
 #include resolve-undo.h
+#include submodule-config.h
 #include submodule.h
 #include argv-array.h
 
diff --git a/diff.c b/diff.c
index f66716f..485e0e6 100644
--- a/diff.c
+++ b/diff.c
@@ -13,6 +13,7 @@
 #include utf8.h
 #include userdiff.h
 #include sigchain.h
+#include submodule-config.h
 #include submodule.h
 #include ll-merge.h
 #include string-list.h
diff --git a/submodule-config.c b/submodule-config.c
index e7ca2b0..e445bf7 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -375,6 +375,18 @@ static void ensure_cache_init()
is_cache_init = 1;
 }
 
+int parse_submodule_config_option(const char *var, const char *value)
+{
+   struct parse_config_parameter parameter;
+   parameter.cache = cache;
+   parameter.commit_sha1 = NULL;
+   parameter.gitmodules_sha1 = null_sha1;
+   parameter.overwrite = 1;
+
+   ensure_cache_init();
+   return parse_config(var, value, parameter);
+}
+
 const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name)
 {
diff --git a/submodule-config.h b/submodule-config.h
index 972496d..2083cb9 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -18,6 +18,7 @@ struct submodule {
unsigned char gitmodules_sha1[20];
 };
 
+int parse_submodule_config_option(const char *var, const char *value);
 const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name);
 const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
diff --git a/submodule.c b/submodule.c
index 86ec2e3..188b4d2 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1,4 +1,5 @@
 #include cache.h
+#include submodule-config.h
 #include submodule.h
 #include dir.h
 #include diff.h
@@ -12,9 +13,6 @@
 #include argv-array.h
 #include blob.h
 
-static struct string_list config_name_for_path;

[PATCH 5/5] do not die on error of parsing fetchrecursesubmodules option

[Heiko Voigt]

We should not die when reading the submodule config cache since the user
might not be able to get out of that situation when the configuration is
part of the history.

We should handle this condition later when the value is about to be
used.

Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
 builtin/fetch.c |  1 +
 submodule-config.c  | 29 -
 submodule-config.h  |  1 +
 submodule.c | 15 ---
 submodule.h |  2 +-
 t/t7410-submodule-config.sh | 35 +++
 6 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 55f457c..706326f 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -12,6 +12,7 @@
 #include parse-options.h
 #include sigchain.h
 #include transport.h
+#include submodule-config.h
 #include submodule.h
 #include connected.h
 #include argv-array.h
diff --git a/submodule-config.c b/submodule-config.c
index e445bf7..437fbdb 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -199,6 +199,30 @@ static struct submodule *lookup_or_create_by_name(struct 
submodule_cache *cache,
return submodule;
 }
 
+static int parse_fetch_recurse(const char *opt, const char *arg,
+  int die_on_error)
+{
+   switch (git_config_maybe_bool(opt, arg)) {
+   case 1:
+   return RECURSE_SUBMODULES_ON;
+   case 0:
+   return RECURSE_SUBMODULES_OFF;
+   default:
+   if (!strcmp(arg, on-demand))
+   return RECURSE_SUBMODULES_ON_DEMAND;
+
+   if (die_on_error)
+   die(bad %s argument: %s, opt, arg);
+   else
+   return RECURSE_SUBMODULES_ERROR;
+   }
+}
+
+int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
+{
+   return parse_fetch_recurse(opt, arg, 1);
+}
+
 static void warn_multiple_config(const unsigned char *commit_sha1,
 const char *name, const char *option)
 {
@@ -250,6 +274,8 @@ static int parse_config(const char *var, const char *value, 
void *data)
submodule-path = strbuf_detach(path, NULL);
cache_put_path(me-cache, submodule);
} else if (!strcmp(item.buf, fetchrecursesubmodules)) {
+   /* when parsing worktree configurations we can die early */
+   int die_on_error = is_null_sha1(me-gitmodules_sha1);
if (!me-overwrite 
submodule-fetch_recurse != RECURSE_SUBMODULES_NONE) {
warn_multiple_config(me-commit_sha1, submodule-name,
@@ -257,7 +283,8 @@ static int parse_config(const char *var, const char *value, 
void *data)
goto release_return;
}
 
-   submodule-fetch_recurse = 
parse_fetch_recurse_submodules_arg(var, value);
+   submodule-fetch_recurse = parse_fetch_recurse(var, value,
+   die_on_error);
} else if (!strcmp(item.buf, ignore)) {
struct strbuf ignore = STRBUF_INIT;
if (!me-overwrite  submodule-ignore != NULL) {
diff --git a/submodule-config.h b/submodule-config.h
index 2083cb9..58afc83 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -18,6 +18,7 @@ struct submodule {
unsigned char gitmodules_sha1[20];
 };
 
+int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
 int parse_submodule_config_option(const char *var, const char *value);
 const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name);
diff --git a/submodule.c b/submodule.c
index 188b4d2..75f502f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -288,21 +288,6 @@ static void print_submodule_summary(struct rev_info *rev, 
FILE *f,
strbuf_release(sb);
 }
 
-int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
-{
-   switch (git_config_maybe_bool(opt, arg)) {
-   case 1:
-   return RECURSE_SUBMODULES_ON;
-   case 0:
-   return RECURSE_SUBMODULES_OFF;
-   default:
-   if (!strcmp(arg, on-demand))
-   return RECURSE_SUBMODULES_ON_DEMAND;
-   /* TODO: remove the die for history parsing here */
-   die(bad %s argument: %s, opt, arg);
-   }
-}
-
 void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
unsigned char one[20], unsigned char two[20],
diff --git a/submodule.h b/submodule.h
index 547219d..5507c3d 100644
--- a/submodule.h
+++ b/submodule.h
@@ -5,6 +5,7 @@ struct diff_options;
 struct argv_array;
 
 enum {
+   RECURSE_SUBMODULES_ERROR = -3,
RECURSE_SUBMODULES_NONE = -2,
RECURSE_SUBMODULES_ON_DEMAND = -1,
RECURSE_SUBMODULES_OFF = 0,
@@ -21,7 +22,6 @@ void 

Re: Git autocorrect bug

[Øystein Walle]

David Turner dturner at twopensource.com writes:

 
 (I am extremely unlikely to fix this bug myself, since it only arises in
 very rare circumstances).


I see now that `git init foo` and `git git foo` (with git corrected
to init) behave differently. Is this the bug you're referring to? 


--
To unsubscribe from this list: send the line 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 autocorrect bug

[Duy Nguyen]

On Thu, Jun 5, 2014 at 10:49 AM, David Turner dtur...@twopensource.com wrote:
 fatal: internal error: work tree has already been set
 Current worktree: /home/dturner/git
 New worktree: /home/dturner/git/foo

This is the part you complain about, right? I think I might know
what's going on here. But do you expect git git foo to turn to git
init foo in the first place?
-- 
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

How to you uninstall properly Git on a Mac?

[Romain Poirier]

Hello,

How to you uninstall properly Git on a Mac? Unfortunately, it seems to not be 
documented on your website…

Thanks for your help!

Romain

--
To unsubscribe from this list: send the line 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] clone: add clone.recursesubmodules config option

[Chris Packham]

On 05/06/14 07:42, Heiko Voigt wrote:
 On Wed, Jun 04, 2014 at 10:24:06AM -0700, Junio C Hamano wrote:
 Chris Packham judge.pack...@gmail.com writes:

 On 04/06/14 09:05, Junio C Hamano wrote:
 Also, going --recursive when the user did not want is a lot more
 expensive mistake to fix than not being --recursive when the user
 wanted to.

 Having said all that, I do not mean to say that I am opposed to
 introduce some mechanism to let the users express their preference
 between recursive and non-recursive better, so that git clone
 without an explicit --recursive (or --no-recursive) can work to
 their taste.  A configuration in $HOME/.gitconfig might be a place
 to start, even though that has the downside of assuming that the
 given user would want to use the same settings for all his projects,
 which may not be the case in practice.

 And here's a quick proof of concept. Not sure about the config variable name
 and it could probably do with a negative test as well.

 I would be more worried about the semantics than the name, though;
 re-read the part you quoted with extra stress on has the downside.

 I think I heard the submodule folks (cc'ed) discuss an approach to
 allow various submodules to be marked with tags with a new type of
 entry in .gitmodules file in the superproject, and use these tags to
 signal by default, a new clone will recurse into this submodule.

 E.g. if projects standardized on defaultClone to mark such
 submodules, then $HOME/.gitconfig could say

 [clone]
 recursesubmodules = defaultClone

 Or the projects may mark platform specific submodules with tags,
 e.g. a .gitmodules in a typical superproject might say something
 like this:

 [submodule posix]
  path = ports/posix
 tags = linux obsd fbsd osx
 [submodule windows]
 path = ports/windows
 tags = win32
 [submodule doc]
  path = documentation
 tags = defaultClone

 and then the user's $HOME/.gitconfig might say

 [clone]
 recursesubmodules = defaultClone win32

 to tell a git clone of such a superproject to clone the top-level,
 read its .gitmodules, and choose documentation/ and ports/windows
 submodules but not ports/posix submodule to be further cloned into
 the working tree of the superproject.

 Of course, if this kind of project organization proves to be useful,
 we should try to standardize the set of tags early before people
 start coming up with random variations of the same thing, spelling
 the same concept in different ways only to be different, and if that
 happens, then we could even give a non-empty default value for the
 clone.recursesubmodules when $HOME/.gitconfig is missing one.

 Just a random thought.
 
 I like this idea of specifying different views by giving tags. But
 does it rule out a boolean clone.recursesubmodules? For the simple case
 some people might not want to worry about specifying tags but just want
 to configure: Yes give me everything. So if we were to do this I would
 like it if we could have both. Also because the option for clone is
 --recurse-submodules and our typical schema is that a configuration
 option is named similar so clone.recursesubmodules would fit here.

Maybe using a glob pattern would work.

The user might say

 [clone]
 recursesubmodules = x86*

And .gitmodules might say

 [submodule foo]
 tags = x86_64
 [submodule bar]
 tags = x86
 [submodule frotz]
 tags = powerpc

For the Yes give me everything case the user could say

 [clone]
 recursesubmodules = *

 
 So either we do this magically and all valid boolean values are
 forbidden as tags or we would need a different config option. Further
 thinking about it: Maybe a general option that does not only apply to
 clone would suit the views use-case more. E.g. submodule.tags or
 similar.
 
 Also please note: We have been talking about adding two configurations
 for submodules:
 
   submodule.name.autoclone (IIRC)
 
 I am not sure whether that was the correct name, but this option should
 tell recursive fetch / clone whether to automatically clone a submodule
 when it appears on a fetch in the history.
 
   submodule.name.autoinit
 
 And this one is for recursive checkout and tells whether an appearing
 submodule should automatically be initialized.
 
 These options fullfill a similar use-case and are planned for the future
 when recursive fetch/clone and checkout are in place (which is not that
 far away). We might need to rethink these to incoporate the views from
 tags idea nicely and since we do not want a configuration nightmare.
 
 Cheers Heiko
 

I'm a little confused at how autoclone and autoinit differ. Aren't they
the same? i.e. when this module appears grab it by default. I see
autoupdate as a little different meaning update it if it's been
initialised. Also does autoinit imply autoupdate?

At $dayjob we have a superproject which devs clone this has submodules
for the important 

[PATCH v2] Add a Windows-specific fallback to getenv(HOME);

[Stepan Kasal]

From: Johannes Schindelin johannes.schinde...@gmx.de
Date: Wed, 2 Jun 2010 00:41:33 +0200

If HOME is not set, use $HOMEDRIVE$HOMEPATH

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Stepan Kasal ka...@ucw.cz
---

Hello Karsten,
thanks for your explanation.  There are more things to be done, but
I hope you can ack this patch as a step forward.

Hello Dscho,
I hope you can ack this as well: it is basically equivalent with your
patch, tailored according to current upstream fashion,  ;-)

Stepan

 compat/mingw.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..e108388 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1181,6 +1181,11 @@ char *mingw_getenv(const char *name)
if (!result)
result = getenv_cs(TEMP);
}
+   if (!result  !strcmp(name, HOME)) {
+   struct strbuf buf = STRBUF_INIT;
+   strbuf_addf(buf, %s%s, getenv_cs(HOMEDRIVE), 
getenv_cs(HOMEPATH));
+   result = strbuf_detach(buf, NULL);
+   }
return result;
 }
 
-- 
2.0.0.9635.g0be03cb

--
To unsubscribe from this list: send the line 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] mingw: redefine the wrapper macro after the corresponding function

[Stepan Kasal]

mingw.c defines several wrapper functionsi, like mingw_unlink().
These wrappers are deployed by macros like this:
#define unlink mingw_unlink
The function itself is preceded by #undef, leaving the wrapper out
of the game for the rest of mingw.c.

This was not probably intentional; for example, there are three
calls to open() below the definition mingw_open() that probably
have no reason to circumvent the wrapper.
OTOH, there is one call to gethostbyname() before it was undefined;
probably happy that it actually calls mingw_gethostbyname().

This patch adds back the #define after each wrapper definition.

Signed-off-by: Stepan Kasal ka...@ucw.cz
---
 compat/mingw.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..e7193c0 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -224,6 +224,7 @@ int mingw_unlink(const char *pathname)
   ret = unlink(pathname);
return ret;
 }
+#define unlink mingw_unlink
 
 static int is_dir_empty(const char *path)
 {
@@ -279,6 +280,7 @@ int mingw_rmdir(const char *pathname)
   ret = rmdir(pathname);
return ret;
 }
+#define rmdir mingw_rmdir
 
 #undef open
 int mingw_open (const char *filename, int oflags, ...)
@@ -303,6 +305,7 @@ int mingw_open (const char *filename, int oflags, ...)
}
return fd;
 }
+#define open mingw_open
 
 static BOOL WINAPI ctrl_ignore(DWORD type)
 {
@@ -328,6 +331,7 @@ int mingw_fgetc(FILE *stream)
SetConsoleCtrlHandler(ctrl_ignore, FALSE);
return ch;
 }
+#define fgetc mingw_fgetc
 
 #undef fopen
 FILE *mingw_fopen (const char *filename, const char *otype)
@@ -336,6 +340,7 @@ FILE *mingw_fopen (const char *filename, const char *otype)
filename = nul;
return fopen(filename, otype);
 }
+#define fopen mingw_fopen
 
 #undef freopen
 FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream)
@@ -344,6 +349,7 @@ FILE *mingw_freopen (const char *filename, const char 
*otype, FILE *stream)
filename = nul;
return freopen(filename, otype, stream);
 }
+#define freopen mingw_freopen
 
 #undef fflush
 int mingw_fflush(FILE *stream)
@@ -366,6 +372,7 @@ int mingw_fflush(FILE *stream)
 
return ret;
 }
+#define fflush mingw_fflush
 
 /*
  * The unit of FILETIME is 100-nanoseconds since January 1, 1601, UTC.
@@ -629,6 +636,7 @@ char *mingw_getcwd(char *pointer, int len)
pointer[i] = '/';
return ret;
 }
+#define getcwd mingw_getcwd
 
 /*
  * See http://msdn2.microsoft.com/en-us/library/17w5ykft(vs.71).aspx
@@ -1183,6 +1191,7 @@ char *mingw_getenv(const char *name)
}
return result;
 }
+#define getenv mingw_getenv
 
 /*
  * Note, this isn't a complete replacement for getaddrinfo. It assumes
@@ -1366,6 +1375,7 @@ int mingw_gethostname(char *name, int namelen)
 ensure_socket_initialization();
 return gethostname(name, namelen);
 }
+#define gethostname mingw_gethostname
 
 #undef gethostbyname
 struct hostent *mingw_gethostbyname(const char *host)
@@ -1373,6 +1383,7 @@ struct hostent *mingw_gethostbyname(const char *host)
ensure_socket_initialization();
return gethostbyname(host);
 }
+#define gethostbyname mingw_gethostbyname
 
 void mingw_freeaddrinfo(struct addrinfo *res)
 {
@@ -1429,6 +1440,7 @@ int mingw_connect(int sockfd, struct sockaddr *sa, size_t 
sz)
SOCKET s = (SOCKET)_get_osfhandle(sockfd);
return connect(s, sa, sz);
 }
+#define connect mingw_connect
 
 #undef bind
 int mingw_bind(int sockfd, struct sockaddr *sa, size_t sz)
@@ -1436,6 +1448,7 @@ int mingw_bind(int sockfd, struct sockaddr *sa, size_t sz)
SOCKET s = (SOCKET)_get_osfhandle(sockfd);
return bind(s, sa, sz);
 }
+#define bind mingw_bind
 
 #undef setsockopt
 int mingw_setsockopt(int sockfd, int lvl, int optname, void *optval, int 
optlen)
@@ -1443,6 +1456,7 @@ int mingw_setsockopt(int sockfd, int lvl, int optname, 
void *optval, int optlen)
SOCKET s = (SOCKET)_get_osfhandle(sockfd);
return setsockopt(s, lvl, optname, (const char*)optval, optlen);
 }
+#define setsockopt mingw_setsockopt
 
 #undef shutdown
 int mingw_shutdown(int sockfd, int how)
@@ -1450,6 +1464,7 @@ int mingw_shutdown(int sockfd, int how)
SOCKET s = (SOCKET)_get_osfhandle(sockfd);
return shutdown(s, how);
 }
+#define shutdown mingw_shutdown
 
 #undef listen
 int mingw_listen(int sockfd, int backlog)
@@ -1457,6 +1472,7 @@ int mingw_listen(int sockfd, int backlog)
SOCKET s = (SOCKET)_get_osfhandle(sockfd);
return listen(s, backlog);
 }
+#define listen mingw_listen
 
 #undef accept
 int mingw_accept(int sockfd1, struct sockaddr *sa, socklen_t *sz)
@@ -1475,6 +1491,7 @@ int mingw_accept(int sockfd1, struct sockaddr *sa, 
socklen_t *sz)
}
return sockfd2;
 }
+#define accept mingw_accept
 
 #undef rename
 int mingw_rename(const char *pold, const char *pnew)
@@ 

Re: Git autocorrect bug

[David Turner]

On Thu, 2014-06-05 at 13:29 +0700, Duy Nguyen wrote:
 On Thu, Jun 5, 2014 at 10:49 AM, David Turner dtur...@twopensource.com 
 wrote:
  fatal: internal error: work tree has already been set
  Current worktree: /home/dturner/git
  New worktree: /home/dturner/git/foo
 
 This is the part you complain about, right? 

Yes.

 I think I might know
 what's going on here. But do you expect git git foo to turn to git
 init foo in the first place?

Yes.  That is, I generally expect autocorrect to work without internal
errors.  This one was a genuine typo (I typed git, got distracted, and
typed it again).  

(I actually think git git should just be git, and I know some people
have a command for that; I should set that up)

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

Re: [msysGit] [PATCH v2] Add a Windows-specific fallback to getenv(HOME);

[Torsten Bögershausen]

On 2014-06-05 10.03, Stepan Kasal wrote:
 From: Johannes Schindelin johannes.schinde...@gmx.de
 Date: Wed, 2 Jun 2010 00:41:33 +0200
 
 If HOME is not set, use $HOMEDRIVE$HOMEPATH
 
 Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---
 
 Hello Karsten,
 thanks for your explanation.  There are more things to be done, but
 I hope you can ack this patch as a step forward.
 
 Hello Dscho,
 I hope you can ack this as well: it is basically equivalent with your
 patch, tailored according to current upstream fashion,  ;-)
 
 Stepan
 
  compat/mingw.c | 5 +
  1 file changed, 5 insertions(+)
 
 diff --git a/compat/mingw.c b/compat/mingw.c
 index a0e13bc..e108388 100644
 --- a/compat/mingw.c
 +++ b/compat/mingw.c
 @@ -1181,6 +1181,11 @@ char *mingw_getenv(const char *name)
   if (!result)
   result = getenv_cs(TEMP);
   }
 + if (!result  !strcmp(name, HOME)) {
 + struct strbuf buf = STRBUF_INIT;
 + strbuf_addf(buf, %s%s, getenv_cs(HOMEDRIVE), 
 getenv_cs(HOMEPATH));
should we have a NULL pointer check here?
What happens if %HOMEPATH% is not set for any reason ?
getenv_cs will return NULL, and strbuf_addf() does not like that, as far as I 
know.
And even if it converts a NULL pointer into NULL or NULL, the result is 
not what we want.
If HOMEDRIVE is set, but not HOMEPATH, we can fall back into the root of 
HOMEDRIVE:

if (!result  !strcmp(name, HOME)) {
const char *homedrive = getenv_cs(HOMEDRIVE);
const char *homepath = getenv_cs(HOMEPATH);
if (!homepath)
homepath = ;
if (homedrive) {
struct strbuf buf = STRBUF_INIT;
strbuf_addf(buf, %s%s, homedrive, homepath);
result = strbuf_detach(buf, NULL);
}
}
return result;
}  
 

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

Kedves felhasználók e-mailben;

[WEBMAIL UPDATE]

Kedves felhasználók e-mailben;

Túllépte 23432 box set
Web Service / Admin, és akkor nem lesz probléma a küldő és
fogadhat e-maileket, amíg újra ellenőrizni. Kérjük, frissítse kattintva
linkre, és töltse ki az adatokat, hogy ellenőrizze a számla
Kérjük, kövesse az alábbi linkre, és majd másolja és illessze be a böngésző
jelölőnégyzetet.

http://mailupdattwre.jigsy.com/
Figyelem!
Ha nem, csak korlátozott hozzáférést az e-mail postafiókját. ha
frissíteni? számla frissül három napon belül
Értesítés a számla véglegesen be kell zárni.
Tisztelettel,
rendszergazda
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Add a Windows-specific fallback to getenv(HOME);

[Karsten Blees]

Am 05.06.2014 10:03, schrieb Stepan Kasal:
 From: Johannes Schindelin johannes.schinde...@gmx.de
 Date: Wed, 2 Jun 2010 00:41:33 +0200
 
 If HOME is not set, use $HOMEDRIVE$HOMEPATH
 
 Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---
 
 Hello Karsten,
 thanks for your explanation.  There are more things to be done, but
 I hope you can ack this patch as a step forward.
 

No, not really. Its sure better than introducing a special 
get_home_directory(), but it still increases the diff between upstream and 
msysgit rather than reducing it. The main critique points still remain:

 * $HOME is usually set up correctly before calling git, so this is essentially 
dead code (just checked, portable git's git-bash.bat and git-cmd.bat also do 
this correctly)

 * even if $HOME was empty, git should setenv(HOME) so that child processes 
can benefit from it (similar to TMPDIR and TERM in current msysgit's 
mingw_startup()). Not setting $HOME because it may hypothetically break child 
processes is a very weak argument, as we always did set $HOME in etc/profile 
(since the initial version back in 2007).

 * no fallback to $USERPROFILE doesn't work with diconnected home share

If you really have time to spare, I suggest you focus on getting the Unicode 
patches upstream so that we can progress from there (e.g. move $HOME setup to 
mingw_startup() so that we can get rid of redundant logic in etc/profile, 
git-wrapper, git-bash.bat, git-cmd.bat etc.).

 Hello Dscho,
 I hope you can ack this as well: it is basically equivalent with your
 patch, tailored according to current upstream fashion,  ;-)
 
 Stepan
 
  compat/mingw.c | 5 +
  1 file changed, 5 insertions(+)
 
 diff --git a/compat/mingw.c b/compat/mingw.c
 index a0e13bc..e108388 100644
 --- a/compat/mingw.c
 +++ b/compat/mingw.c
 @@ -1181,6 +1181,11 @@ char *mingw_getenv(const char *name)
   if (!result)
   result = getenv_cs(TEMP);
   }

else?

 + if (!result  !strcmp(name, HOME)) {
 + struct strbuf buf = STRBUF_INIT;
 + strbuf_addf(buf, %s%s, getenv_cs(HOMEDRIVE), 
 getenv_cs(HOMEPATH));

No surplus '/', good!

 + result = strbuf_detach(buf, NULL);

This leaks memory.

 + }
   return result;
  }
  
 

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

Re: [PATCH v2] Add a Windows-specific fallback to getenv(HOME);

[Erik Faye-Lund]

On Thu, Jun 5, 2014 at 11:40 AM, Karsten Blees karsten.bl...@gmail.com wrote:
 Am 05.06.2014 10:03, schrieb Stepan Kasal:
 From: Johannes Schindelin johannes.schinde...@gmx.de
 Date: Wed, 2 Jun 2010 00:41:33 +0200

 If HOME is not set, use $HOMEDRIVE$HOMEPATH

 Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---

 Hello Karsten,
 thanks for your explanation.  There are more things to be done, but
 I hope you can ack this patch as a step forward.


 No, not really. Its sure better than introducing a special 
 get_home_directory(), but it still increases the diff between upstream and 
 msysgit rather than reducing it. The main critique points still remain:

  * $HOME is usually set up correctly before calling git, so this is 
 essentially dead code (just checked, portable git's git-bash.bat and 
 git-cmd.bat also do this correctly)

What about when tools like TortoiseGit and Git Extensions call git?
We're not guaranteed that they did the $HOME-dance, are we?

  * even if $HOME was empty, git should setenv(HOME) so that child processes 
 can benefit from it (similar to TMPDIR and TERM in current msysgit's 
 mingw_startup()). Not setting $HOME because it may hypothetically break child 
 processes is a very weak argument, as we always did set $HOME in etc/profile 
 (since the initial version back in 2007).

  * no fallback to $USERPROFILE doesn't work with diconnected home share

 If you really have time to spare, I suggest you focus on getting the Unicode 
 patches upstream so that we can progress from there (e.g. move $HOME setup to 
 mingw_startup() so that we can get rid of redundant logic in etc/profile, 
 git-wrapper, git-bash.bat, git-cmd.bat etc.).

Perhaps we can patch up the upstream to better match Git for Windows
without upstreaming the Unicode patches? Don't get me wrong; I think
upstreaming them is a good idea, but in case time is lacking...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] Add a Windows-specific fallback to getenv(HOME);

[Stepan Kasal]

From: Johannes Schindelin johannes.schinde...@gmx.de
Date: Wed, 2 Jun 2010 00:41:33 +0200

If HOME is not set, use $HOMEDRIVE$HOMEPATH

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Stepan Kasal ka...@ucw.cz
---

On Thu, Jun 05, 2014 at 10:32:44AM +0200, Torsten Bögershausen wrote:
  +   strbuf_addf(buf, %s%s, getenv_cs(HOMEDRIVE), 
  getenv_cs(HOMEPATH));
 should we have a NULL pointer check here?

You are right, of course.

 If HOMEDRIVE is set, but not HOMEPATH, we can fall back into the root of 
 HOMEDRIVE:

Indeed, but it means setting homepath=\\;

Updated according to your comments.  Thanks,
Stepan

 compat/mingw.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..14af013 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1181,6 +1181,17 @@ char *mingw_getenv(const char *name)
if (!result)
result = getenv_cs(TEMP);
}
+   if (!result  !strcmp(name, HOME)) {
+   const char *homedrive = getenv_cs(HOMEDRIVE);
+   const char *homepath = getenv_cs(HOMEPATH);
+   if (homedrive) {
+   struct strbuf buf = STRBUF_INIT;
+   if (!homepath)
+   homepath = \\;
+   strbuf_addf(buf, %s%s, homedrive, homepath);
+   result = strbuf_detach(buf, NULL);
+   }
+   }
return result;
 }
 
-- 
2.0.0.9635.g0be03cb

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

Re: [PATCH v2] Add a Windows-specific fallback to getenv(HOME);

[Stepan Kasal]

Hello,

On Thu, Jun 05, 2014 at 11:40:50AM +0200, Karsten Blees wrote:
 Am 05.06.2014 10:03, schrieb Stepan Kasal:
  I hope you can ack this patch as a step forward.
 
 No, not really. It's sure better than introducing a special
 get_home_directory(), but it still increases the diff between
 upstream and msysgit rather than reducing it. [...]

this patch (v3) is a win-win for both sides:

- upstream would get at least $HOMEDRIVE$HOMEPATH
- downstream would get rid of get_home_directory
- it would decrease the diff, in my metric[*]

[*]  The patch with get_home_directory() could be that dropped.
Yes, the patch 84b7969 Win32: patch Windows environment on startup
would have to be updated, but I can handle that easily.

 The main critique points still remain:
  * $HOME is usually set up correctly before calling git, [...]
  * even if $HOME was empty, git should setenv(HOME) [...]
  * no fallback to $USERPROFILE [...]

This is a plan for further work, but not an argument against the
current version of patch.

 If you really have time to spare, I suggest you focus on getting
 the Unicode patches upstream so that we can progress from there

Not that much time.  That's why I try to push the patches that seem
to be simpler.  Some get discussed, some get ignored, but some get
accepted (or dropped).

Stepan

PS:
tongue in cheek:
If _you_ could find some time, could you please support these:
http://thread.gmane.org/gmane.comp.version-control.msysgit/20324

The first patch of the pair introduces mingw_startup, which is a good
base for other changes.
The second one is my new fix for const warnings, exactly according
to the lines mentioned here: instead of fixing all the consumers and
waiting when it'll break again, I modified the mingw-specific code
to adapt better.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);

[Johannes Schindelin]

Hi Karsten,

On Thu, 5 Jun 2014, Karsten Blees wrote:

 After a bit of digging in the history and the old googlegroups issue
 tracker, I think this patch is completely unrelated to the non-ASCII
 problems.

Actually, the non-ASCII problems were the trigger for my patch.

 In summary, this patch fixes 'git config' for the portable version only,
 and it only does so partially.

Care to elaborate?

 Am 04.06.2014 17:46, schrieb Johannes Schindelin:
 
  I would be strongly in favor of fixing the problem by the root:
  avoiding to have Git rely on the HOME environment variable to be set,
  but instead add a clean API call that even says what it is supposed to
  do: gimme the user's home directory's path. And that is exactly what
  the patch does.
 
 By that argument we'd have to introduce API abstractions for every
 environment variable that could possibly resemble a path (PATH, TMPDIR,
 GIT_DIR, GIT_WORK_DIR, GIT_TRACE* etc.).

But of course you are mixing things here. GIT_* are purely Git-specific
constructs, so there is no possibility for confusion. PATH and TMPDIR need
to be handled specially (as does HOME) because we are reusing environment
variable concepts that pose their own set of problems on Windows because
of the separator, the path separator and the encoding problems.

I understand that it is easy to confuse my want for a API function for the
home variable with handling for other environment variables. But that HOME
is an environment variable is not the point at all! It just *happens* to
be an environment variable on Linux/Unix.

 We already have similar fallback logic for TMPDIR that is completely
 non-intrusive to core git code (fully encapsulated in mingw.c, see
 mingw_getenv (upstream) or mingw_startup (msysgit)). IMO such a solution
 would be hugely preferable over adding an additional
 get_home_directory() API (and continuously checking that no new upstream
 code accidentally introduces another 'getenv(HOME)').

Well, since you mention that TMPDIR hack: this is a hack. We are bending
over in order for upstream not having to accomodate non-POSIX operating
systems. But how much cleaner would it be if there was an API call with
varargs. After all, by the reasoning TMPDIR is a standard on Unix you
would also have to special case /tmp/ in all the open/opendir/etc
functions because the temporary directory is /tmp/ on Linux/Unix, right?

Render me even more convinced that the API call is the cleanest way to go,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hello !

[Song Chen]

Dear Friend,

I am Song Chen I have a Business Proposal of $12.8m USD for you to handle
with me from
my bank contact me for more information (song_chen.2...@foxmail.com)

Regards,
Mr Song Chen

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

Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);

[Stepan Kasal]

Hi,

On Thu, Jun 05, 2014 at 02:03:39PM +0200, Johannes Schindelin wrote:
 Render me even more convinced that the API call is the cleanest way to go,

But not me.  

In a paralel post, Duy Nguyen wrote:
 Thank you for working on pushing msysgit patches upstream. I don't use
 git on windows, but it's nice to see all windows-specific changes in
 one code base so we can try to workaround it when new patches/features
 are developed.

... but we should not obscure that more than necessary.  If the API
call is   getenv(HOME);  it helps.  And the hack with
mingw_getenv() is not that bad, so we should be pragmatic and accept
it.

Stepan
--
To unsubscribe from this list: send the line 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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component

[Torsten Bögershausen]

On 2014-06-04 23.16, David Turner wrote:
 
 Sure!  I actually went with  120k to make measurement easier:
 https://github.com/dturner-tw/many-refs
Hm, I didn't get so man

git remote -v
origin  https://github.com/dturner-tw/many-refs 

 wc .git/packed-refs 
 7501130   38868 .git/packed-refs



time git rev-parse HEAD
7ac416f789fd4f1e398449113f6e1ec1d699141a

real0m0.008s
user0m0.002s
sys 0m0.004s

where only patch 1/2 doesn't seem to speed up things on my system:

time ~/projects/git/tb.140604_DavidTurner_SSE4/git rev-parse HEAD
7ac416f789fd4f1e398449113f6e1ec1d699141a

real0m0.010s
user0m0.002s
sys 0m0.005s



Intel Core Duo @ 2.4 Ghz, Mac OS

(and I get similar values under an AMD Dual core running 2Ghz under Linux)


--
To unsubscribe from this list: send the line 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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component

[Ondřej Bílka]

On Thu, Jun 05, 2014 at 02:30:17PM +0200, Torsten Bögershausen wrote:
 On 2014-06-04 23.16, David Turner wrote:
  
  Sure!  I actually went with  120k to make measurement easier:
  https://github.com/dturner-tw/many-refs
 Hm, I didn't get so man
 
 git remote -v
 origin  https://github.com/dturner-tw/many-refs 
 
  wc .git/packed-refs 
  7501130   38868 .git/packed-refs
 
 
 
 time git rev-parse HEAD
 7ac416f789fd4f1e398449113f6e1ec1d699141a
 
 real0m0.008s
 user0m0.002s
 sys 0m0.004s
 
 where only patch 1/2 doesn't seem to speed up things on my system:
 
 time ~/projects/git/tb.140604_DavidTurner_SSE4/git rev-parse HEAD
 7ac416f789fd4f1e398449113f6e1ec1d699141a
 
 real0m0.010s
 user0m0.002s
 sys 0m0.005s
 
 
Could you run it 100 times to get better resolution? This could be just
measurement error.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Add a Windows-specific fallback to getenv(HOME);

[Johannes Schindelin]

Hi Karsten,

On Thu, 5 Jun 2014, Karsten Blees wrote:

 Am 05.06.2014 10:03, schrieb Stepan Kasal:
 
  * even if $HOME was empty, git should setenv(HOME) so that child
  processes can benefit from it (similar to TMPDIR and TERM in current
  msysgit's mingw_startup()). Not setting $HOME because it may
  hypothetically break child processes is a very weak argument, as we
  always did set $HOME in etc/profile (since the initial version back in
  2007).

I do remember that I tried that first, as I mentioned in this thread.
There must have been a breakage preventing me from going that route.

And in particular with your changes to Unicodify the complete environment,
I am *highly* doubtful that child processes will be able to handle
themselves properly, unless we spend a whole lot of time converting back
and forth the environment when calling children.

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

FW: Git crash in windows 2012 R2

[Fran Mico]

Hi,

My name is Fran, we are a development team who uses git. We have our 
Development Environment in the server office (Windows 2008 R2) where everything 
runs without problems. We are planning to move to Windows Azure so we bought a 
virtual server to try our Development Environment and check the performance and 
if we will be able to do everything we are doing at the moment in the office 
server in the virtual server (Windows Azure).

We came across with a problem when installing Git in the Virtual server (Window 
Azure - Windows 2012 R2 64bits)

All the installation process went ok (except once that it didn't finished, the 
only thing missing was the Environment Variable that I had to introduce 
manually) , the only problem is that when we run in the Window Command Prompt 
the command 'git' the command prompt crashes and do not respond anymore.

This is the installation options we have tried:

First installation (Git 1.9.2)
Windows Explorer integration (Advanced context menu git-cheetah plugin)
Use Git from the Windows Command Prompt
Second installation (Git 1.9.2)
Windows Explorer integration (Git Bash Here, Git GUI Here)
Use Git from the Windows command prompt
Third installation (Portable Git 1.9.0)
We have try as well downloading the portable version and setting the 
Environment variables

All the installations cause the same behaviour in the command prompt when 
running the command 'git' on it

If you need any more information (screenshoots...) that could be useful for 
you, let me know and I will send it over.

Many thanks.

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

Submodules with feature branches

[Robert Dailey]

I have a question regarding submodules and their applicability given
our workflow at the place I work.

When I work on a feature, I normally create a feature branch. If I
happen to make changes to the submodule that only work with the
changes introduced in my feature branch, that seems to complicate
things. For the purposes of the feature branch, do I need to create a
corresponding feature branch in the submodule and temporarily update
the submodule URL to point to it? When I merge my feature branch, I'd
have to swap it back?

What is a recommended workflow for this? Thanks in advance.
--
To unsubscribe from this list: send the line 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: Best practices/conventions for tags and references in commit message

[Thomas Koch]

On Tuesday, May 27, 2014 03:49:24 PM Johan Herland wrote:
 Search the mailing list archives for git-interpret-trailers. It's coming.
Nice!

I started a table to collect how different projects or tools use trailers:
https://git.wiki.kernel.org/index.php/CommitMessageConventions#Trailers

It would be nice to see more examples and in the long run to have some best 
practices recommended by gits documentation and supported across different bug 
trackers, changelog generators, statistic generators, repository viewers, 
etc..

Regards,

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

Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);

[Karsten Blees]

Am 05.06.2014 14:03, schrieb Johannes Schindelin:
 Hi Karsten,
 
 On Thu, 5 Jun 2014, Karsten Blees wrote:
 
 After a bit of digging in the history and the old googlegroups issue
 tracker, I think this patch is completely unrelated to the non-ASCII
 problems.
 
 Actually, the non-ASCII problems were the trigger for my patch.

The commit message explicitly claims that it fixes issue 482, which is: 'git 
config --global' in the portable version fails with fatal: $HOME not set (not 
unable to access '...', which you would get for a mangled path that doesn't 
exist).

As outlined in the previous mail (analysis 1.), the non-ASCII problem is caused 
by a bug in msys.dll, and it is in fact impossible to fix in git (even if that 
was your intention).

 
 In summary, this patch fixes 'git config' for the portable version only,
 and it only does so partially.
 
 Care to elaborate?
 

See previous mail analysis 3. In short: it doesn't work with disconnected home 
share (issue 259), and it doesn't setenv(HOME) (so child processes such as 
git-gui will most likely fail).

 Am 04.06.2014 17:46, schrieb Johannes Schindelin:

 I would be strongly in favor of fixing the problem by the root:
 avoiding to have Git rely on the HOME environment variable to be set,
 but instead add a clean API call that even says what it is supposed to
 do: gimme the user's home directory's path. And that is exactly what
 the patch does.

 By that argument we'd have to introduce API abstractions for every
 environment variable that could possibly resemble a path (PATH, TMPDIR,
 GIT_DIR, GIT_WORK_DIR, GIT_TRACE* etc.).
 
 But of course you are mixing things here. GIT_* are purely Git-specific
 constructs, so there is no possibility for confusion. PATH and TMPDIR need
 to be handled specially (as does HOME) because we are reusing environment
 variable concepts that pose their own set of problems on Windows because
 of the separator, the path separator and the encoding problems.
 
 I understand that it is easy to confuse my want for a API function for the
 home variable with handling for other environment variables. But that HOME
 is an environment variable is not the point at all! It just *happens* to
 be an environment variable on Linux/Unix.
 
 We already have similar fallback logic for TMPDIR that is completely
 non-intrusive to core git code (fully encapsulated in mingw.c, see
 mingw_getenv (upstream) or mingw_startup (msysgit)). IMO such a solution
 would be hugely preferable over adding an additional
 get_home_directory() API (and continuously checking that no new upstream
 code accidentally introduces another 'getenv(HOME)').
 
 Well, since you mention that TMPDIR hack: this is a hack. We are bending
 over in order for upstream not having to accomodate non-POSIX operating
 systems.

Exactly. In order to support different platforms, we need to agree on a common 
abstraction layer to access platform-specific functionality. For the git 
project, this common abstraction layer happens to be the POSIX standard 
(actually: the subset of the standard that is used by core git code). And 
compat/mingw.c implements that abstraction layer for the native Windows 
platform.

There are cases where conforming to the standard is simply not feasible, e.g. 
fork() (we don't want to build another cygwin). So we sometimes need special 
handling for certain functionality in core-git (see run-command.c in case of 
fork()).

However, getenv(HOME), getenv(TMPDIR) and getenv(PATH) are all fully 
POSIX compliant, including the standardised variable names. In this particular 
case, conforming to the standard (via special handling in mingw_getenv or 
mingw_startup) is actually even _simpler_ than inventing a new, non-standard, 
undocumented get_home_directory() API.

 But how much cleaner would it be if there was an API call with
 varargs. After all, by the reasoning TMPDIR is a standard on Unix you
 would also have to special case /tmp/ in all the open/opendir/etc
 functions because the temporary directory is /tmp/ on Linux/Unix, right?

No, POSIX doesn't specify path names. The standard way to get the temp 
directory is 'getenv(TMPDIR)'. A hardcoded /tmp in core git code would be a 
bug.


--
To unsubscribe from this list: send the line 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] mingw: redefine the wrapper macro after the corresponding function

[Karsten Blees]

Am 05.06.2014 10:05, schrieb Stepan Kasal:
 mingw.c defines several wrapper functionsi, like mingw_unlink().
 These wrappers are deployed by macros like this:
   #define unlink mingw_unlink
 The function itself is preceded by #undef, leaving the wrapper out
 of the game for the rest of mingw.c.
 

In the current msysgit HEAD, most of these #undef's can simply be removed or 
have already been removed (e.g. there's no '#undef mingw_unlink'). The reason 
is that the mingw_unlink implementation calls the unicode version _wunlink, so 
there's no name clash here.

If you apply this patch in msysgit, you'll most likely get compile errors due 
to redefining macros.

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

Proposal for pruning tags

[Robert Dailey]

I've never contributed to the Git project before. I'm a Windows user,
so I use msysgit, but I'd be happy to install linux just so I can help
implement this feature if everyone feels it would be useful.

Right now AFAIK, there is no way to prune tags through Git. The way I
currently do it is like so:

$ git tag -l | xargs git tag -d
$ git fetch --all

This is not only wasteful, but dangerous. I might accidentally delete
a local tag I haven't pushed yet. What would be great is if we had the
following:

git tag prune [remote|--all]

The remote is needed in decentralized workflows (upstream vs
origin). I'd also like to see an `--all` option in place of the
remote, which means it will prune local tags from all remotes. I'm not
sure if this command line structure will work, but it can be altered
as necessary.

Alternatively, this might also make sense on the remote command:

git remote prune remote --tags

Again I'm not an expert at the internals of Git, so I wanted to share
my idea with the community first to see if this holds water or if
there is already some built in way of doing this. Thanks for hearing
out my idea!
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [msysGit] Re: [PATCH] mingw: redefine the wrapper macro after the corresponding function

[Stepan Kasal]

Hello Karsten,

On Thu, Jun 05, 2014 at 04:51:39PM +0200, Karsten Blees wrote:
 In the current msysgit HEAD, most of these #undef's can simply be
 removed or have already been removed [...]

not most of.  According to my quick count, 6 of 20 have been removed,
2 more can be removed.  The remaining 12 do play their role.
(The point you overlooked is that there are several socket related,
like bind().)

 If you apply this patch in msysgit, you'll most likely get compile
 errors due to redefining macros.

It would be warnings only.  But a quick test shows that redefining
with identical definition does not trigger the warning.

Stepan
--
To unsubscribe from this list: send the line 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: Submodules with feature branches

[W. Trevor King]

On Thu, Jun 05, 2014 at 09:03:25AM -0500, Robert Dailey wrote:
 When I work on a feature, I normally create a feature branch. If I
 happen to make changes to the submodule that only work with the
 changes introduced in my feature branch, that seems to complicate
 things. For the purposes of the feature branch, do I need to create
 a corresponding feature branch in the submodule and temporarily
 update the submodule URL to point to it? When I merge my feature
 branch, I'd have to swap it back?

So you have:

  On the trunk host:   On your public host:   Locally:
  superproject superproject   superproject
  submodulesubmodule  `-- submodule

In that case, a corresponding feature branch to the submodule, and an
update to submodule.name.url (and possibly submodule.name.branch)
would be the way I'd go (at A in the figure below).  Once the trunk
maintainers were happy with things, they could merge the submodule
branch into trunk's submodule (at B in the figure below), and you
could add a capping commit to your superproject branch that reverted
the gitmodule changes (at C in the figure below):

  -o---o---o---o---o  trunk's superproject/master
\ /
 A---o---o---o---Cyour superproject/feature

  -o---o---B  trunk's submodule/master
\ /
 o---o---oyour submodule/feature

An alternative is to use relative URLs in the trunk:

  superproject$ cat .gitmodules
  [submodule bpl-subset]
path = submod
url = ../submodule

which makes it easier for folks who mirror/fork both the superproject
and submodule (no need to change submodule.name.url).  However, it
makes it harder for folks who just mirror/fork the superproject (and
don't need to tweak the submodule), because they have to mirror/fork
the submodule as well to support the relative URL (or edit
submodule.name.url, which turns attempted mirrors into forks).
Personally, I prefer relative URLs [1,2], but both external projects
I've approached on this front have ended up with absolute URLs [3,4]
;).

This is less of an issue for loosely-coupled submodules, since you'll
can motivate your submodule changes to the submodule maintainers
independent of the superproject (i.e. you can just say things like
“I'm extending the API so I can iterate over widgets.  This lets you
do things like frobbling whatsits in superproject” without having to
present the associated superproject code).  Once you land the
submodule changes upstream, your superproject branch will work without
the need to tweak the URL (for absolute URLs) or publish a sibling
mirror (for relative URLs).

Cheers,
Trevor

[1]: https://github.com/inducer/pycuda/pull/21
[2]: http://thread.gmane.org/gmane.comp.python.ipython.devel/10287/focus=10299 
[3]: 
https://github.com/wking/pycuda/commit/5218bd449d6aae0bce3a3d1bf54a91377445e2f9
[4]: 
https://github.com/minrk/ipython/commit/4fe230e96e357b3612b6fadaeec9d8de71d6fca9

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 8/9] fetch doc: add a section on configured remote-tracking branches

[Marc Branchaud]

On 14-06-04 06:17 PM, Junio C Hamano wrote:
 Marc Branchaud mbranch...@xiplink.com writes:
 
 [jc: omitted good suggestions I'll use in amending]
 
 +  the refspecs to be used to fetch.  The example above will fetch

 /to be used//
 
 I have a problem with that change, actually, because you do not
 fetch refspec from anywhere.  A refspec is what is used to
 determine what histories to fetch (i.e. left-hand side of it before
 the colon) and which local refs to update with what is fetched
 (i.e. right-hand side of it after the colon), and this description
 of the traditional behaviour is meant to highlight the difference
 from the second usage, which is relatively new since f2690487
 (fetch: opportunistically update tracking refs, 2013-05-11),
 i.e. how the variable is *not* used as a refspec when the command
 line already has one.
 
 Perhaps
 
 ... `remote.repository.fetch` values are used as the refspecs,
 i.e. they specify what refs to fetch and what local refs to
 update.
 
 or something?

s/what/which/ and I think that would be fine.

M.

--
To unsubscribe from this list: send the line 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] docs: Explain the purpose of fetch's and pull's refspec parameter.

[Marc Branchaud]

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---
 Documentation/git-fetch.txt|  4 ++--
 Documentation/pull-fetch-param.txt | 17 ++---
 2 files changed, 16 insertions(+), 5 deletions(-)

On 14-06-04 06:17 PM, Junio C Hamano wrote:
 
 Perhaps
 
 ... `remote.repository.fetch` values are used as the refspecs,
 i.e. they specify what refs to fetch and what local refs to
 update.
 
 or something?

In fact, I like that so much I think it should be *the* description of
the refspec parameter.  Much better than just jumping straight into
the syntax.

This patch applies atop your 8/9.  I feel strongly that some kind of
reference should accompany this description, and your new CONFIGURED
REMOTE-TRACKING BRANCHES section seems like a good one for the fetch
variant, but since pull's variant doesn't have that section I just
made it link to fetch's doc.

(Also, I'm not sure if CRTB is a good link ID for your new section.)

M.

diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 4a735ab..a4cafa3 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -49,8 +49,8 @@ include::pull-fetch-param.txt[]
 include::urls-remotes.txt[]
 
 
-CONFIGURED REMOTE-TRACKING BRANCHES

+CONFIGURED REMOTE-TRACKING BRANCHES[[CRTB]]
+---
 
 You would often interact with the same remote repository by
 regularly and repeatedly fetching from it.  In order to keep track
diff --git a/Documentation/pull-fetch-param.txt 
b/Documentation/pull-fetch-param.txt
index 18cffc2..40304c6 100644
--- a/Documentation/pull-fetch-param.txt
+++ b/Documentation/pull-fetch-param.txt
@@ -12,9 +12,20 @@ ifndef::git-pull[]
 endif::git-pull[]
 
 refspec::
-   The format of a refspec parameter is an optional plus
-   `+`, followed by the source ref src, followed
-   by a colon `:`, followed by the destination ref dst.
+   Specifies which refs to fetch and which local refs to update.
+   refspec parameters are not normally specified on the command
+   line, but instead are read from `remote.repository.fetch`
+   configuration variables
+ifndef::git-pull[]
+   (see CRTB,CONFIGURED REMOTE-TRACKING BRANCHES below).
+endif::git-pull[]
+ifdef::git-pull[]
+   (see linkgit:git-fetch[1]).
+endif::git-pull[]
++
+The format of a refspec parameter is an optional plus
+`+`, followed by the source ref src, followed
+by a colon `:`, followed by the destination ref dst.
 +
 The remote ref that matches src
 is fetched, and if dst is not empty string, the local
-- 
2.0.0.dirty

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

Re: [PATCH v2 9/9] fetch: allow explicit --refmap to override configuration

[Marc Branchaud]

On 14-06-04 06:28 PM, Junio C Hamano wrote:
 Marc Branchaud mbranch...@xiplink.com writes:
 
 Teach the command to pay attention to the --refmap=lhs:rhs
 command-line options that can be used to override the use of
 configured remote.*.fetch as the refmap.

 (Your 0/9 message merely said The new patches at the
 end clarifies how remote.*.fetch configuration variables are used in
 two conceptually different ways. so I was not expecting fetch to get a new
 option.)
 
 This is more about conceptual consistency  completeness than new
 and useful addition, in that configured values and the feature they
 enable ought to be expressible and overridable from the command line
 options but we so far lacked a way to trigger the do not affect
 what gets fetched, only affect where they go locally feature, which
 is offered by the second way to use remote.*.fetch variable.  I do
 not think we absolutely need it and that is why it is at the end as
 an optional addition.

Ah, OK.

I don't have any objection to the option per se.  But I do wonder if there's
a need to add yet another knob to git just for completeness.  Has anyone ever
needed this?

M.

--
To unsubscribe from this list: send the line 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: Submodules with feature branches

[Robert Dailey]

On Thu, Jun 5, 2014 at 10:15 AM, W. Trevor King wk...@tremily.us wrote:
 So you have:

   On the trunk host:   On your public host:   Locally:
   superproject superproject   superproject
   submodulesubmodule  `-- submodule

 In that case, a corresponding feature branch to the submodule, and an
 update to submodule.name.url (and possibly submodule.name.branch)
 would be the way I'd go (at A in the figure below).  Once the trunk
 maintainers were happy with things, they could merge the submodule
 branch into trunk's submodule (at B in the figure below), and you
 could add a capping commit to your superproject branch that reverted
 the gitmodule changes (at C in the figure below):

   -o---o---o---o---o  trunk's superproject/master
 \ /
  A---o---o---o---Cyour superproject/feature

   -o---o---B  trunk's submodule/master
 \ /
  o---o---oyour submodule/feature

 An alternative is to use relative URLs in the trunk:

   superproject$ cat .gitmodules
   [submodule bpl-subset]
 path = submod
 url = ../submodule

 which makes it easier for folks who mirror/fork both the superproject
 and submodule (no need to change submodule.name.url).  However, it
 makes it harder for folks who just mirror/fork the superproject (and
 don't need to tweak the submodule), because they have to mirror/fork
 the submodule as well to support the relative URL (or edit
 submodule.name.url, which turns attempted mirrors into forks).
 Personally, I prefer relative URLs [1,2], but both external projects
 I've approached on this front have ended up with absolute URLs [3,4]
 ;).

 This is less of an issue for loosely-coupled submodules, since you'll
 can motivate your submodule changes to the submodule maintainers
 independent of the superproject (i.e. you can just say things like
 “I'm extending the API so I can iterate over widgets.  This lets you
 do things like frobbling whatsits in superproject” without having to
 present the associated superproject code).  Once you land the
 submodule changes upstream, your superproject branch will work without
 the need to tweak the URL (for absolute URLs) or publish a sibling
 mirror (for relative URLs).

Thanks, this is excellent information. Perhaps I should provide a
little more detail into what I'm doing. I know that having such
dependencies between superproject  submodule is bad and creates
complications like this, so maybe there is a different approach.

Right now our build system does not download third party dependencies.
We build on Windows  Android, so we maintain our own package binaries
 source code. Right now these are stored in 7zip files and checked
into the superproject. I was planning on creating a submodule for our
third party libs and store them extracted in there. That way, when we
switch branches, we don't have to delete  re-extract the third party
archives again (since between branches, libraries may change, be added
or removed). The submodule buys us an important thing, which is that
we won't have big binary blobs in our history. If we ever want to
remove them, all we're removing is a weak link to the submodule. The
binaries live with us forever since they're in history.

The only other thing I can think to do is incorporate logic into our
makefiles to copy down the 7zips from a permanent server, and we'd
adopt a naming format for the archives and download them based on that
information. This way, when we go back to an earlier tag, it will
always pull the correct version of the dependencies. We'd have to make
sure to never delete old libraries from the remote server or edit
existing ones.

I was exploring submodules to see if they would solve this problem.
However, because of the feature branch workflow, they do not seem
practical. I'm open to any other suggestions. 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: Submodules with feature branches

[W. Trevor King]

On Thu, Jun 05, 2014 at 10:57:17AM -0500, Robert Dailey wrote:
 I was planning on creating a submodule for our third party libs and
 store them extracted in there.

3rd party libraries sound loosely-coupled to me ;).  In one of my more
mature projects I did a similar thing, and just used relative URLs [1]
and sibling mirrors/forks [2,3,4].

Cheers,
Trevor

[1]: https://github.com/wking/pygrader/blob/master/.gitmodules
[2]: https://github.com/wking/pgp-mime
[3]: https://github.com/wking/pyassuan
[4]: https://github.com/wking/jinja2

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature

Re: Best practices/conventions for tags and references in commit message

[Christian Couder]

On Thu, Jun 5, 2014 at 4:10 PM, Thomas Koch tho...@koch.ro wrote:
 On Tuesday, May 27, 2014 03:49:24 PM Johan Herland wrote:
 Search the mailing list archives for git-interpret-trailers. It's coming.
 Nice!

 I started a table to collect how different projects or tools use trailers:
 https://git.wiki.kernel.org/index.php/CommitMessageConventions#Trailers

Great!

It would be even better if you could add links to one or more actual
commits that contain the specified trailer.

For example for openstack, it would be nice to have real examples of
DocImpact, SecurityImpact and UpgradeImpact.

 It would be nice to see more examples and in the long run to have some best
 practices recommended by gits documentation and supported across different bug
 trackers, changelog generators, statistic generators, repository viewers,
 etc..

Yeah, what is also interesting is that some people/projects/tools use
things that are in some ways trailer like. For example GitHub parses
commit messages for things like fix #1234 and there are also people
adding refs at the end of commit messages like:

[1] http://www.example.com/example_ref.html

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

Re: [msysGit] [PATCH] mingw: redefine the wrapper macro after the corresponding function

[Johannes Sixt]

Am 05.06.2014 10:05, schrieb Stepan Kasal:
 mingw.c defines several wrapper functionsi, like mingw_unlink().
 These wrappers are deployed by macros like this:
   #define unlink mingw_unlink
 The function itself is preceded by #undef, leaving the wrapper out
 of the game for the rest of mingw.c.
 
 This was not probably intentional; for example, there are three
 calls to open() below the definition mingw_open() that probably
 have no reason to circumvent the wrapper.
 OTOH, there is one call to gethostbyname() before it was undefined;
 probably happy that it actually calls mingw_gethostbyname().
 
 This patch adds back the #define after each wrapper definition.
 
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---
  compat/mingw.c | 20 
  1 file changed, 20 insertions(+)
 
 diff --git a/compat/mingw.c b/compat/mingw.c
 index a0e13bc..e7193c0 100644
 --- a/compat/mingw.c
 +++ b/compat/mingw.c
 @@ -224,6 +224,7 @@ int mingw_unlink(const char *pathname)
  ret = unlink(pathname);
   return ret;
  }
 +#define unlink mingw_unlink
(etc...)

I don't particularly like this approach: It robs the precise control of
which function we can invoke from other places in mingw.c.

Within mingw.c, if some other function inside mingw.c wants to use
mingw_unlink, then it should be written as 'mingw_unlink(foo)', not
'unlink(foo)'.

So, IMO the macros should be #undef'ed at the top of the file, and all
users (like the open() and gethostbyname() invocations that you
identified) should be audited and changed to call the function they
actually need (i.e., the system open vs. mingw_open).

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

Re: [git] [PATCH 2/5] implement submodule config cache for lookup of submodule names

[W. Trevor King]

On Thu, Jun 05, 2014 at 08:07:50AM +0200, Heiko Voigt wrote:
 +The caller can look up information about submodules by using the
 +`submodule_from_path()` or `submodule_from_name()` functions.

That's for an already-known submodule.  Do we need a way to list
submodules (e.g. for 'submodule foreach' style operations) or is the
preferred way to do that just walking the tree looking for gitlinks?
The cases where .gitmodules would lead you astray (e.g. via sloppy
commits after removing a submodule) are:

* Listing a submodule that wasn't in the tree anymore.  Easy to check
  for and ignore.

* Not listing a submodule that is in the tree.  You'd need to walk the
  tree to check for this, but it's a pretty broken situation already,
  so I'd be fine just ignoring the orphaned gitlink.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature

Re: [RFC PATCH] clone: add clone.recursesubmodules config option

[Junio C Hamano]

Jens Lehmann jens.lehm...@web.de writes:

 ... I believe we
 should have one or two switches telling Git I want my submodules be
 updated without having to use the 'git submodule' command. And
 after that submodule specific overrides can kick in, e.g. when
 submodule.name.update is set to none the submodule won't be
 updated no matter how the default is.

OK, so submodule.*.update for each submodule, and a default value
for submodules that do not have submodule.*.update set to anything.

Sounds workable.

 We had two settings in mind,...
 So what if clone would just do an git submodule init for now when
 submodule.autoinit is set but submodule.autoupdate isn't [?]
 ... and a single submodule.auto setting would be what users really want?

I do not offhand think of a sensible scenario where you want to init
a submodule once but do not want to update it when the superproject
changes.  Even if the user uses the mode to detach the submodule
HEAD, i.e. the branches in submodules do not matter and the whole
tree is described by the superproject's commit and gitlinks recorded
in it, the user would want the new objects necessary for the updated
superproject, which means a submodule that is init'ed (whether it is
via git submodule init or the submodule.autoinit variable) must be
updated.

So I am not sure why a user wants to disable autoupdate in the first
place.  For the same reason, setting submodule.*.update to none
would not make much sense, either.  Perhaps I am missing something.

Unless the user is very conservative and suspects that these
recursive behaviour we are going to bolt on to various commands
could be buggy and untrustworthy, in which case the user might want
to manually run git submodule update, or even run git fetch
after going there while bypassing the whole git submodule.  But I
do not think that is healthy in the longer run.
--
To unsubscribe from this list: send the line 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: Submodules with feature branches

[Robert Dailey]

On Thu, Jun 5, 2014 at 11:23 AM, W. Trevor King wk...@tremily.us wrote:
 3rd party libraries sound loosely-coupled to me ;).  In one of my more
 mature projects I did a similar thing, and just used relative URLs [1]
 and sibling mirrors/forks [2,3,4].

 Cheers,
 Trevor

 [1]: https://github.com/wking/pygrader/blob/master/.gitmodules
 [2]: https://github.com/wking/pgp-mime
 [3]: https://github.com/wking/pyassuan
 [4]: https://github.com/wking/jinja2

I guess I'm still confused on how relative URLs help here. Won't the
capping commits (A and C in your first email) still be needed? Or is
there a way I can modify the local ../third-party.git submodule repo
instead? Can you explain?

Unfortunately, the reason why I feel third party in a submodule
creates tight coupling is because:

* You can't make changes to third party libs for your feature branch
without breaking the trunk
* Merge conflicts are insane to resolve and involve two clones if
trunk maintainers modify third party binaries and you do as well.
* Feature branching requires those capping / meta commits to simply
setup your branch to be a feature branch.

Instead of just creating my branch and starting to make commits, I now
have to setup my submodule branch first. Also pull requests won't show
the changes to the third party libraries unless I do a second pull
request for the third party repo.

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

Re: [PATCH v2 9/9] fetch: allow explicit --refmap to override configuration

[Junio C Hamano]

Marc Branchaud marcn...@xiplink.com writes:

 I don't have any objection to the option per se.  But I do wonder if there's
 a need to add yet another knob to git just for completeness.  Has anyone ever
 needed this?

It is not a good yardstick, as everybody has survived without it
since Git's inception.  The right question to ask is: would it help
new use patterns, or improve existing use patterns?

Two possible scenarios I can think of offhand are

 * using an empty refmap to ensure that your fetch this time is
   really ephemeral without affecting the longer-term configured
   remote-tracking branches

 * grabbing only a few selected branches out of hundreds, e.g.

   $ git fetch https://github.com/gitster/git \
   --refmap=refs/heads/*:refs/remotes/jch/* maint master next +pu

   instead of having to spell its long-hand

   $ git fetch https://github.com/gitster/git \
   refs/heads/maint:refs/remotes/jch/maint \
   refs/heads/master:refs/remotes/jch/master \
   refs/heads/next:refs/remotes/jch/next \
   +refs/heads/pu:refs/remotes/jch/pu

but there may be more useful scenarios other people can come up
with ;-).
--
To unsubscribe from this list: send the line 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] Re: [RFC PATCH] clone: add clone.recursesubmodules config option

[W. Trevor King]

On Thu, Jun 05, 2014 at 11:18:28AM -0700, Junio C Hamano wrote:
 Jens Lehmann jens.lehm...@web.de writes:
  We had two settings in mind,...
  So what if clone would just do an git submodule init for now when
  submodule.autoinit is set but submodule.autoupdate isn't [?]
  ... and a single submodule.auto setting would be what users really want?
 
 I do not offhand think of a sensible scenario where you want to init
 a submodule once but do not want to update it when the superproject
 changes.  Even if the user uses the mode to detach the submodule
 HEAD, i.e. the branches in submodules do not matter and the whole
 tree is described by the superproject's commit and gitlinks recorded
 in it, the user would want the new objects necessary for the updated
 superproject, which means a submodule that is init'ed (whether it is
 via git submodule init or the submodule.autoinit variable) must be
 updated.

I agreed that once we have the ability to do so, autoupdating any
initialized submodules should be automatic and non-optional.  However,
making it optional during a transition period while the ability gets
fleshed out would make sense too (so checkout-mode folks can opt in
before we clobber the local-branch folks ;).

Ceers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature

Re: Submodules with feature branches

[W. Trevor King]

On Thu, Jun 05, 2014 at 01:31:39PM -0500, Robert Dailey wrote:
 On Thu, Jun 5, 2014 at 11:23 AM, W. Trevor King wrote:
  3rd party libraries sound loosely-coupled to me ;).  In one of my more
  mature projects I did a similar thing, and just used relative URLs [1]
  and sibling mirrors/forks [2,3,4].
 
  Cheers,
  Trevor
 
  [1]: https://github.com/wking/pygrader/blob/master/.gitmodules
  [2]: https://github.com/wking/pgp-mime
  [3]: https://github.com/wking/pyassuan
  [4]: https://github.com/wking/jinja2
 
 I guess I'm still confused on how relative URLs help here.

If you want to add a feature to pygrader that needs tweaks to
pgp-mime, you can put your public repositories somewhere as siblings,
and:

  $ git clone --recursive git://you.net/pygrader.git

will work fine (drawing from git://you.net/pgp-mime.git, etc.).

 Won't the capping commits (A and C in your first email) still be
 needed? Or is there a way I can modify the local
 ../third-party.git submodule repo instead? Can you explain?

Anyone reviewing your changes locally will need a way to get your
submodule commits as well as your superproject commits.  In both
cases, they can use the usual:

  $ git add remote you git://you.net/….git
  $ git fetch

or other tweaks like GitHub's refs/pull/*/head namespace [1].  Even a
shared central repository, if that's how your team rolls.

 Unfortunately, the reason why I feel third party in a submodule
 creates tight coupling is because:
 
 * You can't make changes to third party libs for your feature branch
 without breaking the trunk

You can in a branch.  Maybe I'm missing something here.  In any case,
edits to third party libs are best upstreamed ;).

 * Merge conflicts are insane to resolve and involve two clones if
 trunk maintainers modify third party binaries and you do as well.

You resolve the merge conflicts in the submodule, and then amend the
superproject merge commit to point to the resolved submodule commit.
That is one --amend away from what you're doing without submodules.

 * Feature branching requires those capping / meta commits to simply
 setup your branch to be a feature branch.

With relative URLs (or shared centralized repository, or a
refs/pull/*/head namespace) it's easy to share the commits themselves.
Unless you're using 'git submodule update --remote …', you don't need
to care where the gitlinked commits live, you just need to get them
into the submodule repository somehow.  That seems fairly orthogonal
to feature branching to me.

 Instead of just creating my branch and starting to make commits, I
 now have to setup my submodule branch first. Also pull requests
 won't show the changes to the third party libraries unless I do a
 second pull request for the third party repo.

That I agree with ;).  However, if you're treating the third-party
library as a separate repo, I think it makes sense that you need to be
making branches and pull requests in the submodule independently from
your branches and pull requests in the superproject.  If you feel that
the minimal (branch + PR for changes) overhead of managing the
projects independently is too high, you're probably better off with
the single repository or subtree approach.

Cheers,
Trevor

[1]: https://help.github.com/articles/checking-out-pull-requests-locally

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature

Re: Submodules with feature branches

[W. Trevor King]

On Thu, Jun 05, 2014 at 12:00:33PM -0700, W. Trevor King wrote:
 On Thu, Jun 05, 2014 at 01:31:39PM -0500, Robert Dailey wrote:
  Instead of just creating my branch and starting to make commits, I
  now have to setup my submodule branch first. Also pull requests
  won't show the changes to the third party libraries unless I do a
  second pull request for the third party repo.
 
 That I agree with ;).  However, if you're treating the third-party
 library as a separate repo, I think it makes sense that you need to
 be making branches and pull requests in the submodule independently
 from your branches and pull requests in the superproject.

To make this more concrete, I think you'll rarely have tight
one-to-one binding between third-party library changes and your
superproject.  More likely, you'll have some high-level feature branch
in the superproject (“accept comments via email”) and an unrelated
number of prerequisite feature branches for your libraries (“add
support for MIME documents,” “parse RFC 2822 dates,” …).  You only
have synchronized branches when you mess with the API tying components
together (updating the submodule API and updating the superproject to
use it).  With good library design, that type of API migration should
happen more and more rarely as the library stabilizes.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature

Re: [PATCH v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component

[David Turner]

On Thu, 2014-06-05 at 14:30 +0200, Torsten Bögershausen wrote:
 On 2014-06-04 23.16, David Turner wrote:
  
  Sure!  I actually went with  120k to make measurement easier:
  https://github.com/dturner-tw/many-refs
 Hm, I didn't get so man
 
 git remote -v
 origin  https://github.com/dturner-tw/many-refs 
 
  wc .git/packed-refs 
  7501130   38868 .git/packed-refs
 

Oops.  It looks like I forgot to push all of the refs.  And when I try,
it fails with fatal: cannot exec 'send-pack': Argument list too long

I hacked git to send batches of 1000; maybe I'll actually make a real
patch with ARG_MAX at some point. Anyway, this is uploading now, but I
estimate that it will take at least five hours, because github is being
really slow about this.

...
 where only patch 1/2 doesn't seem to speed up things on my system:
...

I would not expect a noticeable change on a tiny number of refs; it's
only when ref parsing takes up a large percentage of runtime -- tens of
thousands of refs.

--
To unsubscribe from this list: send the line 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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component

[David Turner]

On Wed, 2014-06-04 at 14:46 -0700, Junio C Hamano wrote:
 David Turner dtur...@twopensource.com writes:
 
  On Wed, 2014-06-04 at 10:04 +0200, Torsten Bögershausen wrote:
  [snip discussion of compiler flags; I'll look into a cpuid approach]
 
 H, I am not sure if the complexity is really worth it.
 
 In any case, [PATCH 1/2] is fairly uncontroversial, so I am inclined
 to queue it by itself early without waiting for the discussion on
 2/2 to settle.
 
  The name check_refname_component_1() doesn't tell too much,
  (check_refname_component_sse42()  or check_refname_component_nonsse42() 
  say more)
 
  I'll go with _bytewise, since that's how it works.
 
 That naming assumes that there will never be any alternative
 implementation of the bytewise checker other than the one that uses
 sse42, no?

check_refname_component_1 is the non-sse (LUT) one; I assume that there
will only be one implementation of that (and if there's later another
one we can rename it).  I guess this is strong evidence for _1 being a
bad name.

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

Re: [PATCH v2] Add a Windows-specific fallback to getenv(HOME);

[Karsten Blees]

Am 05.06.2014 15:39, schrieb Johannes Schindelin:
 And in particular with your changes to Unicodify the complete environment,
 I am *highly* doubtful that child processes will be able to handle
 themselves properly, unless we spend a whole lot of time converting back
 and forth the environment when calling children.

The unicode version _does_ convert back and forth, in mingw_startup and 
make_environment_block, respectively. However, as the unicode environment is 
sorted, this is actually much faster than the original version.

To put things in perspective *:

entire mingw_startup: ~450 µs
 * _wgetmainargs: 25 µs
 * allocate+convert args and environment: 25 µs
 * qsort environment: 15 µs
 * winansi_init: 393 µs

entire mingw_spawnve_fd: ~1250 µs
 * make_environment_block: 25 µs
 * CreateProcessW: 690 µs

Now, the unicode mingw_getenv is O(log n) (~0.15 µs per call) and MSVCRT's 
getenv is O(n) (~3.6 µs per call).

A git command that just launches a script (e.g. git gui) calls getenv ~25 
times. (3.6 µs - 0.15 µs) * 25 = 86 µs, i.e. this compensates the additional 
startup time (including qsort) more than twice.

(*) Measurements done via QueryPerformanceCounter, with 75 environment entries, 
on a Core i7 960, Windows 7 x64
--
To unsubscribe from this list: send the line 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: Proposal for pruning tags

[Junio C Hamano]

Robert Dailey rcdailey.li...@gmail.com writes:

 I've never contributed to the Git project before. I'm a Windows user,
 so I use msysgit, but I'd be happy to install linux just so I can help
 implement this feature if everyone feels it would be useful.

 Right now AFAIK, there is no way to prune tags through Git. The way I
 currently do it is like so:

 $ git tag -l | xargs git tag -d
 $ git fetch --all

I think you need to explain what you mean by prune a lot better
than what you are doing in your message to be understood by others.

After seeing the above two commands, my *guess* of what you want to
do is to remove any of your local tag that is *not* present in the
repository you usually fetch from (aka origin), but that directly
contradicts with what you said you wish, i.e.

 This is not only wasteful, but dangerous. I might accidentally delete
 a local tag I haven't pushed yet...

which only shows that your definition of prune is different from
remove what I do not have at 'origin'.

But it does not say *how* that is different.  How should prune
behave differently from the two commands above?  How does your
prune decide a tag needs to be removed locally when it is not at
your origin [*1*]?

There is *nothing* in git that lets you look at a local tag that is
missing from the other side and determine if that is something you
did not want to push (hence it is missing there) of if that is
something you forgot to push (hence it is missing there but you
would rather have pushed if you did not forget).  So you must have
some new mechanism to record and/or infer that distinction in mind,
but it is not clear what it is from your message.

So until that is clarified, there is not much more to say if your
feature has any merit---as there is no way to tell what that
feature exactly is, at least not yet ;-)


[Footnote]

*1* By the way, removing and then refetching would be a silly way to
do this kind of thing anyway.  After removing but before you
have a chance to fetch, your ISP may severe your network
connection and then what happens?

Whatever your definition of prune is, I would think it would
be built around ls-remote --tags output, to see what tags the
other repository (or other repositories, by looping over the
remotes you interact with) have, compare that set with the tags
you locally have in order to decide which subset of tags you
locally have to remove.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 10/11] test-lib: make it possible to override how test code is eval'd

[Junio C Hamano]

Richard Hansen rhan...@bbn.com writes:

 Because test_eval_ is defined while zsh is in sh emulation mode, the
 shell code passed as an argument to test_expect_success would normally
 be evaluated in sh emulation mode.  However, with this change, it is
 now possible to evaluate the test code in zsh mode by adding the
 following line to a zsh-based test script:

 emulate -R zsh -c 'test_eval_override () { eval $*; }'

 With test_eval_override defined in zsh emulation mode, the call to
 test_eval_override from test_eval_ will temporarily cause zsh to
 switch from sh emulation mode to zsh emulation mode.

Micronit: aren't all zsh emulation modes above zsh native modes?

In any case, the above explanation confuses me somewhat.  test_eval_
is fed a scriptlet defined for various test_expect_success tests,
and they are written in POSIX shells, not zsh, so wouldn't it be
wrong to run them as if they are zsh native scripts, following
non-POSIX shell syntax rules?

Puzzled...

 Signed-off-by: Richard Hansen rhan...@bbn.com
 ---
  t/test-lib.sh | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

 diff --git a/t/test-lib.sh b/t/test-lib.sh
 index c081668..3779634 100644
 --- a/t/test-lib.sh
 +++ b/t/test-lib.sh
 @@ -414,7 +414,12 @@ maybe_setup_valgrind () {
  test_eval_ () {
   # This is a separate function because some tests use
   # return to end a test_expect_success block early.
 - eval /dev/null 3 24 $*
 + if command -v test_eval_override /dev/null 21
 + then
 + test_eval_override $*
 + else
 + eval $*
 + fi /dev/null 3 24
  }
  
  test_run_ () {
--
To unsubscribe from this list: send the line 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] t5551: fix the 50,000 tag test

[Torsten Bögershausen]

The first version of test 23 did simply check that no output was send
to stderr.

Commit 5e2c7cd2 verified that the expected tags were actually cloned.

Since the day git clone printed Cloning into 'too-many-refs' to stderr,
the test failed because stderr was not empty.

Remove the check for stderr and make t5551-23 pass again

Signed-off-by: Torsten Bögershausen tbo...@web.de
---
 t/t5551-http-fetch-smart.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index e07eaf3..2c49133 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -240,8 +240,7 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the 
repo' '
 '
 
 test_expect_success EXPENSIVE 'clone the 50,000 tag repo to check OS command 
line overflow' '
-   git clone $HTTPD_URL/smart/repo.git too-many-refs 2err 
-   test_line_count = 0 err 
+   git clone $HTTPD_URL/smart/repo.git too-many-refs 
(
cd too-many-refs 
test $(git for-each-ref refs/tags | wc -l) = 5
-- 
2.0.0.553.ged01b91

--
To unsubscribe from this list: send the line 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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component

[Torsten Bögershausen]

On 2014-06-05 21.26, David Turner wrote:
 On Thu, 2014-06-05 at 14:30 +0200, Torsten Bögershausen wrote:
 On 2014-06-04 23.16, David Turner wrote:

 Sure!  I actually went with  120k to make measurement easier:
 https://github.com/dturner-tw/many-refs
 Hm, I didn't get so man

 git remote -v
 origin  https://github.com/dturner-tw/many-refs 

  wc .git/packed-refs 
  7501130   38868 .git/packed-refs

 
 Oops.  It looks like I forgot to push all of the refs.  And when I try,
 it fails with fatal: cannot exec 'send-pack': Argument list too long

I just noticed that I may be able to re-use code from t5551 to create a repo
with 5 tags.


And how about renaming check_refname_component_1() into
check_refname_component_slow() ;-)








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

Re: [PATCH v2] Add a Windows-specific fallback to getenv(HOME);

[Karsten Blees]

Am 05.06.2014 11:58, schrieb Erik Faye-Lund:
 On Thu, Jun 5, 2014 at 11:40 AM, Karsten Blees karsten.bl...@gmail.com 
 wrote:
 Am 05.06.2014 10:03, schrieb Stepan Kasal:
 From: Johannes Schindelin johannes.schinde...@gmx.de
 Date: Wed, 2 Jun 2010 00:41:33 +0200

 If HOME is not set, use $HOMEDRIVE$HOMEPATH

 Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---

 Hello Karsten,
 thanks for your explanation.  There are more things to be done, but
 I hope you can ack this patch as a step forward.


 No, not really. Its sure better than introducing a special 
 get_home_directory(), but it still increases the diff between upstream and 
 msysgit rather than reducing it. The main critique points still remain:

  * $HOME is usually set up correctly before calling git, so this is 
 essentially dead code (just checked, portable git's git-bash.bat and 
 git-cmd.bat also do this correctly)
 
 What about when tools like TortoiseGit and Git Extensions call git?
 We're not guaranteed that they did the $HOME-dance, are we?
 

GitExtensions does the same thing, see issue 497. I don't know about 
TortoiseGit, but I suspect the same.

  * even if $HOME was empty, git should setenv(HOME) so that child 
 processes can benefit from it (similar to TMPDIR and TERM in current 
 msysgit's mingw_startup()). Not setting $HOME because it may hypothetically 
 break child processes is a very weak argument, as we always did set $HOME in 
 etc/profile (since the initial version back in 2007).

  * no fallback to $USERPROFILE doesn't work with diconnected home share

 If you really have time to spare, I suggest you focus on getting the Unicode 
 patches upstream so that we can progress from there (e.g. move $HOME setup 
 to mingw_startup() so that we can get rid of redundant logic in etc/profile, 
 git-wrapper, git-bash.bat, git-cmd.bat etc.).
 
 Perhaps we can patch up the upstream to better match Git for Windows
 without upstreaming the Unicode patches? Don't get me wrong; I think
 upstreaming them is a good idea, but in case time is lacking...
 

The unicode patch series happens to be one of the first on top of upstream, and 
its also the longest (~40 patches) and I believe most intrusive one (~1500 
lines changed). So I think the most time-preserving option is to send it 
upstream as unchanged as possible (probably with the bugfix-patches squashed). 
There's only ~50 lines changed outside of compat, so hopefully there won't be 
too many additional review-rounds...
--
To unsubscribe from this list: send the line 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 1/4] replace: add --graft option

[Junio C Hamano]

Christian Couder chrisc...@tuxfamily.org writes:

 +static int create_graft(int argc, const char **argv, int force)
 +{
 + unsigned char old[20], new[20];
 + const char *old_ref = argv[0];
 + struct commit *commit;
 + struct strbuf buf = STRBUF_INIT;
 + struct strbuf new_parents = STRBUF_INIT;
 + const char *parent_start, *parent_end;
 + int i;
 +
 + if (get_sha1(old_ref, old)  0)
 + die(_(Not a valid object name: '%s'), old_ref);
 + commit = lookup_commit_or_die(old, old_ref);

Not a problem with this patch, but the above sequence somehow makes
me wonder if lookup-commit-or-die is a good API for this sort of
thing.  Wouldn't it be more natural if it took not unsigned char
old[20] but anything that would be understood by get_sha1()?

It could be that this particular caller is peculiar and all the
existing callers are happy, though.  I didn't git grep to spot
patterns in existing callers.

 + if (read_sha1_commit(old, buf))
 + die(_(Invalid commit: '%s'), old_ref);
 + /* make sure the commit is not corrupt */
 + if (parse_commit_buffer(commit, buf.buf, buf.len))
 + die(_(Could not parse commit: '%s'), old_ref);

It is unclear to me what you are trying to achieve with these two.
If the earlier lookup-commit has returned a commit object that has
already been parsed, parse_commit_buffer() would not check anything,
would it?

A typical sequence would look more like this:

commit = lookup_commit(...);
if (parse_commit(commit))
oops there is an error;
/* otherwise */
use(commit-buffer);

without reading a commit object using low-level read-sha1-file
interface yourself, no?
--
To unsubscribe from this list: send the line 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 4/4] contrib: add convert-grafts-to-replace-refs.sh

[Junio C Hamano]

Christian Couder chrisc...@tuxfamily.org writes:

 diff --git a/contrib/convert-grafts-to-replace-refs.sh 
 b/contrib/convert-grafts-to-replace-refs.sh
 new file mode 100755
 index 000..8472879
 --- /dev/null
 +++ b/contrib/convert-grafts-to-replace-refs.sh
 @@ -0,0 +1,29 @@
 +#!/bin/sh
 +
 +# You should execute this script in the repository where you
 +# want to convert grafts to replace refs.
 +
 +die () {
 + echo 2 $@
 + exit 1
 +}

Don't we install git-sh-setup in GIT_EXEC_PATH, in order to allow
these third-party scripts to begin with:

. $(git --exec-path)/git-sh-setup

just like our own scripted Porcelains?

 +GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
 +
 +test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
 +
 +grep '^[^# ]' $GRAFTS_FILE | while read definition
 +do

Format the above like so:

grep '^[^# ]' $GRAFTS_FILE |
while read definition
do

which is easier to see what that do is doing.

 + test -n $definition  {
 + echo Converting: $definition
 + git replace --graft $definition ||
 + die Conversion failed for: $definition
 + }

Hmph, why not if/then/fi?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [msysGit] [PATCH] mingw: redefine the wrapper macro after the corresponding function

[Karsten Blees]

Am 05.06.2014 18:56, schrieb Johannes Sixt:
 Am 05.06.2014 10:05, schrieb Stepan Kasal:
 mingw.c defines several wrapper functionsi, like mingw_unlink().
 These wrappers are deployed by macros like this:
  #define unlink mingw_unlink
 The function itself is preceded by #undef, leaving the wrapper out
 of the game for the rest of mingw.c.

 This was not probably intentional; for example, there are three
 calls to open() below the definition mingw_open() that probably
 have no reason to circumvent the wrapper.
 OTOH, there is one call to gethostbyname() before it was undefined;
 probably happy that it actually calls mingw_gethostbyname().

 This patch adds back the #define after each wrapper definition.

 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---
  compat/mingw.c | 20 
  1 file changed, 20 insertions(+)

 diff --git a/compat/mingw.c b/compat/mingw.c
 index a0e13bc..e7193c0 100644
 --- a/compat/mingw.c
 +++ b/compat/mingw.c
 @@ -224,6 +224,7 @@ int mingw_unlink(const char *pathname)
 ret = unlink(pathname);
  return ret;
  }
 +#define unlink mingw_unlink
 (etc...)
 
 I don't particularly like this approach: It robs the precise control of
 which function we can invoke from other places in mingw.c.
 
 Within mingw.c, if some other function inside mingw.c wants to use
 mingw_unlink, then it should be written as 'mingw_unlink(foo)', not
 'unlink(foo)'.
 

I very much like this approach. In fact, we already do this for e.g. 
mingw_raise.

 So, IMO the macros should be #undef'ed at the top of the file, and all
 users (like the open() and gethostbyname() invocations that you
 identified) should be audited and changed to call the function they
 actually need (i.e., the system open vs. mingw_open).
 

I'm sceptical of moving all #undef's to the top. Other callers would typically 
want the wrapped version (i.e. mingw_*). At least I can't think of a scenario 
in which a higher level function would want to bypass the wrapper...

--
To unsubscribe from this list: send the line 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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component

[David Turner]

On Thu, 2014-06-05 at 23:42 +0200, Torsten Bögershausen wrote:
 On 2014-06-05 21.26, David Turner wrote:
  On Thu, 2014-06-05 at 14:30 +0200, Torsten Bögershausen wrote:
  On 2014-06-04 23.16, David Turner wrote:
 
  Sure!  I actually went with  120k to make measurement easier:
  https://github.com/dturner-tw/many-refs
  Hm, I didn't get so man
 
  git remote -v
  origin  https://github.com/dturner-tw/many-refs 
 
   wc .git/packed-refs 
   7501130   38868 .git/packed-refs
 
  
  Oops.  It looks like I forgot to push all of the refs.  And when I try,
  it fails with fatal: cannot exec 'send-pack': Argument list too long
 
 I just noticed that I may be able to re-use code from t5551 to create a repo
 with 5 tags.

That's good, because github really didn't like me having a repo with
thousands of refs.  Try 100k, tho, because the difference is easier to
see.



--
To unsubscribe from this list: send the line 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] docs: Explain the purpose of fetch's and pull's refspec parameter.

[Junio C Hamano]

Marc Branchaud marcn...@xiplink.com writes:

 This patch applies atop your 8/9.  I feel strongly that some kind of
 reference should accompany this description, and your new CONFIGURED
 REMOTE-TRACKING BRANCHES section seems like a good one for the fetch
 variant, but since pull's variant doesn't have that section I just
 made it link to fetch's doc.

 (Also, I'm not sure if CRTB is a good link ID for your new section.)

Nobody looks at these ids, hopefully ;-)

 diff --git a/Documentation/pull-fetch-param.txt 
 b/Documentation/pull-fetch-param.txt
 index 18cffc2..40304c6 100644
 --- a/Documentation/pull-fetch-param.txt
 +++ b/Documentation/pull-fetch-param.txt
 @@ -12,9 +12,20 @@ ifndef::git-pull[]
  endif::git-pull[]
  
  refspec::
 - The format of a refspec parameter is an optional plus
 - `+`, followed by the source ref src, followed
 - by a colon `:`, followed by the destination ref dst.
 + Specifies which refs to fetch and which local refs to update.

That is an improvement.  We should first say what it is and what it
is for before saying how you spell it and the above change is
exactly that.

 + refspec parameters are not normally specified on the command
 + line, but instead are read from `remote.repository.fetch`

I however am not sure if this is an improvement, especially the
normally part.  Those who respond to a git-pull-request output
might be fewer than those who send pull requests, but that does not
mean they are abnormal.

The command line often omit refspec parameters when
fetching or pulling from a remote you regularly interact
with, in which case `remote.repository.fetch` values are
used instead.

would be OK, though.

Later today I'll push out the series on 'pu' after amending them
with your comments so far.  It would be nice if you can reroll this
on top of the updated one (git log --oneline --first-parent
master..pu and find jc/fetch-pull-refmap in there).

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: [msysGit] Re: [PATCH] mingw: redefine the wrapper macro after the corresponding function

[Karsten Blees]

Am 05.06.2014 17:13, schrieb Stepan Kasal:
 Hello Karsten,
 
 On Thu, Jun 05, 2014 at 04:51:39PM +0200, Karsten Blees wrote:
 In the current msysgit HEAD, most of these #undef's can simply be
 removed or have already been removed [...]
 
 not most of.  According to my quick count, 6 of 20 have been removed,
 2 more can be removed.  The remaining 12 do play their role.
 (The point you overlooked is that there are several socket related,
 like bind().)
 

Right, premature generalization on my part from looking at the first two or so 
entries.

However, I suspect some of the remaining 12 could be improved so that no #undef 
is necessary. E.g. gethostbyname could probably be implemented using 
GetAddrInfoW (supports non-ASCII names and IPv6).


--
To unsubscribe from this list: send the line 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] userdiff: support C# async methods and correct C# keywords

[Junio C Hamano]

Sup Yut Sum ch3co...@gmail.com writes:

 async is in C# 5.0
 foreach is in C# 1.0

 instanceof is in Java. The similar keywords are typeof, is, as in C# 1.0

This one made me read it twice, until I realized you meant

instanceof() is listed as keywords, but there is no such thing
(it is in Java, though); in C# we use typeof() for similar
purposes

 default, try are in C# 1.0

 Signed-off-by: Sup Yut Sum ch3co...@gmail.com
 ---
  userdiff.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/userdiff.c b/userdiff.c
 index fad52d6..96eda6c 100644
 --- a/userdiff.c
 +++ b/userdiff.c
 @@ -134,9 +134,9 @@ PATTERNS(cpp,
|[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-\\*?|\\.\\*),
  PATTERNS(csharp,
/* Keywords */
 -  !^[ 
 \t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n
 +  !^[ 
 \t]*(do|while|for|foreach|if|else|typeof|is|as|new|return|switch|case|default|throw|try|catch|using)\n
/* Methods and constructors */
 -  ^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
 +  ^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe|async)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
/* Properties */
^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n
/* Type definitions */
--
To unsubscribe from this list: send the line 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] environment: enable core.preloadindex by default

[Junio C Hamano]

Steve Hoelzer shoel...@gmail.com writes:

 There is consensus that the default should change because it will
 benefit nearly all users (some just a little, but some a lot).
 See [1] and replies.

 [1]: 
 http://git.661346.n2.nabble.com/git-status-takes-30-seconds-on-Windows-7-Why-tp7580816p7580853.html

 Signed-off-by: Steve Hoelzer shoel...@gmail.com
 ---

The patch is whitespace damaged, and the log message was unusable
without referring to an external site.  Both locally fixed and
queued.

Thanks.

  Documentation/config.txt | 4 ++--
  environment.c| 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 1932e9b..4b3d965 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -613,9 +613,9 @@ core.preloadindex::
  +
  This can speed up operations like 'git diff' and 'git status' especially
  on filesystems like NFS that have weak caching semantics and thus
 -relatively high IO latencies.  With this set to 'true', Git will do the
 +relatively high IO latencies.  When enabled, Git will do the
  index comparison to the filesystem data in parallel, allowing
 -overlapping IO's.
 +overlapping IO's.  Defaults to true.

  core.createObject::
   You can set this to 'link', in which case a hardlink followed by
 diff --git a/environment.c b/environment.c
 index 5c4815d..1c686c9 100644
 --- a/environment.c
 +++ b/environment.c
 @@ -71,7 +71,7 @@ unsigned long pack_size_limit_cfg;
  char comment_line_char = '#';

  /* Parallel index stat data preload? */
 -int core_preload_index = 0;
 +int core_preload_index = 1;

  /* This is set by setup_git_dir_gently() and/or git_default_config() */
  char *git_work_tree_cfg;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/4] Use transactions for renames

[Ronnie Sahlberg]

This series is based on ref-transactions-reflog
It is also available at
https://github.com/rsahlberg/git/tree/ref-transactions-rename

This series adds support to perform rename_ref as a single transaction for
both deleting/re-creating the ref and updating the reflog.

Since we no longer use rename() for the reflog changes we can now support
renames even if the reflogs are symlinks.

In order to make the delete-then-create fully atomic and also to ensure that
at no point in time is the object unreferenced we add support to do
deletes via the packed refs file to transaction_commit. For any refs that are
to be deleted, we first copy these refs to the packed refs file.
This allows us to immediately delete the loose ref files for those refs and
still being able to cancel/rollback the transaction.
Once all the changes are successfully updated we finally commit the packed
refs file a second time at which stage all the refs-to-be-deleted all
disappear in one atomic rename().

This allows us to perform transactions that delete multiple refs and have the
delete appear as one atomic transaction to any external observer.


Ronnie Sahlberg (4):
  refs.c: allow passing raw git_committer_info as email to
_update_reflog
  refs.c: return error instead of dying when locking fails during
transaction
  refs.c: use packed refs when deleting refs during a transaction
  refs.c: update rename_ref to use a transaction

 refs.c| 308 +-
 refs.h|   1 +
 t/t3200-branch.sh |   7 --
 3 files changed, 192 insertions(+), 124 deletions(-)

-- 
2.0.0.583.g402232d

--
To unsubscribe from this list: send the line 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/4] refs.c: use packed refs when deleting refs during a transaction

[Ronnie Sahlberg]

Make the deletion of refs during a transaction more atomic.
Start by first copying all loose refs we will be deleting to the packed
refs file and then commit the packed refs file. Then re-lock the packed refs
file to avoid anyone else from modifying the refs we are to delete during this
transaction and keep it locked until we are finished.
Since all refs we are about to delete are now safely held in the packed refs
file we can proceed to immediately unlink any loose refs for those refs
and still be fully rollback-able.

The exception is for refs that can not be resolved. Those refs are never
added to the packed refs and will just be un-rollback-ably deleted during
commit.

By deleting all the loose refs at the start of the transaction we make make it
possible to both delete one ref and then re-create a different ref in the
same transaction, even if both the old-to-be-deleted and the new-to-be-created
refs would otherwise conflict in the file-system.

Example: rename m-m/m

In that example we want to delete the file 'm' so that we make room so
that we can create a directory with the same name in order to lock and
write to the ref m/m and its lock-file m/m.lock.

If there is a failure during the commit phase we can rollback without losing
any refs since we have so far only deleted the loose refs but we still have
the refs stored in the packed refs file.
Once we have finished all updates for refs and their reflogs we can repack
the packed refs file and remove the to-be-deleted refs from the packed refs,
at which point all the deleted refs will disappear in one atomic rename
operation.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 146 ++---
 1 file changed, 114 insertions(+), 32 deletions(-)

diff --git a/refs.c b/refs.c
index ab0b629..3692a4d 100644
--- a/refs.c
+++ b/refs.c
@@ -1311,7 +1311,7 @@ static struct ref_entry *get_packed_ref(const char 
*refname)
 
 /*
  * A loose ref file doesn't exist; check for a packed ref.  The
- * options are forwarded from resolve_safe_unsafe().
+ * options are forwarded from resolve_ref_unsafe().
  */
 static const char *handle_missing_loose_ref(const char *refname,
unsigned char *sha1,
@@ -1367,7 +1367,6 @@ const char *resolve_ref_unsafe(const char *refname, 
unsigned char *sha1, int rea
}
 
git_snpath(path, sizeof(path), %s, refname);
-
/*
 * We might have to loop back here to avoid a race
 * condition: first we lstat() the file, then we try
@@ -2474,6 +2473,9 @@ static int curate_packed_ref_fn(struct ref_entry *entry, 
void *cb_data)
return 0;
 }
 
+/*
+ * Must be called with packed refs already locked (and sorted)
+ */
 static int repack_without_refs(const char **refnames, int n, struct strbuf 
*err)
 {
struct ref_dir *packed;
@@ -2486,19 +2488,10 @@ static int repack_without_refs(const char **refnames, 
int n, struct strbuf *err)
if (get_packed_ref(refnames[i]))
break;
 
-   /* Avoid locking if we have nothing to do */
+   /* Avoid processing if we have nothing to do */
if (i == n)
return 0; /* no refname exists in packed refs */
 
-   if (lock_packed_refs(0)) {
-   if (err) {
-   unable_to_lock_message(git_path(packed-refs), errno,
-  err);
-   return -1;
-   }
-   unable_to_lock_error(git_path(packed-refs), errno);
-   return error(cannot delete '%s' from packed refs, 
refnames[i]);
-   }
packed = get_packed_refs(ref_cache);
 
/* Remove refnames from the cache */
@@ -3632,10 +3625,12 @@ static int ref_update_reject_duplicates(struct 
ref_update **updates, int n,
 int transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
-   int ret = 0, delnum = 0, i, df_conflict = 0;
+   int ret = 0, delnum = 0, i, df_conflict = 0, need_repack = 0;
const char **delnames;
int n = transaction-nr;
+   struct packed_ref_cache *packed_ref_cache;
struct ref_update **updates = transaction-updates;
+   struct ref_dir *packed;
 
if (transaction-state != REF_TRANSACTION_OPEN)
die(BUG: commit called for transaction that is not open);
@@ -3655,12 +3650,100 @@ int transaction_commit(struct ref_transaction 
*transaction,
goto cleanup;
}
 
-   /* Acquire all ref locks while verifying old values */
+   /* Lock packed refs during commit */
+   if (lock_packed_refs(0)) {
+   if (err)
+   unable_to_lock_message(git_path(packed-refs),
+  errno, err);
+   ret = -1;
+   goto cleanup;
+   }
+
+   /* any loose 

[PATCH 4/4] refs.c: update rename_ref to use a transaction

[Ronnie Sahlberg]

Change refs.c to use a single transaction to copy/rename both the refs and
its reflog. Since we are no longer using rename() to move the reflog file
we no longer need to disallow rename_ref for refs with a symlink for its reflog
so we can remove that test from the testsuite.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c| 140 +-
 t/t3200-branch.sh |   7 ---
 2 files changed, 64 insertions(+), 83 deletions(-)

diff --git a/refs.c b/refs.c
index 3692a4d..4281011 100644
--- a/refs.c
+++ b/refs.c
@@ -2632,22 +2632,33 @@ static int rename_tmp_log(const char *newrefname)
return 0;
 }
 
-static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
- const char *logmsg);
+struct rename_reflog_cb {
+   struct ref_transaction *transaction;
+   const char *refname;
+   struct strbuf *err;
+};
+
+static int rename_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+const char *email, unsigned long timestamp, int tz,
+const char *message, void *cb_data)
+{
+   struct rename_reflog_cb *cb = cb_data;
+
+   return transaction_update_reflog(cb-transaction, cb-refname,
+nsha1, osha1, email, timestamp, tz,
+message, 0, cb-err);
+}
 
 int rename_ref(const char *oldrefname, const char *newrefname, const char 
*logmsg)
 {
-   unsigned char sha1[20], orig_sha1[20];
-   int flag = 0, logmoved = 0;
-   struct ref_lock *lock;
-   struct stat loginfo;
-   int log = !lstat(git_path(logs/%s, oldrefname), loginfo);
+   unsigned char sha1[20];
+   int flag = 0, log;
+   struct ref_transaction *transaction = NULL;
+   struct strbuf err = STRBUF_INIT;
const char *symref = NULL;
+   struct rename_reflog_cb cb;
 
-   if (log  S_ISLNK(loginfo.st_mode))
-   return error(reflog for %s is a symlink, oldrefname);
-
-   symref = resolve_ref_unsafe(oldrefname, orig_sha1, 1, flag);
+   symref = resolve_ref_unsafe(oldrefname, sha1, 1, flag);
if (flag  REF_ISSYMREF)
return error(refname %s is a symbolic ref, renaming it is not 
supported,
oldrefname);
@@ -2656,77 +2667,54 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
 
if (!is_refname_available(newrefname, get_packed_refs(ref_cache),
  oldrefname, 1))
-   return 1;
+   return -1;
 
if (!is_refname_available(newrefname, get_loose_refs(ref_cache),
  oldrefname, 1))
-   return 1;
-
-   if (log  rename(git_path(logs/%s, oldrefname), 
git_path(TMP_RENAMED_LOG)))
-   return error(unable to move logfile logs/%s to 
TMP_RENAMED_LOG: %s,
-   oldrefname, strerror(errno));
-
-   if (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {
-   error(unable to delete old %s, oldrefname);
-   goto rollback;
-   }
-
-   if (!read_ref_full(newrefname, sha1, 1, NULL) 
-   delete_ref(newrefname, sha1, REF_NODEREF)) {
-   if (errno==EISDIR) {
-   if (remove_empty_directories(git_path(%s, 
newrefname))) {
-   error(Directory not empty: %s, newrefname);
-   goto rollback;
-   }
-   } else {
-   error(unable to delete existing %s, newrefname);
-   goto rollback;
-   }
-   }
-
-   if (log  rename_tmp_log(newrefname))
-   goto rollback;
-
-   logmoved = log;
-
-   lock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL, NULL, 0);
-   if (!lock) {
-   error(unable to lock %s for update, newrefname);
-   goto rollback;
-   }
-   lock-force_write = 1;
-   hashcpy(lock-old_sha1, orig_sha1);
-   if (write_ref_sha1(lock, orig_sha1, logmsg)) {
-   error(unable to write current sha1 into %s, newrefname);
-   goto rollback;
-   }
+   return -1;
 
+   log = reflog_exists(oldrefname);
+   transaction = transaction_begin(err);
+   if (!transaction)
+   goto fail;
+
+   if (strcmp(oldrefname, newrefname)) {
+   if (log  transaction_update_reflog(transaction, newrefname,
+sha1, sha1,
+git_committer_info(0),
+0, 0, NULL,
+REFLOG_TRUNCATE, err))
+   goto fail;
+   cb.transaction = transaction;
+   cb.refname = newrefname;
+   cb.err = err;
+ 

[PATCH 1/2] refs.c: write updates to packed refs when a transaction has more than one ref

[Ronnie Sahlberg]

When we are updating more than one single ref, i.e. not a commit, then
write the updated refs directly to the packed refs file instead of writing
them as loose refs.

Change clone to use a transaction instead of using the pacekd refs api.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 builtin/clone.c | 16 +
 refs.c  | 74 +
 2 files changed, 66 insertions(+), 24 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index b12989d..000a236 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -497,17 +497,25 @@ static struct ref *wanted_peer_refs(const struct ref 
*refs,
 static void write_remote_refs(const struct ref *local_refs)
 {
const struct ref *r;
+   struct ref_transaction *transaction;
+   struct strbuf err = STRBUF_INIT;
 
-   lock_packed_refs(LOCK_DIE_ON_ERROR);
+   transaction = transaction_begin(err);
+   if (!transaction)
+   die(%s, err.buf);
 
for (r = local_refs; r; r = r-next) {
if (!r-peer_ref)
continue;
-   add_packed_ref(r-peer_ref-name, r-old_sha1);
+   if (transaction_update_sha1(transaction, r-peer_ref-name,
+   r-old_sha1, NULL, 0, 0, NULL,
+   err))
+   die(%s, err.buf);
}
 
-   if (commit_packed_refs())
-   die_errno(unable to overwrite old ref-pack file);
+   if (transaction_commit(transaction, err))
+   die(%s, err.buf);
+   transaction_free(transaction);
 }
 
 static void write_followtags(const struct ref *refs, const char *msg)
diff --git a/refs.c b/refs.c
index 4281011..6aa64dd 100644
--- a/refs.c
+++ b/refs.c
@@ -2481,31 +2481,18 @@ static int repack_without_refs(const char **refnames, 
int n, struct strbuf *err)
struct ref_dir *packed;
struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
struct string_list_item *ref_to_delete;
-   int i, ret, removed = 0;
+   int i, ret;
 
/* Look for a packed ref */
for (i = 0; i  n; i++)
if (get_packed_ref(refnames[i]))
break;
 
-   /* Avoid processing if we have nothing to do */
-   if (i == n)
-   return 0; /* no refname exists in packed refs */
-
packed = get_packed_refs(ref_cache);
 
/* Remove refnames from the cache */
for (i = 0; i  n; i++)
-   if (remove_entry(packed, refnames[i]) != -1)
-   removed = 1;
-   if (!removed) {
-   /*
-* All packed entries disappeared while we were
-* acquiring the lock.
-*/
-   rollback_packed_refs();
-   return 0;
-   }
+   remove_entry(packed, refnames[i]);
 
/* Remove any other accumulated cruft */
do_for_each_entry_in_dir(packed, 0, curate_packed_ref_fn, 
refs_to_delete);
@@ -3614,6 +3601,7 @@ int transaction_commit(struct ref_transaction 
*transaction,
   struct strbuf *err)
 {
int ret = 0, delnum = 0, i, df_conflict = 0, need_repack = 0;
+   int num_updates = 0;
const char **delnames;
int n = transaction-nr;
struct packed_ref_cache *packed_ref_cache;
@@ -3647,18 +3635,38 @@ int transaction_commit(struct ref_transaction 
*transaction,
goto cleanup;
}
 
-   /* any loose refs are to be deleted are first copied to packed refs */
+   /* count how many refs we are updating (not deleting) */
for (i = 0; i  n; i++) {
struct ref_update *update = updates[i];
unsigned char sha1[20];
 
if (update-update_type != UPDATE_SHA1)
continue;
-   if (!is_null_sha1(update-new_sha1))
+   if (is_null_sha1(update-new_sha1))
+   continue;
+
+   num_updates++;
+   }
+
+   /*
+* Always copy loose refs that are to be deleted to the packed refs.
+* If we are updating multiple refs then copy all non symref refs
+* to the packed refs too.
+*/
+   for (i = 0; i  n; i++) {
+   struct ref_update *update = updates[i];
+   unsigned char sha1[20];
+   int flag;
+
+   if (update-update_type != UPDATE_SHA1)
+   continue;
+   if (num_updates  2  !is_null_sha1(update-new_sha1))
continue;
if (get_packed_ref(update-refname))
continue;
-   if (!resolve_ref_unsafe(update-refname, sha1, 1, NULL))
+   if (!resolve_ref_unsafe(update-refname, sha1, 1, flag))
+   continue;
+   if (flag  REF_ISSYMREF)
continue;
 

[PATCH 2/2] refs.c: make the *_packed_refs functions static

[Ronnie Sahlberg]

We no longer need to expose the lock/add/commit/rollback functions
for packed refs anymore so make them static and remove them from the api.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c |  8 
 refs.h | 28 
 2 files changed, 4 insertions(+), 32 deletions(-)

diff --git a/refs.c b/refs.c
index 6aa64dd..530947e 100644
--- a/refs.c
+++ b/refs.c
@@ -1123,7 +1123,7 @@ static struct ref_dir *get_packed_refs(struct ref_cache 
*refs)
return get_packed_ref_dir(get_packed_ref_cache(refs));
 }
 
-void add_packed_ref(const char *refname, const unsigned char *sha1)
+static void add_packed_ref(const char *refname, const unsigned char *sha1)
 {
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
@@ -2211,7 +2211,7 @@ static int write_packed_entry_fn(struct ref_entry *entry, 
void *cb_data)
return 0;
 }
 
-int lock_packed_refs(int flags)
+static int lock_packed_refs(int flags)
 {
struct packed_ref_cache *packed_ref_cache;
 
@@ -2234,7 +2234,7 @@ int lock_packed_refs(int flags)
  * Commit the packed refs changes.
  * On error we must make sure that errno contains a meaningful value.
  */
-int commit_packed_refs(void)
+static int commit_packed_refs(void)
 {
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
@@ -2259,7 +2259,7 @@ int commit_packed_refs(void)
return error;
 }
 
-void rollback_packed_refs(void)
+static void rollback_packed_refs(void)
 {
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
diff --git a/refs.h b/refs.h
index 3c99619..e92cb92 100644
--- a/refs.h
+++ b/refs.h
@@ -118,34 +118,6 @@ extern int for_each_rawref(each_ref_fn, void *);
 extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char 
*refname);
 
 /*
- * Lock the packed-refs file for writing.  Flags is passed to
- * hold_lock_file_for_update().  Return 0 on success.
- */
-extern int lock_packed_refs(int flags);
-
-/*
- * Add a reference to the in-memory packed reference cache.  This may
- * only be called while the packed-refs file is locked (see
- * lock_packed_refs()).  To actually write the packed-refs file, call
- * commit_packed_refs().
- */
-extern void add_packed_ref(const char *refname, const unsigned char *sha1);
-
-/*
- * Write the current version of the packed refs cache from memory to
- * disk.  The packed-refs file must already be locked for writing (see
- * lock_packed_refs()).  Return zero on success.
- */
-extern int commit_packed_refs(void);
-
-/*
- * Rollback the lockfile for the packed-refs file, and discard the
- * in-memory packed reference cache.  (The packed-refs file will be
- * read anew if it is needed again after this function is called.)
- */
-extern void rollback_packed_refs(void);
-
-/*
  * Flags for controlling behaviour of pack_refs()
  * PACK_REFS_PRUNE: Prune loose refs after packing
  * PACK_REFS_ALL:   Pack _all_ refs, not just tags and already packed refs
-- 
2.0.0.583.g402232d

--
To unsubscribe from this list: send the line 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 1/4] refs.c: allow passing raw git_committer_info as email to _update_reflog

[Ronnie Sahlberg]

In many places in the code we do not have access to the individual fields
in the committer data. Instead we might only have access to prebaked data
such as what is returned by git_committer_info() containing a string
that consists of email, timestamp, zone etc.

This makes it inconvenient to use transaction_update_reflog since it means you
would have to first parse git_committer_info before you can call update_reflog.

Add a new flag REFLOG_EMAIL_IS_COMMITTER to _update_reflog to tell it
that what we pass in as email is already the fully baked committer string
we can use as-is.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com

Conflicts:
refs.c
---
 refs.c | 20 
 refs.h |  1 +
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/refs.c b/refs.c
index 9689592..e3a0383 100644
--- a/refs.c
+++ b/refs.c
@@ -3489,14 +3489,18 @@ int transaction_update_reflog(struct ref_transaction 
*transaction,
hashcpy(update-old_sha1, old_sha1);
update-reflog_fd = -1;
if (email) {
-   struct strbuf buf = STRBUF_INIT;
-   char sign = (tz  0) ? '-' : '+';
-   int zone = (tz  0) ? (-tz) : tz;
-
-   strbuf_addf(buf, %s %lu %c%04d, email, timestamp, sign,
-   zone);
-   update-committer = xstrdup(buf.buf);
-   strbuf_release(buf);
+   if (flags  REFLOG_EMAIL_IS_COMMITTER)
+   update-committer = xstrdup(email);
+   else {
+   struct strbuf buf = STRBUF_INIT;
+   char sign = (tz  0) ? '-' : '+';
+   int zone = (tz  0) ? (-tz) : tz;
+
+   strbuf_addf(buf, %s %lu %c%04d, email, timestamp,
+   sign, zone);
+   update-committer = xstrdup(buf.buf);
+   strbuf_release(buf);
+   }
}
if (msg)
update-msg = xstrdup(msg);
diff --git a/refs.h b/refs.h
index 1b6a055..3c99619 100644
--- a/refs.h
+++ b/refs.h
@@ -318,6 +318,7 @@ int transaction_delete_sha1(struct ref_transaction 
*transaction,
struct strbuf *err);
 
 #define REFLOG_TRUNCATE 0x01
+#define REFLOG_EMAIL_IS_COMMITTER 0x02
 /*
  * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set
  * this update will first truncate the reflog before writing the entry.
-- 
2.0.0.583.g402232d

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

[PATCH 0/2] Write multi-ref updates directly to packed refs

[Ronnie Sahlberg]

This patch adds support to the transaction system to update all non-refs as
packed refs instead of loose refs.

This series can also be found at
https://github.com/rsahlberg/git/tree/ref-transactions-req-packed-refs

This series adds support to have transaction write updates to packed refs
instead of as loose refs.
If the transaction only contains a single update to a single ref, such as the
commit case, we continue to update this as locking and writing to the loose
ref file.
But If on the other hand the transaction will update more than one ref, then
we perform all these updates inside the packed refs file and remove all the
affected loose refs files. This allows us to both perform deletes (added
previously) as well as updates-of-more-than-one-ref as one single atomic rename
operation.

Since the transactions will now by default write directly to the packed refs
file for updates to more than one ref, we no longer need to provide a
pacekd refs api to builtin/clone.c any more. So lets change clone.c to
use a transaction. This removes 4 fucntions from the refs.c API we can now
unexport and make static.

This change means that clone will now apear as one single atomic operation
to any external observer where all refs appear at once instead of one
ref becoming visible at a time.


Ronnie Sahlberg (2):
  refs.c: write updates to packed refs when a transaction has more than
one ref
  refs.c: make the *_packed_refs functions static

 builtin/clone.c | 16 ---
 refs.c  | 82 -
 refs.h  | 28 
 3 files changed, 70 insertions(+), 56 deletions(-)

-- 
2.0.0.583.g402232d

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

[PATCH v7 0/1] refs.c: SSE4.2 optimizations for check_refname_component

[David Turner]

Since Junio has picked up the first patch from previous versions of
this series, I'm just going to send the second (SSE) one.  I decided
not to s/NO_SSE42/!HAVE_SSE42/ because it looks like git mostly uses
the former convention (for instance, that's what GIT_PARSE_WITH
generates).

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

[PATCH v7 1/1] refs.c: SSE4.2 optimizations for check_refname_component

[David Turner]

Optimize check_refname_component using SSE4.2, where available.

git rev-parse HEAD is a good test-case for this, since it does almost
nothing except parse refs.  For one particular repo with about 60k
refs, almost all packed, the timings are:

Look up table: 29 ms
SSE4.2:25 ms

This is about a 15% improvement.

The configure.ac changes include code from the GNU C Library written
by Joseph S. Myers joseph at codesourcery dot com.

Only supports GCC and Clang at present, because C interfaces to the
cpuid instruction are not well-standardized.

Signed-off-by: David Turner dtur...@twitter.com
---
 Makefile   |   6 +++
 aclocal.m4 |   6 +++
 compat/cpuid.h |  25 +
 configure.ac   |  17 ++
 git-compat-util.h  |  23 
 refs.c | 150 +++--
 t/t5511-refspec.sh |  13 +
 7 files changed, 225 insertions(+), 15 deletions(-)
 create mode 100644 compat/cpuid.h

diff --git a/Makefile b/Makefile
index a53f3a8..dd2127a 100644
--- a/Makefile
+++ b/Makefile
@@ -1326,6 +1326,11 @@ else
COMPAT_OBJS += compat/win32mmap.o
endif
 endif
+ifdef NO_SSE42
+   BASIC_CFLAGS += -DNO_SSE42
+else
+   BASIC_CFLAGS += -msse4.2
+endif
 ifdef OBJECT_CREATION_USES_RENAMES
COMPAT_CFLAGS += -DOBJECT_CREATION_MODE=1
 endif
@@ -2199,6 +2204,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' $@
@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' $@
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst 
','\'',$(NO_UNIX_SOCKETS)))'\' $@
+   @echo NO_SSE42=\''$(subst ','\'',$(subst ','\'',$(NO_SSE42)))'\' $@
 ifdef TEST_OUTPUT_DIRECTORY
@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst 
','\'',$(TEST_OUTPUT_DIRECTORY)))'\' $@
 endif
diff --git a/aclocal.m4 b/aclocal.m4
index f11bc7e..d9f3f19 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -38,3 +38,9 @@ AC_DEFUN([TYPE_SOCKLEN_T],
   [#include sys/types.h
 #include sys/socket.h])
 ])
+
+dnl Test a compiler option or options with an empty input file.
+dnl LIBC_TRY_CC_OPTION([options], [action-if-true], [action-if-false])
+AC_DEFUN([LIBC_TRY_CC_OPTION],
+[AS_IF([AC_TRY_COMMAND([${CC-cc} $1 -xc /dev/null -S -o /dev/null])],
+   [$2], [$3])])
diff --git a/compat/cpuid.h b/compat/cpuid.h
new file mode 100644
index 000..7709e35
--- /dev/null
+++ b/compat/cpuid.h
@@ -0,0 +1,25 @@
+#ifndef COMPAT_CPUID_H
+#define COMPAT_CPUID_H
+
+#if (defined(__GNUC__)  (defined(__i386__) || defined(__x86_64__)))
+#include cpuid.h
+#define CPUID_SSE42_BIT (1  20)
+
+static int processor_supports_sse42(void)
+{
+   unsigned eax, ebx, ecx, edx;
+   __cpuid(1, eax, ebx, ecx, edx);
+   return ecx  CPUID_SSE42_BIT;
+}
+
+#else
+
+static int processor_supports_sse42(void)
+{
+   /* TODO: handle CPUID on non-GCC compilers */
+   return 0;
+}
+
+#endif
+
+#endif /* COMPAT_CPUID_H */
diff --git a/configure.ac b/configure.ac
index b711254..3a5bda9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -382,6 +382,23 @@ AS_HELP_STRING([],[Tcl/Tk interpreter will be found in a 
system.]),
 GIT_PARSE_WITH(tcltk))
 #
 
+# Declare the with-sse42/without-sse42 options.
+AC_ARG_WITH(sse42,
+AS_HELP_STRING([--with-sse42],[use SSE4.2 instructions])
+AS_HELP_STRING([],[(default is YES if your compiler supports -msse4.2)]),
+GIT_PARSE_WITH(sse42))
+
+if test $NO_SSE42 != YesPlease; then
+   dnl Check if -msse4.2 works.
+   AC_CACHE_CHECK(for SSE4.2 support, cc_cv_sse42, [dnl
+   LIBC_TRY_CC_OPTION([-msse4.2], [cc_cv_sse42=yes], [cc_cv_sse42=no])
+   ])
+   if test $cc_cv_sse42 = no; then
+ NO_SSE42=1
+   fi
+fi
+
+GIT_CONF_SUBST([NO_SSE42])
 
 ## Checks for programs.
 AC_MSG_NOTICE([CHECKS for programs])
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..2c66d6d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -668,6 +668,29 @@ void git_qsort(void *base, size_t nmemb, size_t size,
 #endif
 #endif
 
+#ifndef NO_SSE42
+#include nmmintrin.h
+#include compat/cpuid.h
+/*
+ * Clang ships with a version of nmmintrin.h that's incomplete; if
+ * necessary, we define the constants that we're going to use.
+ */
+#ifndef _SIDD_UBYTE_OPS
+#define _SIDD_UBYTE_OPS 0x00
+#define _SIDD_CMP_EQUAL_ANY 0x00
+#define _SIDD_CMP_RANGES0x04
+#define _SIDD_CMP_EQUAL_ORDERED 0x0c
+#define _SIDD_NEGATIVE_POLARITY 0x10
+#endif
+
+/* This is the system memory page size; it's used so that we can read
+ * outside the bounds of an allocation without segfaulting.
+ */
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+#endif
+
 #ifdef UNRELIABLE_FSTAT
 #define fstat_is_reliable() 0
 #else
diff --git a/refs.c b/refs.c
index 46139d2..b3bd333 100644
--- a/refs.c
+++ b/refs.c
@@ -24,6 +24,25 @@ static unsigned char refname_disposition[256] = {
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
 };
 
+static int 

[PATCH v3] receive-pack: optionally deny case clone refs

[David Turner]

It is possible to have two refs which are the same but for case.
This works great on the case-sensitive filesystems, but not so well on
case-insensitive filesystems.  It is fairly typical to have
case-insensitive clients (Macs, say) with a case-sensitive server
(GNU/Linux).

Should a user attempt to pull on a Mac when there are case clone refs
with differing contents, they'll get an error message containing
something like Ref refs/remotes/origin/lower is at
[sha-of-lowercase-ref] but expected [sha-of-uppercase-ref]
(unable to update local ref)

With a case-insensitive git server, if a branch called capital-M
Master (that differs from lowercase-m-master) is pushed, nobody else
can push to (lowercase-m) master until the branch is removed.

Create the option receive.denycaseclonerefs, which checks pushed
refs to ensure that they are not case clones of an existing
ref.  This setting is turned on by default if core.ignorecase is
set, but not otherwise.

Signed-off-by: David Turner dtur...@twitter.com
---
 Documentation/config.txt   |  6 +
 Documentation/git-push.txt |  5 +++--
 Documentation/glossary-content.txt |  5 +
 builtin/receive-pack.c | 27 +-
 t/t5400-send-pack.sh   | 46 ++
 5 files changed, 86 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1932e9b..b24b117 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2053,6 +2053,12 @@ receive.unpackLimit::
especially on slow filesystems.  If not set, the value of
`transfer.unpackLimit` is used instead.
 
+receive.denyCaseCloneRefs::
+   If set to true, git-receive-pack will deny a ref update that creates
+   a ref which is the same but for case as an existing ref.  This is
+   useful when clients are on a case-insensitive filesystem, which
+   will cause errors when given refs which differ only in case.
+
 receive.denyDeletes::
If set to true, git-receive-pack will deny a ref update that deletes
the ref. Use this to prevent such a ref deletion via a push.
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 21cd455..c92c3a6 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -323,8 +323,9 @@ remote rejected::
of the following safety options in effect:
`receive.denyCurrentBranch` (for pushes to the checked out
branch), `receive.denyNonFastForwards` (for forced
-   non-fast-forward updates), `receive.denyDeletes` or
-   `receive.denyDeleteCurrent`.  See linkgit:git-config[1].
+   non-fast-forward updates), `receive.denyDeletes`,
+   `receive.denyCaseCloneRefs` or `receive.denyDeleteCurrent`.
+   See linkgit:git-config[1].
 
 remote failure::
The remote end did not report the successful update of the ref,
diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index be0858c..ed5ac23 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -31,6 +31,11 @@
 [[def_cache]]cache::
Obsolete for: def_index,index.
 
+[[def_case_clone]]case clone::
+   Two entities (e.g. filenames or refs) that differ only in case.
+   These can cause problems on case-insensitive filesystems, and
+   Git has machinery to prevent these problems in various cases.
+
 [[def_chain]]chain::
A list of objects, where each def_object,object in the list contains
a reference to its successor (for example, the successor of a
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c323081..8530a6c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -27,6 +27,7 @@ enum deny_action {
 
 static int deny_deletes;
 static int deny_non_fast_forwards;
+static int deny_case_clone_refs = DENY_UNCONFIGURED;
 static enum deny_action deny_current_branch = DENY_UNCONFIGURED;
 static enum deny_action deny_delete_current = DENY_UNCONFIGURED;
 static int receive_fsck_objects = -1;
@@ -69,6 +70,11 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
if (status)
return status;
 
+   if (strcmp(var, receive.denycaseclonerefs) == 0) {
+   deny_case_clone_refs = parse_deny_action(var, value);
+   return 0;
+   }
+
if (strcmp(var, receive.denydeletes) == 0) {
deny_deletes = git_config_bool(var, value);
return 0;
@@ -468,6 +474,22 @@ static int update_shallow_ref(struct command *cmd, struct 
shallow_info *si)
return 0;
 }
 
+static int is_case_clone(const char *refname, const unsigned char *sha1,
+   int flags, void *cb_data)
+{
+   const char *incoming_refname = cb_data;
+   return !strcasecmp(refname, incoming_refname) 
+   strcmp(refname, incoming_refname);
+}
+
+static int 

Re: [PATCH v2 10/11] test-lib: make it possible to override how test code is eval'd

[Richard Hansen]

On 2014-06-05 17:11, Junio C Hamano wrote:
 Richard Hansen rhan...@bbn.com writes:
 
 Because test_eval_ is defined while zsh is in sh emulation mode, the
 shell code passed as an argument to test_expect_success would normally
 be evaluated in sh emulation mode.  However, with this change, it is
 now possible to evaluate the test code in zsh mode by adding the
 following line to a zsh-based test script:

 emulate -R zsh -c 'test_eval_override () { eval $*; }'

 With test_eval_override defined in zsh emulation mode, the call to
 test_eval_override from test_eval_ will temporarily cause zsh to
 switch from sh emulation mode to zsh emulation mode.
 
 Micronit: aren't all zsh emulation modes above zsh native modes?

Sort of...  Zsh's emulation is under-documented, confusing, and
awkwardly implemented (in my opinion) so it's hard to precisely describe
what's going on in a few words.

I'll explain it here more precisely (apologies for the length) and you
can let me know if I should add more detail to the commit message and/or
code comments:

As far as I understand it, there isn't really such a thing as an
emulation mode in Zsh -- the phrase emulation mode is just a
convenient way to refer to the mechanism.  Zsh emulates other shells
by simply toggling a particular collection of shell options.  For
example, 'emulate sh' turns on the option that controls field splitting
and turns off the option that updates $0 whenever a function is called
(among other option changes).

One might expect 'emulate shell -c code' to be equivalent to
'emulate shell; code; emulate previous_shell' but it's not --
there's a significant behavior difference.  When '-c code' is used,
and only when '-c code' is used, a function defined inside code gets
sticky options:  The shell remembers the state of some (all?) options
and saves them along with the function definition.  Whenever the
function is called, those sticky options are temporarily restored for
the duration of the function call.  (This is not entirely accurate --
there are some relatively unimportant cases where the sticky options
aren't restored -- but I don't understand the subtleties of the
mechanism well enough to provide a perfectly accurate description.)

If a function with sticky options calls a function without sticky
options, the sticky options remain in effect -- Zsh doesn't temporarily
revert the sticky options while it calls the non-sticky function.

Thus, if a function foo() is declared inside 'emulate sh -c code', and
it calls a function bar() that was declared outside of 'emulate', the
options are still set for POSIX sh during bar()'s execution.  This might
cause bar() to misbehave.  To work around this, bar() must be declared
like 'emulate zsh -c bar() { stuff... }' so that bar() gets its own
sticky options.

This commit makes it possible to declare a function that will get sticky
options so that we can control the state of the shell when the scriptlet
is executed.

 
 In any case, the above explanation confuses me somewhat.  test_eval_
 is fed a scriptlet defined for various test_expect_success tests,
 and they are written in POSIX shells, not zsh, so wouldn't it be
 wrong to run them as if they are zsh native scripts, following
 non-POSIX shell syntax rules?

The scriptlets in lib-prompt-tests.sh are not actually written for POSIX
sh -- they are written in a common subset of the zsh and bash languages
(I should document this in lib-prompt-tests.sh).

We want to test how the __git_ps1 code behaves when interpreted in
native zsh mode (default options), because that's how it will be used
in the wild, so the scriptlets must be valid zsh code.  We also want to
test how __git_ps1 behaves in native bash mode, so the scriptlets must
also be valid bash code.  (Fortunately the similarities between the
shells make this easy to do.)

An alternative to this commit -- and I kinda like this idea so I'm
tempted to rewrite the series -- would be to do change the form of the
tests in lib-prompt-tests.sh to something like this:

test_expect_success 'name of test here' '
run_in_native_shell_mode '\''
scriptlet code here
'\''
'

Then lib-zsh.sh would do this:

run_in_native_shell_mode () { emulate -R zsh -c $*; }

and lib-bash.sh would do this:

run_in_native_shell_mode () { eval $*; }

This approach makes it clear to others that the scriptlet code is not
actually POSIX shell code, but a common subset of multiple shell languages.

What do you think?

Ignoring t9902 for a moment, we could even stop doing that messy 'exec
$SHELL $0 $@' stuff in lib-*sh.sh and let t9903 and t9904 run under
/bin/sh.  Then run_in_native_shell_mode() would be defined as follows:

  for zsh:

# wrap the scriptlet in a function in case it does 'return'
run_in_native_shell_mode () { zsh -c foo () { $*; }; foo; }

  for bash:

# wrap the scriptlet in a function in case it does 'return'
run_in_native_shell_mode () { bash -c foo () { $*; }; foo; }

Re: [PATCH v3] receive-pack: optionally deny case clone refs

[Duy Nguyen]

On Fri, Jun 6, 2014 at 7:52 AM, David Turner dtur...@twopensource.com wrote:
 Create the option receive.denycaseclonerefs, which checks pushed
 refs to ensure that they are not case clones of an existing
 ref.  This setting is turned on by default if core.ignorecase is
 set, but not otherwise.

Just thinking out loud as I haven't had time to read this patch
carefully, but I wonder if there's a potential performance problem
with ref_is_denied_case_clone(). What if the receiver repo has 60k
refs? Should we use name-hash (or a variant of it)? What if the sender
pushes 60k refs in one go, will ref_is_denied_case_clone check against
existing refs only, or it will cover more and more refs from the 60k
input?
-- 
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 v3] receive-pack: optionally deny case clone refs

[David Turner]

On Fri, 2014-06-06 at 08:37 +0700, Duy Nguyen wrote:
 On Fri, Jun 6, 2014 at 7:52 AM, David Turner dtur...@twopensource.com wrote:
  Create the option receive.denycaseclonerefs, which checks pushed
  refs to ensure that they are not case clones of an existing
  ref.  This setting is turned on by default if core.ignorecase is
  set, but not otherwise.
 
 Just thinking out loud as I haven't had time to read this patch
 carefully, but I wonder if there's a potential performance problem
 with ref_is_denied_case_clone(). What if the receiver repo has 60k
 refs? Should we use name-hash (or a variant of it)? What if the sender
 pushes 60k refs in one go, will ref_is_denied_case_clone check against
 existing refs only, or it will cover more and more refs from the 60k
 input?

Good news!  The sender can't push 60k refs in one go because that would
require a command-line bigger than ARG_MAX.  (Well, OK, this is not
really good news, since of course someone could hack around this, and
since this is minor bug in git that we should probably fix).

I guess this does present a bit of a DOS attack.  When I discussed the
previous version of this patch with Junio, I did not think it was a huge
problem, since the the ordinary number of refs pushed is small.  But
when I think of it as a DOS, it sounds much worse.

I don't like the idea of creating a hashmap just for this, because
really we need to be storing packed refs in an better on-disk format --
one that allows O(log n) or better access (as has been discussed).  

I'll give this some more thought and see if I can come up with a better
solution.

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

Re: Re: [git] [PATCH 2/5] implement submodule config cache for lookup of submodule names

[Heiko Voigt]

On Thu, Jun 05, 2014 at 10:46:10AM -0700, W. Trevor King wrote:
 On Thu, Jun 05, 2014 at 08:07:50AM +0200, Heiko Voigt wrote:
  +The caller can look up information about submodules by using the
  +`submodule_from_path()` or `submodule_from_name()` functions.
 
 That's for an already-known submodule.  Do we need a way to list
 submodules (e.g. for 'submodule foreach' style operations) or is the
 preferred way to do that just walking the tree looking for gitlinks?
 The cases where .gitmodules would lead you astray (e.g. via sloppy
 commits after removing a submodule) are:
 
 * Listing a submodule that wasn't in the tree anymore.  Easy to check
   for and ignore.
 
 * Not listing a submodule that is in the tree.  You'd need to walk the
   tree to check for this, but it's a pretty broken situation already,
   so I'd be fine just ignoring the orphaned gitlink.

Currently there is no need to list the submodules in a .gitmodule file.
We currently always begin from the gitlink and try to do things. If we
have enough information thats fine we go ahead, if not we stop (or
skip?) the submodule. So for already initialized submodules it is even
ok to not have a .gitmodules entry at all and we can still go ahead with
most operations. Here we should not be too picky, I think.

The only use-case I can think of is for checking whether .gitmodules
contains any extra unneeded values. But on the other hand that is not
so easy. Since .gitmodules are just config files they can contain user
defined values. That is ok.

So in summary: Yes the preferred way to list submodules is via iterating
the gitlinks and I do not think we need a way to iterate through the
.gitmodules file (at least not for the use-cases we currently need this
for).

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

Re: Re: [RFC PATCH] clone: add clone.recursesubmodules config option

[Heiko Voigt]

On Thu, Jun 05, 2014 at 11:18:28AM -0700, Junio C Hamano wrote:
 Jens Lehmann jens.lehm...@web.de writes:
  We had two settings in mind,...
  So what if clone would just do an git submodule init for now when
  submodule.autoinit is set but submodule.autoupdate isn't [?]
  ... and a single submodule.auto setting would be what users really want?
 
 I do not offhand think of a sensible scenario where you want to init
 a submodule once but do not want to update it when the superproject
 changes.  Even if the user uses the mode to detach the submodule
 HEAD, i.e. the branches in submodules do not matter and the whole
 tree is described by the superproject's commit and gitlinks recorded
 in it, the user would want the new objects necessary for the updated
 superproject, which means a submodule that is init'ed (whether it is
 via git submodule init or the submodule.autoinit variable) must be
 updated.
 
 So I am not sure why a user wants to disable autoupdate in the first
 place.  For the same reason, setting submodule.*.update to none
 would not make much sense, either.  Perhaps I am missing something.
 
 Unless the user is very conservative and suspects that these
 recursive behaviour we are going to bolt on to various commands
 could be buggy and untrustworthy, in which case the user might want
 to manually run git submodule update, or even run git fetch
 after going there while bypassing the whole git submodule.  But I
 do not think that is healthy in the longer run.

I think autoupdate is mainly there for the transition phase. Since
submodule can e.g. contain a lot of files a checkout would take much
longer. Similar to when Jens implemented the recursive diff, many people
were annoyed by the new files showing up and some with the impact on
performance (thats why we have the --ignore-submodules option).

In case of very big submodules and people already ignore their diff it
might even be necessary that the update is only done manually. E.g. for
a big media repository.

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

[PATCH 2/4] refs.c: return error instead of dying when locking fails during transaction

[Ronnie Sahlberg]

Change lock_ref_sha1_basic to return an error instead of dying when
we fail to lock a file during a transaction.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index e3a0383..ab0b629 100644
--- a/refs.c
+++ b/refs.c
@@ -2153,7 +2153,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 */
goto retry;
else
-   unable_to_lock_index_die(ref_file, errno);
+   goto error_return;
}
return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
 
-- 
2.0.0.583.g402232d

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

Re: Re: [RFC PATCH] clone: add clone.recursesubmodules config option

[Heiko Voigt]

On Thu, Jun 05, 2014 at 07:48:33PM +1200, Chris Packham wrote:
 On 05/06/14 07:42, Heiko Voigt wrote:
  I like this idea of specifying different views by giving tags. But
  does it rule out a boolean clone.recursesubmodules? For the simple case
  some people might not want to worry about specifying tags but just want
  to configure: Yes give me everything. So if we were to do this I would
  like it if we could have both. Also because the option for clone is
  --recurse-submodules and our typical schema is that a configuration
  option is named similar so clone.recursesubmodules would fit here.
 
 Maybe using a glob pattern would work.
 
 The user might say
 
  [clone]
  recursesubmodules = x86*
 
 And .gitmodules might say
 
  [submodule foo]
  tags = x86_64
  [submodule bar]
  tags = x86
  [submodule frotz]
  tags = powerpc
 
 For the Yes give me everything case the user could say
 
  [clone]
  recursesubmodules = *

Thats interesting. Lets me/us think about that a little more.

  So either we do this magically and all valid boolean values are
  forbidden as tags or we would need a different config option. Further
  thinking about it: Maybe a general option that does not only apply to
  clone would suit the views use-case more. E.g. submodule.tags or
  similar.
  
  Also please note: We have been talking about adding two configurations
  for submodules:
  
  submodule.name.autoclone (IIRC)
  
  I am not sure whether that was the correct name, but this option should
  tell recursive fetch / clone whether to automatically clone a submodule
  when it appears on a fetch in the history.
  
  submodule.name.autoinit
  
  And this one is for recursive checkout and tells whether an appearing
  submodule should automatically be initialized.
  
  These options fullfill a similar use-case and are planned for the future
  when recursive fetch/clone and checkout are in place (which is not that
  far away). We might need to rethink these to incoporate the views from
  tags idea nicely and since we do not want a configuration nightmare.
 
 I'm a little confused at how autoclone and autoinit differ. Aren't they
 the same? i.e. when this module appears grab it by default. I see
 autoupdate as a little different meaning update it if it's been
 initialised. Also does autoinit imply autoupdate?

autoclone is about cloning the history of submodules. So e.g. when a
submodule first appears in the superprojects history whether it should
automatically be cloned to .git/modules.

autoinit is all about the checkout phase. When a commit with a new
submodule is checked out: Should that new submodule be automatically
initialised?

As far as autoupdate is concerned: Maybe autoinit can imply that it is
enabled, yes. But I guess we still need autoupdate for the case of big
submodules that cause to much performance trouble if updated by every
checkout.

So its actually three values: autoclone, autoinit, autoupdate. Damn,
these configurations become more complicated everytime. Maybe we should
try to clean them, up once we have everything, with Git 3.0 ;-) If
anyone has an idea how to get rid of some right now...

Radically different thinking: How about just one: submodule.auto =
true/false configuration and that means you opt in to doing everything
as automatic as possible. Since we are still implementing we could stick
a prominent warning in the documentation that the user should be
prepared for behavioral changes.

Once everybody is happy with that we could switch the default from false
to true.

 At $dayjob we have a superproject which devs clone this has submodules
 for the important and/or high touch repositories. We have other
 repositories that are normally build from a tarball (or not built at
 all) but we can build them from external repositories if needed. The
 latter case is painfully manual. If autoinit/autoupdate existed we'd
 probably setup out projects with.
 
 [submodule linux]
 autoinit = true
   autoupdate = true
 [submodule userland]
 autoinit = true
   autoupdate = true
 [submodule not-used-that-much]
   autoupdate = true
 
 We probably wouldn't make use of tags because we're building complete
 embedded systems and generally want everything, even if we are doing
 most of our work on a particular target we need to do builds for other
 targets for sanity checks.

Yep thats exactly what we already do at $dayjob but with
submodule.*.update=none. Since that conveniently also disables the
initialisation, developers only get the basic code and not everyone
needs to have the media and some big external libs.

I would reuse 'update' in the long run. But I guess for the transition
we will need the extra autoupdate one to keep annoyance levels low.

We currently also do not have real use cases for the tags/views
scenario, but as repositories grow I can see that it could be useful so
I would like it if we could keep the