Re: [PATCH v3] sha1_file: do not add own object directory as alternate
On 15.07.2014 21:26, Junio C Hamano wrote: + strbuf_addstr(objdirbuf, absolute_path(get_object_directory())); + normalize_path_copy(objdirbuf.buf, objdirbuf.buf); This is somewhat a strange usage of a strbuf. There might be a more elegant way, but I tried to mimic the local coding style and simply copied how the alternate paths were normalized. Let me know if you want this re-rolled without strbuf, otherwise I'll leave it as-is. diff --git a/t/t7702-repack-cyclic-alternate.sh b/t/t7702-repack-cyclic-alternate.sh new file mode 100755 index 000..8341d46 --- /dev/null +++ b/t/t7702-repack-cyclic-alternate.sh Do we really have to waste a new test file only for this test? Absolutely not. I'll add it to 7700, which seems appropriate. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] sha1_file: do not add own object directory as alternate
On 15.07.2014 21:48, Junio C Hamano wrote: Ephrim Khong dr.kh...@gmail.com writes: +test_expect_success setup ' + GIT_OBJECT_DIRECTORY=.git//../.git/objects + export GIT_OBJECT_DIRECTORY Do you need this artificially strange environment settings for the problem to manifest itself, or is it sufficient to have a non-canonical pathname in the info/alternates file? The test should check the normalization of both the paths in info/alternates and of GIT_OBJECT_DIRECTORY, so I'd prefer to leave it in. It is not necessary to reproduce the original problem, though. Exporting an environment early in the test and having later tests in the file depend on it makes it harder to debug when things go wrong, than leaving an info/alternates file in the repository, primarily because the latter can be inspected more easily in the trash directory after t7702-*.sh -i dies, hence the above question. That sounds reasonable. The variable is not required at the initialization anyway, so I'll set it right before the actual test and wrap it into a subshell to make debugging easier. -- To unsubscribe from this list: send the line 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] annotate: use argv_array
Simplify the code and get rid of some magic constants by using
argv_array to build the argument list for cmd_blame. Be lazy and let
the OS release our allocated memory, as before.
Signed-off-by: Rene Scharfe l@web.de
---
builtin/annotate.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/builtin/annotate.c b/builtin/annotate.c
index fc43eed..da413ae 100644
--- a/builtin/annotate.c
+++ b/builtin/annotate.c
@@ -5,20 +5,18 @@
*/
#include git-compat-util.h
#include builtin.h
+#include argv-array.h
int cmd_annotate(int argc, const char **argv, const char *prefix)
{
- const char **nargv;
+ struct argv_array args = ARGV_ARRAY_INIT;
int i;
- nargv = xmalloc(sizeof(char *) * (argc + 2));
- nargv[0] = annotate;
- nargv[1] = -c;
+ argv_array_pushl(args, annotate, -c, NULL);
for (i = 1; i argc; i++) {
- nargv[i+1] = argv[i];
+ argv_array_push(args, argv[i]);
}
- nargv[argc + 1] = NULL;
- return cmd_blame(argc + 1, nargv, prefix);
+ return cmd_blame(args.argc, args.argv, prefix);
}
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] fix test suite with mingw-unicode patches
Hello Karsten, thanks for your analysis. Most of the patches you refer to are simply switching off tests for MINGW; let me comment on the remaining ones: * t0110-urlmatch-normalization: 1 Passing binary data on the command line...would have to teach test-urlmatch-normalization.c to read from stdin or file. https://github.com/msysgit/git/commit/be0d6dee Indeed, that would be better solution. For now, I'm going to submit the switch-off patch you mention. * t4036-format-patch-signer-mime: 1 Passing non-ASCII by environment variable, will be fixed by Unicode environment support. Will submit that patch series soon. * t7001-mv: 6 cp -P fails in MinGW - perhaps use the long option forms (--no-dereference)? cp -P fails with our 2001-edition of cp, so msysgit had to revert: https://github.com/msysgit/git/commit/6d3e23d4 But I was ashamed to mention that upstream; and I hope mingwGitDevEnv is going to solve that. * t8001-annotate/t8002-blame: 5 Msys.dll thinks '-L/regex/' is an absolute path and expands to '-LC:/msysgit/regex/'. https://github.com/msysgit/git/commit/2d52168a Nice! But I'm afraid the patch cannot be submitted upstream as it is. I think the hack could be automated by processing options -L* this way: sed 'sX\(^-L\|,\)\^\?/X\\;*Xg' Then it would become only few lines at the top of the script, executed on mingw only. I hope to submit the patch in this form soon. Have a nice day, Stepan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 4/4] cache-tree: Write updated cache-tree after commit
On Tue, Jul 15, 2014 at 11:45 PM, Junio C Hamano gits...@pobox.com wrote: What is the real point of writing into *.lock and renaming? It serves two purposes: (1) everybody adheres to that convention---if we managed to take the lock index.lock, nobody else will compete and interfere with us until we rename it to index. (2) in the meantime, others can still read the old contents in the original index. With take lock, write to a temporary, commit by rename or rollback by delete, we can have the usual transactional semantics. While we are working on it, nobody is allowed to muck with the same file, because everybody pays attention to *.lock. People will not see what we are doing until we release the lock because we are not writing into the primary file. And people can see what we did in its entirety once we are done because we close and rename to commit our changes atomically. True. Think what CLOSE_LOCK adds to that and you would appreciate its usefulness and at the same time realize its limitation. By allowing us to flush what we wrote to the disk without releasing the lock, we can give others (e.g. subprograms we spawn) controlled access to the new contents we prepared before we commit the result to the outside world. The access is controlled because we are in control when we tell these others to peek into or update the temporary file *.lock. The original implementaiton of CLOSE_LOCK is limited in that you can do so only once; you take a lock, you do your thing, you close, you let (one or more) others see it, and you commit (or rollback). You cannot do another of your thing once you close with the original implementation because there was no way to reopen. This is probably where our opinions differ. Yes, if you are sure nobody else is looking at the lock file any more, then you can do whatever you want. And because this is a .lock file, nobody is supposed to look at it unless you tell them too (in contrast $GIT_DIR/index can be read at any time). The format of the index makes it impossible to just edit one byte and be done with it. You always write a full new file. By sticking to transaction-style update, you need no extra code, and you have a back up file as a side effect. What do you gain by your proposal to lock index.lock file? We know we already have index.lock, so nobody should be competing on mucking with its contents with us and we gain nothing by writing into index.lock.lock and renaming it to index.lock. We are in total control of the lifetime of index.lock, when we spawn subprograms on it to let them write to it, when we ourselves write to it, when we spawn subprograms on it to let them read from it, all under the lock we have on the index, i.e. index.lock. The only thing use of another temporary file (that does not have to be a lock on index.lock, by the way, because we have total control over when and who updates the file while we hold the index.lock) would give you is that it allows you to make the success of the do another of your thing step optional. While holding the lock, you close and let add -i work on it, and after it returns, instead of reopening, you write into yet another index.lock.lock, expecting that it might fail and when it fails you can roll it back, leaving the contents add -i left in index.lock intact. If you do not use the extra temporary file, you start from index.lock left by add -i, write the updated index into index.lock and if you fail to write, you have to roll back the entire index---you lose the option to use the index left by add -i without repopulated cache-tree. But in the index update context, I do not think such a complexity is not necessary. If something fails, we should fail and roll back the entire index. I probably look at the problem from a wrong angle. To me the result of commit -p is precious. I'm not a big user of commit -p myself as I prefer add -p but it's the same: it'd be frustrating if after you have carefully added your chunks, the program aborts and you have to start over. And not just a few chunks. Think of reviewing .po files and approve strings by the means of adding them to the index. Perhaps because _I_ as a developer see this cache-tree update step optional and react to it unnecessarily. Ordinary users won't see any difference. And perhaps a better way to save the result of commit/add -p is some sort of index log, not be over-protective at this interactive commit code block. I don't feel strongly either way. So your call. -- 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 2/2] config: use chmod() instead of fchmod()
Am 16.07.2014 07:33, schrieb Johannes Sixt:
Am 16.07.2014 00:54, schrieb Karsten Blees:
There is no fchmod() on native Windows platforms (MinGW and MSVC), and the
equivalent Win32 API (SetFileInformationByHandle) requires Windows Vista.
Use chmod() instead.
Signed-off-by: Karsten Blees bl...@dcon.de
---
config.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/config.c b/config.c
index ba882a1..9767c4b 100644
--- a/config.c
+++ b/config.c
@@ -1636,8 +1636,8 @@ int git_config_set_multivar_in_file(const char
*config_filename,
MAP_PRIVATE, in_fd, 0);
close(in_fd);
-if (fchmod(fd, st.st_mode 0) 0) {
-error(fchmod on %s failed: %s,
+if (chmod(lock-filename, st.st_mode 0) 0) {
+error(chmod on %s failed: %s,
lock-filename, strerror(errno));
ret = CONFIG_NO_WRITE;
goto out_free;
@@ -1815,8 +1815,8 @@ int git_config_rename_section_in_file(const char
*config_filename,
fstat(fileno(config_file), st);
-if (fchmod(out_fd, st.st_mode 0) 0) {
-ret = error(fchmod on %s failed: %s,
+if (chmod(lock-filename, st.st_mode 0) 0) {
+ret = error(chmod on %s failed: %s,
lock-filename, strerror(errno));
goto out;
}
I assume you tested this patch on Windows. I am mildly surprised that
(on Windows) chmod() works on a file that is still open.
-- Hannes
Yes, file attributes can be set independently of open files. In fact, existing
code in git already does that in many places (via adjust_shared_perm(), which
is typically called while the file is open).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff.renameLimit biting/silently ignored in cherry-pick
Hi all, we're running into a problem with the rename detection; we're at num_src=27320 and num_create=46731, which means that 'matrix' would still be enumerable in int32, but... well, I don't yet know where exactly it refuses to perform rename detection. I've tried to set needed_rename_limit to 2^31-2 at the place where it was set to the maximum of num_src and num_create, but that doesn't help. Where could I affect this? Also we have the impression that 'git cherry-pick' silently stops doing rename detection in this situation - it doesn't take nearly long enough to perform it, and it clearly misses renames. I'm trying to do a 'diff --name-status -M' to see whether the rename is properly detected, but I guess the real way would be to make cherry-pick still perform rename detection, and to find out how to do *that*. Cheers, Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line 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 0/3] fix test suite with mingw-unicode patches
Am 16.07.2014 11:29, schrieb Stepan Kasal: * t7001-mv: 6 cp -P fails in MinGW - perhaps use the long option forms (--no-dereference)? cp -P fails with our 2001-edition of cp, so msysgit had to revert: https://github.com/msysgit/git/commit/6d3e23d4 But I was ashamed to mention that upstream; and I hope mingwGitDevEnv is going to solve that. Yes it does. cp in mingwGitDevEnv is from coreutils 5.97 and knows about -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
[PATCH v9r1 1/2] 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 | 264 + 3 files changed, 431 insertions(+) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..fc0e379 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 v9r1 2/2] 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 | 140 +++
4 files changed, 342 insertions(+)
create mode 100755 t/t1308-config-set.sh
create mode 100644 test-config.c
diff --git a/.gitignore b/.gitignore
index 42294e5..7677533 100644
--- a/.gitignore
+++ b/.gitignore
@@ -177,6 +177,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 b92418d..e070eb8 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..cf8f2d7
--- /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
Re: [PATCH v9r1 2/2] test-config: add tests for the config_set API
Tanay Abhra tanay...@gmail.com writes:
+# '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
Broken -chain after shift.
(No time for more review right now, but I'll have time in ~3h from now)
--
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 v9r2 1/2] 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 | 264 + 3 files changed, 431 insertions(+) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..fc0e379 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 v9r2 2/2] 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 | 140 +++
4 files changed, 342 insertions(+)
create mode 100755 t/t1308-config-set.sh
create mode 100644 test-config.c
diff --git a/.gitignore b/.gitignore
index edb1dcf..fcdee42 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 b92418d..e070eb8 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..4752fd9
--- /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
Re: Big repository cannot be reduced
On 07/15/2014 09:43 AM, Woody Wu wrote: I have tried some methods introduced in the network, but always failed. Some big files committed by me to a very old branch then the files deleted and new branches were continuously created. Now the checkout directory has grown to about 80 megabytes. What's the right way to permenently erase those garbage big files? You probably need to use git filter-branch or maybe BFG (http://rtyley.github.io/bfg-repo-cleaner/) to rewrite history as if the big files had never been committed. But beware of the warnings about rewriting history--for example, any collaborators will have to rebase their branches onto the new history. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] rebase -p: Command line option --no-ff is ignored
Hi Marc,
I forgot to cc your mailbox when I posted this patch last week. Do you
still remember whether there was a particular reason why
pick_one_preserving_merges wasn't touched by the commit b499549 (Teach
rebase the --no-ff option.), by any chance?
Kind regards,
Fabian
Fabian Ruch writes:
The --no-ff option instructs git-rebase to always recreate commits as
they are being replayed, even if fast-forwards are possible.
However, if git-rebase is asked to recreate merge commits (via the -p
option), it suddenly ignores the --no-ff option and fast-forwards
both normal and merge commits whenever possible.
git-rebase--interactive, which is responsible for recreating merge
commits during a rebase, maintains a variable fast_forward to decide
whether the current replay should be tried as a fast-forward.
Previously, fast_forward was on by default and would get toggled only
if a parent was rewritten or a squash was in effect. Also turn
fast_forward off if --no-ff is in use, which is signalled by
git-rebase through the variable force_rebase.
If --no-ff is not in use, try to fast-forward HEAD using git-reset as
before. In contrast, if --no-ff is in use, replay normal commits
using git-cherry-pick and merge commits using git-merge. Note that
git-rebase--interactive already provides this machinery for enabling
and disabling fast-forwards, controlled by fast_forward being
assigned either t (for boolean true) or f (for boolean false).
As mentioned above, git-rebase--interactive needs to detect when a
squash is in effect. If several commits are squashed into one, each
of them is picked using the git-cherry-pick option -n and they get
all rewritten to the same commit, the squash commit. Previously,
fast_forward was assigned f if and only if -n was specified. This no
longer holds for fast_forward might be turned off due to a use of
--no-ff. To correctly notice squashes, explicitly check for -n.
Add test.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
Hi,
The code checking force_rebase is copied from pick_one, although
using a ternary operator to initialise fast_forward might be more
readable. Moreover, the code snippet used to detect squash mode is
copied from the f arm of the fast_forward case switch, although the
code base prefers to spell out test(1).
The test recreates a topic branch that merged a second topic branch.
Therefore, the test case tests the recreation of both normal and
merge commits.
Commit b499549 first introduced the --no-ff option to git-rebase and
since then force_rebase seems to respected only by pick_one but not
by its sibling pick_one_preserving_merges. I couldn't find a reason
why. Was pick_one_preserving_merges merely overlooked?
Is it a usability issue that conflicting merges will have to be
resolved again when being replayed now? The same applies to -p and
the replay of merges with rewritten parents. Should the possibly
required resolution be mentioned alongside git-rerere in the
git-rebase manual page?
Fabian
git-rebase--interactive.sh| 3 ++-
t/t3409-rebase-preserve-merges.sh | 12
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f267d8b..264a768 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -266,10 +266,11 @@ pick_one_preserving_merges () {
;;
esac
sha1=$(git rev-parse $sha1)
+ case $force_rebase in '') ;; ?*) fast_forward=f ;; esac
if test -f $state_dir/current-commit
then
- if test $fast_forward = t
+ if [ $1 != -n ]
then
while read current_commit
do
diff --git a/t/t3409-rebase-preserve-merges.sh
b/t/t3409-rebase-preserve-merges.sh
index 8c251c5..838937b 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -81,6 +81,18 @@ test_expect_success 'setup for merge-preserving rebase' \
git commit -a -m Modify B2
'
+test_expect_success '--no-ff records new commits' '
+ (
+ cd clone3
+ test_when_finished 'cd clone3 git checkout topic'
+ git checkout -b recreated-topic
+ # recreate topic with merged topic2 (branching-off point A1)
+ git rebase -p --no-ff HEAD~2
+ test $(git rev-parse new-topic^) != $(git rev-parse topic^)
+ test $(git rev-parse new-topic) != $(git rev-parse topic)
+ )
+'
+
test_expect_success '--continue works after a conflict' '
(
cd clone2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] fixup for patch 2: minor style fix
Signed-off-by: Matthieu Moy matthieu@imag.fr --- t/t1308-config-set.sh | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 4752fd9..ea031bf 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -150,8 +150,8 @@ test_expect_success 'find value from a configset' ' ' test_expect_success 'find value with highest priority from a configset' ' - echo hask expect - test-config configset_get_value case.baz config2 .git/config actual + echo hask expect + test-config configset_get_value case.baz config2 .git/config actual test_cmp expect actual ' @@ -163,7 +163,7 @@ test_expect_success 'find value_list for a key from a configset' ' lama ball EOF - test-config configset_get_value case.baz config2 .git/config actual + test-config configset_get_value case.baz config2 .git/config actual test_cmp expect actual ' @@ -173,7 +173,7 @@ test_expect_success 'proper error on non-existant files' ' test_cmp expect actual ' -test_expect_success 'proper error on non-accessible files' ' +test_expect_success 'proper error on non-accessible files' ' chmod -r .git/config test_when_finished chmod +r .git/config echo Error reading configuration file .git/config. expect @@ -184,14 +184,14 @@ test_expect_success 'proper error on non-accessible files' ' test_expect_success 'proper error on error in default config files' ' cp .git/config .git/config.old test_when_finished mv .git/config.old .git/config - echo [ .git/config + echo [ .git/config echo fatal: bad config file line 35 in .git/config expect test_expect_code 128 test-config get_value foo.bar 2actual test_cmp expect actual ' test_expect_success 'proper error on error in custom config files' ' - echo [ syntax-error + echo [ syntax-error echo fatal: bad config file line 1 in syntax-error expect test_expect_code 128 test-config configset_get_value foo.bar syntax-error 2actual test_cmp expect actual -- 2.0.0.262.gdafc651 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] fixup for patch 2: actually check the return value
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
I won't fight for this, but I think it makes sense.
t/t1308-config-set.sh | 4 ++--
test-config.c | 10 ++
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
index ea031bf..f0307b7 100755
--- a/t/t1308-config-set.sh
+++ b/t/t1308-config-set.sh
@@ -168,7 +168,7 @@ test_expect_success 'find value_list for a key from a
configset' '
'
test_expect_success 'proper error on non-existant files' '
- echo Error reading configuration file non-existant-file. expect
+ echo Error (-1) reading configuration file non-existant-file. expect
test_expect_code 2 test-config configset_get_value foo.bar
non-existant-file 2actual
test_cmp expect actual
'
@@ -176,7 +176,7 @@ test_expect_success 'proper error on non-existant files' '
test_expect_success 'proper error on non-accessible files' '
chmod -r .git/config
test_when_finished chmod +r .git/config
- echo Error reading configuration file .git/config. expect
+ 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
'
diff --git a/test-config.c b/test-config.c
index cad35f4..9dd1b22 100644
--- a/test-config.c
+++ b/test-config.c
@@ -86,8 +86,9 @@ int main(int argc, char **argv)
}
} else if (!strcmp(argv[1], configset_get_value)) {
for (i = 3; i argc; i++) {
- if (git_configset_add_file(cs, argv[i])) {
- fprintf(stderr, Error reading configuration
file %s.\n, argv[i]);
+ int err;
+ if ((err = git_configset_add_file(cs, argv[i]))) {
+ fprintf(stderr, Error (%d) reading
configuration file %s.\n, err, argv[i]);
goto exit2;
}
}
@@ -103,8 +104,9 @@ int main(int argc, char **argv)
}
} else if (!strcmp(argv[1], configset_get_value_multi)) {
for (i = 3; i argc; i++) {
- if (git_configset_add_file(cs, argv[i])) {
- fprintf(stderr, Error reading configuration
file %s.\n, argv[i]);
+ int err;
+ if ((err = git_configset_add_file(cs, argv[i]))) {
+ fprintf(stderr, Error (%d) reading
configuration file %s.\n, err, argv[i]);
goto exit2;
}
}
--
2.0.0.262.gdafc651
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] fixup for patch 1: typo
Signed-off-by: Matthieu Moy matthieu@imag.fr --- Documentation/technical/api-config.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index fc0e379..8a86e45 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -242,7 +242,7 @@ Configset API provides functions for the above mentioned work flow, including: Parses the file and adds the variable-value pairs to the `config_set`, dies if there is an error in parsing the file. Returns 0 on success, or - -1 if the file doesnot exist or is inaccessible. The user has to decide + -1 if the file does not exist or is inaccessible. The user has to decide if he wants to free the incomplete configset or continue using it when the function returns -1. -- 2.0.0.262.gdafc651 -- To unsubscribe from this list: send the line 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 v9r2 1/2] add `config_set` API for caching config-like files
On 7/16/2014 9:36 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
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 | 264
+
3 files changed, 431 insertions(+)
(you broke the patch by removing the ---)
Yikes, sorry about that.
+static void git_config_check_init(void)
+{
+if (the_config_set.hash_initialized)
+return;
+git_configset_init(the_config_set);
+git_config(config_set_callback, the_config_set);
+}
So, you're now ignoring the return value of git_config. What is the
rationale for this? In particular, why did you reject the die
possibility (I understood that you were inclined to take this option, so
I'm curious why you changed your mind).
The errors (non accessible, non existent files etc) were already being caught by
git_config_early(). Since git_config() only returns positive values except
the weird race case you mentioned, I thought the die confused the reader
of the patch more than it provided error checking. I also tried myself
simulating the race condition but failed. All the callers of git_config()
also ignore the return value, so I ended up ignoring the return value myself.
OTOH, you're transmitting the return value without dying here:
+int git_configset_add_file(struct config_set *cs, const char *filename)
+{
+ return git_config_from_file(config_set_callback, filename, cs);
+}
and I think this one is correct, as we cannot tell in advance how
serious an error would be for any callers. And we do test this (I think
we can improve a bit, I'll send a fixup patch).
After reading the commit log that you mentioned and some previous ones before
that I surmised that the official slant was to silently ignore nonexistent
files. Though an access_or_warn() check was placed on most of the files
like git attributes, since non accessible file errors may be a user
configuration
error. So, I decided to ignore the return value.
But I do think that an access_or_warn() check should be put on git config --file
and git_configset_add_file since other parts of git follow it. What do
you think about it, still I will send followup patch correcting the git config
--file condition where it silently ignores the file access error and continues?
--
To unsubscribe from this list: send the line 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/3] fixup for patch 2: actually check the return value
Tanay Abhra tanay...@gmail.com writes: I think it would be unnecessary for the current iteration. Currently git_configset_add_file has only two possible return values -1 or 0. Yes. My point is just to check that statement in tests. (I'm usually wary of statements like is obviously true so I don't need to test it ;-) ) I could add specialized error values for ENOENT or ENOTDIR or EACCES, but the logs show that we silently ignore the first two. I can add an access warn for the third. What do you think? I think the code is fine as it is. But anyway, it's not terribly important. -- 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 v8 4/4] cache-tree: Write updated cache-tree after commit
Duy Nguyen pclo...@gmail.com writes: If you do not use the extra temporary file, you start from index.lock left by add -i, write the updated index into index.lock and if you fail to write, you have to roll back the entire index---you lose the option to use the index left by add -i without repopulated cache-tree. But in the index update context, I do not think such a complexity is not necessary. If something fails, we should fail and roll back the entire index. I probably look at the problem from a wrong angle. To me the result of commit -p is precious. I'm not a big user of commit -p myself as I prefer add -p but it's the same... Oh, we agree that the result of commit -p is precious. But the point of David's series is to change the definition of the precious result to not just commit is made as asked, but now also to include that the index the user will use for continued work will have populated cache-tree. The series thinks both are precious. As you can probably read from my review responses, I am not sold to the idea that spending extra cycles to pre-populate cache-tree is 100% good idea, but if we _were_ to accept that it is a good idea, it logically follows that failing to populate cache-tree is just as a failure as failing to commit. In any case, it is unlikely for writing out the updated index with refreshed cache-tree to fail and blow away the partially built index (we do not even attempt to reopen/update if we cannot prepare in-core cache-tree), so I do not think it is such a big deal either way. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] MinGW: fix compile error due to missing ELOOP
Karsten Blees karsten.bl...@gmail.com writes: MinGW and MSVC before 2010 don't define ELOOP, use EMLINK (aka Too many links) instead. Signed-off-by: Karsten Blees bl...@dcon.de --- Thanks; will apply directly to 'master'. compat/mingw.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compat/mingw.h b/compat/mingw.h index 405c08f..510530c 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -35,6 +35,9 @@ typedef int socklen_t; #ifndef EWOULDBLOCK #define EWOULDBLOCK EAGAIN #endif +#ifndef ELOOP +#define ELOOP EMLINK +#endif #define SHUT_WR SD_SEND #define SIGHUP 1 -- 2.0.1.779.g26aeac4.dirty -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
Keller, Jacob E jacob.e.kel...@intel.com writes:
On Tue, 2014-07-15 at 19:42 -0400, Jeff King wrote:
On Tue, Jul 15, 2014 at 04:32:59PM -0700, Jacob Keller wrote:
+static void error_bad_sort_config(const char *err, va_list params)
+{
+ vreportf(warning: tag.sort has , err, params);
+}
This feels a little like an abuse of the prefix field of vreportf, but
as you probably saw in my for fun patch, doing it right means
formatting into a buffer and then reformatting that (which we're
already doing again in vreportf, but less flexibly). I dunno.
At any rate, this should be marked for translation, no?
-Peff
Oh, yes it probably should. It's definitely a little bit abusive of the
prefix field, but that seemed easier.
And i18n would automatically mean this will not work, no? There is
no guarantee that the translation of warning: will be followed
directly by the translation of tag.sort has without any words from
variable part in all languages.
After all, it seems to me that the one in
http://thread.gmane.org/gmane.comp.version-control.git/253346
struck the right balance among various abuses; let's use the error
reporter from that version, instead of going down this rabbit hole.
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 v1] rebase -p: Command line option --no-ff is ignored
On 14-07-16 12:01 PM, Fabian Ruch wrote: Hi Marc, I forgot to cc your mailbox when I posted this patch last week. Do you still remember whether there was a particular reason why pick_one_preserving_merges wasn't touched by the commit b499549 (Teach rebase the --no-ff option.), by any chance? I think it was simply overlooked. (Though it was very long ago and my brain can barely keep track of what I did last week...) M. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v20 40/48] refs.c: add an err argument to delete_ref_loose
On Tue, Jul 8, 2014 at 7:19 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
Add an err argument to delete_loose_ref so that we can pass a descriptive
error string back to the caller. Pass the err argument from transaction
commit to this function so that transaction users will have a nice error
string if the transaction failed due to delete_loose_ref.
Add a new function unlink_or_err that we can call from delete_ref_loose. This
function is similar to unlink_or_warn except that we can pass it an err
argument. If err is non-NULL the function will populate err instead of
printing a warning().
Simplify warn_if_unremovable.
The change to warn_if_unremovable() is orthogonal to the rest of the
commit and should be a separate commit.
Done.
I split it into three patches.
One patch for the warn_if_removable change
another patch to add unlink_or_msg to wrapper.c
and a final patch to change delete_loose_ref.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 33 -
wrapper.c | 14 ++
2 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/refs.c b/refs.c
index 92a06d4..c7d1f3e 100644
--- a/refs.c
+++ b/refs.c
@@ -2544,16 +2544,38 @@ int repack_without_refs(const char **refnames, int
n, struct strbuf *err)
return ret;
}
-static int delete_ref_loose(struct ref_lock *lock, int flag)
+static int add_err_if_unremovable(const char *op, const char *file,
+ struct strbuf *e, int rc)
This function is only used once. Given also that its purpose is not
that obvious from its signature, it seems to me that the code would be
easier to read if it were inlined.
+{
+ int err = errno;
+ if (rc 0 errno != ENOENT) {
+ strbuf_addf(e, unable to %s %s: %s,
+ op, file, strerror(errno));
+ errno = err;
+ return -1;
+ }
+ return 0;
+}
+
+static int unlink_or_err(const char *file, struct strbuf *err)
The name of this function is misleading; it sounds like it will try to
unlink the file and if not possible call error(). Maybe a name like
unlink_or_report would be less prejudicial.
It might also make sense to move this function to wrapper.c and
implement unlink_or_warn() in terms of it rather than vice versa.
+{
+ if (err)
+ return add_err_if_unremovable(unlink, file, err,
+ unlink(file));
+ else
+ return unlink_or_warn(file);
+}
+
+static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf
*err)
{
if (!(flag REF_ISPACKED) || flag REF_ISSYMREF) {
/* loose */
- int err, i = strlen(lock-lk-filename) - 5; /* .lock */
+ int res, i = strlen(lock-lk-filename) - 5; /* .lock */
lock-lk-filename[i] = 0;
- err = unlink_or_warn(lock-lk-filename);
+ res = unlink_or_err(lock-lk-filename, err);
lock-lk-filename[i] = '.';
- if (err errno != ENOENT)
+ if (res)
return 1;
}
return 0;
@@ -3603,7 +3625,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (update-lock) {
- ret |= delete_ref_loose(update-lock, update-type);
+ ret |= delete_ref_loose(update-lock, update-type,
+ err);
if (!(update-flags REF_ISPRUNING))
delnames[delnum++] = update-lock-ref_name;
}
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;
}
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] rebase--am: use --cherry-pick instead of --ignore-if-in-upstream
When using `git format-patch --ignore-if-in-upstream` we are only
allowed to give a single revision range. In the next commit we will
want to add an additional exclusion revision in order to handle fork
points correctly, so convert `git-rebase--am` to use a symmetric
difference with `--cherry-pick --right-only`.
This does not change the result of the format-patch invocation, just how
we spell the arguments.
Signed-off-by: John Keeping j...@keeping.me.uk
---
Unchanged from v1.
git-rebase--am.sh | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index ca20e1e..902bf2d 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -29,7 +29,13 @@ skip)
;;
esac
-test -n $rebase_root root_flag=--root
+if test -z $rebase_root
+ # this is now equivalent to ! -z $upstream
+then
+ revisions=$upstream...$orig_head
+else
+ revisions=$onto...$orig_head
+fi
ret=0
if test -n $keep_empty
@@ -38,14 +44,15 @@ then
# empty commits and even if it didn't the format doesn't really lend
# itself well to recording empty patches. fortunately, cherry-pick
# makes this easy
- git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} --allow-empty
$revisions
+ git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} --allow-empty \
+ --right-only $revisions
ret=$?
else
rm -f $GIT_DIR/rebased-patches
- git format-patch -k --stdout --full-index --ignore-if-in-upstream \
+ git format-patch -k --stdout --full-index --cherry-pick --right-only \
--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
- $root_flag $revisions $GIT_DIR/rebased-patches
+ $revisions $GIT_DIR/rebased-patches
ret=$?
if test 0 != $ret
--
2.0.1.472.g6f92e5f.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1] rebase --root: sentinel commit cloaks empty commits
git-rebase supports the option `--root` both with and without
`--onto`. In rebase root mode it replays all commits reachable from a
branch on top of another branch, including the very first commit. In
case `--onto` is not specified, that other branch is temporarily
provided by creating an empty commit. When the first commit on the
to-do list is being replayed, the so-called sentinel commit is
amended using the log message and patch of the replayed commit. Since
the sentinel commit is empty, this results in a replacement of the
sentinel commit with the new root commit of the rebased branch.
The combination of `--root` and no `--onto` implies an interactive
rebase. When `--preserve-merges` is not specified on the command
line, git-rebase--interactive uses `--cherry-pick` with git-rev-list
to put the initial to-do list together. The left side is given by the
fake base and the right side by the branch being rebased. What
happens now is that any empty commit on the original branch is
treated as a cherry-pick of the sentinel commit and subsequently
omitted from the to-do list. This is a bug if `--keep-empty` is
specified also.
Even without `--keep-empty`, using the sentinel commit as left side
with git-rev-list can result in a faulty rebased branch. Indeed, in
the unlikely case that the original branch consists solely of empty
commits, the bug crops up in the strangest fashion as all commits are
skipped and the sentinel commit is not replaced. As a result,
git-rebase produces a branch with a single empty commit.
To trigger the replacement of the sentinel commit, git-rebase assigns
the variable `squash_onto`. Special case a second time regarding
`squash_onto` and run git-rev-list without a left side if the
variable is assigned. The latter is the case if and only if `--root`
is used without `--onto`, that is `upstream` points to the sentinel
commit and `$upstream...$orig_head` would subtract a commit that is
not actually there from the original branch.
Fix a typo in `is_empty_commit`. It always found root commits
non-empty so that empty root commits were scheduled even without
`--keep-empty`. The POSIX specification states that command
substitutions are to be executed in sub-shells, which makes exit(1)
and variable assignments not affect the script execution state. That
was the reason why `ptree` was null for parentless commits and the
test `$tree = $ptree` always false for them.
Add tests.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
Hi,
Three test cases were added to the bug report to account for the
additional cases in which the bug strikes (raised by Michael on the
other sub-thread). A bugfix is included now as well.
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`). However, the following two bugs, not related to
empty commits, seem to suggest that git-rebase--interactive cannot
work obliviously to non-existent bases.
1) git-rebase--interactive when used with `--root` and the to-do
list `noop` results in the original branch's history being
rewritten to contain only the sentinel commit.
git-rebase--interactive correctly checkouts `$onto` and replays
no commits on top of it but git-rebase has forgotten that `$onto`
was fake.
2) git-rebase--interactive when used with `--root` always creates a
fresh root commit, regardless of `--no-ff` being specified.
With the current meaning of `squash_onto`,
git-rebase--interactive cannot just reset the branch to the old
root commit. It is really the fault of git-rebase to start off
with a new commit.
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.
Thanks for your time,
Fabian
git-rebase--interactive.sh | 20 ++-
t/t3412-rebase-root.sh | 49 ++
2 files changed, 64 insertions(+), 5 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f267d8b..71ca0f0 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -201,10 +201,10 @@ has_action () {
}
is_empty_commit() {
- tree=$(git rev-parse -q --verify $1^{tree} 2/dev/null ||
- die $1: not a commit that can be picked)
-
Re: [PATCH v8 1/4] usage: make error functions a stack
There was no way to get the current error routine now, and I figured that a stack was a simple way of saving the old routine. Essentially these two paths would be the same as a save/restore except we manage it via a stack. I don't really see how that would end up any different. I mean I don't mind either approach. Thanks, Jake On Tue, 2014-07-15 at 17:49 -0700, Junio C Hamano wrote: I actually am not a big fan of stack for a thing like this, to be honest. Wouldn't it be sufficient for the callers who want specific behaviour from its callees to - save away the current error/warning routines; - set error/warning routines to its own custom versions; - call the callees; - set error/warning routines back to their original; and - return from it at least in the code paths under discussion? On Tue, Jul 15, 2014 at 4:26 PM, Keller, Jacob E jacob.e.kel...@intel.com wrote: On Tue, 2014-07-15 at 15:47 -0700, Junio C Hamano wrote: Jacob Keller jacob.e.kel...@intel.com writes: extern void set_error_routine(void (*routine)(const char *err, va_list params)); +extern void pop_error_routine(void); pop that undoes set smells somewhat weird. Perhaps we should rename set to push? That would allow us catch possible topics that add new calls to set_error_routine() as well by forcing the system not to link when they are merged without necessary fixes. Also curious what your thoughts on making every set_*_routine to be a stack? For now I was only planning on error but it maybe makes sense to change them all? Thanks, Jake
Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
On Wed, 2014-07-16 at 10:59 -0700, Junio C Hamano wrote:
Keller, Jacob E jacob.e.kel...@intel.com writes:
On Tue, 2014-07-15 at 19:42 -0400, Jeff King wrote:
On Tue, Jul 15, 2014 at 04:32:59PM -0700, Jacob Keller wrote:
+static void error_bad_sort_config(const char *err, va_list params)
+{
+vreportf(warning: tag.sort has , err, params);
+}
This feels a little like an abuse of the prefix field of vreportf, but
as you probably saw in my for fun patch, doing it right means
formatting into a buffer and then reformatting that (which we're
already doing again in vreportf, but less flexibly). I dunno.
At any rate, this should be marked for translation, no?
-Peff
Oh, yes it probably should. It's definitely a little bit abusive of the
prefix field, but that seemed easier.
And i18n would automatically mean this will not work, no? There is
no guarantee that the translation of warning: will be followed
directly by the translation of tag.sort has without any words from
variable part in all languages.
After all, it seems to me that the one in
http://thread.gmane.org/gmane.comp.version-control.git/253346
struck the right balance among various abuses; let's use the error
reporter from that version, instead of going down this rabbit hole.
Thanks.
This is fine with me :)
Thanks,
Jake
Re: [PATCH v2 2/2] rebase: omit patch-identical commits with --fork-point
John Keeping j...@keeping.me.uk writes:
When the `--fork-point` argument was added to `git rebase`, we changed
the value of $upstream to be the fork point instead of the point from
which we want to rebase. When $orig_head..$upstream is empty this does
not change the behaviour, but when there are new changes in the upstream
we are no longer checking if any of them are patch-identical with
changes in $upstream..$orig_head.
Fix this by introducing a new variable to hold the fork point and using
this to restrict the range as an extra (negative) revision argument so
that the set of desired revisions becomes (in fork-point mode):
git rev-list --cherry-pick --right-only \
$upstream...$orig_head ^$fork_point
This allows us to correctly handle the scenario where we have the
following topology:
C --- D --- E - dev
/
B - master@{1}
/
o --- B' --- C* --- D* - master
where:
- B' is a fixed-up version of B that is not patch-identical with B;
- C* and D* are patch-identical to C and D respectively and conflict
textually if applied in the wrong order;
- E depends textually on D.
The correct result of `git rebase master dev` is that B is identified as
the fork-point of dev and master, so that C, D, E are the commits that
need to be replayed onto master; but C and D are patch-identical with C*
and D* and so can be dropped, so that the end result is:
o --- B' --- C* --- D* --- E - dev
If the fork-point is not identified, then picking B onto a branch
containing B' results in a conflict and if the patch-identical commits
are not correctly identified then picking C onto a branch containing D
(or equivalently D*) results in a conflict.
This change allows us to handle both of these cases, where previously we
either identified the fork-point (with `--fork-point`) but not the
patch-identical commits *or* (with `--no-fork-point`) identified the
patch-identical commits but not the fact that master had been rewritten.
Reported-by: Ted Felix t...@tedfelix.com
Signed-off-by: John Keeping j...@keeping.me.uk
---
Change from v1:
- add a test case
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 80e0a95..47b5682 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -169,6 +169,29 @@ test_expect_success 'default to common base in
@{upstream}s reflog if no upstrea
test_cmp expect actual
'
+test_expect_success 'cherry-picked commits and fork-point work together' '
+ git checkout default-base
+ echo Amended A
+ git commit -a --no-edit --amend
+ test_commit B B
+ test_commit new_B B New B
+ test_commit C C
+ git checkout default
+ git reset --hard default-base@{4}
+ test_commit D D
+ git cherry-pick -2 default-base^
+ test_commit final_B B Final B
+ git rebase
mental note: this rebases default (i.e. the current branch) on
default-base; it depends on branch.default.{remote,merge} being set
by the previous test piece.
+ echo Amended expect
+ test_cmp A expect
+ echo Final B expect
+ test_cmp B expect
+ echo C expect
+ test_cmp C expect
+ echo D expect
+ test_cmp D expect
+'
Thanks. Do these labels on the commits have any relation to the
topology illustrated in the log message?
It looks like the above produces this:
a'---D---B'--new_B'---final_B (default)
/
oa---B---new_B---C (default-base)
\
D''---final_B''
where 'a' is Modify A. from the original set-up and B and new_B
are the cherry-picks to be filtered out during the rebase. Am I
reading the test correctly?
test_expect_success 'rebase -q is quiet' '
git checkout -b quiet topic
git rebase -q master output.out 21
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
On Wed, 2014-07-16 at 10:59 -0700, Junio C Hamano wrote:
Keller, Jacob E jacob.e.kel...@intel.com writes:
On Tue, 2014-07-15 at 19:42 -0400, Jeff King wrote:
On Tue, Jul 15, 2014 at 04:32:59PM -0700, Jacob Keller wrote:
+static void error_bad_sort_config(const char *err, va_list params)
+{
+vreportf(warning: tag.sort has , err, params);
+}
This feels a little like an abuse of the prefix field of vreportf, but
as you probably saw in my for fun patch, doing it right means
formatting into a buffer and then reformatting that (which we're
already doing again in vreportf, but less flexibly). I dunno.
At any rate, this should be marked for translation, no?
-Peff
Oh, yes it probably should. It's definitely a little bit abusive of the
prefix field, but that seemed easier.
And i18n would automatically mean this will not work, no? There is
no guarantee that the translation of warning: will be followed
directly by the translation of tag.sort has without any words from
variable part in all languages.
After all, it seems to me that the one in
http://thread.gmane.org/gmane.comp.version-control.git/253346
struck the right balance among various abuses; let's use the error
reporter from that version, instead of going down this rabbit hole.
Thanks.
So is that patch series version acceptable then? Or should I spin one
again that is in that vein?
Thanks,
Jake
Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
Keller, Jacob E jacob.e.kel...@intel.com writes: After all, it seems to me that the one in http://thread.gmane.org/gmane.comp.version-control.git/253346 struck the right balance among various abuses; let's use the error reporter from that version, instead of going down this rabbit hole. Thanks. So is that patch series version acceptable then? Or should I spin one again that is in that vein? I do not offhand know what other changes you had in v9 since $gmane/253346, so I'll leave it up to you. If the only difference is the error reporting codepath, and you are happy with what I have in my tree $ git log -p --reverse -3 a93d886b9 then we can proceed with that version. Have there been any issues raised on that older version other than error reporting? -- To unsubscribe from this list: send the line 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 2/2] rebase: omit patch-identical commits with --fork-point
On 07/16/2014 03:23 PM, John Keeping wrote: Change from v1: - add a test case Test case is working fine for me. It passes with the patch and fails without. However, it does seem to cause all the rest of the test cases to fail if it fails. Is there some cleanup missing? Ted. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
On Wed, 2014-07-16 at 14:40 -0700, Junio C Hamano wrote: Keller, Jacob E jacob.e.kel...@intel.com writes: After all, it seems to me that the one in http://thread.gmane.org/gmane.comp.version-control.git/253346 struck the right balance among various abuses; let's use the error reporter from that version, instead of going down this rabbit hole. Thanks. So is that patch series version acceptable then? Or should I spin one again that is in that vein? I do not offhand know what other changes you had in v9 since $gmane/253346, so I'll leave it up to you. If the only difference is the error reporting codepath, and you are happy with what I have in my tree $ git log -p --reverse -3 a93d886b9 then we can proceed with that version. Have there been any issues raised on that older version other than error reporting? I'll double check. Thanks, Jake
[PATCH v10] tag: support configuring --sort via .gitconfig
Add support for configuring default sort ordering for git tags. Command
line option will override this configured value, using the exact same
syntax.
Cc: Jeff King p...@peff.net
Signed-off-by: Jacob Keller jacob.e.kel...@intel.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Based on what's in Junio's tree, this patch includes some minor changes to fix
up other non-error reporting changes included in previous versions. The other
patches in his tree already match so we can leave them as they are.
Documentation/config.txt | 5
Documentation/git-tag.txt | 5 +++-
builtin/tag.c | 68 ++-
t/t7004-tag.sh| 36 +
4 files changed, 95 insertions(+), 19 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9f467d3820a4..9d4f249606b3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2351,6 +2351,11 @@ submodule.name.ignore::
--ignore-submodules option. The 'git submodule' commands are not
affected by this setting.
+tag.sort::
+ This variable controls the sort ordering of tags when displayed by
+ linkgit:git-tag[1]. Without the --sort=value option provided, the
+ value of this variable will be used as the default.
+
tar.umask::
This variable can be used to restrict the permission bits of
tar archive entries. The default is 0002, which turns off the
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index b424a1bc48bb..320908369f06 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -99,7 +99,9 @@ OPTIONS
Sort in a specific order. Supported type is refname
(lexicographic order), version:refname or v:refname (tag
names are treated as versions). Prepend - to reverse sort
- order.
+ order. When this option is not given, the sort order defaults to the
+ value configured for the 'tag.sort' variable if it exists, or
+ lexicographic order otherwise. See linkgit:git-config[1].
--column[=options]::
--no-column::
@@ -317,6 +319,7 @@ include::date-formats.txt[]
SEE ALSO
linkgit:git-check-ref-format[1].
+linkgit:git-config[1].
GIT
---
diff --git a/builtin/tag.c b/builtin/tag.c
index 1101c19596c5..e063035515d9 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -32,6 +32,8 @@ static const char * const git_tag_usage[] = {
#define SORT_MASK 0x7fff
#define REVERSE_SORT0x8000
+static int tag_sort;
+
struct tag_filter {
const char **patterns;
int lines;
@@ -346,9 +348,51 @@ static const char tag_template_nocleanup[] =
Lines starting with '%c' will be kept; you may remove them
yourself if you want to.\n);
+/*
+ * Parse a sort string, and return 0 if parsed successfully. Will return
+ * non-zero when the sort string does not parse into a known type. If var is
+ * given, the error message becomes a warning and includes information about
+ * the configuration value.
+ */
+static int parse_sort_string(const char *var, const char *arg, int *sort)
+{
+ int type = 0, flags = 0;
+
+ if (skip_prefix(arg, -, arg))
+ flags |= REVERSE_SORT;
+
+ if (skip_prefix(arg, version:, arg) || skip_prefix(arg, v:, arg))
+ type = VERCMP_SORT;
+ else
+ type = STRCMP_SORT;
+
+ if (strcmp(arg, refname)) {
+ if (!var)
+ return error(_(unsupported sort specification '%s'),
arg);
+ else {
+ warning(_(unsupported sort specification '%s' in
variable '%s'),
+ var, arg);
+ return -1;
+ }
+ }
+
+ *sort = (type | flags);
+
+ return 0;
+}
+
static int git_tag_config(const char *var, const char *value, void *cb)
{
- int status = git_gpg_config(var, value, cb);
+ int status;
+
+ if (!strcmp(var, tag.sort)) {
+ if (!value)
+ return config_error_nonbool(var);
+ parse_sort_string(var, value, tag_sort);
+ return 0;
+ }
+
+ status = git_gpg_config(var, value, cb);
if (status)
return status;
if (starts_with(var, column.))
@@ -522,20 +566,8 @@ static int parse_opt_points_at(const struct option *opt
__attribute__((unused)),
static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
{
int *sort = opt-value;
- int flags = 0;
- if (skip_prefix(arg, -, arg))
- flags |= REVERSE_SORT;
-
- if (skip_prefix(arg, version:, arg) || skip_prefix(arg, v:, arg))
- *sort = VERCMP_SORT;
- else
- *sort = STRCMP_SORT;
-
- if (strcmp(arg, refname))
- die(_(unsupported sort specification %s), arg);
- *sort |= flags;
- return 0;
+ return
[PATCH] run-command: use internal argv_array of struct child_process in run_hook_ve()
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
---
run-command.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/run-command.c b/run-command.c
index be07d4a..576dfea 100644
--- a/run-command.c
+++ b/run-command.c
@@ -770,28 +770,21 @@ char *find_hook(const char *name)
int run_hook_ve(const char *const *env, const char *name, va_list args)
{
struct child_process hook;
- struct argv_array argv = ARGV_ARRAY_INIT;
const char *p;
- int ret;
p = find_hook(name);
if (!p)
return 0;
- argv_array_push(argv, p);
-
- while ((p = va_arg(args, const char *)))
- argv_array_push(argv, p);
-
memset(hook, 0, sizeof(hook));
- hook.argv = argv.argv;
+ argv_array_push(hook.args, p);
+ while ((p = va_arg(args, const char *)))
+ argv_array_push(hook.args, p);
hook.env = env;
hook.no_stdin = 1;
hook.stdout_to_stderr = 1;
- ret = run_command(hook);
- argv_array_clear(argv);
- return ret;
+ return run_command(hook);
}
int run_hook_le(const char *const *env, const char *name, ...)
--
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
What's cooking in git.git (Jul 2014, #03; Wed, 16)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. We would need to start slowing down to prepare for -rc0 preview at the end of this week and then feature freeze. Some topics that joined 'next' late may want to stay there for the remainder of this cycle. Many of the accumulated fixes have been flushed to 'maint' and Git 2.0.2 has been tagged. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * ek/alt-odb-entry-fix (2014-07-15) 1 commit - sha1_file: do not add own object directory as alternate Will merge to 'next'. * jk/rebase-am-fork-point (2014-07-16) 2 commits - rebase: omit patch-identical commits with --fork-point - rebase--am: use --cherry-pick instead of --ignore-if-in-upstream Will merge to 'next'. * jk/stable-prio-queue (2014-07-15) 4 commits - t5539: update a flaky test - paint_down_to_common: use prio_queue - prio-queue: make output stable with respect to insertion - prio-queue: factor out compare and swap operations * jk/tag-sort (2014-07-13) 2 commits - tag: support configuring --sort via .gitconfig - tag: fix --sort tests to use cat-\EOF format v10 ($gmane/253695) needs to be picked up and replace these. * nd/path-max-must-go (2014-07-14) 3 commits (merged to 'next' on 2014-07-15 at ce68dde) + prep_exclude: remove the artificial PATH_MAX limit + dir.h: move struct exclude declaration to top level + dir.c: coding style fix Will merge to 'master'. * sk/mingw-uni-fix (2014-07-15) 3 commits - tests: do not pass iso8859-1 encoded parameter - Win32: Unicode file name support (dirent) - Win32: Unicode file name support (except dirent) Will merge to 'next'. * ta/config-set (2014-07-15) 2 commits - test-config: add tests for the config_set API - add `config_set` API for caching config-like files Still being discussed. * kb/avoid-fchmod-for-now (2014-07-16) 1 commit - config: use chmod() instead of fchmod() Replaces the only two uses of fchmod() with chmod() because the former does not work on Windows port and because luckily we can. * rs/ref-transaction-1 (2014-07-16) 20 commits - refs.c: make delete_ref use a transaction - refs.c: make prune_ref use a transaction to delete the ref - refs.c: remove lock_ref_sha1 - refs.c: remove the update_ref_write function - refs.c: remove the update_ref_lock function - refs.c: make lock_ref_sha1 static - walker.c: use ref transaction for ref updates - fast-import.c: use a ref transaction when dumping tags - receive-pack.c: use a reference transaction for updating the refs - refs.c: change update_ref to use a transaction - branch.c: use ref transaction for all ref updates - fast-import.c: change update_branch to use ref transactions - sequencer.c: use ref transactions for all ref updates - commit.c: use ref transactions for updates - replace.c: use the ref transaction functions for updates - tag.c: use ref transactions when doing updates - refs.c: add transaction.status and track OPEN/CLOSED/ERROR - refs.c: make ref_transaction_begin take an err argument - refs.c: update ref_transaction_delete to check for error and return status - refs.c: change ref_transaction_create to do error checking and return status (this branch is used by rs/ref-transaction; uses rs/ref-transaction-0.) The second batch of the transactional ref update series. * rs/unify-is-branch (2014-07-16) 1 commit - refs.c: add a public is_branch function Will merge to 'next'. -- [Graduated to master] * ah/fix-http-push (2014-07-13) 1 commit (merged to 'next' on 2014-07-14 at 5d06516) + http-push.c: make CURLOPT_IOCTLDATA a usable pointer An ancient rewrite passed a wrong pointer to a curl library function in a rarely used code path. * cb/filter-branch-prune-empty-degenerate-merges (2014-07-01) 1 commit (merged to 'next' on 2014-07-10 at 860cfea) + filter-branch: eliminate duplicate mapped parents filter-branch left an empty single-parent commit that results when all parents of a merge commit gets mapped to the same commit, even under --prune-empty. * cc/replace-edit (2014-06-25) 3 commits (merged to 'next' on 2014-07-10 at 097cd5e) + replace: use argv_array in export_object + avoid double close of descriptors handed to run_command + replace: replace spaces with tabs in indentation (this branch is used by jk/replace-edit-raw.) Teach git replace an --edit mode. * ep/submodule-code-cleanup (2014-06-30) 1 commit (merged to 'next' on 2014-07-10 at d4de30a) + submodule.c: use the ARRAY_SIZE macro * jk/replace-edit-raw (2014-06-25) 1 commit (merged to 'next' on 2014-07-10 at b934bb0) + replace: add a --raw mode for --edit (this branch uses cc/replace-edit.) Teach
[ANNOUNCE] Git v2.0.2
The latest maintenance release Git v2.0.2 is now available at the usual places. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/ The following public repositories all have a copy of the 'v2.0.2' tag and the 'maint' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v2.0.2 Release Notes * Documentation for git submodule sync forgot to say that the subcommand can take the --recursive option. * Mishandling of patterns in .gitignore that has trailing SPs quoted with backslashes (e.g. ones that end with \ ) have been corrected. * Recent updates to git repack started to duplicate objects that are in packfiles marked with .keep flag into the new packfile by mistake. * git clone -b brefs/tags/bar would have mistakenly thought we were following a single tag, even though it was a name of the branch, because it incorrectly used strstr(). * %G (nothing after G) is an invalid pretty format specifier, but the parser did not notice it as garbage. * Code to avoid adding the same alternate object store twice was subtly broken for a long time, but nobody seems to have noticed. * A handful of code paths had to read the commit object more than once when showing header fields that are usually not parsed. The internal data structure to keep track of the contents of the commit object has been updated to reduce the need for this double-reading, and to allow the caller find the length of the object. * During git rebase --merge, a conflicted patch could not be skipped with --skip if the next one also conflicted. Changes since v2.0.1 are as follows: Jeff King (27): repack: do not accidentally pack kept objects by default repack: respect pack.writebitmaps repack: s/write_bitmap/s/ in code commit_tree: take a pointer/len pair rather than a const strbuf replace dangerous uses of strbuf_attach alloc: include any-object allocations in alloc_report commit: push commit_index update into alloc_commit_node do not create struct commit with xcalloc logmsg_reencode: return const buffer sequencer: use logmsg_reencode in get_message provide a helper to free commit buffer provide a helper to set the commit buffer provide helpers to access the commit buffer use get_cached_commit_buffer where appropriate use get_commit_buffer to avoid duplicate code convert logmsg_reencode to get_commit_buffer use get_commit_buffer everywhere commit-slab: provide a static initializer commit: convert commit-buffer to a slab commit: record buffer length in cache reuse cached commit buffer when parsing signatures t7510: stop referring to master in later tests t7510: test a commit signed by an unknown key t7510: check %G* pretty-format output pretty: avoid reading past end-of-string with %G move %G format test from t7510 to t6006 t7300: repair filesystem permissions with test_when_finished Junio C Hamano (4): t0008: do not depend on 'echo' handling backslashes specially builtin/clone.c: detect a clone starting at a tag correctly Start preparing for 2.0.2 Git 2.0.2 Matthew Chen (1): submodule: document sync --recursive Michael J Gruber (1): t7510: use consistent -chains in loop Pasha Bolokhov (1): dir.c:trim_trailing_spaces(): fix for \ sequence René Scharfe (2): sha1_file: avoid overrunning alternate object base string annotate: use argv_array Ronnie Sahlberg (1): enums: remove trailing ',' after last item in enum brian m. carlson (1): rebase--merge: fix --skip with two conflicts in a row -- To unsubscribe from this list: send the line 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/2] config: use chmod() instead of fchmod()
Karsten Blees karsten.bl...@gmail.com writes:
There is no fchmod() on native Windows platforms (MinGW and MSVC), and the
equivalent Win32 API (SetFileInformationByHandle) requires Windows Vista.
Use chmod() instead.
Signed-off-by: Karsten Blees bl...@dcon.de
---
I am wondering if it is saner to just revert the fchmod() patch and
replace it with something along the lines of
http://thread.gmane.org/gmane.comp.version-control.git/251682/focus=253219
Having said that, these are the only two callers of fchmod()
currently in our code base, so I'll queue this patch to allow us to
kick the problem-can down the road ;-)
Thanks.
config.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/config.c b/config.c
index ba882a1..9767c4b 100644
--- a/config.c
+++ b/config.c
@@ -1636,8 +1636,8 @@ int git_config_set_multivar_in_file(const char
*config_filename,
MAP_PRIVATE, in_fd, 0);
close(in_fd);
- if (fchmod(fd, st.st_mode 0) 0) {
- error(fchmod on %s failed: %s,
+ if (chmod(lock-filename, st.st_mode 0) 0) {
+ error(chmod on %s failed: %s,
lock-filename, strerror(errno));
ret = CONFIG_NO_WRITE;
goto out_free;
@@ -1815,8 +1815,8 @@ int git_config_rename_section_in_file(const char
*config_filename,
fstat(fileno(config_file), st);
- if (fchmod(out_fd, st.st_mode 0) 0) {
- ret = error(fchmod on %s failed: %s,
+ if (chmod(lock-filename, st.st_mode 0) 0) {
+ ret = error(chmod on %s failed: %s,
lock-filename, strerror(errno));
goto out;
}
--
2.0.1.779.g26aeac4.dirty
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/20] ref transactions part 2
Ronnie Sahlberg sahlb...@google.com writes: On Tue, Jul 15, 2014 at 4:33 PM, Ronnie Sahlberg sahlb...@google.com wrote: This is the next 20 patches from my originally big patch series and follow the previous 19 patches that is now in juns tree. These patches were numbered 20-39 in the original 48-patch series. Changes since these patches were in the original series: - Addressing concerns from mhagger's review One patch in the series did not apply cleanly on top of the tip of the previous series (now queued as rs/ref-transaction-0) and I had to wiggle it. Please check the result (queued as three topics, this one is rs/ref-transaction-1 which is built on the abovementioned -0, and the remainder from the previous round is rebased on -1 as rs/ref-transaction), all of which are queued on 'jch' (which is part of 'pu'). 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 12/12] refs.c: fix handling of badly named refs
We currently do not handle badly named refs well :
$ cp .git/refs/heads/master .git/refs/heads/master.@\*@\\.
$ git branch
fatal: Reference has invalid format: 'refs/heads/master.@*@\.'
$ git branch -D master.@\*@\\.
error: branch 'master.@*@\.' not found.
But we can not really recover from a badly named ref with less than
manually deleting the .git/refs/heads/refname file.
Change resolve_ref_unsafe to take a flags field instead of a 'reading'
boolean and update all callers that used a non-zero value for reading
to pass the flag RESOLVE_REF_READING instead.
Add another flag RESOLVE_REF_ALLOW_BAD_NAME that will make
resolve_ref_unsafe skip checking the refname for sanity and use this
from branch.c so that we will be able to call resolve_ref_unsafe on such
refs when trying to delete it.
Add checks for refname sanity when updating (not deleting) a ref in
ref_transaction_update and in ref_transaction_create to make the transaction
fail if an attempt is made to create/update a badly named ref.
Since all ref changes will later go through the transaction layer this means
we no longer need to check for and fail for bad refnames in
lock_ref_sha1_basic.
Change lock_ref_sha1_basic to not fail for bad refnames. Just check the
refname, and print an error, and remember that the refname is bad so that
we can skip calling verify_lock().
Allow reading refs with bad names from loose refs and packed refs
but flag them as broken. This means that the refs will at least show up in
git branch even if their name is invalid/broken.
Since we now do the refname checks, when we need to, before the call
to create_ref_entry we can remove the check_name argument to the function.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/blame.c | 2 +-
builtin/branch.c| 6 ++-
builtin/clone.c | 2 +-
builtin/fmt-merge-msg.c | 2 +-
builtin/for-each-ref.c | 6 ++-
builtin/log.c | 3 +-
builtin/remote.c| 5 +-
builtin/show-branch.c | 6 ++-
bundle.c| 2 +-
cache.h | 18 ---
http-backend.c | 3 +-
reflog-walk.c | 3 +-
refs.c | 126 ++--
remote.c| 6 ++-
sequencer.c | 2 +-
transport-helper.c | 2 +-
transport.c | 5 +-
17 files changed, 123 insertions(+), 76 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 662e3fe..76340e2 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2278,7 +2278,7 @@ static struct commit *fake_working_tree_commit(struct
diff_options *opt,
commit-object.type = OBJ_COMMIT;
parent_tail = commit-parents;
- if (!resolve_ref_unsafe(HEAD, head_sha1, 1, NULL))
+ if (!resolve_ref_unsafe(HEAD, head_sha1, RESOLVE_REF_READING, NULL))
die(no such ref: HEAD);
parent_tail = append_parent(parent_tail, head_sha1);
diff --git a/builtin/branch.c b/builtin/branch.c
index 652b1d2..5c95656 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -129,7 +129,8 @@ static int branch_merged(int kind, const char *name,
branch-merge[0]
branch-merge[0]-dst
(reference_name = reference_name_to_free =
-resolve_refdup(branch-merge[0]-dst, sha1, 1, NULL)) !=
NULL)
+resolve_refdup(branch-merge[0]-dst, sha1,
+ RESOLVE_REF_READING, NULL)) != NULL)
reference_rev = lookup_commit_reference(sha1);
}
if (!reference_rev)
@@ -233,7 +234,8 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
free(name);
name = mkpathdup(fmt, bname.buf);
- target = resolve_ref_unsafe(name, sha1, 0, flags);
+ target = resolve_ref_unsafe(name, sha1,
+ RESOLVE_REF_ALLOW_BAD_NAME, flags);
if (!target ||
(!(flags REF_ISSYMREF) is_null_sha1(sha1))) {
error(remote_branch
diff --git a/builtin/clone.c b/builtin/clone.c
index b12989d..f7307e6 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -622,7 +622,7 @@ static int checkout(void)
if (option_no_checkout)
return 0;
- head = resolve_refdup(HEAD, sha1, 1, NULL);
+ head = resolve_refdup(HEAD, sha1, RESOLVE_REF_READING, NULL);
if (!head) {
warning(_(remote HEAD refers to nonexistent ref,
unable to checkout.\n));
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 3906eda..d8ab177 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -602,7 +602,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
/* get current branch */
current_branch = current_branch_to_free =
-
[PATCH 08/12] refs.c: pass a skip list to name_conflict_fn
Allow passing a list of refs to skip checking to name_conflict_fn.
There are some conditions where we want to allow a temporary conflict and skip
checking those refs. For example if we have a transaction that
1, guarantees that m is a packed refs and there is no loose ref for m
2, the transaction will delete m from the packed ref
3, the transaction will create conflicting m/m
For this case we want to be able to lock and create m/m since we know that the
conflict is only transient. I.e. the conflict will be automatically resolved
by the transaction when it deletes m.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 41 ++---
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/refs.c b/refs.c
index d3fedbb..a115478 100644
--- a/refs.c
+++ b/refs.c
@@ -801,15 +801,18 @@ static int names_conflict(const char *refname1, const
char *refname2)
struct name_conflict_cb {
const char *refname;
- const char *oldrefname;
const char *conflicting_refname;
+ const char **skip;
+ int skipnum;
};
static int name_conflict_fn(struct ref_entry *entry, void *cb_data)
{
struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
- if (data-oldrefname !strcmp(data-oldrefname, entry-name))
- return 0;
+ int i;
+ for (i = 0; i data-skipnum; i++)
+ if (!strcmp(entry-name, data-skip[i]))
+ return 0;
if (names_conflict(data-refname, entry-name)) {
data-conflicting_refname = entry-name;
return 1;
@@ -822,15 +825,18 @@ static int name_conflict_fn(struct ref_entry *entry, void
*cb_data)
* conflicting with the name of an existing reference in dir. If
* oldrefname is non-NULL, ignore potential conflicts with oldrefname
* (e.g., because oldrefname is scheduled for deletion in the same
- * operation).
+ * operation). skip contains a list of refs we want to skip checking for
+ * conflicts with.
*/
-static int is_refname_available(const char *refname, const char *oldrefname,
- struct ref_dir *dir)
+static int is_refname_available(const char *refname,
+ struct ref_dir *dir,
+ const char **skip, int skipnum)
{
struct name_conflict_cb data;
data.refname = refname;
- data.oldrefname = oldrefname;
data.conflicting_refname = NULL;
+ data.skip = skip;
+ data.skipnum = skipnum;
sort_ref_dir(dir);
if (do_for_each_entry_in_dir(dir, 0, name_conflict_fn, data)) {
@@ -2077,7 +2083,8 @@ int dwim_log(const char *str, int len, unsigned char
*sha1, char **log)
/* This function should make sure errno is meaningful on error */
static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
- int flags, int *type_p)
+ int flags, int *type_p,
+ const char **skip, int skipnum)
{
char *ref_file;
const char *orig_refname = refname;
@@ -2126,7 +2133,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
* name is a proper prefix of our refname.
*/
if (missing
-!is_refname_available(refname, NULL, get_packed_refs(ref_cache)))
{
+!is_refname_available(refname, get_packed_refs(ref_cache),
+ skip, skipnum)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -2184,7 +2192,7 @@ struct ref_lock *lock_any_ref_for_update(const char
*refname,
const unsigned char *old_sha1,
int flags, int *type_p)
{
- return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
+ return lock_ref_sha1_basic(refname, old_sha1, flags, type_p, NULL, 0);
}
/*
@@ -2654,10 +2662,12 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
if (!symref)
return error(refname %s not found, oldrefname);
- if (!is_refname_available(newrefname, oldrefname,
get_packed_refs(ref_cache)))
+ if (!is_refname_available(newrefname, get_packed_refs(ref_cache),
+ oldrefname, 1))
return 1;
- if (!is_refname_available(newrefname, oldrefname,
get_loose_refs(ref_cache)))
+ if (!is_refname_available(newrefname, get_loose_refs(ref_cache),
+ oldrefname, 1))
return 1;
if (log rename(git_path(logs/%s, oldrefname),
git_path(TMP_RENAMED_LOG)))
@@ -2687,7 +2697,7 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
logmoved = log;
- lock =
[PATCH 01/12] wrapper.c: simplify warn_if_unremovable
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;
}
--
2.0.1.527.gc6b782e
--
To unsubscribe from this list: send the line 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 03/12] refs.c: add an err argument to delete_ref_loose
Add an err argument to delete_loose_ref so that we can pass a descriptive
error string back to the caller. Pass the err argument from transaction
commit to this function so that transaction users will have a nice error
string if the transaction failed due to delete_loose_ref.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 0017d9c..24f9546 100644
--- a/refs.c
+++ b/refs.c
@@ -2544,16 +2544,16 @@ int repack_without_refs(const char **refnames, int n,
struct strbuf *err)
return ret;
}
-static int delete_ref_loose(struct ref_lock *lock, int flag)
+static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf
*err)
{
if (!(flag REF_ISPACKED) || flag REF_ISSYMREF) {
/* loose */
- int err, i = strlen(lock-lk-filename) - 5; /* .lock */
+ int res, i = strlen(lock-lk-filename) - 5; /* .lock */
lock-lk-filename[i] = 0;
- err = unlink_or_warn(lock-lk-filename);
+ res = unlink_or_msg(lock-lk-filename, err);
lock-lk-filename[i] = '.';
- if (err errno != ENOENT)
+ if (res)
return 1;
}
return 0;
@@ -3602,7 +3602,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (update-lock) {
- ret |= delete_ref_loose(update-lock, update-type);
+ ret |= delete_ref_loose(update-lock, update-type,
+ err);
if (!(update-flags REF_ISPRUNING))
delnames[delnum++] = update-lock-ref_name;
}
--
2.0.1.527.gc6b782e
--
To unsubscribe from this list: send the line 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 04/12] refs.c: pass the ref log message to _create/delete/update instead of _commit
Change the reference transactions so that we pass the reflog message
through to the create/delete/update function instead of the commit message.
This allows for individual messages for each change in a multi ref
transaction.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
branch.c | 4 ++--
builtin/commit.c | 4 ++--
builtin/fetch.c| 3 +--
builtin/receive-pack.c | 5 +++--
builtin/replace.c | 4 ++--
builtin/tag.c | 4 ++--
builtin/update-ref.c | 13 +++--
fast-import.c | 8
refs.c | 34 +-
refs.h | 8
sequencer.c| 4 ++--
walker.c | 5 ++---
12 files changed, 52 insertions(+), 44 deletions(-)
diff --git a/branch.c b/branch.c
index c1eae00..e0439af 100644
--- a/branch.c
+++ b/branch.c
@@ -301,8 +301,8 @@ void create_branch(const char *head,
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, sha1,
- null_sha1, 0, !forcing, err) ||
- ref_transaction_commit(transaction, msg, err))
+ null_sha1, 0, !forcing, msg, err) ||
+ ref_transaction_commit(transaction, err))
die(%s, err.buf);
ref_transaction_free(transaction);
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 668fa6a..c499826 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1767,8 +1767,8 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
ref_transaction_update(transaction, HEAD, sha1,
current_head ?
current_head-object.sha1 : NULL,
- 0, !!current_head, err) ||
- ref_transaction_commit(transaction, sb.buf, err)) {
+ 0, !!current_head, sb.buf, err) ||
+ ref_transaction_commit(transaction, err)) {
rollback_index_files();
die(%s, err.buf);
}
diff --git a/builtin/fetch.c b/builtin/fetch.c
index dd46b61..92fad2d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -702,10 +702,9 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
}
}
}
-
if (rc STORE_REF_ERROR_DF_CONFLICT)
error(_(some local refs could not be updated; try running\n
- 'git remote prune %s' to remove any old, conflicting
+ 'git remote prune %s' to remove any old, conflicting
branches), remote_name);
abort:
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 91099ad..4752225 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -585,8 +585,9 @@ static char *update(struct command *cmd, struct
shallow_info *si)
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, namespaced_name,
- new_sha1, old_sha1, 0, 1, err) ||
- ref_transaction_commit(transaction, push, err)) {
+ new_sha1, old_sha1, 0, 1, push,
+ err) ||
+ ref_transaction_commit(transaction, err)) {
char *str = strbuf_detach(err, NULL);
ref_transaction_free(transaction);
diff --git a/builtin/replace.c b/builtin/replace.c
index 7528f3d..df060f8 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -170,8 +170,8 @@ static int replace_object_sha1(const char *object_ref,
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, ref, repl, prev,
- 0, !is_null_sha1(prev), err) ||
- ref_transaction_commit(transaction, NULL, err))
+ 0, !is_null_sha1(prev), NULL, err) ||
+ ref_transaction_commit(transaction, err))
die(%s, err.buf);
ref_transaction_free(transaction);
diff --git a/builtin/tag.c b/builtin/tag.c
index 1aa88a2..3834b06 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -705,8 +705,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
- 0, 1, err) ||
- ref_transaction_commit(transaction, NULL, err))
+ 0, 1, NULL, err) ||
+ ref_transaction_commit(transaction, err))
[PATCH 05/12] refs.c: pass NULL as *flags to read_ref_full
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.
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))) {
--
2.0.1.527.gc6b782e
--
To unsubscribe from this list: send the line 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 00/12] Use ref transactions part 3
This is the third and final part of the original 48 patch series for basic transaction support. It is used ontop of the previous two series : * rs/ref-transaction-0 (2014-07-14) 19 commits * rs/ref-transaction-1 (2014-07-16) 20 commits This version implements some changes suggested by mhagger for the warn_if_removable changes. It also adds a new patch fix handling of badly named refs that repairs the handling of badly named refs. Ronnie Sahlberg (12): wrapper.c: simplify warn_if_unremovable wrapper.c: add a new function unlink_or_msg refs.c: add an err argument to delete_ref_loose refs.c: pass the ref log message to _create/delete/update instead of _commit refs.c: pass NULL as *flags to read_ref_full refs.c: move the check for valid refname to lock_ref_sha1_basic refs.c: call lock_ref_sha1_basic directly from commit refs.c: pass a skip list to name_conflict_fn refs.c: propagate any errno==ENOTDIR from _commit back to the callers fetch.c: change s_update_ref to use a ref transaction refs.c: make write_ref_sha1 static refs.c: fix handling of badly named refs branch.c| 4 +- builtin/blame.c | 2 +- builtin/branch.c| 6 +- builtin/clone.c | 2 +- builtin/commit.c| 4 +- builtin/fetch.c | 36 --- builtin/fmt-merge-msg.c | 2 +- builtin/for-each-ref.c | 6 +- builtin/log.c | 3 +- builtin/receive-pack.c | 5 +- builtin/remote.c| 5 +- builtin/replace.c | 4 +- builtin/show-branch.c | 6 +- builtin/tag.c | 4 +- builtin/update-ref.c| 13 +-- bundle.c| 2 +- cache.h | 18 ++-- fast-import.c | 8 +- git-compat-util.h | 6 ++ http-backend.c | 3 +- reflog-walk.c | 3 +- refs.c | 247 +++- refs.h | 17 ++-- remote.c| 6 +- sequencer.c | 6 +- transport-helper.c | 2 +- transport.c | 5 +- walker.c| 5 +- wrapper.c | 30 -- 29 files changed, 291 insertions(+), 169 deletions(-) -- 2.0.1.527.gc6b782e -- To unsubscribe from this list: send the line 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 07/12] refs.c: call lock_ref_sha1_basic directly from commit
Skip using the lock_any_ref_for_update wrapper and call lock_ref_sha1_basic
directly from the commit function.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index f29f18a..d3fedbb 100644
--- a/refs.c
+++ b/refs.c
@@ -3575,12 +3575,12 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
for (i = 0; i n; i++) {
struct ref_update *update = updates[i];
- update-lock = lock_any_ref_for_update(update-refname,
- (update-have_old ?
- update-old_sha1 :
- NULL),
- update-flags,
- update-type);
+ update-lock = lock_ref_sha1_basic(update-refname,
+ (update-have_old ?
+ update-old_sha1 :
+ NULL),
+ update-flags,
+ update-type);
if (!update-lock) {
if (err)
strbuf_addf(err, Cannot lock the ref '%s'.,
--
2.0.1.527.gc6b782e
--
To unsubscribe from this list: send the line 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 10/12] fetch.c: change s_update_ref to use a ref transaction
Change s_update_ref to use a ref transaction for the ref update.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/fetch.c | 33 +++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 92fad2d..383c385 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -404,23 +404,36 @@ static int s_update_ref(const char *action,
{
char msg[1024];
char *rla = getenv(GIT_REFLOG_ACTION);
- static struct ref_lock *lock;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
+ int ret, df_conflict = 0;
if (dry_run)
return 0;
if (!rla)
rla = default_rla.buf;
snprintf(msg, sizeof(msg), %s: %s, rla, action);
- lock = lock_any_ref_for_update(ref-name,
- check_old ? ref-old_sha1 : NULL,
- 0, NULL);
- if (!lock)
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
- if (write_ref_sha1(lock, ref-new_sha1, msg) 0)
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
+
+ transaction = ref_transaction_begin(err);
+ if (!transaction ||
+ ref_transaction_update(transaction, ref-name, ref-new_sha1,
+ ref-old_sha1, 0, check_old, msg, err))
+ goto fail;
+
+ ret = ref_transaction_commit(transaction, err);
+ if (ret == UPDATE_REFS_NAME_CONFLICT)
+ df_conflict = 1;
+ if (ret)
+ goto fail;
+
+ ref_transaction_free(transaction);
return 0;
+fail:
+ ref_transaction_free(transaction);
+ error(%s, err.buf);
+ strbuf_release(err);
+ return df_conflict ? STORE_REF_ERROR_DF_CONFLICT
+ : STORE_REF_ERROR_OTHER;
}
#define REFCOL_WIDTH 10
--
2.0.1.527.gc6b782e
--
To unsubscribe from this list: send the line 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 02/12] wrapper.c: add a new function unlink_or_msg
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);
+}
+
int unlink_or_warn(const char *file)
{
return warn_if_unremovable(unlink, file, unlink(file));
--
2.0.1.527.gc6b782e
--
To unsubscribe from this list: send the line 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 06/12] refs.c: move the check for valid refname to lock_ref_sha1_basic
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.
This changes semantics for lock_ref_sha1_basic slightly. With this change
it is no longer possible to open a ref that has a badly name which breaks
any codepaths that tries to open and repair badly named refs. The normal refs
API should not allow neither creating nor accessing refs with invalid names.
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.
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);
}
--
2.0.1.527.gc6b782e
--
To unsubscribe from this list: send the line 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 09/12] refs.c: propagate any errno==ENOTDIR from _commit back to the callers
In _commit, ENOTDIR can happen in the call to lock_ref_sha1_basic, either when
we lstat the new refname and it returns ENOTDIR or if the name checking
function reports that the same type of conflict happened. In both cases it
means that we can not create the new ref due to a name conflict.
For these cases, save the errno value and abort and make sure that the caller
can see errno==ENOTDIR.
Also start defining specific return codes for _commit, assign -1 as a generic
error and -2 as the error that refers to a name conflict. Callers can (and
should) use that return value inspecting errno directly.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 22 +++---
refs.h | 6 ++
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index a115478..69cbca5 100644
--- a/refs.c
+++ b/refs.c
@@ -3559,7 +3559,7 @@ static int ref_update_reject_duplicates(struct ref_update
**updates, int n,
int ref_transaction_commit(struct ref_transaction *transaction,
struct strbuf *err)
{
- int ret = 0, delnum = 0, i;
+ int ret = 0, delnum = 0, i, df_conflict = 0;
const char **delnames;
int n = transaction-nr;
struct ref_update **updates = transaction-updates;
@@ -3577,9 +3577,10 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
/* Copy, sort, and reject duplicate refs */
qsort(updates, n, sizeof(*updates), ref_update_compare);
- ret = ref_update_reject_duplicates(updates, n, err);
- if (ret)
+ if (ref_update_reject_duplicates(updates, n, err)) {
+ ret = -1;
goto cleanup;
+ }
/* Acquire all locks while verifying old values */
for (i = 0; i n; i++) {
@@ -3593,10 +3594,12 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-type,
delnames, delnum);
if (!update-lock) {
+ if (errno == ENOTDIR)
+ df_conflict = 1;
if (err)
strbuf_addf(err, Cannot lock the ref '%s'.,
update-refname);
- ret = 1;
+ ret = -1;
goto cleanup;
}
}
@@ -3614,6 +3617,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
if (err)
strbuf_addf(err, str, update-refname);
+ ret = -1;
goto cleanup;
}
}
@@ -3624,14 +3628,16 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (update-lock) {
- ret |= delete_ref_loose(update-lock, update-type,
- err);
+ if (delete_ref_loose(update-lock, update-type, err))
+ ret = -1;
+
if (!(update-flags REF_ISPRUNING))
delnames[delnum++] = update-lock-ref_name;
}
}
- ret |= repack_without_refs(delnames, delnum, err);
+ if (repack_without_refs(delnames, delnum, err))
+ ret = -1;
for (i = 0; i delnum; i++)
unlink_or_warn(git_path(logs/%s, delnames[i]));
clear_loose_ref_cache(ref_cache);
@@ -3644,6 +3650,8 @@ cleanup:
if (updates[i]-lock)
unlock_ref(updates[i]-lock);
free(delnames);
+ if (df_conflict)
+ ret = -2;
return ret;
}
diff --git a/refs.h b/refs.h
index 6a4d1a7..fc7942c 100644
--- a/refs.h
+++ b/refs.h
@@ -326,7 +326,13 @@ int ref_transaction_delete(struct ref_transaction
*transaction,
* Commit all of the changes that have been queued in transaction, as
* atomically as possible. Return a nonzero value if there is a
* problem.
+ * If the transaction is already in failed state this function will return
+ * an error.
+ * Function returns 0 on success, -1 for generic failures and
+ * UPDATE_REFS_NAME_CONFLICT (-2) if the failure was due to a name
+ * collision (ENOTDIR).
*/
+#define UPDATE_REFS_NAME_CONFLICT -2
int ref_transaction_commit(struct ref_transaction *transaction,
struct strbuf *err);
--
2.0.1.527.gc6b782e
--
To unsubscribe from this list: send the line 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 11/12] refs.c: make write_ref_sha1 static
No external users call write_ref_sha1 any more so lets declare it static.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 10 --
refs.h | 3 ---
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 69cbca5..6c7a9d2 100644
--- a/refs.c
+++ b/refs.c
@@ -2643,6 +2643,9 @@ static int rename_tmp_log(const char *newrefname)
return 0;
}
+static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
+ const char *logmsg);
+
int rename_ref(const char *oldrefname, const char *newrefname, const char
*logmsg)
{
unsigned char sha1[20], orig_sha1[20];
@@ -2892,8 +2895,11 @@ static int is_branch(const char *refname)
return !strcmp(refname, HEAD) || starts_with(refname, refs/heads/);
}
-/* This function must return a meaningful errno */
-int write_ref_sha1(struct ref_lock *lock,
+/*
+ * Writes sha1 into the ref specified by the lock. Makes sure that errno
+ * is sane on error.
+ */
+static int write_ref_sha1(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
{
static char term = '\n';
diff --git a/refs.h b/refs.h
index fc7942c..b0476c1 100644
--- a/refs.h
+++ b/refs.h
@@ -196,9 +196,6 @@ extern int commit_ref(struct ref_lock *lock);
/** Release any lock taken but not written. **/
extern void unlock_ref(struct ref_lock *lock);
-/** Writes sha1 into the ref specified by the lock. **/
-extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
const char *msg);
-
/*
* Setup reflog before using. Set errno to something meaningful on failure.
*/
--
2.0.1.527.gc6b782e
--
To unsubscribe from this list: send the line 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: What's cooking in git.git (Jul 2014, #03; Wed, 16)
On Wed, Jul 16, 2014 at 3:01 PM, Junio C Hamano gits...@pobox.com wrote: Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. We would need to start slowing down to prepare for -rc0 preview at the end of this week and then feature freeze. Some topics that joined 'next' late may want to stay there for the remainder of this cycle. Many of the accumulated fixes have been flushed to 'maint' and Git 2.0.2 has been tagged. Oops; I've been hoping that we will have -rc0 at the end of next week, not this week. -- To unsubscribe from this list: send the line 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 00/20] ref transactions part 2
I had a look at the changes in origin/pu and they look sane to me. make test passes all tests too. regards ronnie sahlberg On Wed, Jul 16, 2014 at 3:16 PM, Junio C Hamano gits...@pobox.com wrote: Ronnie Sahlberg sahlb...@google.com writes: On Tue, Jul 15, 2014 at 4:33 PM, Ronnie Sahlberg sahlb...@google.com wrote: This is the next 20 patches from my originally big patch series and follow the previous 19 patches that is now in juns tree. These patches were numbered 20-39 in the original 48-patch series. Changes since these patches were in the original series: - Addressing concerns from mhagger's review One patch in the series did not apply cleanly on top of the tip of the previous series (now queued as rs/ref-transaction-0) and I had to wiggle it. Please check the result (queued as three topics, this one is rs/ref-transaction-1 which is built on the abovementioned -0, and the remainder from the previous round is rebased on -1 as rs/ref-transaction), all of which are queued on 'jch' (which is part of 'pu'). 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] strbuf: use strbuf_addstr() for adding C strings
Avoid code duplication and let strbuf_addstr() call strlen() for us.
Signed-off-by: Rene Scharfe l@web.de
---
builtin/commit.c | 2 +-
diff.c | 12 ++--
path.c | 6 +++---
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 72eb3be..f2d7979 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -702,7 +702,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
char *buffer;
buffer = strstr(use_message_buffer, \n\n);
if (buffer)
- strbuf_add(sb, buffer + 2, strlen(buffer + 2));
+ strbuf_addstr(sb, buffer + 2);
hook_arg1 = commit;
hook_arg2 = use_message;
} else if (fixup_message) {
diff --git a/diff.c b/diff.c
index 06bdfb8..867f034 100644
--- a/diff.c
+++ b/diff.c
@@ -525,9 +525,9 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
ep += 2; /* skip over @@ */
/* The hunk header in fraginfo color */
- strbuf_add(msgbuf, frag, strlen(frag));
+ strbuf_addstr(msgbuf, frag);
strbuf_add(msgbuf, line, ep - line);
- strbuf_add(msgbuf, reset, strlen(reset));
+ strbuf_addstr(msgbuf, reset);
/*
* trailing \r\n
@@ -541,15 +541,15 @@ static void emit_hunk_header(struct emit_callback
*ecbdata,
if (*ep != ' ' *ep != '\t')
break;
if (ep != cp) {
- strbuf_add(msgbuf, plain, strlen(plain));
+ strbuf_addstr(msgbuf, plain);
strbuf_add(msgbuf, cp, ep - cp);
- strbuf_add(msgbuf, reset, strlen(reset));
+ strbuf_addstr(msgbuf, reset);
}
if (ep line + len) {
- strbuf_add(msgbuf, func, strlen(func));
+ strbuf_addstr(msgbuf, func);
strbuf_add(msgbuf, ep, line + len - ep);
- strbuf_add(msgbuf, reset, strlen(reset));
+ strbuf_addstr(msgbuf, reset);
}
strbuf_add(msgbuf, line + len, org_len - len);
diff --git a/path.c b/path.c
index bc804a3..c3883d4 100644
--- a/path.c
+++ b/path.c
@@ -277,16 +277,16 @@ char *expand_user_path(const char *path)
const char *home = getenv(HOME);
if (!home)
goto return_null;
- strbuf_add(user_path, home, strlen(home));
+ strbuf_addstr(user_path, home);
} else {
struct passwd *pw = getpw_str(username, username_len);
if (!pw)
goto return_null;
- strbuf_add(user_path, pw-pw_dir, strlen(pw-pw_dir));
+ strbuf_addstr(user_path, pw-pw_dir);
}
to_copy = first_slash;
}
- strbuf_add(user_path, to_copy, strlen(to_copy));
+ strbuf_addstr(user_path, to_copy);
return strbuf_detach(user_path, NULL);
return_null:
strbuf_release(user_path);
--
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] strbuf: use strbuf_addstr() for adding C strings
René Scharfe wrote: Avoid code duplication and let strbuf_addstr() call strlen() for us. Nice. Signed-off-by: Rene Scharfe l@web.de --- builtin/commit.c | 2 +- diff.c | 12 ++-- path.c | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) 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
[PATCH] use commit_list_count() to count the members of commit_lists
Call commit_list_count() instead of open-coding it repeatedly.
Signed-off-by: Rene Scharfe l@web.de
---
builtin/blame.c| 5 +
builtin/for-each-ref.c | 16 ++--
commit.c | 7 +--
line-log.c | 13 +
pretty.c | 7 +--
5 files changed, 6 insertions(+), 42 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index c59e702..339a8d0 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1371,11 +1371,8 @@ static struct commit_list *first_scapegoat(struct
rev_info *revs, struct commit
static int num_scapegoats(struct rev_info *revs, struct commit *commit)
{
- int cnt;
struct commit_list *l = first_scapegoat(revs, commit);
- for (cnt = 0; l; l = l-next)
- cnt++;
- return cnt;
+ return commit_list_count(l);
}
/* Distribute collected unsorted blames to the respected sorted lists
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 4135980..47bd624 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -283,18 +283,6 @@ static void grab_tag_values(struct atom_value *val, int
deref, struct object *ob
}
}
-static int num_parents(struct commit *commit)
-{
- struct commit_list *parents;
- int i;
-
- for (i = 0, parents = commit-parents;
-parents;
-parents = parents-next)
- i++;
- return i;
-}
-
/* See grab_values */
static void grab_commit_values(struct atom_value *val, int deref, struct
object *obj, void *buf, unsigned long sz)
{
@@ -315,12 +303,12 @@ static void grab_commit_values(struct atom_value *val,
int deref, struct object
}
if (!strcmp(name, numparent)) {
char *s = xmalloc(40);
- v-ul = num_parents(commit);
+ v-ul = commit_list_count(commit-parents);
sprintf(s, %lu, v-ul);
v-s = s;
}
else if (!strcmp(name, parent)) {
- int num = num_parents(commit);
+ int num = commit_list_count(commit-parents);
int i;
struct commit_list *parents;
char *s = xmalloc(41 * num + 1);
diff --git a/commit.c b/commit.c
index f43970d..e9c40f7 100644
--- a/commit.c
+++ b/commit.c
@@ -987,12 +987,7 @@ struct commit_list *get_merge_bases_many(struct commit
*one,
}
/* There are more than one */
- cnt = 0;
- list = result;
- while (list) {
- list = list-next;
- cnt++;
- }
+ cnt = commit_list_count(result);
rslt = xcalloc(cnt, sizeof(*rslt));
for (list = result, i = 0; list; list = list-next)
rslt[i++] = list-item;
diff --git a/line-log.c b/line-log.c
index afcc98d..1008e72 100644
--- a/line-log.c
+++ b/line-log.c
@@ -766,17 +766,6 @@ void line_log_init(struct rev_info *rev, const char
*prefix, struct string_list
}
}
-static int count_parents(struct commit *commit)
-{
- struct commit_list *parents = commit-parents;
- int count = 0;
- while (parents) {
- count++;
- parents = parents-next;
- }
- return count;
-}
-
static void move_diff_queue(struct diff_queue_struct *dst,
struct diff_queue_struct *src)
{
@@ -1150,7 +1139,7 @@ static int process_ranges_merge_commit(struct rev_info
*rev, struct commit *comm
struct commit **parents;
struct commit_list *p;
int i;
- int nparents = count_parents(commit);
+ int nparents = commit_list_count(commit-parents);
diffqueues = xmalloc(nparents * sizeof(*diffqueues));
cand = xmalloc(nparents * sizeof(*cand));
diff --git a/pretty.c b/pretty.c
index eb676d6..3a1da6f 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1554,12 +1554,7 @@ static void pp_header(struct pretty_print_context *pp,
}
if (!parents_shown) {
- struct commit_list *parent;
- int num;
- for (parent = commit-parents, num = 0;
-parent;
-parent = parent-next, num++)
- ;
+ unsigned num = commit_list_count(commit-parents);
/* with enough slop */
strbuf_grow(sb, num * 50 + 20);
add_merge_info(pp, sb, commit);
--
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
git update-index not delete lock file when using different worktree
This is a [issue from TortoiseGit](https://code.google.com/p/tortoisegit/issues/detail?id=2233). After doing some test, I report it here. The following is the testing information I have tested. ### Folder Structure ``` Test |-- myrepo | |-- bar.txt | |-- foo.txt | |-- myrepo.git |-- .git ``` Testing repository is [here](https://code.google.com/p/tortoisegit/issues/detail?id=2233#c2). ### Using different worktree Set the config file (in the .git folder) ``` [core] worktree = ../../myrepo ``` ### Test 1 - Git Bash ``` User@PC /d/Repo/myrepo.git (master) $ git --version git version 1.9.4.msysgit.0 User@PC /d/Repo/myrepo.git (master) $ git update-index --refresh fatal: Unable to write new index file ``` D:\Repo\myrepo.git\\**index.lock** is not deleted. ### Test 2 - Git 2.0.0 Copy testing repository into ```C:\msysgit\MyTest``` Execute```msys.bat``` ```$ vagrant up``` ```$ vagrant ssh``` ```vagrant@precise64:/vagrant/git$ cd /vagrant``` ```vagrant@precise64:/vagrant$ cd mytest/myrepo.git``` ``` vagrant@precise64:/vagrant/mytest/myrepo.git$ git --version git version 2.0.0 ``` ``` vagrant@precise64:/vagrant/mytest/myrepo.git$ git update-index --refresh fatal: Unable to write new index file ``` Also, **index.lock is not deleted.** ### Test 3 - gitdll of TortoiseGit (git version 1.9.0) Tracing the source code into **compat/mingw.c** line 289 : xutftowcs_canonical_path() get the value of var. wpathname ``` D:\Repo\myrepo\.git\index.lock ``` It should be ``` D:\Repo\myrepo.git\.git\index.lock ``` line 294 : _wunlink() try to delete the file.(```D:\Repo\myrepo\.git\index.lock```) line 295 : GetLastError() return 3(ERROR_PATH_NOT_FOUND) (Actually, there is no ```D:\Repo\myrepo\.git``` folder.) -- View this message in context: http://git.661346.n2.nabble.com/git-update-index-not-delete-lock-file-when-using-different-worktree-tp7615300.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
Syncing Git Repositories
On all my laptops and desktops, I have a directory at /home/sajan/Code where all my active projects and repositories live. /home/sajan/Code/repository1 /home/sajan/Code/repository2 /home/sajan/Code/repository3 ...etc... Up until now I've relied on pushing and pulling to and from my Gitlab server to keep my projects in sync across all my laptops and desktops. It's worked great. However, today I decided to add my code folder to my ownCloud server and sync it across all my laptops and desktops the same way I do for /home/sajan/Documents, /home/sajan/Music, and a few application config directories to keep all my devices in sync. By syncing my code folder and git repositories in this way, do I risk borking any repositories? I'm 99% confident I'm not, since everything is in .git/, and there are not external databases or log files that need to be updated. Just making sure though. I'm only doing this because sometimes I forget to pull changes down from my Gitlab server on a different laptop or desktop and start making local changes. Which is fine, I can merge easily, but if everything were sync'd automatically when I logged into my computer it would be great. Another option I thought of would be to write a bash script that executed at login and went into each of my repositories and ran git pull, but I decided against this because of legitimate non-fast-forward merges. TLDR; If I sync my repositories across computers using something similar to Dropbox, rather than pushing/pulling to and from an central repository, am I risking borking any respository? -- Sajan Parikh smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v7 22/31] checkout: support checking out into a new working directory
Hi. On Sun, Jul 13, 2014 at 11:50:59AM +0700, Nguyễn Thái Ngọc Duy wrote: +MULTIPLE CHECKOUT MODE +--- This generates incorrect html for me, making all section until next heading EXAMPLES into a preformatted text. If I justify the line of dashes to be the exactly same width as header it starts generating correctly looking html. I'm not sure which software to refer to. I mostly use Debian stable. The installed version of asciidocs is 8.6.7-1. I suppose the line really should be justified because for all other headings they are. -- 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
