Re: git_inetd_server: run git-http-backend using inetd
On Jul 17, 2014, at 19:22, Jonathan Nieder wrote: Thanks for these details. I'll file a bug and mull it over some more. RFC 6762 makes it clear that what the package is currently doing is wrong. Given that Debian's libc knows nothing about mdns on its own, I think I'll need to parse resolv.conf (that's what libc-ares does). You might also want to take a look at [1] which suggests that when doing SRV lookups for URLs they should be done regardless of whether or not a port number is present (which then eliminates the RFC 3986 issue the current SRV lookup code has). It's only a draft (and expired now and looks like it received a rather chilly reception from some), but I haven't noticed anything else that addresses what the patch code is trying to do. The thread at [2] is related to what the patch is trying to do. [1] http://tools.ietf.org/html/draft-ohta-urlsrv [2] http://www.ietf.org/mail-archive/web/pcp/current/msg01727.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git update-index not delete lock file when using different worktree
Hi Duy: I tested your patch. It works. :) (only one case.) Thank you. There are 26 hold_locked_index() in these files: Line 475 of builtin\add.c Line 4234 of \builtin\apply.c Line 259 of \builtin\checkout.c Line 448 of \builtin\checkout.c Line 139 of \builtin\checkout-index.c Line 643 of \builtin\clone.c Line 323 of \builtin\commit.c Line 362 of \builtin\commit.c Line 383 of \builtin\commit.c Line 434 of \builtin\commit.c Line 1295 of \builtin\commit.c Line 479 of \builtin\describe.c Line 211 of \builtin\diff.c Line 660 of \builtin\merge.c Line 700 of \builtin\merge.c Line 88 of \builtin\mv.c Line 152 of \builtin\read-tree.c Line 338 of \builtin\reset.c Line 296 of \builtin\rm.c Line 808 of \builtin\update-index.c Line 588 of \cache-tree.c Line 75 of \merge.c Line 2004 of \merge-recursive.c Line 482 of \rerere.c Line 301 of \sequencer.c Line 671 of \sequencer.c Yue Lin -- View this message in context: http://git.661346.n2.nabble.com/git-update-index-not-delete-lock-file-when-using-different-worktree-tp7615300p7615378.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 08/19] rebase -i: Root commits are replayed with an unnecessary option
Hi Junio,
Junio C Hamano writes:
Fabian Ruch baf...@gmail.com writes:
The command line used to recreate root commits specifies the
effectless option `-C`. It is used to reuse commit message and
authorship from the named commit but the commit being amended here,
which is the sentinel commit, already carries the authorship and log
message of the processed commit. Note that the committer email and
commit date fields do not match the root commit either way. Remove
the option. However, `-C` (other than `-c`) does not invoke the
editor and the `--amend` option invokes it by default. Disable editor
invocation again by specifying `--no-edit`.
I'd say this is a no-value change, in the sense that it can be
written either way for the same effect and if the original were
written with --amend --no-edit then there would be no reason to
change it to -C $1 because such a change would also be equally a
no-value change.
What exactly are we gaining? Performance? Correctness?
I submitted this change separately from the next (rebase -i: Commit
only once when rewriting picks) for the following reasons, at least I
thought they were.
It makes the next patch easier to understand because the finalising
command line git commit --allow-empty --amend --no-post-rewrite -n
--no-edit seems to be simply moved to the end of do_pick. Substituting
--no-edit for -C there would make that log message overly long and the
paragraph saying why this substitution is correct would be distracting
(it's a little unfortunate that there is this special case in the first
place). While the resulting history stays the same of course, it might
be confusing to someone reading the code that the log message gets
replaced with itself.
I know the last point is rather weak but aren't the two patches and log
messages easier to read?
Fabian
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ff04d5d..17836d5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -511,7 +511,7 @@ do_pick () {
--no-post-rewrite -n -q -C $1
pick_one -n $1
git commit --allow-empty \
- --amend --no-post-rewrite -n -C $1 \
+ --amend --no-post-rewrite -n --no-edit \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_with_patch $1 Could not apply $1... $2
else
--
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 v10 0/4] git config cache special querying api utilizing the cache
Hi, [PATCH v10]: Minor fixes according to [12]. Re added string_list initializer function. Thanks to Junio and Matthieu for their suggestions. [PATCH v9]: Applied most of the review comments mentioned in [11]. Mostly asthetic changes. test-config now clears the config_set before exiting. Most of the tests now use the check-config function. check_config_init() now handles return values correctly. Diff between v8 and v9 is at the bottom. Thanks to Junio and Matthieu for the review. [PATCH V8]: Moved the contents of config-set.c to config.c for future convenience. Reverted test 'find value with misspelled key' to the one in v5. See [10] for the discussion. [PATCH V7]: Style nits and a broken chain corrected in `t/t1308-config-set.sh`. See [9] for the nits. [PATCH V6]: Style nits and mistakes corrected. Diff between v6 and v5[8] is at the bottom. Thanks to Matthieu, Ramsay and Ram for their suggestions. [PATCH V5]: `config_set` now uses a single hashmap. Corrected style nits raised in the thread[7]. Thanks to Junio and Matthieu for their suggestions. [PATCH v4]: Introduced `config_set` construct which points to a ordered set of config-files cached as hashmaps. Added relevant API functions. For more details see the documentation. Rewrote the git_config_get* family to use `config_set` internally. Added tests for both config_set API and git_config_get family. Added type specific API functions which parses the found value and converts it into a specific type. Most of the changes implemented are the result of discussion in [6]. Thanks to Eric, Ramsay, Junio, Matthieu Karsten for their suggestions and review. [PATCH v3]: Added flag for NULL values that were causing segfaults in some cases. Added test-config for usage examples. Minor changes and corrections. Refer to discussion thread[5] for more details. Thanks to Matthieu, Jeff and Junio for their valuable suggestions. [PATCH v2]:Changed the string_list to a struct instead of pointer to a struct. Added string-list initilization functions. Minor mistakes corrected acoording to review comments[4]. Thanks to Eric and Matthieu for their review. [PATCH V1]:Most of the invaluable suggestions by Eric Sunshine, Torsten Bogershausen and Jeff King has been implemented[1]. Complete rewrite of config_cache*() family using git_config() as hook as suggested by Jeff. Thanks for the review. [RFC V2]: Improved according to the suggestions by Eric Sunshine and Torsten Bogershausen. Added cache invalidation when config file is changed.[2] I am using git_config_set_multivar_in_file() as an update hook. This is patch is for this year's GSoC. My project is Git Config API improvements. The link of my proposal is appended below [3]. The aim of this patch series is to generate a cache for querying values from the config files in a non-callback manner as the current method reads and parses the config files every time a value is queried for. The cache is generated from hooking the update_cache function to the current parsing and callback mechanism in config.c. It is implemented as an hashmap using the hashmap-api with variables and its corresponding values list as its members. The values in the list are sorted in order of increasing priority. The cache is initialised the first time when any of the new query functions is called. It is invalidated by using git_config_set_multivar_in_file() as an update hook. We add two new functions to the config-api, git_config_get_string() and git_config_get_string_multi() for querying in a non callback manner from the cache. [1] http://marc.info/?t=14017206626r=1w=2 [2] http://git.661346.n2.nabble.com/RFC-PATCH-0-2-Git-config-cache-amp-special-querying-api-utilizing-the-cache-td7611691.html [3] https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing [4] http://thread.gmane.org/gmane.comp.version-control.git/251073/focus=251369 [5] http://thread.gmane.org/gmane.comp.version-control.git/251704/ [6] http://thread.gmane.org/gmane.comp.version-control.git/252329/ [7] http://marc.info/?t=14042811521r=1w=2 [8] http://article.gmane.org/gmane.comp.version-control.git/252942/ [9] http://thread.gmane.org/gmane.comp.version-control.git/252959/ [10] http://article.gmane.org/gmane.comp.version-control.git/253113 [11] http://thread.gmane.org/gmane.comp.version-control.git/253248 [12] http://thread.gmane.org/gmane.comp.version-control.git/253562 Tanay Abhra (4): string-list: add string_list initialiser helper functions Use string-list initializaer functions to rewrite config set test-config .gitignore | 1 + Documentation/technical/api-config.txt | 137 +++ Documentation/technical/api-string-list.txt | 5 +
[PATCH v10 1/4] string-list: add string_list initializer helper function
The string-list API has STRING_LIST_INIT_* macros to be used
to define variables with initializers, but lacks functions
to initialise an uninitialized piece of memory to be used as
a string-list at the run-time.
Introduce `string_list_init()` function for that.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Documentation/technical/api-string-list.txt | 5 +
string-list.c | 6 ++
string-list.h | 2 ++
3 files changed, 13 insertions(+)
diff --git a/Documentation/technical/api-string-list.txt
b/Documentation/technical/api-string-list.txt
index f1add51..d51a657 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -68,6 +68,11 @@ Functions
* General ones (works with sorted and unsorted lists as well)
+`string_list_init`::
+
+ Initialize the members of the string_list, set `strdup_strings`
+ member according to the value of the second parameter.
+
`filter_string_list`::
Apply a function to each item in a list, retaining only the
diff --git a/string-list.c b/string-list.c
index aabb25e..db38b62 100644
--- a/string-list.c
+++ b/string-list.c
@@ -1,6 +1,12 @@
#include cache.h
#include string-list.h
+void string_list_init(struct string_list *list, int strdup_strings)
+{
+ memset(list, 0, sizeof(*list));
+ list-strdup_strings = strdup_strings;
+}
+
/* if there is no exact match, point to the index where the entry could be
* inserted */
static int get_entry_index(const struct string_list *list, const char *string,
diff --git a/string-list.h b/string-list.h
index dd5e294..494eb5d 100644
--- a/string-list.h
+++ b/string-list.h
@@ -18,6 +18,8 @@ struct string_list {
#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0, NULL }
#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1, NULL }
+void string_list_init(struct string_list *list, int strdup_strings);
+
void print_string_list(const struct string_list *p, const char *text);
void string_list_clear(struct string_list *list, int free_util);
--
1.9.0.GIT
--
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 v10 4/4] test-config: add tests for the config_set API
Expose the `config_set` C API as a set of simple commands in order to
facilitate testing. Add tests for the `config_set` API as well as for
`git_config_get_*()` family for the usual config files.
Signed-off-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
.gitignore| 1 +
Makefile | 1 +
t/t1308-config-set.sh | 200 ++
test-config.c | 142 +++
4 files changed, 344 insertions(+)
create mode 100755 t/t1308-config-set.sh
create mode 100644 test-config.c
diff --git a/.gitignore b/.gitignore
index 81e12c0..5bfb234 100644
--- a/.gitignore
+++ b/.gitignore
@@ -178,6 +178,7 @@
/gitweb/static/gitweb.min.*
/test-chmtime
/test-ctype
+/test-config
/test-date
/test-delta
/test-dump-cache-tree
diff --git a/Makefile b/Makefile
index 840057c..8d8da72 100644
--- a/Makefile
+++ b/Makefile
@@ -549,6 +549,7 @@ PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
TEST_PROGRAMS_NEED_X += test-chmtime
TEST_PROGRAMS_NEED_X += test-ctype
+TEST_PROGRAMS_NEED_X += test-config
TEST_PROGRAMS_NEED_X += test-date
TEST_PROGRAMS_NEED_X += test-delta
TEST_PROGRAMS_NEED_X += test-dump-cache-tree
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
new file mode 100755
index 000..f0307b7
--- /dev/null
+++ b/t/t1308-config-set.sh
@@ -0,0 +1,200 @@
+#!/bin/sh
+
+test_description='Test git config-set API in different settings'
+
+. ./test-lib.sh
+
+# 'check_config get_* section.key value' verifies that the entry for
+# section.key is 'value'
+check_config () {
+ if test $1 = expect_code
+ then
+ expect_code=$2 shift shift
+ else
+ expect_code=0
+ fi
+ op=$1 key=$2 shift shift
+ if test $# != 0
+ then
+ printf %s\n $@
+ fi expect
+ test_expect_code $expect_code test-config $op $key actual
+ test_cmp expect actual
+}
+
+test_expect_success 'setup default config' '
+ cat .git/config \EOF
+ [case]
+ penguin = very blue
+ Movie = BadPhysics
+ UPPERCASE = true
+ MixedCase = true
+ my =
+ foo
+ baz = sam
+ [Cores]
+ WhatEver = Second
+ baz = bar
+ [cores]
+ baz = bat
+ [CORES]
+ baz = ball
+ [my Foo bAr]
+ hi = mixed-case
+ [my FOO BAR]
+ hi = upper-case
+ [my foo bar]
+ hi = lower-case
+ [case]
+ baz = bat
+ baz = hask
+ [lamb]
+ chop = 65
+ head = none
+ [goat]
+ legs = 4
+ head = true
+ skin = false
+ nose = 1
+ horns
+ EOF
+'
+
+test_expect_success 'get value for a simple key' '
+ check_config get_value case.penguin very blue
+'
+
+test_expect_success 'get value for a key with value as an empty string' '
+ check_config get_value case.my
+'
+
+test_expect_success 'get value for a key with value as NULL' '
+ check_config get_value case.foo (NULL)
+'
+
+test_expect_success 'upper case key' '
+ check_config get_value case.UPPERCASE true
+ check_config get_value case.uppercase true
+'
+
+test_expect_success 'mixed case key' '
+ check_config get_value case.MixedCase true
+ check_config get_value case.MIXEDCASE true
+ check_config get_value case.mixedcase true
+'
+
+test_expect_success 'key and value with mixed case' '
+ check_config get_value case.Movie BadPhysics
+'
+
+test_expect_success 'key with case sensitive subsection' '
+ check_config get_value my.Foo bAr.hi mixed-case
+ check_config get_value my.FOO BAR.hi upper-case
+ check_config get_value my.foo bar.hi lower-case
+'
+
+test_expect_success 'key with case insensitive section header' '
+ check_config get_value cores.baz ball
+ check_config get_value Cores.baz ball
+ check_config get_value CORES.baz ball
+ check_config get_value coreS.baz ball
+'
+
+test_expect_success 'key with case insensitive section header variable' '
+ check_config get_value CORES.BAZ ball
+ check_config get_value cores.baz ball
+ check_config get_value cores.BaZ ball
+ check_config get_value cOreS.bAz ball
+'
+
+test_expect_success 'find value with misspelled key' '
+ check_config expect_code 1 get_value my.fOo Bar.hi Value not found
for \my.fOo Bar.hi\
+'
+
+test_expect_success 'find value with the highest priority' '
+ check_config get_value case.baz hask
+'
+
+test_expect_success 'find integer value for a key' '
+ check_config get_int lamb.chop 65
+'
+
+test_expect_success 'find integer if value is non parse-able' '
+ check_config expect_code 128 get_int lamb.head
+'
+
+test_expect_success
[PATCH v10 3/4] add `config_set` API for caching config-like files
Currently `git_config()` uses a callback mechanism and file rereads for config values. Due to this approach, it is not uncommon for the config files to be parsed several times during the run of a git program, with different callbacks picking out different variables useful to themselves. Add a `config_set`, that can be used to construct an in-memory cache for config-like files that the caller specifies (i.e., files like `.gitmodules`, `~/.gitconfig` etc.). Add two external functions `git_configset_get_value` and `git_configset_get_value_multi` for querying from the config sets. `git_configset_get_value` follows `last one wins` semantic (i.e. if there are multiple matches for the queried key in the files of the configset the value returned will be the last entry in `value_list`). `git_configset_get_value_multi` returns a list of values sorted in order of increasing priority (i.e. last match will be at the end of the list). Add type specific query functions like `git_configset_get_bool` and similar. Add a default `config_set`, `the_config_set` to cache all key-value pairs read from usual config files (repo specific .git/config, user wide ~/.gitconfig, XDG config and the global /etc/gitconfig). `the_config_set` is populated using `git_config()`. Add two external functions `git_config_get_value` and `git_config_get_value_multi` for querying in a non-callback manner from `the_config_set`. Also, add type specific query functions that are implemented as a thin wrapper around the `config_set` API. Signed-off-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Tanay Abhra tanay...@gmail.com --- Documentation/technical/api-config.txt | 137 + cache.h| 30 config.c | 263 + 3 files changed, 430 insertions(+) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..8a86e45 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -77,6 +77,81 @@ To read a specific file in git-config format, use `git_config_from_file`. This takes the same callback and data parameters as `git_config`. +Querying For Specific Variables +--- + +For programs wanting to query for specific variables in a non-callback +manner, the config API provides two functions `git_config_get_value` +and `git_config_get_value_multi`. They both read values from an internal +cache generated previously from reading the config files. + +`int git_config_get_value(const char *key, const char **value)`:: + + Finds the highest-priority value for the configuration variable `key`, + stores the pointer to it in `value` and returns 0. When the + configuration variable `key` is not found, returns 1 without touching + `value`. The caller should not free or modify `value`, as it is owned + by the cache. + +`const struct string_list *git_config_get_value_multi(const char *key)`:: + + Finds and returns the value list, sorted in order of increasing priority + for the configuration variable `key`. When the configuration variable + `key` is not found, returns NULL. The caller should not free or modify + the returned pointer, as it is owned by the cache. + +`void git_config_clear(void)`:: + + Resets and invalidates the config cache. + +The config API also provides type specific API functions which do conversion +as well as retrieval for the queried variable, including: + +`int git_config_get_int(const char *key, int *dest)`:: + + Finds and parses the value to an integer for the configuration variable + `key`. Dies on error; otherwise, stores the value of the parsed integer in + `dest` and returns 0. When the configuration variable `key` is not found, + returns 1 without touching `dest`. + +`int git_config_get_ulong(const char *key, unsigned long *dest)`:: + + Similar to `git_config_get_int` but for unsigned longs. + +`int git_config_get_bool(const char *key, int *dest)`:: + + Finds and parses the value into a boolean value, for the configuration + variable `key` respecting keywords like true and false. Integer + values are converted into true/false values (when they are non-zero or + zero, respectively). Other values cause a die(). If parsing is successful, + stores the value of the parsed result in `dest` and returns 0. When the + configuration variable `key` is not found, returns 1 without touching + `dest`. + +`int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)`:: + + Similar to `git_config_get_bool`, except that integers are copied as-is, + and `is_bool` flag is unset. + +`int git_config_get_maybe_bool(const char *key, int *dest)`:: + + Similar to `git_config_get_bool`, except that it returns -1 on error + rather than dying. + +`int
[PATCH v10 2/4] replace memset with string-list initializers
Using memset and then manually setting values of the string-list
members is not future proof as the internal representation of
string-list may change any time.
Use `string_list_init()` or STRING_LIST_INIT_* macros instead of
memset.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
builtin/commit.c | 3 +--
merge-recursive.c | 9 +++--
submodule.c | 5 +
transport.c | 4 +---
4 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 72eb3be..5b196ca 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -420,8 +420,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
die(_(cannot do a partial commit during a
cherry-pick.));
}
- memset(partial, 0, sizeof(partial));
- partial.strdup_strings = 1;
+ string_list_init(partial, 1);
if (list_paths(partial, !current_head ? NULL : HEAD, prefix,
pathspec))
exit(1);
diff --git a/merge-recursive.c b/merge-recursive.c
index 5814d05..1d332b8 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2059,12 +2059,9 @@ void init_merge_options(struct merge_options *o)
if (o-verbosity = 5)
o-buffer_output = 0;
strbuf_init(o-obuf, 0);
- memset(o-current_file_set, 0, sizeof(struct string_list));
- o-current_file_set.strdup_strings = 1;
- memset(o-current_directory_set, 0, sizeof(struct string_list));
- o-current_directory_set.strdup_strings = 1;
- memset(o-df_conflict_file_set, 0, sizeof(struct string_list));
- o-df_conflict_file_set.strdup_strings = 1;
+ string_list_init(o-current_file_set, 1);
+ string_list_init(o-current_directory_set, 1);
+ string_list_init(o-df_conflict_file_set, 1);
}
int parse_merge_opt(struct merge_options *o, const char *s)
diff --git a/submodule.c b/submodule.c
index 48e3b44..c3a61e7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -544,10 +544,7 @@ static int push_submodule(const char *path)
int push_unpushed_submodules(unsigned char new_sha1[20], const char
*remotes_name)
{
int i, ret = 1;
- struct string_list needs_pushing;
-
- memset(needs_pushing, 0, sizeof(struct string_list));
- needs_pushing.strdup_strings = 1;
+ struct string_list needs_pushing = STRING_LIST_INIT_DUP;
if (!find_unpushed_submodules(new_sha1, remotes_name, needs_pushing))
return 1;
diff --git a/transport.c b/transport.c
index 59c9727..d32aaf2 100644
--- a/transport.c
+++ b/transport.c
@@ -1188,10 +1188,8 @@ int transport_push(struct transport *transport,
if ((flags (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |
TRANSPORT_RECURSE_SUBMODULES_CHECK))
!is_bare_repository()) {
struct ref *ref = remote_refs;
- struct string_list needs_pushing;
+ struct string_list needs_pushing = STRING_LIST_INIT_DUP;
- memset(needs_pushing, 0, sizeof(struct string_list));
- needs_pushing.strdup_strings = 1;
for (; ref; ref = ref-next)
if (!is_null_sha1(ref-new_sha1)
find_unpushed_submodules(ref-new_sha1,
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 3/4] add `config_set` API for caching config-like files
- Original Message - Documentation/technical/api-config.txt | 137 + cache.h| 30 config.c | 263 + 3 files changed, 430 insertions(+) I think the added call to git_config_clear() I proposed yesterday in setup_git_directory_gently_1 should be part of this patch (with the associated comment), just like this call: @@ -1707,6 +1967,9 @@ int git_config_set_multivar_in_file(const char *config_filename, lock = NULL; ret = 0; + /* Invalidate the config cache */ + git_config_clear(); + out_free: if (lock) rollback_lock_file(lock); I have limited access to my email and no way to apply the patches today, so I can't do a detailed review. But other than the remark above, I guess the patch series is now all right and ready to cook in pu. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] checkout: indicate when a detached head is checked out for a branch
In a setup with more than 1 workdir, it can happen that a user requests
to checkout a branch that is already checked out somewhere else. The
current hint about this gives the impression that it is merely warning
about this fact.
Actually, checkout changes its behavior in such a situation from
checking out the requested branch to checking out its head as a detached
head. The only indication is the usual checkout message which appears
before the hint.
Change the hint to make it clearer that checkout changed its behavior to
something that is possible to do in that situation, but may or may not
be what the user intended.
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
I really like the new --to feature and will convert all my new workdir
checkouts to that. But that detached checkout is so easy to miss - in fact
I noticed it only when I compared new-workdir to checkout --to for a
test repo with one branch, to see what a converter would need to do.
I'm even wondering whether we should do this DWIMmery at all, given how
dangerous a detached head is for those who are not aware of it
before gc kicks in.
(Sorry if that dupes something on the list, can't keep up these days;
so this is coming from a mere user ;-)
builtin/checkout.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index cfc6db7..38a5670 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -645,9 +645,9 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
detach_advice(new-name);
describe_detached_head(_(HEAD is now at),
new-commit);
if (new-checkout !*new-checkout)
- fprintf(stderr, _(hint: the main checkout is
holding this branch\n));
+ fprintf(stderr, _(hint: the main checkout is
holding this branch; detaching branch head instead.\n));
else if (new-checkout)
- fprintf(stderr, _(hint: the linked checkout %s
is holding this branch\n),
+ fprintf(stderr, _(hint: the linked checkout %s
is holding this branch; detaching branch head instead.\n),
new-checkout);
}
} else if (new-path) { /* Switch branches. */
--
2.0.1.583.gbe96573
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 3/4] add `config_set` API for caching config-like files
On 7/18/2014 3:10 PM, Matthieu Moy wrote: - Original Message - Documentation/technical/api-config.txt | 137 + cache.h| 30 config.c | 263 + 3 files changed, 430 insertions(+) I think the added call to git_config_clear() I proposed yesterday in setup_git_directory_gently_1 should be part of this patch (with the associated comment), just like this call: Oh, my bad, I thought you meant that I incorporate it with the git_config_raw() patch. I wanted to ask, can we call setup_git_directory_gently() earlier in execv_dashed_external() in git.c, which calls check_pager_config() for the first time which causes the incomplete cache to formed. If we can do it, we won't have to clear the cache every time setup_git_directory_gently_1() is called. @@ -1707,6 +1967,9 @@ int git_config_set_multivar_in_file(const char *config_filename, lock = NULL; ret = 0; +/* Invalidate the config cache */ +git_config_clear(); + out_free: if (lock) rollback_lock_file(lock); I have limited access to my email and no way to apply the patches today, so I can't do a detailed review. But other than the remark above, I guess the patch series is now all right and ready to cook in pu. -- 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 4/6] t4210: skip command-line encoding tests on mingw
On Thu, Jul 17, 2014 at 5:37 PM, Stepan Kasal ka...@ucw.cz wrote: From: Pat Thoyts pattho...@users.sourceforge.net On Windows the application command line is provided as unicode and in mingw-git we convert that to utf-8. So these tests that require a iso-8859-1 input are being subverted by the encoding transformations we perform and should be skipped. Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net Signed-off-by: Stepan Kasal ka...@ucw.cz --- t/t4210-log-i18n.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t4210-log-i18n.sh b/t/t4210-log-i18n.sh index 52a7472..9110404 100755 --- a/t/t4210-log-i18n.sh +++ b/t/t4210-log-i18n.sh @@ -34,7 +34,7 @@ test_expect_success 'log --grep searches in log output encoding (utf8)' ' test_cmp expect actual ' -test_expect_success 'log --grep searches in log output encoding (latin1)' ' +test_expect_success NOT_MINGW 'log --grep searches in log output encoding (latin1)' ' cat expect -\EOF latin1 utf8 @@ -43,7 +43,7 @@ test_expect_success 'log --grep searches in log output encoding (latin1)' ' test_cmp expect actual ' -test_expect_success 'log --grep does not find non-reencoded values (utf8)' ' +test_expect_success NOT_MINGW 'log --grep does not find non-reencoded values (utf8)' ' Perhaps these checks would be more readable a few years in the future, if we make a separate capability along the lines of NON_UNICODE_LOCALE? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 3/4] add `config_set` API for caching config-like files
I think my proposal is a fix, and yours is a workaround. Semantically, calling setup_git_directory changes the config, so it should invalidate the cache. Your proposal consists in not filling-in the cache before it is invalidated, which works, but isn't very future-proof: if anybody fills-in the cache before the setup in the future, it will break it. Now, both options can go together, then moving the check_pager_config call after setup_git_directory would avoid re-filling-in the cache. Intuitively, I'd say it's even needed in case git is started from a subdirectory and the .git/config contains pager-relevant stuff, but there might be complications. - Original Message - On 7/18/2014 3:10 PM, Matthieu Moy wrote: - Original Message - Documentation/technical/api-config.txt | 137 + cache.h| 30 config.c | 263 + 3 files changed, 430 insertions(+) I think the added call to git_config_clear() I proposed yesterday in setup_git_directory_gently_1 should be part of this patch (with the associated comment), just like this call: Oh, my bad, I thought you meant that I incorporate it with the git_config_raw() patch. I wanted to ask, can we call setup_git_directory_gently() earlier in execv_dashed_external() in git.c, which calls check_pager_config() for the first time which causes the incomplete cache to formed. If we can do it, we won't have to clear the cache every time setup_git_directory_gently_1() is called. @@ -1707,6 +1967,9 @@ int git_config_set_multivar_in_file(const char *config_filename, lock = NULL; ret = 0; + /* Invalidate the config cache */ + git_config_clear(); + out_free: if (lock) rollback_lock_file(lock); I have limited access to my email and no way to apply the patches today, so I can't do a detailed review. But other than the remark above, I guess the patch series is now all right and ready to cook in pu. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Why is git submodule slow under windows
Hello, We split a monolithic repository into ~50 submodules. The stored data has the same size. In the 1:1 comparison to the monolithic repository, the submodule handling is very slow. Under Linux everything remains fast, but windows is slow. So, why is git getting slow when it has to deal with a lot of submodules? I read something about the lack of the underlying cygwin to handle NTFS in a efficient way. Is this the root cause, or are there other causes also? -- 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] abspath.c: use PATH_MAX in real_path_internal()
On Fri, Jul 18, 2014 at 6:03 AM, Karsten Blees karsten.bl...@gmail.com wrote: Am 17.07.2014 19:05, schrieb René Scharfe: Am 17.07.2014 14:45, schrieb Nguyễn Thái Ngọc Duy: [...] These routines have traditionally been used by programs to save the name of a working directory for the purpose of returning to it. A much faster and less error-prone method of accomplishing this is to open the current directory (.) and use the fchdir(2) function to return. fchdir() is part of the POSIX-XSI extension, as is realpath(). So why not use realpath() directly (which would also be thread-safe)? But realpath still needs a given buffer (of PATH_MAX at least again). Unless you pass a NULL pointer as resolved_name, then Linux can allocate the buffer but that's implementation specific [1]. I guess we can make a wrapper git_getcwd that returns a new buffer. The default implementation returns one of PATH_MAX chars, certain platforms like Linux can use realpath (or something else) to return a longer path? [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/realpath.html For non-XSI-compliant platforms, we could keep the current implementation. Or re-implement a thread-safe version, e.g. applying resolve_symlink() from lockfile.c to all path components. If I may bother you with the Windows point of view: There is no fchdir(), and I'm pretty sure open(.) won't work either. fchdir() could be emulated using GetFileInformationByHandleEx(FileNameInfo). realpath() is pretty much what GetFinalPathNameByHandle() does. However, both of these APIs require Windows Vista. Opening a handle to a directory can be done using FILE_FLAG_BACKUP_SEMANTICS, which AFAICT MSVCRT.dll's open() implementation does _not_ do (could be emulated in our mingw_open() wrapper, though). ...lots of work for little benefit, I would think. We could wrap this get cwd, cd back pattern as well. So save_cwd returns an opaque pointer, go_back takes the pointer, (f)chdir back and free the pointer if needed. On platforms that support fchdir, it can be used, else we fall back to chdir. I think there are only four places that follow this pattern, here, setup.c (.git discovery), git.c (restore_env) and unix-socket.c. Enough call sites to make it worth the effort? -- 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] checkout: indicate when a detached head is checked out for a branch
On Fri, Jul 18, 2014 at 4:50 PM, Michael J Gruber g...@drmicha.warpmail.net wrote: I really like the new --to feature and will convert all my new workdir checkouts to that. But that detached checkout is so easy to miss - in fact I noticed it only when I compared new-workdir to checkout --to for a test repo with one branch, to see what a converter would need to do. I'm even wondering whether we should do this DWIMmery at all, This is what this series needs, user's opinions (bad or good). The other option is abort the checkout immediately. I think I made detach behavior default is because it's more work (and needs to be proven feasible). How about a config key that lets user decide what to do here, abort or detach. We may change the default behavior too if people think the current one is not good. given how dangerous a detached head is for those who are not aware of it before gc kicks in. Wait, what danger are we talking about? I thought gc pays attention to detached heads as well.. -- 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] abspath.c: use PATH_MAX in real_path_internal()
Am 18.07.2014 01:03, schrieb Karsten Blees: Am 17.07.2014 19:05, schrieb René Scharfe: Am 17.07.2014 14:45, schrieb Nguyễn Thái Ngọc Duy: [...] These routines have traditionally been used by programs to save the name of a working directory for the purpose of returning to it. A much faster and less error-prone method of accomplishing this is to open the current directory (.) and use the fchdir(2) function to return. fchdir() is part of the POSIX-XSI extension, as is realpath(). So why not use realpath() directly (which would also be thread-safe)? That's a good question; thanks for stepping back and looking at the bigger picture. If there is widespread OS support for a functionality then we should use it and just provide a compatibility implementation for those platforms lacking it. The downside is that compat code gets less testing. Seeing that readlink() is left as a stub in compat/mingw.h that only errors out, would the equivalent function on Windows be PathCanonicalize (http://msdn.microsoft.com/en-us/library/windows/desktop/bb773569%28v=vs.85%29.aspx)? For non-XSI-compliant platforms, we could keep the current implementation. OK, so realpath() for Linux and the BSDs, mingw_realpath() wrapping PathCanonicalize() for Windows and the current code for the rest? Or re-implement a thread-safe version, e.g. applying resolve_symlink() from lockfile.c to all path components. Thread safety sounds good. We'd also need something like normalize_path_copy() but without the conversion of backslashes to slashes, in order to get rid of . and .. path components and something like absolute_path() that doesn't die on error, no? If I may bother you with the Windows point of view: There is no fchdir(), and I'm pretty sure open(.) won't work either. On Windows, there *is* an absolute path length limit of 260 in the normal case and a bit more than 32000 for some functions using the \\?\ namespace. So one could get away with using a constant-sized buffer for a remember the place and return later function here. Also, _getcwd can be asked to allocate an appropriately-sized buffer for use, like GNU's get_current_dir_name, by specifying NULL as its first parameter (http://msdn.microsoft.com/en-us/library/sf98bd4y.aspx). Not having to move around at all as mentioned above is still better, of course. René -- 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: Why is git submodule slow under windows
Am 18.07.2014 12:14, schrieb Armbruster Joachim (BEG/EMS1): Hello, We split a monolithic repository into ~50 submodules. The stored data has the same size. In the 1:1 comparison to the monolithic repository, the submodule handling is very slow. Under Linux everything remains fast, but windows is slow. So, why is git getting slow when it has to deal with a lot of submodules? I read something about the lack of the underlying cygwin to handle NTFS in a efficient way. Is this the root cause, or are there other causes also? Hi, I assume you are using the latetst git from https://msysgit.github.io on windows. I would guess that submodules on windows are slow because git-submodules.sh is a shell script, and bash on windows is not really that fast. There has been some (albeit older) discussion on the msysgit mailinglist, see [1]. You can play around with core.fscache [2] maybe that helps. Thomas [1]: https://groups.google.com/d/msg/msysgit/AuPA4EbwchU/N42tsb6GousJ [2]: https://github.com/msysgit/msysgit/releases/tag/Git-1.8.5.2-preview20131230 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 00/19] Enable options --signoff, --reset-author for pick, reword
Fabian Ruch baf...@gmail.com writes: On 07/08/2014 10:45 PM, Junio C Hamano wrote: Fabian Ruch baf...@gmail.com writes: Fabian Ruch (19): rebase -i: Failed reword prints redundant error message rebase -i: reword complains about empty commit despite --keep-empty rebase -i: reword executes pre-commit hook on interim commit rebase -i: Teach do_pick the option --edit rebase -i: Implement reword in terms of do_pick rebase -i: Stop on root commits with empty log messages rebase -i: The replay of root commits is not shown with --verbose rebase -i: Root commits are replayed with an unnecessary option rebase -i: Commit only once when rewriting picks rebase -i: Do not die in do_pick rebase -i: Teach do_pick the option --amend rebase -i: Teach do_pick the option --file rebase -i: Prepare for squash in terms of do_pick --amend rebase -i: Implement squash in terms of do_pick rebase -i: Explicitly distinguish replay commands and exec tasks rebase -i: Parse to-do list command line options rebase -i: Teach do_pick the option --reset-author rebase -i: Teach do_pick the option --signoff rebase -i: Enable options --signoff, --reset-author for pick, reword After rebase -i:, some begin with lowercase and many begin with capital, which makes the short-log output look distracting. The ones that begin with lower-case letters are the ones that begin with the command name reword. All first lines are typed in lower case now. You could spell it 'reword' (with the quotes), which also disambiguates the command from the verb. -- Thomas Rast t...@thomasrast.ch -- 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 v1] rebase --root: sentinel commit cloaks empty commits
Hi Fabian
Impressive analysis!
Concerning the bugfix: Obviously, the patch misuses the `squash_onto`
flag because it assumes that the new base is empty except for the
sentinel commit. The variable name does not imply anything close to
that. An additional flag to disable the use of the git-rev-list
option `--cherry-pick` would work and make sense again (for instance,
`keep_redundant`).
Seeing as there are only two existing uses of the variable, you could
also rename it to make it more obvious what is going on. I think either
way is fine.
[...]
Please take a closer look at the last two test cases that specify the
expected behaviour of rebasing a branch that tracks the empty tree.
At this point they expect the Nothing to do error (aborts with
untouched history). This is consistent with rebasing only empty
commits without `--root`, which also doesn't just delete them from
the history. Furthermore, I think the two alternatives adding a note
that all commits in the range were empty, and removing the empty
commits (thus making the branch empty) are better discussed in a
separate bug report.
Makes sense to me, though I have never thought much about rebasing empty
commits. Maybe Chris has a more informed opinion?
is_empty_commit() {
- tree=$(git rev-parse -q --verify $1^{tree} 2/dev/null ||
- die $1: not a commit that can be picked)
- ptree=$(git rev-parse -q --verify $1^^{tree} 2/dev/null ||
- ptree=4b825dc642cb6eb9a060e54bf8d69288fbee4904)
+ tree=$(git rev-parse -q --verify $1^{tree} 2/dev/null) ||
+ die $1: not a commit that can be picked
+ ptree=$(git rev-parse -q --verify $1^^{tree} 2/dev/null) ||
+ ptree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
test $tree = $ptree
}
Nice catch!
@@ -958,7 +958,17 @@ then
revisions=$upstream...$orig_head
shortrevisions=$shortupstream..$shorthead
else
- revisions=$onto...$orig_head
+ if test -n $squash_onto
+ then
+ # $onto points to an empty commit (the sentinel
+ # commit) which was not created by the user.
+ # Exclude it from the rev list to avoid skipping
+ # empty user commits prematurely, i. e. before
+ # --keep-empty can take effect.
+ revisions=$orig_head
+ else
+ revisions=$onto...$orig_head
+ fi
shortrevisions=$shorthead
Nit: I think this would be clearer if you phrased it using an 'elif',
instead of nesting (but keep the comment!).
--
Thomas Rast
t...@thomasrast.ch
--
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: Why is git submodule slow under windows
On Fri, Jul 18, 2014 at 02:08:36PM +0200, Thomas Braun wrote: Am 18.07.2014 12:14, schrieb Armbruster Joachim (BEG/EMS1): Hello, We split a monolithic repository into ~50 submodules. The stored data has the same size. In the 1:1 comparison to the monolithic repository, the submodule handling is very slow. Under Linux everything remains fast, but windows is slow. So, why is git getting slow when it has to deal with a lot of submodules? I read something about the lack of the underlying cygwin to handle NTFS in a efficient way. Is this the root cause, or are there other causes also? Hi, I assume you are using the latetst git from https://msysgit.github.io on windows. I would guess that submodules on windows are slow because git-submodules.sh is a shell script, and bash on windows is not really that fast. My guess is that because the shell script uses fork() heavily and fork() is an expensive operation on Windows, that alone causes the slowddown. I did a quick test a while back when I rewrote part of git-submodule.sh in lua and runned it on my repo with ~45 submodules. The speedup was significant and should be even bigger on windows. -- Med vänlig hälsning Fredrik Gustafsson tel: 0733-608274 e-post: iv...@iveqy.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Make locked paths absolute when current directory is changed
Locked paths are saved in a linked list so that if something wrong
happens, *.lock are removed. This works fine if we keep cwd the same,
which is true 99% of time except:
- update-index and read-tree hold the lock on $GIT_DIR/index really
early, then later on may call setup_work_tree() to move cwd.
- Suppose a lock is being held (e.g. by git add) then somewhere
down the line, somebody calls real_path (e.g. link_alt_odb_entry),
which temporarily moves cwd away and back.
During that time when cwd is moved (either permanently or temporarily)
and we decide to die(), attempts to remove relative *.lock will fail,
and the next operation will complain that some files are still locked.
Avoid this case by turning relative paths to absolute when chdir() is
called (or soon to be called, in setup_git_directory_gently case).
Reported-by: Yue Lin Ho yuelinho...@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
It occurred to me while writing this commit message that it would be
better if we can unlink via a file descriptor so we don't have to
convert paths. But I'm not sure about the availability of unlinkat(),
especially on Windows.
abspath.c | 2 +-
cache.h | 6 ++
git.c | 6 +++---
lockfile.c| 16
path.c| 4 ++--
run-command.c | 2 +-
setup.c | 3 ++-
unix-socket.c | 2 +-
8 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/abspath.c b/abspath.c
index ca33558..78c963f 100644
--- a/abspath.c
+++ b/abspath.c
@@ -87,7 +87,7 @@ static const char *real_path_internal(const char *path, int
die_on_error)
goto error_out;
}
- if (chdir(buf)) {
+ if (chdir_safe(buf)) {
if (die_on_error)
die_errno(Could not switch to '%s',
buf);
else
diff --git a/cache.h b/cache.h
index 44aa439..d3f2596 100644
--- a/cache.h
+++ b/cache.h
@@ -564,6 +564,12 @@ extern int hold_lock_file_for_update(struct lock_file *,
const char *path, int);
extern int hold_lock_file_for_append(struct lock_file *, const char *path,
int);
extern int commit_lock_file(struct lock_file *);
extern void update_index_if_able(struct index_state *, struct lock_file *);
+extern void make_locked_paths_absolute(void);
+static inline int chdir_safe(const char *path)
+{
+ make_locked_paths_absolute();
+ return chdir(path);
+}
extern int hold_locked_index(struct lock_file *, int);
extern int commit_locked_index(struct lock_file *);
diff --git a/git.c b/git.c
index 5b6c761..27766c3 100644
--- a/git.c
+++ b/git.c
@@ -48,7 +48,7 @@ static void save_env(void)
static void restore_env(void)
{
int i;
- if (*orig_cwd chdir(orig_cwd))
+ if (*orig_cwd chdir_safe(orig_cwd))
die_errno(could not move to %s, orig_cwd);
for (i = 0; i ARRAY_SIZE(env_names); i++) {
if (orig_env[i])
@@ -206,7 +206,7 @@ static int handle_options(const char ***argv, int *argc,
int *envchanged)
fprintf(stderr, No directory given for -C.\n
);
usage(git_usage_string);
}
- if (chdir((*argv)[1]))
+ if (chdir_safe((*argv)[1]))
die_errno(Cannot change to '%s', (*argv)[1]);
if (envchanged)
*envchanged = 1;
@@ -292,7 +292,7 @@ static int handle_alias(int *argcp, const char ***argv)
ret = 1;
}
- if (subdir chdir(subdir))
+ if (subdir chdir_safe(subdir))
die_errno(Cannot change to '%s', subdir);
errno = saved_errno;
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..a70d107 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -280,3 +280,19 @@ void rollback_lock_file(struct lock_file *lk)
}
lk-filename[0] = 0;
}
+
+void make_locked_paths_absolute(void)
+{
+ struct lock_file *lk;
+ const char *abspath;
+ for (lk = lock_file_list; lk != NULL; lk = lk-next) {
+ if (!lk-filename[0] || lk-filename[0] == '/')
+ continue;
+ abspath = absolute_path(lk-filename);
+ if (strlen(abspath) = sizeof(lk-filename))
+ warning(locked path %s is relative when current
directory
+ is changed, lk-filename);
+ else
+ strcpy(lk-filename, abspath);
+ }
+}
diff --git a/path.c b/path.c
index bc804a3..9e8e101 100644
--- a/path.c
+++ b/path.c
@@ -372,11 +372,11 @@ const char *enter_repo(const char *path, int strict)
gitfile = read_gitfile(used_path) ;
if (gitfile)
strcpy(used_path, gitfile);
- if
Re: [PATCH] checkout: indicate when a detached head is checked out for a branch
Duy Nguyen venit, vidit, dixit 18.07.2014 12:58: On Fri, Jul 18, 2014 at 4:50 PM, Michael J Gruber g...@drmicha.warpmail.net wrote: I really like the new --to feature and will convert all my new workdir checkouts to that. But that detached checkout is so easy to miss - in fact I noticed it only when I compared new-workdir to checkout --to for a test repo with one branch, to see what a converter would need to do. I'm even wondering whether we should do this DWIMmery at all, This is what this series needs, user's opinions (bad or good). The other option is abort the checkout immediately. I think I made detach behavior default is because it's more work (and needs to be proven feasible). How about a config key that lets user decide what to do here, abort or detach. We may change the default behavior too if people think the current one is not good. Uh, config bloat :) I think DWIMmery is OK if it's made clear to the user what happened, and it is somewhat expected/probably intended behavior. Do we have a precedent where a detached head is produced when a branch checkout is requested, or something similar? I think checking out remote tracking branches is somehow in that same boat. given how dangerous a detached head is for those who are not aware of it before gc kicks in. Wait, what danger are we talking about? I thought gc pays attention to detached heads as well.. As long as HEAD points to it, of course. I think detached head is one of the killer features of git, in both senses of the meaning... Don't we DWIM (or suggest) git checkout origin/master to git checkout --track origin/master which creates master with upstream origin/master? Maybe I'm mixing things up, but I think we try to produce detached heads only on special requests. New users get confused by them, some don't understand the (well crafted) message you get when you switch away from them, and while you can recover them from HEAD's reflog, they are gone with the next gc unless they remain checked out (or get referenced). I think I've just convinced myself that we shouldn't DWIm to a detached head, and rather tell the user how to produce one if she really intended to: git checkout --detach... That one seems to be broken by multiple workdir setup (in the sense of producing an unnecessary hint). Michael -- 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] checkout: indicate when a detached head is checked out for a branch
Hi. On Fri, Jul 18, 2014 at 4:27 PM, Michael J Gruber g...@drmicha.warpmail.net wrote: Duy Nguyen venit, vidit, dixit 18.07.2014 12:58: This is what this series needs, user's opinions (bad or good). Actually, if options -b branch works with the --to (does it?), then user probably shouldn't need to create detached checkouts (I need them only for scripts), so this action is probably a mistake. And when user does want to create detached checkout he can use the --detach option. So I would say checkout of already checked out branch should fail, suggesting using -b or --detach options. -- Max -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-svn: Initialize SVN::Client with svn config instead of, auth for git svn branch.
If a client certificate is required to connect to svn, git svn branch
always prompt the user for the certificate location and password,
even though those parameters are stored in svn file server
located in svn config dir (generally ~/.subversion).
On the opposite, git svn info/init/dcommit read the config dir
and do not prompt if the parameters are set.
This commit initializes for git svn branch, the SVN::Client with the
'config'
option instead of 'auth'. As decribed in the SVN documentation,
http://search.cpan.org/~mschwern/Alien-SVN-v1.7.17.1/src/subversion/subversion/bindings/swig/perl/native/Client.pm#METHODS
the SVN::Client will then read cached authentication options.
Signed-off-by: Monard Vong travelingsou...@gmail.com
---
git-svn.perl | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/git-svn.perl b/git-svn.perl
index 0a32372..1f41ee1 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1161,7 +1161,9 @@ sub cmd_branch {
::_req_svn();
my $ctx = SVN::Client-new(
-auth= Git::SVN::Ra::_auth_providers(),
+config = SVN::Core::config_get_config(
+$Git::SVN::Ra::config_dir
+),
log_msg = sub {
${ $_[0] } = defined $_message
? $_message
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()
Am 18.07.2014 12:49, schrieb Duy Nguyen:
On Fri, Jul 18, 2014 at 6:03 AM, Karsten Blees karsten.bl...@gmail.com
wrote:
Am 17.07.2014 19:05, schrieb René Scharfe:
Am 17.07.2014 14:45, schrieb Nguyễn Thái Ngọc Duy:
[...]
These routines have traditionally been used by programs to save the
name of a working directory for the purpose of returning to it. A much
faster and less error-prone method of accomplishing this is to open the
current directory (.) and use the fchdir(2) function to return.
fchdir() is part of the POSIX-XSI extension, as is realpath(). So why not
use realpath() directly (which would also be thread-safe)?
But realpath still needs a given buffer (of PATH_MAX at least again).
Unless you pass a NULL pointer as resolved_name, then Linux can
allocate the buffer but that's implementation specific [1]. I guess we
can make a wrapper git_getcwd that returns a new buffer. The default
implementation returns one of PATH_MAX chars, certain platforms like
Linux can use realpath (or something else) to return a longer path?
[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/realpath.html
realpath() can be used to implement the whole of real_path_internal(),
IIUC, so there would be no need to change the current directory anymore.
The realpath(3) manpage for Linux mentions that behaviour of realpath()
with resolved_path being NULL is not standardized by POSIX.1-2001 but
by POSIX.1-2008. At least Linux, OpenBSD and FreeBSD seem to support
that, based on their manpages. Here's the standard doc:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html
For non-XSI-compliant platforms, we could keep the current implementation.
Or re-implement a thread-safe version, e.g. applying resolve_symlink() from
lockfile.c to all path components.
If I may bother you with the Windows point of view:
There is no fchdir(), and I'm pretty sure open(.) won't work either.
fchdir() could be emulated using GetFileInformationByHandleEx(FileNameInfo).
realpath() is pretty much what GetFinalPathNameByHandle() does. However,
both of these APIs require Windows Vista.
Opening a handle to a directory can be done using FILE_FLAG_BACKUP_SEMANTICS,
which AFAICT MSVCRT.dll's open() implementation does _not_ do (could be
emulated in our mingw_open() wrapper, though).
...lots of work for little benefit, I would think.
We could wrap this get cwd, cd back pattern as well. So save_cwd
returns an opaque pointer, go_back takes the pointer, (f)chdir back
and free the pointer if needed. On platforms that support fchdir, it
can be used, else we fall back to chdir. I think there are only four
places that follow this pattern, here, setup.c (.git discovery), git.c
(restore_env) and unix-socket.c. Enough call sites to make it worth
the effort?
On a cursory look I didn't find any more potential users. Something
like this? Applying it to setup.c looks difficult to impossible,
though.
---
Makefile | 8
abspath.c | 10 ++
cache.h | 1 +
git.c | 9 +
save-cwd-fchdir.c | 25 +
save-cwd.c| 22 ++
save-cwd.h| 3 +++
unix-socket.c | 11 ++-
8 files changed, 76 insertions(+), 13 deletions(-)
create mode 100644 save-cwd-fchdir.c
create mode 100644 save-cwd.c
create mode 100644 save-cwd.h
diff --git a/Makefile b/Makefile
index 840057c..ecf3129 100644
--- a/Makefile
+++ b/Makefile
@@ -32,6 +32,8 @@ all::
#
# Define HAVE_ALLOCA_H if you have working alloca(3) defined in that header.
#
+# Define HAVE_FCHDIR if you have fchdir(2).
+#
# Define NO_CURL if you do not have libcurl installed. git-http-fetch and
# git-http-push are not built, and you cannot use http:// and https://
# transports (neither smart nor dumb).
@@ -1118,6 +1120,12 @@ ifdef HAVE_ALLOCA_H
BASIC_CFLAGS += -DHAVE_ALLOCA_H
endif
+ifdef HAVE_FCHDIR
+ COMPAT_OBJS += save-cwd-fchdir.o
+else
+ COMPAT_OBJS += save-cwd.o
+endif
+
ifdef NO_CURL
BASIC_CFLAGS += -DNO_CURL
REMOTE_CURL_PRIMARY =
diff --git a/abspath.c b/abspath.c
index ca33558..f49bacc 100644
--- a/abspath.c
+++ b/abspath.c
@@ -41,7 +41,7 @@ static const char *real_path_internal(const char *path, int
die_on_error)
* here so that we can chdir() back to it at the end of the
* function:
*/
- char cwd[1024] = ;
+ struct saved_cwd *cwd = NULL;
int buf_index = 1;
@@ -80,7 +80,9 @@ static const char *real_path_internal(const char *path, int
die_on_error)
}
if (*buf) {
- if (!*cwd !getcwd(cwd, sizeof(cwd))) {
+ if (!cwd)
+ cwd = save_cwd();
+ if (!cwd) {
if (die_on_error)
die_errno(Could not get current
working directory);
[PATCH] transport: simplify fetch_objs_via_rsync() using argv_array
Use the existing argv_array member instead of building the arguments
list using a string array and a strbuf. This way we don't need magic
number constants and allocations are cleaned up for us automatically
by run_command().
Signed-off-by: Rene Scharfe l@web.de
---
transport.c | 28
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/transport.c b/transport.c
index 59c9727..3e42570 100644
--- a/transport.c
+++ b/transport.c
@@ -263,32 +263,20 @@ static struct ref *get_refs_via_rsync(struct transport
*transport, int for_push)
static int fetch_objs_via_rsync(struct transport *transport,
int nr_objs, struct ref **to_fetch)
{
- struct strbuf buf = STRBUF_INIT;
struct child_process rsync;
- const char *args[8];
- int result;
-
- strbuf_addstr(buf, rsync_url(transport-url));
- strbuf_addstr(buf, /objects/);
memset(rsync, 0, sizeof(rsync));
- rsync.argv = args;
rsync.stdout_to_stderr = 1;
- args[0] = rsync;
- args[1] = (transport-verbose 1) ? -rv : -r;
- args[2] = --ignore-existing;
- args[3] = --exclude;
- args[4] = info;
- args[5] = buf.buf;
- args[6] = get_object_directory();
- args[7] = NULL;
+ argv_array_push(rsync.args, rsync);
+ argv_array_push(rsync.args, (transport-verbose 1) ? -rv : -r);
+ argv_array_push(rsync.args, --ignore-existing);
+ argv_array_push(rsync.args, --exclude);
+ argv_array_push(rsync.args, info);
+ argv_array_pushf(rsync.args, %s/objects/, rsync_url(transport-url));
+ argv_array_push(rsync.args, get_object_directory());
/* NEEDSWORK: handle one level of alternates */
- result = run_command(rsync);
-
- strbuf_release(buf);
-
- return result;
+ return run_command(rsync);
}
static int write_one_ref(const char *name, const unsigned char *sha1,
--
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] remote-testsvn: use internal argv_array of struct child_process in cmd_import()
Use the existing argv_array member instead of providing our own. This
way we don't have to initialize or clean it up explicitly.
Signed-off-by: Rene Scharfe l@web.de
---
remote-testsvn.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/remote-testsvn.c b/remote-testsvn.c
index 6be55cb..31415bd 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -175,7 +175,6 @@ static int cmd_import(const char *line)
char *note_msg;
unsigned char head_sha1[20];
unsigned int startrev;
- struct argv_array svndump_argv = ARGV_ARRAY_INIT;
struct child_process svndump_proc;
if (read_ref(private_ref, head_sha1))
@@ -202,11 +201,10 @@ static int cmd_import(const char *line)
} else {
memset(svndump_proc, 0, sizeof(struct child_process));
svndump_proc.out = -1;
- argv_array_push(svndump_argv, svnrdump);
- argv_array_push(svndump_argv, dump);
- argv_array_push(svndump_argv, url);
- argv_array_pushf(svndump_argv, -r%u:HEAD, startrev);
- svndump_proc.argv = svndump_argv.argv;
+ argv_array_push(svndump_proc.args, svnrdump);
+ argv_array_push(svndump_proc.args, dump);
+ argv_array_push(svndump_proc.args, url);
+ argv_array_pushf(svndump_proc.args, -r%u:HEAD, startrev);
code = start_command(svndump_proc);
if (code)
@@ -227,7 +225,6 @@ static int cmd_import(const char *line)
code = finish_command(svndump_proc);
if (code)
warning(%s, returned %d, svndump_proc.argv[0], code);
- argv_array_clear(svndump_argv);
}
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] fast-import: use hashcmp() for SHA1 hash comparison
Signed-off-by: Rene Scharfe l@web.de
---
fast-import.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fast-import.c b/fast-import.c
index fa635f7..d73f58c 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2324,7 +2324,7 @@ static void file_change_m(const char *p, struct branch *b)
}
/* Git does not track empty, non-toplevel directories. */
- if (S_ISDIR(mode) !memcmp(sha1, EMPTY_TREE_SHA1_BIN, 20) *p) {
+ if (S_ISDIR(mode) !hashcmp(sha1, EMPTY_TREE_SHA1_BIN) *p) {
tree_content_remove(b-branch_tree, p, NULL, 0);
return;
}
--
2.0.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()
Karsten Blees karsten.bl...@gmail.com writes: Am 17.07.2014 14:45, schrieb Nguyễn Thái Ngọc Duy: e.g. git init. Make it static too to reduce stack usage. But wouldn't this increase overall memory usage? Stack memory will be reused by subsequent code, while static memory cannot be reused (but still increases the process working set). Correct. And this is a function that is not involved in deep recursion, so avoiding stack allocation seems quite misguided. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 08/19] rebase -i: Root commits are replayed with an unnecessary option
Fabian Ruch baf...@gmail.com writes: It makes the next patch easier to understand because the finalising command line git commit --allow-empty --amend --no-post-rewrite -n --no-edit seems to be simply moved to the end of do_pick. Substituting --no-edit for -C there would make that log message overly long ... And the reason why the same -C $1 is not used and --no-edit is used in the final version that is moved to the end of do_pick is...? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 4/4] test-config: add tests for the config_set API
Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'proper error on non-existant files' ' + echo Error (-1) reading configuration file non-existant-file. expect s/tant/tent/ perhaps? cf. http://en.wiktionary.org/wiki/non-existant +test_expect_success 'proper error on non-accessible files' ' + chmod -r .git/config + test_when_finished chmod +r .git/config + echo Error (-1) reading configuration file .git/config. expect + test_expect_code 2 test-config configset_get_value foo.bar .git/config 2actual + test_cmp expect actual +' Doesn't this one need some prerequisite? -- 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] rebase -i: Remember merge options beyond continue actions
Hi, Thanks for the patch. I've had this issue today and the patch has fixed it. I hope the patch makes its way to Git. Ralf Hi Eric, thanks a lot for the reference. I added the Reported-by: and Signed-off-by: lines to the commit message. Fabian -- 8 -- Subject: rebase -i: Remember merge options beyond continue actions If the user explicitly specified a merge strategy or strategy options, rebase --interactive started using the default merge strategy again after rebase --continue. This problem gets fixed by this commit. Add test. Since the rebase options -s and -X imply --merge, we can simply remove the do_merge guard in the interactive mode and always compile the cherry-pick arguments from the rebase state variables strategy and strategy_opts. ... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 1/4] string-list: add string_list initializer helper function
Tanay Abhra tanay...@gmail.com writes:
The string-list API has STRING_LIST_INIT_* macros to be used
to define variables with initializers, but lacks functions
to initialise an uninitialized piece of memory to be used as
a string-list at the run-time.
Introduce `string_list_init()` function for that.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Looks sensible, modulo s/initialise/initialize/ which perhaps is
originally my fault.
Will queue; thanks.
Documentation/technical/api-string-list.txt | 5 +
string-list.c | 6 ++
string-list.h | 2 ++
3 files changed, 13 insertions(+)
diff --git a/Documentation/technical/api-string-list.txt
b/Documentation/technical/api-string-list.txt
index f1add51..d51a657 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -68,6 +68,11 @@ Functions
* General ones (works with sorted and unsorted lists as well)
+`string_list_init`::
+
+ Initialize the members of the string_list, set `strdup_strings`
+ member according to the value of the second parameter.
+
`filter_string_list`::
Apply a function to each item in a list, retaining only the
diff --git a/string-list.c b/string-list.c
index aabb25e..db38b62 100644
--- a/string-list.c
+++ b/string-list.c
@@ -1,6 +1,12 @@
#include cache.h
#include string-list.h
+void string_list_init(struct string_list *list, int strdup_strings)
+{
+ memset(list, 0, sizeof(*list));
+ list-strdup_strings = strdup_strings;
+}
+
/* if there is no exact match, point to the index where the entry could be
* inserted */
static int get_entry_index(const struct string_list *list, const char
*string,
diff --git a/string-list.h b/string-list.h
index dd5e294..494eb5d 100644
--- a/string-list.h
+++ b/string-list.h
@@ -18,6 +18,8 @@ struct string_list {
#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0, NULL }
#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1, NULL }
+void string_list_init(struct string_list *list, int strdup_strings);
+
void print_string_list(const struct string_list *p, const char *text);
void string_list_clear(struct string_list *list, int free_util);
--
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] checkout: indicate when a detached head is checked out for a branch
Michael J Gruber g...@drmicha.warpmail.net writes:
I really like the new --to feature and will convert all my new workdir
checkouts to that. But that detached checkout is so easy to miss - in fact
I noticed it only when I compared new-workdir to checkout --to for a
test repo with one branch, to see what a converter would need to do.
I'm even wondering whether we should do this DWIMmery at all, given how
dangerous a detached head is for those who are not aware of it
before gc kicks in.
As long as the amount of warning about 'detached HEAD' is about the
same between this case and a git checkout v1.2.3 in a normal
repository, I do not think there is no additional danger you need
to be worried about.
But I do agree that there should not be any DWIM here.
The reason to introduce this new set of rather intrusive changes is
so that working trees can be aware of branches other working trees
have checked out. And the whole point of wanting to have that
mutual awareness is to enable us to forbid users from mucking with
the same branch from two different places.
But Git is not in the position to dictate which alternative action
the user would want to take, when her git checkout foo is
prevented by this mechanism. In one scenario, she may say I only
wanted to take a peek and run git checkout foo^0 instead. In
another, she may say Ah, I forgot I already was doing this change
in the other one and run cd ../foo. There may be other classes
of alternative actions.
Don't make it easier for the first class of scenario and make it
less useful and more dangerous for the second class, especially the
second class involve forgetful users who are likely to forget seeing
the we've warned you that we detached without being asked message.
Please fix it to always just error out.
(Sorry if that dupes something on the list, can't keep up these days;
so this is coming from a mere user ;-)
builtin/checkout.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index cfc6db7..38a5670 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -645,9 +645,9 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
detach_advice(new-name);
describe_detached_head(_(HEAD is now at),
new-commit);
if (new-checkout !*new-checkout)
- fprintf(stderr, _(hint: the main checkout is
holding this branch\n));
+ fprintf(stderr, _(hint: the main checkout is
holding this branch; detaching branch head instead.\n));
else if (new-checkout)
- fprintf(stderr, _(hint: the linked checkout %s
is holding this branch\n),
+ fprintf(stderr, _(hint: the linked checkout %s
is holding this branch; detaching branch head instead.\n),
new-checkout);
}
} else if (new-path) { /* Switch branches. */
--
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] Make locked paths absolute when current directory is changed
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
Locked paths are saved in a linked list so that if something wrong
happens, *.lock are removed. This works fine if we keep cwd the same,
which is true 99% of time except:
- update-index and read-tree hold the lock on $GIT_DIR/index really
early, then later on may call setup_work_tree() to move cwd.
- Suppose a lock is being held (e.g. by git add) then somewhere
down the line, somebody calls real_path (e.g. link_alt_odb_entry),
which temporarily moves cwd away and back.
During that time when cwd is moved (either permanently or temporarily)
and we decide to die(), attempts to remove relative *.lock will fail,
and the next operation will complain that some files are still locked.
Avoid this case by turning relative paths to absolute when chdir() is
called (or soon to be called, in setup_git_directory_gently case).
The rationale makes sense.
+extern void make_locked_paths_absolute(void);
+static inline int chdir_safe(const char *path)
+{
+ make_locked_paths_absolute();
+ return chdir(path);
+}
Clever ;-). Instead of making paths absolute when you receive
requests to lock them, you lazily turn the ones relative to cwd()
absolute just before they are about to become invalid/problematic
because the program wants to chdir.
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..a70d107 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -280,3 +280,19 @@ void rollback_lock_file(struct lock_file *lk)
}
lk-filename[0] = 0;
}
+
+void make_locked_paths_absolute(void)
+{
+ struct lock_file *lk;
+ const char *abspath;
+ for (lk = lock_file_list; lk != NULL; lk = lk-next) {
+ if (!lk-filename[0] || lk-filename[0] == '/')
+ continue;
Do we have to worry about Windows?
+ abspath = absolute_path(lk-filename);
+ if (strlen(abspath) = sizeof(lk-filename))
+ warning(locked path %s is relative when current
directory
+ is changed, lk-filename);
Shouldn't this be a die() or an error return (which will kill the
caller anyway)?
@@ -636,6 +636,7 @@ static const char *setup_git_directory_gently_1(int
*nongit_ok)
die_errno(Unable to read current working directory);
offset = len = strlen(cwd);
+ make_locked_paths_absolute();
Just being curious, but this early in the start-up sequence, what
files do we have locks on?
--
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: Why is git submodule slow under windows
Am 18.07.2014 15:02, schrieb Fredrik Gustafsson: On Fri, Jul 18, 2014 at 02:08:36PM +0200, Thomas Braun wrote: Am 18.07.2014 12:14, schrieb Armbruster Joachim (BEG/EMS1): Hello, We split a monolithic repository into ~50 submodules. The stored data has the same size. In the 1:1 comparison to the monolithic repository, the submodule handling is very slow. Under Linux everything remains fast, but windows is slow. So, why is git getting slow when it has to deal with a lot of submodules? I read something about the lack of the underlying cygwin to handle NTFS in a efficient way. Is this the root cause, or are there other causes also? Hi, I assume you are using the latetst git from https://msysgit.github.io on windows. I would guess that submodules on windows are slow because git-submodules.sh is a shell script, and bash on windows is not really that fast. My guess is that because the shell script uses fork() heavily and fork() is an expensive operation on Windows, that alone causes the slowddown. I did a quick test a while back when I rewrote part of git-submodule.sh in lua and runned it on my repo with ~45 submodules. The speedup was significant and should be even bigger on windows. Without having looked at your Lua rewrite I suspect my recursive submodule checkout series could speed that up even more, as it only needs to fork a git status followed by a git checkout or git read-tree for each submodule. Now that my submodule test harness and Heiko's submodule config lookup API did hit next, this is the thing I'm currently working on. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 25/31] prune: strategies for linked checkouts
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
(alias R=$GIT_COMMON_DIR/repos/id)
- linked checkouts are supposed to keep its location in $R/gitdir up
to date. The use case is auto fixup after a manual checkout move.
- linked checkouts are supposed to update mtime of $R/gitdir. If
$R/gitdir's mtime is older than a limit, and it points to nowhere,
repos/id is to be pruned.
- If $R/locked exists, repos/id is not supposed to be pruned. If
$R/locked exists and $R/gitdir's mtime is older than a really long
limit, warn about old unused repo.
- git checkout --to is supposed to make a hard link named $R/link
pointing to the .git file on supported file systems to help detect
the user manually deleting the checkout. If $R/link exists and its
link count is greated than 1, the repo is kept.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/git-prune.txt| 3 +
Documentation/gitrepository-layout.txt | 19 ++
builtin/checkout.c | 14 +
builtin/prune.c| 99
++
setup.c| 13
t/t2026-prune-linked-checkouts.sh (new +x) | 84 +
I get this from t2026.2 under valgrind:
==21334== Conditional jump or move depends on uninitialised value(s)
==21334==at 0x46D49B: prune_repos_dir (prune.c:182)
==21334==by 0x46D8C0: cmd_prune (prune.c:252)
==21334==by 0x405C2F: run_builtin (git.c:351)
==21334==by 0x405E47: handle_builtin (git.c:530)
==21334==by 0x405F6B: run_argv (git.c:576)
==21334==by 0x40610B: main (git.c:663)
==21334== Uninitialised value was created by a stack allocation
==21334==at 0x46D3BB: prune_repos_dir (prune.c:169)
==21334==
{
insert_a_suppression_name_here
Memcheck:Cond
fun:prune_repos_dir
fun:cmd_prune
fun:run_builtin
fun:handle_builtin
fun:run_argv
fun:main
}
not ok 2 - prune files inside $GIT_DIR/repos
#
# mkdir .git/repos
# : .git/repos/abc
# git prune --repos --verbose actual
# cat expect EOF
# Removing repos/abc: not a valid directory
# EOF
# test_i18ncmp expect actual
# ! test -f .git/repos/abc
# ! test -d .git/repos
#
I think it's because of the early 'return 0' ...
+static int prune_repo_dir(const char *id, struct stat *st, struct strbuf
*reason)
+{
+ char *path;
+ int fd, len;
+
+ if (!is_directory(git_path(repos/%s, id))) {
+ strbuf_addf(reason, _(Removing repos/%s: not a valid
directory), id);
+ return 1;
+ }
+ if (file_exists(git_path(repos/%s/locked, id)))
+ return 0;
in this line, before the stat() actually runs, which then in the
condition ...
+ if (stat(git_path(repos/%s/gitdir, id), st)) {
+ st-st_mtime = expire;
+ strbuf_addf(reason, _(Removing repos/%s: gitdir file does not
exist), id);
+ return 1;
+ }
[...]
+}
+
+static void prune_repos_dir(void)
+{
[...]
+ struct stat st;
[...]
+ if (!prune_repo_dir(d-d_name, st, reason) ||
+ st.st_mtime expire)
causes the second arm to be evaluated when st.st_mtime is not
initialized yet. Can you look into this?
--
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] Disable t0110's high-bit test on Windows
Karsten Blees karsten.bl...@gmail.com writes: Am 17.07.2014 17:37, schrieb Stepan Kasal: From: Johannes Schindelin johannes.schinde...@gmx.de The bash Git for Windows uses (i.e. the MSys bash) cannot pass command-line arguments with high bits set verbatim to non-MSys programs, but instead converts those characters with high bits set to their hex representation. The description is not entirely correct...the Unicode-enabled MSYS.dll expects the command line to be UTF-8. Only *invalid* UTF-8 is converted to hex code for convenience. So its not the high bits that cause trouble, but specifying 0x80 without proper UTF-8 lead byte. I believe the last line of the test may actually work: test $(test-urlmatch-normalization -p $(cat $tu-11)) = x://q/%C2%80%DF%BF%E0%A0%80%EF%BF%BD%F0%90%80%80%F0%AF%BF%BD -- Can somebody send a tested replacement then? Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fast-import: use hashcmp() for SHA1 hash comparison
René Scharfe wrote: Signed-off-by: Rene Scharfe l@web.de --- fast-import.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Before: $ size git-fast-import textdata bss dec hex filename 8041386768 754160 1565066 17e18a git-fast-import After: $ size git-fast-import textdata bss dec hex filename 8041546768 754160 1565082 17e19a git-fast-import So this makes the text size worse on this machine (amd64, building with gcc 4.8.2 -O2). That's probably because the old code does 'call memcmp', while the new code inlines it. Inlining is presumably the better choice. More importantly, the new code is more readable. For what it's worth, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH 00/13] mingw unicode environment
Hello Karsten, you wrote: However, if it *did* compile for you, I wonder where ALLOC_GROW (as of #02/13) and alloc_nr (as of #10/13) came from? Or did we recently remove '#include cache.h' from upstream mingw.c? you are right, the include needs to be added. To test my modifications, I rebased all the rest of msysGit collection, built and run the test suite. As you pointed out, https://github.com/msysgit/git/commit/d85d2b75 adds the include. Unfortunately, I won't get to this for several weeks. Could you or Hannes be so kind and post the fixup? Thank you for this comment. And big thanks for the orignal work. (I was only moving it and even that took some time. ;-) 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
a more helpful message on git status output
Hi, when running git status, if the current branch can be fast-forwarded, the user get this message: On branch master Your branch is behind 'origin/master' by 6 commits, and can be fast-forwarded. (use git pull to update your local branch) The suggestion of using git pull for updating the local branch, although it will work, might not be a proper choice or advice in terms of what is really needed to just update the local branch. As the user already has the newer commits locally (ie. the commits have been already git fetched), he just needs to merge them. Running git pull will unnecessarily try to fetch commits from upstream again. So... Proposal: suggesting the user to run git merge or git merge origin/master seems a more helpful suggestion, imo. Also, for beginners, it helps for better understanding on what's going on, and goes in hand with the usual advice of using git fetch + git merge (instead of git pull) to grasp better some Git concepts and how Git works. So, the output of git status would read like this: On branch master Your branch is behind 'origin/master' by 6 commits, and can be fast-forwarded. (use git merge origin/master to update your local branch) (it could read git merge too, if it defaults to merging upstream, tracked branch). -- 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] remote-testsvn: use internal argv_array of struct child_process in cmd_import()
René Scharfe l@web.de writes:
Use the existing argv_array member instead of providing our own. This
way we don't have to initialize or clean it up explicitly.
Signed-off-by: Rene Scharfe l@web.de
---
The change below looks so trivial and I cannot offhand see why it
would break t9020 in a reproducible way.
Puzzled...
remote-testsvn.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/remote-testsvn.c b/remote-testsvn.c
index 6be55cb..31415bd 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -175,7 +175,6 @@ static int cmd_import(const char *line)
char *note_msg;
unsigned char head_sha1[20];
unsigned int startrev;
- struct argv_array svndump_argv = ARGV_ARRAY_INIT;
struct child_process svndump_proc;
if (read_ref(private_ref, head_sha1))
@@ -202,11 +201,10 @@ static int cmd_import(const char *line)
} else {
memset(svndump_proc, 0, sizeof(struct child_process));
svndump_proc.out = -1;
- argv_array_push(svndump_argv, svnrdump);
- argv_array_push(svndump_argv, dump);
- argv_array_push(svndump_argv, url);
- argv_array_pushf(svndump_argv, -r%u:HEAD, startrev);
- svndump_proc.argv = svndump_argv.argv;
+ argv_array_push(svndump_proc.args, svnrdump);
+ argv_array_push(svndump_proc.args, dump);
+ argv_array_push(svndump_proc.args, url);
+ argv_array_pushf(svndump_proc.args, -r%u:HEAD, startrev);
code = start_command(svndump_proc);
if (code)
@@ -227,7 +225,6 @@ static int cmd_import(const char *line)
code = finish_command(svndump_proc);
if (code)
warning(%s, returned %d, svndump_proc.argv[0], code);
- argv_array_clear(svndump_argv);
}
return 0;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fast-import: use hashcmp() for SHA1 hash comparison
Am 18.07.2014 20:42, schrieb Jonathan Nieder: René Scharfe wrote: Signed-off-by: Rene Scharfe l@web.de --- fast-import.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Before: $ size git-fast-import textdata bss dec hex filename 8041386768 754160 1565066 17e18a git-fast-import After: $ size git-fast-import textdata bss dec hex filename 8041546768 754160 1565082 17e19a git-fast-import So this makes the text size worse on this machine (amd64, building with gcc 4.8.2 -O2). That's probably because the old code does 'call memcmp', while the new code inlines it. Inlining is presumably the better choice. More importantly, the new code is more readable. Yes, the latter point is the important one. If inlining is really better is another matter; I don't understand how 1a812f3a (hashcmp(): inline memcmp() by hand to optimize) could have made git gc 18% faster, as it claimed. I would expect memcmp(), which can compare more than a byte at a time, to be significantly faster -- or at least just as fast as whatever the compiler does with the inlined version. René -- 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: a more helpful message on git status output
Julián Landerreche mani...@gmail.com writes: when running git status, if the current branch can be fast-forwarded, the user get this message: On branch master Your branch is behind 'origin/master' by 6 commits, and can be fast-forwarded. (use git pull to update your local branch) The suggestion of using git pull for updating the local branch, although it will work, might not be a proper choice or advice in terms of what is really needed to just update the local branch. As the user already has the newer commits locally (ie. the commits have been already git fetched), he just needs to merge them. Running git pull will unnecessarily try to fetch commits from upstream again. By running git pull, the user may obtain yet newer commits from the upstream, which very likely will happen in an active project, or git fetch launched by git pull will return without doing anything after noticing there is nothing new. As long as the updates to the upstream is also a fast-forward, it will still fast-forward you, but to an even newer state of the upstream. There is no harm done[*1*] by suggesting git pull over git merge, no? [Footnote] *1* There is a bigger problem with this message, especially when the user sees it on 'master', but your message is about the case where you are strictly behind and that bigger problem will not be an issue, so I won't discuss it further. -- 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] remote-testsvn: use internal argv_array of struct child_process in cmd_import()
Am 18.07.2014 21:10, schrieb Junio C Hamano:
René Scharfe l@web.de writes:
Use the existing argv_array member instead of providing our own. This
way we don't have to initialize or clean it up explicitly.
Signed-off-by: Rene Scharfe l@web.de
---
The change below looks so trivial and I cannot offhand see why it
would break t9020 in a reproducible way.
Puzzled...
remote-testsvn.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/remote-testsvn.c b/remote-testsvn.c
index 6be55cb..31415bd 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -175,7 +175,6 @@ static int cmd_import(const char *line)
char *note_msg;
unsigned char head_sha1[20];
unsigned int startrev;
-struct argv_array svndump_argv = ARGV_ARRAY_INIT;
struct child_process svndump_proc;
if (read_ref(private_ref, head_sha1))
@@ -202,11 +201,10 @@ static int cmd_import(const char *line)
} else {
memset(svndump_proc, 0, sizeof(struct child_process));
svndump_proc.out = -1;
-argv_array_push(svndump_argv, svnrdump);
-argv_array_push(svndump_argv, dump);
-argv_array_push(svndump_argv, url);
-argv_array_pushf(svndump_argv, -r%u:HEAD, startrev);
-svndump_proc.argv = svndump_argv.argv;
+argv_array_push(svndump_proc.args, svnrdump);
+argv_array_push(svndump_proc.args, dump);
+argv_array_push(svndump_proc.args, url);
+argv_array_pushf(svndump_proc.args, -r%u:HEAD, startrev);
code = start_command(svndump_proc);
if (code)
@@ -227,7 +225,6 @@ static int cmd_import(const char *line)
code = finish_command(svndump_proc);
if (code)
warning(%s, returned %d, svndump_proc.argv[0], code);
-argv_array_clear(svndump_argv);
Unfortunately I don't get a test failure, but I think I see what's
wrong: The warning line above references the argv array after it was
freed by finish_command. Ouch. Fixup below:
---
remote-testsvn.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/remote-testsvn.c b/remote-testsvn.c
index 31415bd..e3ad11b 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -176,6 +176,7 @@ static int cmd_import(const char *line)
unsigned char head_sha1[20];
unsigned int startrev;
struct child_process svndump_proc;
+ const char *command;
if (read_ref(private_ref, head_sha1))
startrev = 0;
@@ -199,16 +200,17 @@ static int cmd_import(const char *line)
if(dumpin_fd 0)
die_errno(Couldn't open svn dump file %s., url);
} else {
+ command = svnrdump;
memset(svndump_proc, 0, sizeof(struct child_process));
svndump_proc.out = -1;
- argv_array_push(svndump_proc.args, svnrdump);
+ argv_array_push(svndump_proc.args, command);
argv_array_push(svndump_proc.args, dump);
argv_array_push(svndump_proc.args, url);
argv_array_pushf(svndump_proc.args, -r%u:HEAD, startrev);
code = start_command(svndump_proc);
if (code)
- die(Unable to start %s, code %d,
svndump_proc.argv[0], code);
+ die(Unable to start %s, code %d, command, code);
dumpin_fd = svndump_proc.out;
}
/* setup marks file import/export */
@@ -224,7 +226,7 @@ static int cmd_import(const char *line)
if (!dump_from_file) {
code = finish_command(svndump_proc);
if (code)
- warning(%s, returned %d, svndump_proc.argv[0], code);
+ warning(%s, returned %d, command, code);
}
return 0;
--
2.0.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: a more helpful message on git status output
By running git pull, the user may obtain yet newer commits from the upstream, which very likely will happen in an active project, or git fetch launched by git pull will return without doing anything after noticing there is nothing new. As long as the updates to the upstream is also a fast-forward, it will still fast-forward you, but to an even newer state of the upstream. There is no harm done[*1*] by suggesting git pull over git merge, no? OK, I'm mostly convinced. A more verbose, educational output could read: (use git pull to fetch newer commits from upstream and update your local branch) (use git merge to update your local branch) [Footnote] *1* There is a bigger problem with this message, especially when the user sees it on 'master', but your message is about the case where you are strictly behind and that bigger problem will not be an issue, so I won't discuss it further. No idea what's this bigger problem with this message. Care to expand? 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
[PATCH v2] remote-testsvn: use internal argv_array of struct child_process in cmd_import()
Use the existing argv_array member instead of providing our own. This
way we don't have to initialize or clean it up explicitly. Because of
that automatic cleanup, we need to keep our own reference to the
command name instead of using .argv[0] to print the warning at the end.
Signed-off-by: Rene Scharfe l@web.de
---
The added command pointer makes the patch more complicated, but I think
it still counts as a cleanup.
remote-testsvn.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/remote-testsvn.c b/remote-testsvn.c
index 6be55cb..e3ad11b 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -175,8 +175,8 @@ static int cmd_import(const char *line)
char *note_msg;
unsigned char head_sha1[20];
unsigned int startrev;
- struct argv_array svndump_argv = ARGV_ARRAY_INIT;
struct child_process svndump_proc;
+ const char *command;
if (read_ref(private_ref, head_sha1))
startrev = 0;
@@ -200,17 +200,17 @@ static int cmd_import(const char *line)
if(dumpin_fd 0)
die_errno(Couldn't open svn dump file %s., url);
} else {
+ command = svnrdump;
memset(svndump_proc, 0, sizeof(struct child_process));
svndump_proc.out = -1;
- argv_array_push(svndump_argv, svnrdump);
- argv_array_push(svndump_argv, dump);
- argv_array_push(svndump_argv, url);
- argv_array_pushf(svndump_argv, -r%u:HEAD, startrev);
- svndump_proc.argv = svndump_argv.argv;
+ argv_array_push(svndump_proc.args, command);
+ argv_array_push(svndump_proc.args, dump);
+ argv_array_push(svndump_proc.args, url);
+ argv_array_pushf(svndump_proc.args, -r%u:HEAD, startrev);
code = start_command(svndump_proc);
if (code)
- die(Unable to start %s, code %d,
svndump_proc.argv[0], code);
+ die(Unable to start %s, code %d, command, code);
dumpin_fd = svndump_proc.out;
}
/* setup marks file import/export */
@@ -226,8 +226,7 @@ static int cmd_import(const char *line)
if (!dump_from_file) {
code = finish_command(svndump_proc);
if (code)
- warning(%s, returned %d, svndump_proc.argv[0], code);
- argv_array_clear(svndump_argv);
+ warning(%s, returned %d, command, code);
}
return 0;
--
2.0.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Make locked paths absolute when current directory is changed
Am 18.07.2014 15:08, schrieb Nguyễn Thái Ngọc Duy:
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..a70d107 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -280,3 +280,19 @@ void rollback_lock_file(struct lock_file *lk)
}
lk-filename[0] = 0;
}
+
+void make_locked_paths_absolute(void)
+{
+ struct lock_file *lk;
+ const char *abspath;
+ for (lk = lock_file_list; lk != NULL; lk = lk-next) {
+ if (!lk-filename[0] || lk-filename[0] == '/')
Please use is_absolute_path().
+ continue;
+ abspath = absolute_path(lk-filename);
+ if (strlen(abspath) = sizeof(lk-filename))
+ warning(locked path %s is relative when current
directory
+ is changed, lk-filename);
+ else
+ strcpy(lk-filename, abspath);
+ }
+}
--- a/run-command.c
+++ b/run-command.c
@@ -399,7 +399,7 @@ fail_pipe:
close(cmd-out);
}
- if (cmd-dir chdir(cmd-dir))
+ if (cmd-dir chdir_safe(cmd-dir))
This one shouldn't be necessary: It's in the child, and the child
process does not release the locks; see the check for the owner in
remove_lock_file.
die_errno(exec '%s': cd to '%s' failed, cmd-argv[0],
cmd-dir);
if (cmd-env) {
-- 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: a more helpful message on git status output
Julián Landerreche wrote: A more verbose, educational output could read: (use git pull to fetch newer commits from upstream and update your local branch) (use git merge to update your local branch) Yes, I like this idea, with a few qualifications: 1. The first line is long. Is there a shorter way to say the same thing? Maybe (use git pull to fetch newer commits and update your local branch) 2. s/from upstream/from $remote/ (e.g., from origin) in the first line? Though that would make problem (1) worse. 3. Is there some way to make it more obvious these two hints are independent suggestions and that the user doesn't need to do both? Maybe something as simple as (or use git merge to update your local branch) 4. Should the advice differ based on whether the current branch is set up for merging or rebasing? Thanks, Jonathan -- 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: a more helpful message on git status output
Julián Landerreche mani...@gmail.com writes: By running git pull, the user may obtain yet newer commits from the upstream, which very likely will happen in an active project, or git fetch launched by git pull will return without doing anything after noticing there is nothing new. As long as the updates to the upstream is also a fast-forward, it will still fast-forward you, but to an even newer state of the upstream. There is no harm done[*1*] by suggesting git pull over git merge, no? OK, I'm mostly convinced. A more verbose, educational output could read: (use git pull to fetch newer commits from upstream and update your local branch) (use git merge to update your local branch) I actually do not like that, to be honest. These brief reminders should be just that, and if anything, we should aim to make them shorter and more concise, not longer and more verbose. They will never be sufficient to replace education (otherwise why would we even have complete manual?)---they should just point the users clearly in the right direction. -- 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] remote-testsvn: use internal argv_array of struct child_process in cmd_import()
René Scharfe l@web.de writes:
Use the existing argv_array member instead of providing our own. This
way we don't have to initialize or clean it up explicitly. Because of
that automatic cleanup, we need to keep our own reference to the
command name instead of using .argv[0] to print the warning at the end.
Signed-off-by: Rene Scharfe l@web.de
---
The added command pointer makes the patch more complicated, but I think
it still counts as a cleanup.
Surely. I'd move the svnrdump assignment to where the variable is
defined, though; we do not switch what command to run depending on
some computed conditions anyway.
remote-testsvn.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/remote-testsvn.c b/remote-testsvn.c
index 6be55cb..e3ad11b 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -175,8 +175,8 @@ static int cmd_import(const char *line)
char *note_msg;
unsigned char head_sha1[20];
unsigned int startrev;
- struct argv_array svndump_argv = ARGV_ARRAY_INIT;
struct child_process svndump_proc;
+ const char *command;
if (read_ref(private_ref, head_sha1))
startrev = 0;
@@ -200,17 +200,17 @@ static int cmd_import(const char *line)
if(dumpin_fd 0)
die_errno(Couldn't open svn dump file %s., url);
} else {
+ command = svnrdump;
memset(svndump_proc, 0, sizeof(struct child_process));
svndump_proc.out = -1;
- argv_array_push(svndump_argv, svnrdump);
- argv_array_push(svndump_argv, dump);
- argv_array_push(svndump_argv, url);
- argv_array_pushf(svndump_argv, -r%u:HEAD, startrev);
- svndump_proc.argv = svndump_argv.argv;
+ argv_array_push(svndump_proc.args, command);
+ argv_array_push(svndump_proc.args, dump);
+ argv_array_push(svndump_proc.args, url);
+ argv_array_pushf(svndump_proc.args, -r%u:HEAD, startrev);
code = start_command(svndump_proc);
if (code)
- die(Unable to start %s, code %d,
svndump_proc.argv[0], code);
+ die(Unable to start %s, code %d, command, code);
dumpin_fd = svndump_proc.out;
}
/* setup marks file import/export */
@@ -226,8 +226,7 @@ static int cmd_import(const char *line)
if (!dump_from_file) {
code = finish_command(svndump_proc);
if (code)
- warning(%s, returned %d, svndump_proc.argv[0], code);
- argv_array_clear(svndump_argv);
+ warning(%s, returned %d, command, code);
}
return 0;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] remote-testsvn: use internal argv_array of struct child_process in cmd_import()
Am 18.07.2014 23:18, schrieb Junio C Hamano:
René Scharfe l@web.de writes:
Use the existing argv_array member instead of providing our own. This
way we don't have to initialize or clean it up explicitly. Because of
that automatic cleanup, we need to keep our own reference to the
command name instead of using .argv[0] to print the warning at the end.
Signed-off-by: Rene Scharfe l@web.de
---
The added command pointer makes the patch more complicated, but I think
it still counts as a cleanup.
Surely. I'd move the svnrdump assignment to where the variable is
defined, though; we do not switch what command to run depending on
some computed conditions anyway.
OK; I see you already did that in pu, thanks. My point was to allow the
compiler to warn if the variable command was used in the case
start_command was not actually called. That's probably too subtle to be
useful.
remote-testsvn.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/remote-testsvn.c b/remote-testsvn.c
index 6be55cb..e3ad11b 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -175,8 +175,8 @@ static int cmd_import(const char *line)
char *note_msg;
unsigned char head_sha1[20];
unsigned int startrev;
- struct argv_array svndump_argv = ARGV_ARRAY_INIT;
struct child_process svndump_proc;
+ const char *command;
if (read_ref(private_ref, head_sha1))
startrev = 0;
@@ -200,17 +200,17 @@ static int cmd_import(const char *line)
if(dumpin_fd 0)
die_errno(Couldn't open svn dump file %s., url);
} else {
+ command = svnrdump;
memset(svndump_proc, 0, sizeof(struct child_process));
svndump_proc.out = -1;
- argv_array_push(svndump_argv, svnrdump);
- argv_array_push(svndump_argv, dump);
- argv_array_push(svndump_argv, url);
- argv_array_pushf(svndump_argv, -r%u:HEAD, startrev);
- svndump_proc.argv = svndump_argv.argv;
+ argv_array_push(svndump_proc.args, command);
+ argv_array_push(svndump_proc.args, dump);
+ argv_array_push(svndump_proc.args, url);
+ argv_array_pushf(svndump_proc.args, -r%u:HEAD, startrev);
code = start_command(svndump_proc);
if (code)
- die(Unable to start %s, code %d,
svndump_proc.argv[0], code);
+ die(Unable to start %s, code %d, command, code);
dumpin_fd = svndump_proc.out;
}
/* setup marks file import/export */
@@ -226,8 +226,7 @@ static int cmd_import(const char *line)
if (!dump_from_file) {
code = finish_command(svndump_proc);
if (code)
- warning(%s, returned %d, svndump_proc.argv[0], code);
- argv_array_clear(svndump_argv);
+ warning(%s, returned %d, command, code);
}
return 0;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 4/4] test-config: add tests for the config_set API
Junio C Hamano gits...@pobox.com writes: Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'proper error on non-existant files' ' +echo Error (-1) reading configuration file non-existant-file. expect s/tant/tent/ perhaps? cf. http://en.wiktionary.org/wiki/non-existant It seems so. My bad. +test_expect_success 'proper error on non-accessible files' ' +chmod -r .git/config +test_when_finished chmod +r .git/config +echo Error (-1) reading configuration file .git/config. expect +test_expect_code 2 test-config configset_get_value foo.bar .git/config 2actual +test_cmp expect actual +' Doesn't this one need some prerequisite? My bad too ;-). There should have been SANITY,POSIXPERM I guess (Tanay: search prereq in t/README if you don't see what we're talking about). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] checkout: indicate when a detached head is checked out for a branch
On vr, 2014-07-18 at 10:36 -0700, Junio C Hamano wrote: Michael J Gruber g...@drmicha.warpmail.net writes: I really like the new --to feature and will convert all my new workdir checkouts to that. But that detached checkout is so easy to miss - in fact I noticed it only when I compared new-workdir to checkout --to for a test repo with one branch, to see what a converter would need to do. I'm even wondering whether we should do this DWIMmery at all, given how dangerous a detached head is for those who are not aware of it before gc kicks in. As long as the amount of warning about 'detached HEAD' is about the same between this case and a git checkout v1.2.3 in a normal repository, I do not think there is no additional danger you need to be worried about. But I do agree that there should not be any DWIM here. The reason to introduce this new set of rather intrusive changes is so that working trees can be aware of branches other working trees have checked out. And the whole point of wanting to have that mutual awareness is to enable us to forbid users from mucking with the same branch from two different places. But Git is not in the position to dictate which alternative action the user would want to take, when her git checkout foo is prevented by this mechanism. In one scenario, she may say I only wanted to take a peek and run git checkout foo^0 instead. In another, she may say Ah, I forgot I already was doing this change in the other one and run cd ../foo. There may be other classes of alternative actions. Don't make it easier for the first class of scenario and make it less useful and more dangerous for the second class, especially the second class involve forgetful users who are likely to forget seeing the we've warned you that we detached without being asked message. Please fix it to always just error out. I really would appreciate it if it wouldn't always error out. Erroring out by default is fine, but please keep it overridable. My use case for this is checking out the same branch (or commit, so already on a detached HEAD) in multiple different places to run independent actions (e.g. make test with different compiler options, or creating several different packages) and I would really appreciate it if that would keep working. -- Dennis Kaarsemaker www.kaarsemaker.net -- 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] checkout: indicate when a detached head is checked out for a branch
Dennis Kaarsemaker den...@kaarsemaker.net writes: My use case for this is checking out the same branch (or commit, so already on a detached HEAD) in multiple different places to run independent actions (e.g. make test with different compiler options, or creating several different packages) and I would really appreciate it if that would keep working. I do not have any problem if multiple working trees have the same commit checked out on their own detached HEADs at all. The should error out was solely for the case where the user asked not to detach but to obtain a state where a named branch is checked out. In such a case, the command should not turn it into a detached HEAD, with or without a warning. -- 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 01/12] wrapper.c: simplify warn_if_unremovable
Ronnie Sahlberg sahlb...@google.com writes:
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
wrapper.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/wrapper.c b/wrapper.c
index bc1bfb8..740e193 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -429,14 +429,12 @@ int xmkstemp_mode(char *template, int mode)
static int warn_if_unremovable(const char *op, const char *file, int rc)
{
- if (rc 0) {
- int err = errno;
- if (ENOENT != err) {
- warning(unable to %s %s: %s,
- op, file, strerror(errno));
- errno = err;
- }
- }
+ int err;
+ if (rc = 0 || errno == ENOENT)
+ return rc;
+ err = errno;
+ warning(unable to %s %s: %s, op, file, strerror(errno));
+ errno = err;
return rc;
}
Looks sensible.
--
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 02/12] wrapper.c: add a new function unlink_or_msg
Ronnie Sahlberg sahlb...@google.com writes:
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
git-compat-util.h | 6 ++
wrapper.c | 18 ++
2 files changed, 24 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index b6f03b3..426bc98 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -704,12 +704,18 @@ void git_qsort(void *base, size_t nmemb, size_t size,
#endif
#endif
+#include strbuf.h
+
/*
* Preserves errno, prints a message, but gives no warning for ENOENT.
* Always returns the return value of unlink(2).
*/
int unlink_or_warn(const char *path);
/*
+ * Like unlink_or_warn but populates a strbuf
+ */
+int unlink_or_msg(const char *file, struct strbuf *err);
+/*
* Likewise for rmdir(2).
*/
int rmdir_or_warn(const char *path);
diff --git a/wrapper.c b/wrapper.c
index 740e193..74a0cc0 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -438,6 +438,24 @@ static int warn_if_unremovable(const char *op, const
char *file, int rc)
return rc;
}
+int unlink_or_msg(const char *file, struct strbuf *err)
+{
+ if (err) {
+ int rc = unlink(file);
+ int save_errno = errno;
+
+ if (rc 0 errno != ENOENT) {
+ strbuf_addf(err, unable to unlink %s: %s,
+ file, strerror(errno));
+ errno = save_errno;
+ return -1;
+ }
+ return 0;
+ }
+
+ return unlink_or_warn(file);
+}
In general, I do not generally like to see messages propagated
upwards from deeper levels of the callchain to the callers to be
used later, primarily because that will easily make it harder to
localize the message-lego.
For this partcular one, shouldn't the caller be doing
if (unlink(file) errno != ENOENT) {
... do its own error message ...
}
instead of calling any of the unlink_or_whatever() helper?
int unlink_or_warn(const char *file)
{
return warn_if_unremovable(unlink, file, unlink(file));
--
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 05/12] refs.c: pass NULL as *flags to read_ref_full
Ronnie Sahlberg sahlb...@google.com writes:
We call read_ref_full with a pointer to flags from rename_ref but since
we never actually use the returned flags we can just pass NULL here instead.
Sensible, at least for the current callers. I had to wonder if
rename_ref() would never want to take advantage of the flags return
parameter in the future, though. For example, would it want to act
differently when the given ref turns out to be a symref? Would it
want to report something when the ref to be overwritten was a broken
one?
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
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 7d65253..0df6894 100644
--- a/refs.c
+++ b/refs.c
@@ -2666,7 +2666,7 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
goto rollback;
}
- if (!read_ref_full(newrefname, sha1, 1, flag)
+ 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))) {
--
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 06/12] refs.c: move the check for valid refname to lock_ref_sha1_basic
Ronnie Sahlberg sahlb...@google.com writes:
Move the check for check_refname_format from lock_any_ref_for_update
to lock_ref_sha1_basic. At some later stage we will get rid of
lock_any_ref_for_update completely.
If lock_ref_sha1_basic fails the check_refname_format test, set errno to
EINVAL before returning NULL. This to guarantee that we will not return an
error without updating errno.
This leaves lock_any_ref_for_updates as a no-op wrapper which could be
removed.
But this wrapper is also called from an external caller and we will soon
make changes to the signature to lock_ref_sha1_basic that we do not want to
expose to that caller.
That might be sensible if our only goal were to remove
lock-any-ref-for-updates, but I wonder what the impact of this
change to other existing callers of lock-ref-sha1-basic. I may be
recalling things incorrectly, but I suspect that it was deliberate
to keep the lowest-level internal helper function (i.e. _basic()) to
be lenient so that those who do not want the format checks can
choose to pass refnames that are not exactly kosher.
If we need such recovery code we could add it as an option to git fsck and
have
git fsck be the only sanctioned way of bypassing the normal API and checks.
But fsck is about checking and never about recovering, isn't it?
Does it offer to remove misnamed refs? Should it?
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 0df6894..f29f18a 100644
--- a/refs.c
+++ b/refs.c
@@ -2088,6 +2088,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
int missing = 0;
int attempts_remaining = 3;
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
+ errno = EINVAL;
+ return NULL;
+ }
+
lock = xcalloc(1, sizeof(struct ref_lock));
lock-lock_fd = -1;
@@ -2179,8 +2184,6 @@ struct ref_lock *lock_any_ref_for_update(const char
*refname,
const unsigned char *old_sha1,
int flags, int *type_p)
{
- if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
- return NULL;
return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
}
--
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 02/12] wrapper.c: add a new function unlink_or_msg
Hmm, the primary reason for this seems to be because you are going to handle
multiple refs at a time, some of them might fail to lock due to this
lowest-level
helper to unlink failing, some others may fail to lock due to some other reason,
and the user may want to be able to differentiate various modes of failure.
But even if that were the case, would it be necessary to buffer the messages
like this and give them all at the end? In the transaction code path,
it is likely
that you would be aborting the whole thing at the first failure, no?
I dunno...
On Fri, Jul 18, 2014 at 3:25 PM, Junio C Hamano gits...@pobox.com wrote:
Ronnie Sahlberg sahlb...@google.com writes:
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
git-compat-util.h | 6 ++
wrapper.c | 18 ++
2 files changed, 24 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index b6f03b3..426bc98 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -704,12 +704,18 @@ void git_qsort(void *base, size_t nmemb, size_t size,
#endif
#endif
+#include strbuf.h
+
/*
* Preserves errno, prints a message, but gives no warning for ENOENT.
* Always returns the return value of unlink(2).
*/
int unlink_or_warn(const char *path);
/*
+ * Like unlink_or_warn but populates a strbuf
+ */
+int unlink_or_msg(const char *file, struct strbuf *err);
+/*
* Likewise for rmdir(2).
*/
int rmdir_or_warn(const char *path);
diff --git a/wrapper.c b/wrapper.c
index 740e193..74a0cc0 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -438,6 +438,24 @@ static int warn_if_unremovable(const char *op, const
char *file, int rc)
return rc;
}
+int unlink_or_msg(const char *file, struct strbuf *err)
+{
+ if (err) {
+ int rc = unlink(file);
+ int save_errno = errno;
+
+ if (rc 0 errno != ENOENT) {
+ strbuf_addf(err, unable to unlink %s: %s,
+ file, strerror(errno));
+ errno = save_errno;
+ return -1;
+ }
+ return 0;
+ }
+
+ return unlink_or_warn(file);
+}
In general, I do not generally like to see messages propagated
upwards from deeper levels of the callchain to the callers to be
used later, primarily because that will easily make it harder to
localize the message-lego.
For this partcular one, shouldn't the caller be doing
if (unlink(file) errno != ENOENT) {
... do its own error message ...
}
instead of calling any of the unlink_or_whatever() helper?
int unlink_or_warn(const char *file)
{
return warn_if_unremovable(unlink, file, unlink(file));
--
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] fast-import: use hashcmp() for SHA1 hash comparison
On Fri, Jul 18, 2014 at 09:14:05PM +0200, René Scharfe wrote: If inlining is really better is another matter; I don't understand how 1a812f3a (hashcmp(): inline memcmp() by hand to optimize) could have made git gc 18% faster, as it claimed. I would expect memcmp(), which can compare more than a byte at a time, to be significantly faster -- or at least just as fast as whatever the compiler does with the inlined version. I looked into this a while ago[1]. I think with glibc 2.13 and up, the memcmp is a win. We should consider switching back if that is what is common now. -Peff [1] http://article.gmane.org/gmane.comp.version-control.git/218396 -- 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_inetd_server: run git-http-backend using inetd
On Jul 18, 2014, at 10:16, Jonathan Nieder wrote: Kyle J. McKay wrote: You might also want to take a look at [1] which suggests that when doing SRV lookups for URLs they should be done regardless of whether or not a port number is present (which then eliminates the RFC 3986 issue the current SRV lookup code has). Git URLs as described e.g. in git-clone(1) weren't intended to be actual URIs. According to RFC 3968 section 1.1.3: A URI can be further classified as a locator, a name, or both. The term Uniform Resource Locator (URL) refers to the subset of URIs [...] So actually they are URIs. What would be the interoperability advantage of making them URIs? According to RFC 3968 they are already considered URIs. This has come up before, with e.g. people asking to introduce a git+ssh:// and git+http:// How is a discussion about changing the scheme name relevant to a discussion about treating a URL with an explicit default port the same as one without (which Git already does but stops doing with the 0010 git SRV patch)? That would seem to be an orthogonal discussion to whether or not to change the scheme name(s) used by Git more than 9 years after it first came out. -- 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_inetd_server: run git-http-backend using inetd
Kyle J. McKay wrote: On Jul 18, 2014, at 10:16, Jonathan Nieder wrote: Kyle J. McKay wrote: You might also want to take a look at [1] which suggests that when doing SRV lookups for URLs they should be done regardless of whether or not a port number is present (which then eliminates the RFC 3986 issue the current SRV lookup code has). Git URLs as described e.g. in git-clone(1) weren't intended to be actual URIs. According to RFC 3968 section 1.1.3: A URI can be further classified as a locator, a name, or both. The term Uniform Resource Locator (URL) refers to the subset of URIs [...] So actually they are URIs. You mean abusing the word URL when we meant URL-shaped thing makes it into a URL? What would be the interoperability advantage of making them URIs? According to RFC 3968 they are already considered URIs. I don't think you understood my question. This has come up before, with e.g. people asking to introduce a git+ssh:// and git+http:// How is a discussion about changing the scheme name relevant to a discussion about treating a URL with an explicit default port the same as one without (which Git already does but stops doing with the 0010 git SRV patch)? It's where the question of whether the things you pass to 'git clone' are URIs came up before. I don't understand what you want in this side-conversation. Do you mean that I should read that RFC and be convinced that what you are saying about ports is the right thing to do? I can easily be convinced some other way, but It's in a standard that you never intended to follow is not particularly convincing or relevant. The same philosophy as the git project applies to POSIX conformance issues applies here, too. We live in the real world. Hoping that clarifies, Jonathan -- 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_inetd_server: run git-http-backend using inetd
On Jul 18, 2014, at 17:19, Jonathan Nieder wrote: Kyle J. McKay wrote: On Jul 18, 2014, at 10:16, Jonathan Nieder wrote: Git URLs as described e.g. in git-clone(1) weren't intended to be actual URIs. According to RFC 3968 section 1.1.3: A URI can be further classified as a locator, a name, or both. The term Uniform Resource Locator (URL) refers to the subset of URIs [...] So actually they are URIs. You mean abusing the word URL when we meant URL-shaped thing makes it into a URL? That is your contention. Do you mean that I should read that RFC and be convinced that what you are saying about ports is the right thing to do? It's the right thing to do because it's the standard for how URLs are expected to behave. It's in a standard that you never intended to follow is not particularly convincing or relevant. That is your contention. If it is truly the case that where the Git documentation uses the URL acronym that Git does not actually intend for URL to be interpreted as a URL as defined by the various standards covering such URLs then explicit text needs to be added to the documentation saying so to avoid confusion. In the absence of such text, expecting a reader of Git documentation to interpret the term URL any other way is irrational. -- 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 v11 0/4] git config cache special querying api utilizing the cache
Hi, [PATCH V11]: very minor fixes. check [13] for discussion. [PATCH v10]: Minor fixes according to [12]. Re added string_list initializer function. Thanks to Junio and Matthieu for their suggestions. [PATCH v9]: Applied most of the review comments mentioned in [11]. Mostly asthetic changes. test-config now clears the config_set before exiting. Most of the tests now use the check-config function. check_config_init() now handles return values correctly. Diff between v8 and v9 is at the bottom. Thanks to Junio and Matthieu for the review. [PATCH V8]: Moved the contents of config-set.c to config.c for future convenience. Reverted test 'find value with misspelled key' to the one in v5. See [10] for the discussion. [PATCH V7]: Style nits and a broken chain corrected in `t/t1308-config-set.sh`. See [9] for the nits. [PATCH V6]: Style nits and mistakes corrected. Diff between v6 and v5[8] is at the bottom. Thanks to Matthieu, Ramsay and Ram for their suggestions. [PATCH V5]: `config_set` now uses a single hashmap. Corrected style nits raised in the thread[7]. Thanks to Junio and Matthieu for their suggestions. [PATCH v4]: Introduced `config_set` construct which points to a ordered set of config-files cached as hashmaps. Added relevant API functions. For more details see the documentation. Rewrote the git_config_get* family to use `config_set` internally. Added tests for both config_set API and git_config_get family. Added type specific API functions which parses the found value and converts it into a specific type. Most of the changes implemented are the result of discussion in [6]. Thanks to Eric, Ramsay, Junio, Matthieu Karsten for their suggestions and review. [PATCH v3]: Added flag for NULL values that were causing segfaults in some cases. Added test-config for usage examples. Minor changes and corrections. Refer to discussion thread[5] for more details. Thanks to Matthieu, Jeff and Junio for their valuable suggestions. [PATCH v2]:Changed the string_list to a struct instead of pointer to a struct. Added string-list initilization functions. Minor mistakes corrected acoording to review comments[4]. Thanks to Eric and Matthieu for their review. [PATCH V1]:Most of the invaluable suggestions by Eric Sunshine, Torsten Bogershausen and Jeff King has been implemented[1]. Complete rewrite of config_cache*() family using git_config() as hook as suggested by Jeff. Thanks for the review. [RFC V2]: Improved according to the suggestions by Eric Sunshine and Torsten Bogershausen. Added cache invalidation when config file is changed.[2] I am using git_config_set_multivar_in_file() as an update hook. This is patch is for this year's GSoC. My project is Git Config API improvements. The link of my proposal is appended below [3]. The aim of this patch series is to generate a cache for querying values from the config files in a non-callback manner as the current method reads and parses the config files every time a value is queried for. The cache is generated from hooking the update_cache function to the current parsing and callback mechanism in config.c. It is implemented as an hashmap using the hashmap-api with variables and its corresponding values list as its members. The values in the list are sorted in order of increasing priority. The cache is initialised the first time when any of the new query functions is called. It is invalidated by using git_config_set_multivar_in_file() as an update hook. We add two new functions to the config-api, git_config_get_string() and git_config_get_string_multi() for querying in a non callback manner from the cache. [1] http://marc.info/?t=14017206626r=1w=2 [2] http://git.661346.n2.nabble.com/RFC-PATCH-0-2-Git-config-cache-amp-special-querying-api-utilizing-the-cache-td7611691.html [3] https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing [4] http://thread.gmane.org/gmane.comp.version-control.git/251073/focus=251369 [5] http://thread.gmane.org/gmane.comp.version-control.git/251704/ [6] http://thread.gmane.org/gmane.comp.version-control.git/252329/ [7] http://marc.info/?t=14042811521r=1w=2 [8] http://article.gmane.org/gmane.comp.version-control.git/252942/ [9] http://thread.gmane.org/gmane.comp.version-control.git/252959/ [10] http://article.gmane.org/gmane.comp.version-control.git/253113 [11] http://thread.gmane.org/gmane.comp.version-control.git/253248 [12] http://thread.gmane.org/gmane.comp.version-control.git/253562 [13] http://thread.gmane.org/gmane.comp.version-control.git/253799 Tanay Abhra (4): string-list: add string_list initialiser helper functions Use string-list initializaer functions to rewrite config set test-config .gitignore | 1 +
[PATCH v11 2/4] replace memset with string-list initializers
Using memset and then manually setting values of the string-list
members is not future proof as the internal representation of
string-list may change any time.
Use `string_list_init()` or STRING_LIST_INIT_* macros instead of
memset.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
builtin/commit.c | 3 +--
merge-recursive.c | 9 +++--
submodule.c | 5 +
transport.c | 4 +---
4 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 72eb3be..5b196ca 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -420,8 +420,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
die(_(cannot do a partial commit during a
cherry-pick.));
}
- memset(partial, 0, sizeof(partial));
- partial.strdup_strings = 1;
+ string_list_init(partial, 1);
if (list_paths(partial, !current_head ? NULL : HEAD, prefix,
pathspec))
exit(1);
diff --git a/merge-recursive.c b/merge-recursive.c
index 5814d05..1d332b8 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2059,12 +2059,9 @@ void init_merge_options(struct merge_options *o)
if (o-verbosity = 5)
o-buffer_output = 0;
strbuf_init(o-obuf, 0);
- memset(o-current_file_set, 0, sizeof(struct string_list));
- o-current_file_set.strdup_strings = 1;
- memset(o-current_directory_set, 0, sizeof(struct string_list));
- o-current_directory_set.strdup_strings = 1;
- memset(o-df_conflict_file_set, 0, sizeof(struct string_list));
- o-df_conflict_file_set.strdup_strings = 1;
+ string_list_init(o-current_file_set, 1);
+ string_list_init(o-current_directory_set, 1);
+ string_list_init(o-df_conflict_file_set, 1);
}
int parse_merge_opt(struct merge_options *o, const char *s)
diff --git a/submodule.c b/submodule.c
index 48e3b44..c3a61e7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -544,10 +544,7 @@ static int push_submodule(const char *path)
int push_unpushed_submodules(unsigned char new_sha1[20], const char
*remotes_name)
{
int i, ret = 1;
- struct string_list needs_pushing;
-
- memset(needs_pushing, 0, sizeof(struct string_list));
- needs_pushing.strdup_strings = 1;
+ struct string_list needs_pushing = STRING_LIST_INIT_DUP;
if (!find_unpushed_submodules(new_sha1, remotes_name, needs_pushing))
return 1;
diff --git a/transport.c b/transport.c
index 59c9727..d32aaf2 100644
--- a/transport.c
+++ b/transport.c
@@ -1188,10 +1188,8 @@ int transport_push(struct transport *transport,
if ((flags (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |
TRANSPORT_RECURSE_SUBMODULES_CHECK))
!is_bare_repository()) {
struct ref *ref = remote_refs;
- struct string_list needs_pushing;
+ struct string_list needs_pushing = STRING_LIST_INIT_DUP;
- memset(needs_pushing, 0, sizeof(struct string_list));
- needs_pushing.strdup_strings = 1;
for (; ref; ref = ref-next)
if (!is_null_sha1(ref-new_sha1)
find_unpushed_submodules(ref-new_sha1,
--
1.9.0.GIT
--
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 v11 1/4] string-list: add string_list initializer helper function
The string-list API has STRING_LIST_INIT_* macros to be used
to define variables with initializers, but lacks functions
to initialize an uninitialized piece of memory to be used as
a string-list at the run-time.
Introduce `string_list_init()` function for that.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Documentation/technical/api-string-list.txt | 5 +
string-list.c | 6 ++
string-list.h | 2 ++
3 files changed, 13 insertions(+)
diff --git a/Documentation/technical/api-string-list.txt
b/Documentation/technical/api-string-list.txt
index f1add51..d51a657 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -68,6 +68,11 @@ Functions
* General ones (works with sorted and unsorted lists as well)
+`string_list_init`::
+
+ Initialize the members of the string_list, set `strdup_strings`
+ member according to the value of the second parameter.
+
`filter_string_list`::
Apply a function to each item in a list, retaining only the
diff --git a/string-list.c b/string-list.c
index aabb25e..db38b62 100644
--- a/string-list.c
+++ b/string-list.c
@@ -1,6 +1,12 @@
#include cache.h
#include string-list.h
+void string_list_init(struct string_list *list, int strdup_strings)
+{
+ memset(list, 0, sizeof(*list));
+ list-strdup_strings = strdup_strings;
+}
+
/* if there is no exact match, point to the index where the entry could be
* inserted */
static int get_entry_index(const struct string_list *list, const char *string,
diff --git a/string-list.h b/string-list.h
index dd5e294..494eb5d 100644
--- a/string-list.h
+++ b/string-list.h
@@ -18,6 +18,8 @@ struct string_list {
#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0, NULL }
#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1, NULL }
+void string_list_init(struct string_list *list, int strdup_strings);
+
void print_string_list(const struct string_list *p, const char *text);
void string_list_clear(struct string_list *list, int free_util);
--
1.9.0.GIT
--
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 v11 3/4] add `config_set` API for caching config-like files
Currently `git_config()` uses a callback mechanism and file rereads for config values. Due to this approach, it is not uncommon for the config files to be parsed several times during the run of a git program, with different callbacks picking out different variables useful to themselves. Add a `config_set`, that can be used to construct an in-memory cache for config-like files that the caller specifies (i.e., files like `.gitmodules`, `~/.gitconfig` etc.). Add two external functions `git_configset_get_value` and `git_configset_get_value_multi` for querying from the config sets. `git_configset_get_value` follows `last one wins` semantic (i.e. if there are multiple matches for the queried key in the files of the configset the value returned will be the last entry in `value_list`). `git_configset_get_value_multi` returns a list of values sorted in order of increasing priority (i.e. last match will be at the end of the list). Add type specific query functions like `git_configset_get_bool` and similar. Add a default `config_set`, `the_config_set` to cache all key-value pairs read from usual config files (repo specific .git/config, user wide ~/.gitconfig, XDG config and the global /etc/gitconfig). `the_config_set` is populated using `git_config()`. Add two external functions `git_config_get_value` and `git_config_get_value_multi` for querying in a non-callback manner from `the_config_set`. Also, add type specific query functions that are implemented as a thin wrapper around the `config_set` API. Signed-off-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Tanay Abhra tanay...@gmail.com --- Documentation/technical/api-config.txt | 137 + cache.h| 30 config.c | 263 + setup.c| 9 ++ 4 files changed, 439 insertions(+) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..8a86e45 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -77,6 +77,81 @@ To read a specific file in git-config format, use `git_config_from_file`. This takes the same callback and data parameters as `git_config`. +Querying For Specific Variables +--- + +For programs wanting to query for specific variables in a non-callback +manner, the config API provides two functions `git_config_get_value` +and `git_config_get_value_multi`. They both read values from an internal +cache generated previously from reading the config files. + +`int git_config_get_value(const char *key, const char **value)`:: + + Finds the highest-priority value for the configuration variable `key`, + stores the pointer to it in `value` and returns 0. When the + configuration variable `key` is not found, returns 1 without touching + `value`. The caller should not free or modify `value`, as it is owned + by the cache. + +`const struct string_list *git_config_get_value_multi(const char *key)`:: + + Finds and returns the value list, sorted in order of increasing priority + for the configuration variable `key`. When the configuration variable + `key` is not found, returns NULL. The caller should not free or modify + the returned pointer, as it is owned by the cache. + +`void git_config_clear(void)`:: + + Resets and invalidates the config cache. + +The config API also provides type specific API functions which do conversion +as well as retrieval for the queried variable, including: + +`int git_config_get_int(const char *key, int *dest)`:: + + Finds and parses the value to an integer for the configuration variable + `key`. Dies on error; otherwise, stores the value of the parsed integer in + `dest` and returns 0. When the configuration variable `key` is not found, + returns 1 without touching `dest`. + +`int git_config_get_ulong(const char *key, unsigned long *dest)`:: + + Similar to `git_config_get_int` but for unsigned longs. + +`int git_config_get_bool(const char *key, int *dest)`:: + + Finds and parses the value into a boolean value, for the configuration + variable `key` respecting keywords like true and false. Integer + values are converted into true/false values (when they are non-zero or + zero, respectively). Other values cause a die(). If parsing is successful, + stores the value of the parsed result in `dest` and returns 0. When the + configuration variable `key` is not found, returns 1 without touching + `dest`. + +`int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)`:: + + Similar to `git_config_get_bool`, except that integers are copied as-is, + and `is_bool` flag is unset. + +`int git_config_get_maybe_bool(const char *key, int *dest)`:: + + Similar to `git_config_get_bool`, except that it returns -1 on error +
[PATCH v11 4/4] test-config: add tests for the config_set API
Expose the `config_set` C API as a set of simple commands in order to
facilitate testing. Add tests for the `config_set` API as well as for
`git_config_get_*()` family for the usual config files.
Signed-off-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
.gitignore| 1 +
Makefile | 1 +
t/t1308-config-set.sh | 200 ++
test-config.c | 142 +++
4 files changed, 344 insertions(+)
create mode 100755 t/t1308-config-set.sh
create mode 100644 test-config.c
diff --git a/.gitignore b/.gitignore
index 81e12c0..5bfb234 100644
--- a/.gitignore
+++ b/.gitignore
@@ -178,6 +178,7 @@
/gitweb/static/gitweb.min.*
/test-chmtime
/test-ctype
+/test-config
/test-date
/test-delta
/test-dump-cache-tree
diff --git a/Makefile b/Makefile
index 840057c..8d8da72 100644
--- a/Makefile
+++ b/Makefile
@@ -549,6 +549,7 @@ PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
TEST_PROGRAMS_NEED_X += test-chmtime
TEST_PROGRAMS_NEED_X += test-ctype
+TEST_PROGRAMS_NEED_X += test-config
TEST_PROGRAMS_NEED_X += test-date
TEST_PROGRAMS_NEED_X += test-delta
TEST_PROGRAMS_NEED_X += test-dump-cache-tree
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
new file mode 100755
index 000..7fdf840
--- /dev/null
+++ b/t/t1308-config-set.sh
@@ -0,0 +1,200 @@
+#!/bin/sh
+
+test_description='Test git config-set API in different settings'
+
+. ./test-lib.sh
+
+# 'check_config get_* section.key value' verifies that the entry for
+# section.key is 'value'
+check_config () {
+ if test $1 = expect_code
+ then
+ expect_code=$2 shift shift
+ else
+ expect_code=0
+ fi
+ op=$1 key=$2 shift shift
+ if test $# != 0
+ then
+ printf %s\n $@
+ fi expect
+ test_expect_code $expect_code test-config $op $key actual
+ test_cmp expect actual
+}
+
+test_expect_success 'setup default config' '
+ cat .git/config \EOF
+ [case]
+ penguin = very blue
+ Movie = BadPhysics
+ UPPERCASE = true
+ MixedCase = true
+ my =
+ foo
+ baz = sam
+ [Cores]
+ WhatEver = Second
+ baz = bar
+ [cores]
+ baz = bat
+ [CORES]
+ baz = ball
+ [my Foo bAr]
+ hi = mixed-case
+ [my FOO BAR]
+ hi = upper-case
+ [my foo bar]
+ hi = lower-case
+ [case]
+ baz = bat
+ baz = hask
+ [lamb]
+ chop = 65
+ head = none
+ [goat]
+ legs = 4
+ head = true
+ skin = false
+ nose = 1
+ horns
+ EOF
+'
+
+test_expect_success 'get value for a simple key' '
+ check_config get_value case.penguin very blue
+'
+
+test_expect_success 'get value for a key with value as an empty string' '
+ check_config get_value case.my
+'
+
+test_expect_success 'get value for a key with value as NULL' '
+ check_config get_value case.foo (NULL)
+'
+
+test_expect_success 'upper case key' '
+ check_config get_value case.UPPERCASE true
+ check_config get_value case.uppercase true
+'
+
+test_expect_success 'mixed case key' '
+ check_config get_value case.MixedCase true
+ check_config get_value case.MIXEDCASE true
+ check_config get_value case.mixedcase true
+'
+
+test_expect_success 'key and value with mixed case' '
+ check_config get_value case.Movie BadPhysics
+'
+
+test_expect_success 'key with case sensitive subsection' '
+ check_config get_value my.Foo bAr.hi mixed-case
+ check_config get_value my.FOO BAR.hi upper-case
+ check_config get_value my.foo bar.hi lower-case
+'
+
+test_expect_success 'key with case insensitive section header' '
+ check_config get_value cores.baz ball
+ check_config get_value Cores.baz ball
+ check_config get_value CORES.baz ball
+ check_config get_value coreS.baz ball
+'
+
+test_expect_success 'key with case insensitive section header variable' '
+ check_config get_value CORES.BAZ ball
+ check_config get_value cores.baz ball
+ check_config get_value cores.BaZ ball
+ check_config get_value cOreS.bAz ball
+'
+
+test_expect_success 'find value with misspelled key' '
+ check_config expect_code 1 get_value my.fOo Bar.hi Value not found
for \my.fOo Bar.hi\
+'
+
+test_expect_success 'find value with the highest priority' '
+ check_config get_value case.baz hask
+'
+
+test_expect_success 'find integer value for a key' '
+ check_config get_int lamb.chop 65
+'
+
+test_expect_success 'find integer if value is non parse-able' '
+ check_config expect_code 128 get_int lamb.head
+'
+
+test_expect_success
