Profile feedback patchkit v2
Fix the bitrotted profile feedback support. Changes to v1: - Remove obsolete comment - Remove controversal diff script. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] Use BASIC_FLAGS for profile feedback
From: Andi Kleen a...@linux.intel.com Use BASIC_CFLAGS instead of CFLAGS to set up the profile feedback option in the Makefile. This allows still overriding CFLAGS on the make command line without disabling profile feedback. Signed-off-by: Andi Kleen a...@linux.intel.com --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 07ea105..a9770ac 100644 --- a/Makefile +++ b/Makefile @@ -1552,13 +1552,13 @@ endif PROFILE_DIR := $(CURDIR) ifeq ($(PROFILE),GEN) - CFLAGS += -fprofile-generate=$(PROFILE_DIR) -DNO_NORETURN=1 + BASIC_CFLAGS += -fprofile-generate=$(PROFILE_DIR) -DNO_NORETURN=1 EXTLIBS += -lgcov export CCACHE_DISABLE = t V = 1 else ifneq ($(PROFILE),) - CFLAGS += -fprofile-use=$(PROFILE_DIR) -fprofile-correction -DNO_NORETURN=1 + BASIC_CFLAGS += -fprofile-use=$(PROFILE_DIR) -fprofile-correction -DNO_NORETURN=1 export CCACHE_DISABLE = t V = 1 endif -- 2.0.1 -- 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 4/4] Fix profile feedback with -jN and add profile-fast
From: Andi Kleen a...@linux.intel.com Profile feedback always failed for me with -jN. The problem was that there was no implicit ordering between the profile generate stage and the profile use stage. So some objects in the later stage would be linked with profile generate objects, and fail due to the missing -lgcov. This adds a new profile target that implicitely enforces the correct ordering by using submakes. Plus a profile-install target to also install. This is also nicer to type that PROFILE=... Plus I always run the performance test suite now for the full profile run. In addition I also added a profile-fast / profile-fast-install target the only runs the performance test suite instead of the whole test suite. This significantly speeds up the profile build, which was totally dominated by test suite run time. However it may have less coverage of course. Signed-off-by: Andi Kleen a...@linux.intel.com --- INSTALL | 14 -- Makefile | 21 + 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/INSTALL b/INSTALL index ba01e74..6ec7a24 100644 --- a/INSTALL +++ b/INSTALL @@ -28,7 +28,7 @@ set up install paths (via config.mak.autogen), so you can write instead If you're willing to trade off (much) longer build time for a later faster git you can also do a profile feedback build with - $ make prefix=/usr PROFILE=BUILD all + $ make prefix=/usr profile # make prefix=/usr PROFILE=BUILD install This will run the complete test suite as training workload and then @@ -36,10 +36,20 @@ rebuild git with the generated profile feedback. This results in a git which is a few percent faster on CPU intensive workloads. This may be a good tradeoff for distribution packagers. +Alternatively you can run profile feedback only with the git benchmark +suite. This runs significantly faster than the full test suite, but +has less coverage: + + $ make prefix=/usr profile-fast + # make prefix=/usr PROFILE=BUILD install + Or if you just want to install a profile-optimized version of git into your home directory, you could run: - $ make PROFILE=BUILD install + $ make profile-install + +or + $ make profile-fast-install As a caveat: a profile-optimized build takes a *lot* longer since the git tree must be built twice, and in order for the profiling diff --git a/Makefile b/Makefile index ba64be9..a760402 100644 --- a/Makefile +++ b/Makefile @@ -1643,13 +1643,20 @@ SHELL = $(SHELL_PATH) all:: shell_compatibility_test ifeq $(PROFILE) BUILD -ifeq ($(filter all,$(MAKECMDGOALS)),all) -all:: profile-clean +all:: profile +endif + +profile:: profile-clean $(MAKE) PROFILE=GEN all $(MAKE) PROFILE=GEN -j1 test $(MAKE) PROFILE=GEN -j1 perf -endif -endif + $(MAKE) PROFILE=USE all + +profile-fast: profile-clean + $(MAKE) PROFILE=GEN all + $(MAKE) PROFILE=GEN -j1 perf + $(MAKE) PROFILE=USE all + all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS ifneq (,$X) @@ -2336,6 +2343,12 @@ mergetools_instdir_SQ = $(subst ','\'',$(mergetools_instdir)) install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X) +profile-install: profile + $(MAKE) install + +profile-fast-install: profile-fast + $(MAKE) install + install: all $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)' $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' -- 2.0.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] Run the perf test suite for profile feedback too
From: Andi Kleen a...@linux.intel.com Signed-off-by: Andi Kleen a...@linux.intel.com --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index a9770ac..ba64be9 100644 --- a/Makefile +++ b/Makefile @@ -1647,6 +1647,7 @@ ifeq ($(filter all,$(MAKECMDGOALS)),all) all:: profile-clean $(MAKE) PROFILE=GEN all $(MAKE) PROFILE=GEN -j1 test + $(MAKE) PROFILE=GEN -j1 perf endif endif -- 2.0.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] Don't define away __attribute__ on gcc
From: Andi Kleen a...@linux.intel.com Profile feedback sets -DNO_NORETURN, which causes the compat header file to go into a default #else block. That #else block defines away __attribute__(). Doing so causes all kinds of problems with the Linux and gcc system headers: in particular it makes the xmmintrin.h headers error out, breaking the build. Don't define away __attribute__ when __GNUC__ is set. Signed-off-by: Andi Kleen a...@linux.intel.com --- git-compat-util.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 96f5554..01e8695 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -291,10 +291,12 @@ extern char *gitbasename(char *); #else #define NORETURN #define NORETURN_PTR +#ifndef __GNUC__ #ifndef __attribute__ #define __attribute__(x) #endif #endif +#endif /* The sentinel attribute is valid from gcc version 4.0 */ #if defined(__GNUC__) (__GNUC__ = 4) -- 2.0.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit
On Tue, Jul 8, 2014 at 7:26 AM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: diff --git a/builtin/commit.c b/builtin/commit.c index 9cfef6c..5981755 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -342,6 +342,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, discard_cache(); read_cache_from(index_lock.filename); +if (update_main_cache_tree(WRITE_TREE_SILENT) = 0) +write_cache(fd, active_cache, active_nr); commit_style = COMMIT_NORMAL; return index_lock.filename; I wonder if we need to update_main_cache_tree() so many times in this function. If I read the code correctly, all roads must lead to update_main_cache_tree(0) in prepare_to_commit(). If we find out that we have incomplete cache-tree before that call, we could write the index one more time after that call, instead of spreading update_main_cache_tree() all over prepare_index(). I know the index_file in prepare_to_commit() is probably index.lock or something, but that does not stop us from locking again (index.lock.lock) if we want to update it. Writing cache tree early in prepare_index() does help hooks, but I would say hooks are uncommon case and we could add an option to update-index to explicitly rebuild cache-tree, then hooks that do diff a lot (or other operations that use cache-tree) could rebuild cache-tree by themselves. When the index file is large, every write pays high penalty so I think trying to write less often is good. -- 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
[PATCH] log: fix indentation for --graph --show-signature
The git log --graph --show-signature command incorrectly indents the gpg information about signed commits and merged signed tags. It does not follow the level of indentation of the current commit. Example of garbled output: $ git log --show-signature --graph * commit 258e0a237cb69aaa587b0a4fb528bb0316b1b776 |\ gpg: Signature made Mon, Jun 30, 2014 13:22:33 EDT using RSA key ID DA08 gpg: Good signature from Jason Pyeron jpye...@pdinc.us Merge: 727c355 1ca13ed | | Author: Jason Pyeron jpye...@pdinc.us | | Date: Mon Jun 30 13:22:29 2014 -0400 | | | | Merge of 1ca13ed2271d60ba9 branch - rebranding | | | * commit 1ca13ed2271d60ba93d40bcc8db17ced8545f172 | | gpg: Signature made Mon, Jun 23, 2014 9:45:47 EDT using RSA key ID DD37 gpg: Good signature from Stephen Robert Guglielmo s...@guglielmo.us gpg: aka Stephen Robert Guglielmo srguglie...@gmail.com Author: Stephen R Guglielmo s...@guglielmo.us | | Date: Mon Jun 23 09:45:27 2014 -0400 | | | | Minor URL updates In log-tree.c modify show_sig_lines() function to call graph_show_oneline() after each line of gpg information it has printed in order to preserve the level of indentation for the next output line. Reported-by: Jason Pyeron jpye...@pdinc.us Signed-off-by: Zoltan Klinger zoltan.klin...@gmail.com --- log-tree.c | 1 + t/t4202-log.sh | 29 + 2 files changed, 30 insertions(+) diff --git a/log-tree.c b/log-tree.c index 10e6844..f13b861 100644 --- a/log-tree.c +++ b/log-tree.c @@ -365,6 +365,7 @@ static void show_sig_lines(struct rev_info *opt, int status, const char *bol) eol = strchrnul(bol, '\n'); printf(%s%.*s%s%s, color, (int)(eol - bol), bol, reset, *eol ? \n : ); + graph_show_oneline(opt-graph); bol = (*eol) ? (eol + 1) : eol; } } diff --git a/t/t4202-log.sh b/t/t4202-log.sh index cb03d28..b429aff 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -3,6 +3,7 @@ test_description='git log' . ./test-lib.sh +. $TEST_DIRECTORY/lib-gpg.sh test_expect_success setup ' @@ -841,4 +842,32 @@ test_expect_success 'dotdot is a parent directory' ' test_cmp expect actual ' +test_expect_success GPG 'log --graph --show-signature' ' + git checkout -b signed master + echo foo foo + git add foo + git commit -S -m signed_commit + git log --graph --show-signature -n1 signed actual + grep ^| gpg: Signature made actual + grep ^| gpg: Good signature actual +' + +test_expect_success GPG 'log --graph --show-signature for merged tag' ' + git checkout -b plain master + echo aaa bar + git add bar + git commit -m bar_commit + git checkout -b tagged master + echo bbb baz + git add baz + git commit -m baz_commit + git tag -s -m signed_tag_msg signed_tag + git checkout plain + git merge --no-ff -m msg signed_tag + git log --graph --show-signature -n1 plain actual + grep ^|\\\ merged tag actual + grep ^| | gpg: Signature made actual + grep ^| | gpg: Good signature actual +' + test_done -- 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 v20 07/48] lockfile.c: make lock_file return a meaningful errno on failurei
On 06/20/2014 04:42 PM, Ronnie Sahlberg wrote: Making errno when returning from lock_file() meaningful, which should fix * an existing almost-bug in lock_ref_sha1_basic where it assumes errno==ENOENT is meaningful and could waste some work on retries * an existing bug in repack_without_refs where it prints strerror(errno) and picks advice based on errno, despite errno potentially being zero and potentially having been clobbered by that point [...] Typo in subject line: s/failurei/failure/ 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 v20 22/48] refs.c: make ref_transaction_begin take an err argument
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote: Add an err argument to _begin so that on non-fatal failures in future ref backends we can report a nice error back to the caller. While _begin can currently never fail for other reasons than OOM, in which case we die() anyway, we may add other types of backends in the future. For example, a hypothetical MySQL backend could fail in _being with s/_being/_begin/ [...] 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 v20 20/48] refs.c: change ref_transaction_create to do error checking and return status
I'm in my next attempt to get through your patch series. Sorry for the
long hiatus.
Patches 1-19 look OK aside from a minor typo that I just reported.
See below for a comment on this patch.
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
Do basic error checking in ref_transaction_create() and make it return
non-zero on error. Update all callers to check the result of
ref_transaction_create(). There are currently no conditions in _create that
will return error but there will be in the future. Add an err argument that
will be updated on failure.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 4 +++-
refs.c | 18 +++--
refs.h | 55
+---
3 files changed, 63 insertions(+), 14 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 3067b11..41121fa 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -226,7 +226,9 @@ static const char *parse_cmd_create(struct strbuf *input,
const char *next)
if (*next != line_termination)
die(create %s: extra input: %s, refname, next);
- ref_transaction_create(transaction, refname, new_sha1, update_flags);
+ if (ref_transaction_create(transaction, refname, new_sha1,
+update_flags, err))
+ die(%s, err.buf);
update_flags = 0;
free(refname);
diff --git a/refs.c b/refs.c
index 3f05e88..c49f1c6 100644
--- a/refs.c
+++ b/refs.c
@@ -3449,18 +3449,24 @@ int ref_transaction_update(struct ref_transaction
*transaction,
return 0;
}
-void ref_transaction_create(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- int flags)
+int ref_transaction_create(struct ref_transaction *transaction,
+const char *refname,
+const unsigned char *new_sha1,
+int flags,
+struct strbuf *err)
{
- struct ref_update *update = add_update(transaction, refname);
+ struct ref_update *update;
+
+ if (!new_sha1 || is_null_sha1(new_sha1))
+ die(BUG: create ref with null new_sha1);
+
+ update = add_update(transaction, refname);
- assert(!is_null_sha1(new_sha1));
hashcpy(update-new_sha1, new_sha1);
hashclr(update-old_sha1);
update-flags = flags;
update-have_old = 1;
+ return 0;
}
void ref_transaction_delete(struct ref_transaction *transaction,
diff --git a/refs.h b/refs.h
index c5376ce..33b4383 100644
--- a/refs.h
+++ b/refs.h
@@ -10,6 +10,45 @@ struct ref_lock {
int force_write;
};
+/*
+ * A ref_transaction represents a collection of ref updates
+ * that should succeed or fail together.
+ *
+ * Calling sequence
+ *
+ * - Allocate and initialize a `struct ref_transaction` by calling
+ * `ref_transaction_begin()`.
+ *
+ * - List intended ref updates by calling functions like
+ * `ref_transaction_update()` and `ref_transaction_create()`.
+ *
+ * - Call `ref_transaction_commit()` to execute the transaction.
+ * If this succeeds, the ref updates will have taken place and
+ * the transaction cannot be rolled back.
+ *
+ * - At any time call `ref_transaction_free()` to discard the
+ * transaction and free associated resources. In particular,
+ * this rolls back the transaction if it has not been
+ * successfully committed.
+ *
+ * Error handling
+ * --
+ *
+ * On error, transaction functions append a message about what
+ * went wrong to the 'err' argument. The message mentions what
+ * ref was being updated (if any) when the error occurred so it
+ * can be passed to 'die' or 'error' as-is.
+ *
+ * The message is appended to err without first clearing err.
+ * This allows the caller to prepare preamble text to the generated
+ * error message:
+ *
+ * strbuf_addf(err, Error while doing foo-bar: );
+ * if (ref_transaction_update(..., err)) {
+ * ret = error(%s, err.buf);
+ * goto cleanup;
+ * }
+ */
I don't have a problem with the API, but I think the idiom suggested in
the comment above is a bit silly. Surely one would do the following
instead:
if (ref_transaction_update(..., err)) {
ret = error(Error while doing foo-bar: %s, err.buf);
goto cleanup;
}
I think it would also be helpful to document whether the error string
that is appended to the strbuf is terminated with a LF.
[...]
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 v20 23/48] refs.c: add transaction.status and track OPEN/CLOSED/ERROR
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
Track the status of a transaction in a new status field. Check the field for
The status field is not set or used anywhere. The field that you use is
state.
sanity, i.e. that status must be OPEN when _commit/_create/_delete or
_update is called or else die(BUG:...)
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 40 +++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 9cb7908..8c695ba 100644
--- a/refs.c
+++ b/refs.c
@@ -3387,6 +3387,25 @@ struct ref_update {
};
/*
+ * Transaction states.
+ * OPEN: The transaction is in a valid state and can accept new updates.
+ * An OPEN transaction can be committed.
+ * CLOSED: If an open transaction is successfully committed the state will
+ * change to CLOSED. No further changes can be made to a CLOSED
+ * transaction.
+ * CLOSED means that all updates have been successfully committed and
+ * the only thing that remains is to free the completed transaction.
+ * ERROR: The transaction has failed and is no longer committable.
+ * No further changes can be made to a CLOSED transaction and it must
+ * be rolled back using transaction_free.
+ */
+enum ref_transaction_state {
+ REF_TRANSACTION_OPEN = 0,
+ REF_TRANSACTION_CLOSED = 1,
+ REF_TRANSACTION_ERROR = 2,
+};
+
+/*
* Data structure for holding a reference transaction, which can
* consist of checks and updates to multiple references, carried out
* as atomically as possible. This structure is opaque to callers.
@@ -3395,6 +3414,8 @@ struct ref_transaction {
struct ref_update **updates;
size_t alloc;
size_t nr;
+ enum ref_transaction_state state;
+ int status;
The status field should probably be deleted.
};
struct ref_transaction *ref_transaction_begin(struct strbuf *err)
@@ -3437,6 +3458,9 @@ int ref_transaction_update(struct ref_transaction
*transaction,
{
struct ref_update *update;
+ if (transaction-state != REF_TRANSACTION_OPEN)
+ die(BUG: update called for transaction that is not open);
+
if (have_old !old_sha1)
die(BUG: have_old is true but old_sha1 is NULL);
@@ -3457,6 +3481,9 @@ int ref_transaction_create(struct ref_transaction
*transaction,
{
struct ref_update *update;
+ if (transaction-state != REF_TRANSACTION_OPEN)
+ die(BUG: create called for transaction that is not open);
+
if (!new_sha1 || is_null_sha1(new_sha1))
die(BUG: create ref with null new_sha1);
@@ -3477,6 +3504,9 @@ int ref_transaction_delete(struct ref_transaction
*transaction,
{
struct ref_update *update;
+ if (transaction-state != REF_TRANSACTION_OPEN)
+ die(BUG: delete called for transaction that is not open);
+
if (have_old !old_sha1)
die(BUG: have_old is true but old_sha1 is NULL);
@@ -3532,8 +3562,13 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
int n = transaction-nr;
struct ref_update **updates = transaction-updates;
- if (!n)
+ if (transaction-state != REF_TRANSACTION_OPEN)
+ die(BUG: commit called for transaction that is not open);
+
+ if (!n) {
+ transaction-state = REF_TRANSACTION_CLOSED;
return 0;
+ }
/* Allocate work space */
delnames = xmalloc(sizeof(*delnames) * n);
@@ -3595,6 +3630,9 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
clear_loose_ref_cache(ref_cache);
cleanup:
+ transaction-state = ret ? REF_TRANSACTION_ERROR
+ : REF_TRANSACTION_CLOSED;
+
for (i = 0; i n; i++)
if (updates[i]-lock)
unlock_ref(updates[i]-lock);
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v20 27/48] sequencer.c: use ref transactions for all ref updates
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
Change to use ref transactions for all updates to refs.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
sequencer.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0a80c58..fd8acaf 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -272,23 +272,31 @@ static int error_dirty_index(struct replay_opts *opts)
static int fast_forward_to(const unsigned char *to, const unsigned char
*from,
int unborn, struct replay_opts *opts)
{
- struct ref_lock *ref_lock;
+ struct ref_transaction *transaction;
struct strbuf sb = STRBUF_INIT;
- int ret;
+ struct strbuf err = STRBUF_INIT;
read_cache();
if (checkout_fast_forward(from, to, 1))
exit(1); /* the callee should have complained already */
- ref_lock = lock_any_ref_for_update(HEAD, unborn ? null_sha1 : from,
-0, NULL);
- if (!ref_lock)
- return error(_(Failed to lock HEAD during fast_forward_to));
I think you've changed the semantics when unborn is set. Please note
that lock_any_ref_for_update() behaves differently if old_sha1 is NULL
(when no check is done) vs. when it is null_sha1 (when it verifies that
the reference didn't previously exist). So when unborn is true, the old
code verifies that the reference previously didn't exist...
strbuf_addf(sb, %s: fast-forward, action_name(opts));
- ret = write_ref_sha1(ref_lock, to, sb.buf);
+
+ transaction = ref_transaction_begin(err);
+ if (!transaction ||
+ ref_transaction_update(transaction, HEAD, to, from,
+0, !unborn, err) ||
...whereas when unborn is true, the new code does no check at all. I
think you want
ref_transaction_update(transaction, HEAD,
to, unborn ? null_sha1 : from,
0, 1, err) ||
+ ref_transaction_commit(transaction, sb.buf, err)) {
+ ref_transaction_free(transaction);
+ error(%s, err.buf);
+ strbuf_release(sb);
+ strbuf_release(err);
+ return -1;
+ }
strbuf_release(sb);
- return ret;
+ ref_transaction_free(transaction);
+ return 0;
}
static int do_recursive_merge(struct commit *base, struct commit *next,
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v20 24/48] tag.c: use ref transactions when doing updates
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
Change tag.c to use ref transactions for all ref updates.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/tag.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index c6e8a71..c9bfc9a 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -548,7 +548,6 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
struct strbuf ref = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
- struct ref_lock *lock;
struct create_tag_options opt;
char *cleanup_arg = NULL;
int annotate = 0, force = 0, lines = -1;
@@ -556,6 +555,8 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
struct option options[] = {
OPT_CMDMODE('l', list, cmdmode, N_(list tag names), 'l'),
{ OPTION_INTEGER, 'n', NULL, lines, N_(n),
@@ -701,11 +702,13 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
if (annotate)
create_tag(object, tag, buf, opt, prev, object);
- lock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);
- if (!lock)
- die(_(%s: cannot lock the ref), ref.buf);
- if (write_ref_sha1(lock, object, NULL) 0)
- die(_(%s: cannot update the ref), ref.buf);
+ transaction = ref_transaction_begin(err);
+ if (!transaction ||
+ ref_transaction_update(transaction, ref.buf, object, prev,
+0, !is_null_sha1(prev), err) ||
Similar to the error in sequencer.c a few patches later (explained in
more detail in my comment on that patch), here you only do a check if
!is_null_sha1(prev), whereas the old code always did the check. I think
you want
ref_transaction_update(transaction, ref.buf, object, prev,
0, 1, err) ||
Please check whether you have made the same mistake in other patches.
[...]
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 v20 25/48] replace.c: use the ref transaction functions for updates
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote: Update replace.c to use ref transactions for updates. Reviewed-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/replace.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 1bb491d..7528f3d 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -153,7 +153,8 @@ static int replace_object_sha1(const char *object_ref, unsigned char prev[20]; enum object_type obj_type, repl_type; char ref[PATH_MAX]; - struct ref_lock *lock; + struct ref_transaction *transaction; + struct strbuf err = STRBUF_INIT; obj_type = sha1_object_info(object, NULL); repl_type = sha1_object_info(repl, NULL); @@ -166,12 +167,14 @@ static int replace_object_sha1(const char *object_ref, check_ref_valid(object, prev, ref, sizeof(ref), force); - lock = lock_any_ref_for_update(ref, prev, 0, NULL); - if (!lock) - die(%s: cannot lock the ref, ref); - if (write_ref_sha1(lock, repl, NULL) 0) - die(%s: cannot update the ref, ref); + transaction = ref_transaction_begin(err); + if (!transaction || + ref_transaction_update(transaction, ref, repl, prev, +0, !is_null_sha1(prev), err) || Same problem here. You need s/!is_null_sha1(prev)/1/ + ref_transaction_commit(transaction, NULL, err)) + die(%s, err.buf); + ref_transaction_free(transaction); return 0; } -- 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] path: for clarity, rename get_pathname to get_path_buffer
The get_pathname function does not really return path name but rather
a buffer to store pathname in. As such, current name is a bit
confusing. Change the name as to make it clearer what the function is
doing.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
path.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/path.c b/path.c
index bc804a3..70e2f85 100644
--- a/path.c
+++ b/path.c
@@ -16,7 +16,7 @@ static int get_st_mode_bits(const char *path, int *mode)
static char bad_path[] = /bad-path/;
-static char *get_pathname(void)
+static char *get_path_buffer(void)
{
static char pathname_array[4][PATH_MAX];
static int index;
@@ -108,7 +108,7 @@ char *mkpath(const char *fmt, ...)
{
va_list args;
unsigned len;
- char *pathname = get_pathname();
+ char *pathname = get_path_buffer();
va_start(args, fmt);
len = vsnprintf(pathname, PATH_MAX, fmt, args);
@@ -120,7 +120,7 @@ char *mkpath(const char *fmt, ...)
char *git_path(const char *fmt, ...)
{
- char *pathname = get_pathname();
+ char *pathname = get_path_buffer();
va_list args;
char *ret;
@@ -158,7 +158,7 @@ void home_config_paths(char **global, char **xdg, char
*file)
char *git_path_submodule(const char *path, const char *fmt, ...)
{
- char *pathname = get_pathname();
+ char *pathname = get_path_buffer();
struct strbuf buf = STRBUF_INIT;
const char *git_dir;
va_list args;
--
2.0.0.526.g5318336
--
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 30/48] refs.c: change update_ref to use a transaction
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
Change the update_ref helper function to use a ref transaction internally.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 8c695ba..4bdccc5 100644
--- a/refs.c
+++ b/refs.c
@@ -3524,11 +3524,31 @@ int update_ref(const char *action, const char
*refname,
const unsigned char *sha1, const unsigned char *oldval,
int flags, enum action_on_err onerr)
{
- struct ref_lock *lock;
- lock = update_ref_lock(refname, oldval, flags, NULL, onerr);
- if (!lock)
+ struct ref_transaction *t;
+ struct strbuf err = STRBUF_INIT;
+
+ t = ref_transaction_begin(err);
+ if (!t ||
+ ref_transaction_update(t, refname, sha1, oldval, flags,
+!!oldval, err) ||
+ ref_transaction_commit(t, action, err)) {
+ const char *str = update_ref failed for ref '%s': %s;
+
+ ref_transaction_free(t);
+ switch (onerr) {
+ case UPDATE_REFS_MSG_ON_ERR:
+ error(str, refname, err.buf);
+ break;
+ case UPDATE_REFS_DIE_ON_ERR:
+ die(str, refname, err.buf);
+ break;
+ case UPDATE_REFS_QUIET_ON_ERR:
+ break;
+ }
+ strbuf_release(err);
return 1;
- return update_ref_write(action, refname, sha1, lock, NULL, onerr);
+ }
+ return 0;
}
Should this function be scheduled for the take strbuf *err argument
treatment instead of continuing to use an action_on_err parameter?
(Maybe you've changed this later in the patch series?)
I'm not saying this change has to be part of the current patch series,
but let's consider it for the future.
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] path: for clarity, rename get_pathname to get_path_buffer
The get_pathname function does not really return path name but rather
a buffer to store pathname in. As such, current name is a bit
confusing. Change the name as to make it clearer what the function is
doing.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
path.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
This time sent with Junio's correct address.
diff --git a/path.c b/path.c
index bc804a3..70e2f85 100644
--- a/path.c
+++ b/path.c
@@ -16,7 +16,7 @@ static int get_st_mode_bits(const char *path, int *mode)
static char bad_path[] = /bad-path/;
-static char *get_pathname(void)
+static char *get_path_buffer(void)
{
static char pathname_array[4][PATH_MAX];
static int index;
@@ -108,7 +108,7 @@ char *mkpath(const char *fmt, ...)
{
va_list args;
unsigned len;
- char *pathname = get_pathname();
+ char *pathname = get_path_buffer();
va_start(args, fmt);
len = vsnprintf(pathname, PATH_MAX, fmt, args);
@@ -120,7 +120,7 @@ char *mkpath(const char *fmt, ...)
char *git_path(const char *fmt, ...)
{
- char *pathname = get_pathname();
+ char *pathname = get_path_buffer();
va_list args;
char *ret;
@@ -158,7 +158,7 @@ void home_config_paths(char **global, char **xdg, char
*file)
char *git_path_submodule(const char *path, const char *fmt, ...)
{
- char *pathname = get_pathname();
+ char *pathname = get_path_buffer();
struct strbuf buf = STRBUF_INIT;
const char *git_dir;
va_list args;
--
2.0.0.526.g5318336
--
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 31/48] receive-pack.c: use a reference transaction for updating the refs
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote: Wrap all the ref updates inside a transaction. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/receive-pack.c | 96 +- 1 file changed, 63 insertions(+), 33 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index c323081..b51f8ae 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c [...] @@ -647,6 +654,9 @@ static void check_aliased_update(struct command *cmd, struct string_list *list) char cmd_oldh[41], cmd_newh[41], dst_oldh[41], dst_newh[41]; int flag; + if (cmd-error_string) + die(BUG: check_alised_update called with failed cmd); s/check_alised_update/check_aliased_update/ [...] 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 v20 33/48] walker.c: use ref transaction for ref updates
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
Switch to using ref transactions in walker_fetch(). As part of the refactoring
to use ref transactions we also fix a potential memory leak where in the
original code if write_ref_sha1() would fail we would end up returning from
the function without free()ing the msg string.
Note that this function is only called when fetching from a remote HTTP
repository onto the local (most of the time single-user) repository which
likely means that the type of collissions that the previous locking would
s/collissions/collisions/
protect against and cause the fetch to fail for to be even more rare.
Grammatico: s/to be/are/ ?
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
walker.c | 59 +++
1 file changed, 35 insertions(+), 24 deletions(-)
diff --git a/walker.c b/walker.c
index 1dd86b8..60d9f9e 100644
--- a/walker.c
+++ b/walker.c
@@ -251,39 +251,36 @@ void walker_targets_free(int targets, char **target,
const char **write_ref)
int walker_fetch(struct walker *walker, int targets, char **target,
const char **write_ref, const char *write_ref_log_details)
{
- struct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *));
+ struct strbuf ref_name = STRBUF_INIT;
+ struct strbuf err = STRBUF_INIT;
+ struct ref_transaction *transaction = NULL;
unsigned char *sha1 = xmalloc(targets * 20);
- char *msg;
- int ret;
+ char *msg = NULL;
int i;
save_commit_buffer = 0;
- for (i = 0; i targets; i++) {
- if (!write_ref || !write_ref[i])
- continue;
-
- lock[i] = lock_ref_sha1(write_ref[i], NULL);
- if (!lock[i]) {
- error(Can't lock ref %s, write_ref[i]);
- goto unlock_and_fail;
+ if (write_ref) {
+ transaction = ref_transaction_begin(err);
+ if (!transaction) {
+ error(%s, err.buf);
+ goto rollback_and_fail;
}
}
-
Is there some reason why the transaction cannot be built up during a
single iteration over targets, thereby also avoiding the need for the
sha1[20*i] stuff? This seems like exactly the kind of situation where
transactions should *save* code. But perhaps I've overlooked a
dependency between the two loops.
if (!walker-get_recover)
for_each_ref(mark_complete, NULL);
for (i = 0; i targets; i++) {
if (interpret_target(walker, target[i], sha1[20 * i])) {
error(Could not interpret response from server '%s' as
something to pull, target[i]);
- goto unlock_and_fail;
+ goto rollback_and_fail;
}
if (process(walker, lookup_unknown_object(sha1[20 * i])))
- goto unlock_and_fail;
+ goto rollback_and_fail;
}
if (loop(walker))
- goto unlock_and_fail;
+ goto rollback_and_fail;
if (write_ref_log_details) {
msg = xmalloc(strlen(write_ref_log_details) + 12);
@@ -294,19 +291,33 @@ int walker_fetch(struct walker *walker, int targets,
char **target,
for (i = 0; i targets; i++) {
if (!write_ref || !write_ref[i])
continue;
- ret = write_ref_sha1(lock[i], sha1[20 * i], msg ? msg : fetch
(unknown));
- lock[i] = NULL;
- if (ret)
- goto unlock_and_fail;
+ strbuf_reset(ref_name);
+ strbuf_addf(ref_name, refs/%s, write_ref[i]);
+ if (ref_transaction_update(transaction, ref_name.buf,
+sha1[20 * i], NULL, 0, 0,
+err)) {
+ error(%s, err.buf);
+ goto rollback_and_fail;
+ }
+ }
+ if (write_ref) {
+ if (ref_transaction_commit(transaction,
+msg ? msg : fetch (unknown),
+err)) {
+ error(%s, err.buf);
+ goto rollback_and_fail;
+ }
+ ref_transaction_free(transaction);
}
- free(msg);
+ free(msg);
return 0;
-unlock_and_fail:
- for (i = 0; i targets; i++)
- if (lock[i])
- unlock_ref(lock[i]);
+rollback_and_fail:
+ ref_transaction_free(transaction);
+ free(msg);
+ strbuf_release(err);
+ strbuf_release(ref_name);
return -1;
}
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to
Re: [PATCH v20 37/48] refs.c: remove lock_ref_sha1
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote: lock_ref_sha1 was only called from one place in refc.c and only provided a check that the refname was sane before adding back the initial refs/ part of the ref path name, the initial refs/ that this caller had already stripped off before calling lock_ref_sha1. [...] I'm especially glad to see this ugly function disappear! 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 v20 39/48] refs.c: make delete_ref use a transaction
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
Change delete_ref to use a ref transaction for the deletion. At the same time
since we no longer have any callers of repack_without_ref we can now delete
this function.
Change delete_ref to return 0 on success and 1 on failure instead of the
previous 0 on success either 1 or -1 on failure.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 34 +-
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/refs.c b/refs.c
index 3d070d5..92a06d4 100644
--- a/refs.c
+++ b/refs.c
@@ -2544,11 +2544,6 @@ int repack_without_refs(const char **refnames, int n,
struct strbuf *err)
return ret;
}
-static int repack_without_ref(const char *refname)
-{
- return repack_without_refs(refname, 1, NULL);
-}
-
static int delete_ref_loose(struct ref_lock *lock, int flag)
{
if (!(flag REF_ISPACKED) || flag REF_ISSYMREF) {
@@ -2566,24 +2561,21 @@ static int delete_ref_loose(struct ref_lock *lock,
int flag)
int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
{
- struct ref_lock *lock;
- int ret = 0, flag = 0;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
- lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
The old code checked that the old value of refname was sha1, regardless
of whether sha1 was null_sha1. Presumably callers never set sha1 to
null_sha1...
- if (!lock)
+ transaction = ref_transaction_begin(err);
+ if (!transaction ||
+ ref_transaction_delete(transaction, refname, sha1, delopt,
+sha1 !is_null_sha1(sha1), err) ||
...But the new code explicitly skips the check if sha1 is null_sha1.
This shouldn't make a practical difference, because presumably callers
never set sha1 to null_sha1. But given that the new policy elsewhere
for delete updates is that it is an error for old_sha1 to equal
null_sha1, it seems to me that this extra check shouldn't be here. So I
think this should be changed to
ref_transaction_delete(transaction, refname, sha1, delopt,
!!sha1, err) ||
[...]
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 v20 40/48] refs.c: add an err argument to delete_ref_loose
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.
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
Re: [PATCH v20 41/48] refs.c: pass the ref log message to _create/delete/update instead of _commit
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote: 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 --- [...] diff --git a/refs.h b/refs.h index 3b321c2..f24b2c1 100644 --- a/refs.h +++ b/refs.h Would you please document the msg parameter in the block comment that precedes these three declarations? Especially important is the fact that the functions make internal copies of msg, so the caller retains ownership of its copy. You might also mention what happens if msg is NULL (which, as far as I can see, is that a reflog entry is created anyway (except in the case of a delete) but that the entry doesn't contain an explanation). @@ -297,7 +297,7 @@ int ref_transaction_update(struct ref_transaction *transaction, const char *refname, const unsigned char *new_sha1, const unsigned char *old_sha1, -int flags, int have_old, +int flags, int have_old, const char *msg, struct strbuf *err); /* @@ -312,7 +312,7 @@ int ref_transaction_update(struct ref_transaction *transaction, int ref_transaction_create(struct ref_transaction *transaction, const char *refname, const unsigned char *new_sha1, -int flags, +int flags, const char *msg, struct strbuf *err); It is noteworthy that ref_transaction_delete() accepts a msg parameter, even though we currently delete a reference's entire reflog when the reference is deleted. I prefer to think of this as a shortcoming of the current reference backend, from which future backends hopefully will not suffer. So I like this design choice. However, I think it is worth noting this dichotomy in the commit message and perhaps also in the function documentation. /* @@ -326,7 +326,7 @@ int ref_transaction_create(struct ref_transaction *transaction, int ref_transaction_delete(struct ref_transaction *transaction, const char *refname, const unsigned char *old_sha1, -int flags, int have_old, +int flags, int have_old, const char *msg, struct strbuf *err); /* @@ -335,7 +335,7 @@ int ref_transaction_delete(struct ref_transaction *transaction, * problem. */ int ref_transaction_commit(struct ref_transaction *transaction, -const char *msg, struct strbuf *err); +struct strbuf *err); /* * Free an existing transaction and all associated data. [...] Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v20 43/48] refs.c: move the check for valid refname to lock_ref_sha1_basic
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
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
s/badly name/bad name,/
any codepaths that tries to open and repair badly named refs. The normal refs
s/tries/try/
API should not allow neither creating nor accessing refs with invalid names.
s/not allow neither/allow neither/
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.
I like the sentiment, but in the real world I'm not sure we can take
such a step based only on good intentions. Which callers would be
affected? Where is this git fsck code that would be needed to help
people rescue their repos?
I can also imagine that we will tighten up the check_refname_format
checks in the future; for example, I think it would be a good idea to
prohibit reference names that start with '-' because it is almost
impossible to work with them (their names look like command-line
options). If we ever make a change like that, we will need some amount
of tolerance in git versions around the transition.
So...I like the idea of enforcing refname checks at the lowest level
possible, but I think that the change you propose is too abrupt. I
think it needs either more careful analysis showing that it won't hurt
anybody, or some kind of tooling or non-strict mode that people can use
to fix their repositories.
Michael
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 389a55f..bccf8c3 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);
}
--
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 v20 44/48] refs.c: call lock_ref_sha1_basic directly from commit
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
Skip using the lock_any_ref_for_update wrapper and call lock_ref_sha1_basic
directly from the commit function.
This commit is obviously hostage to whether commit 43/48 is kept.
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 bccf8c3..f3fab37 100644
--- a/refs.c
+++ b/refs.c
@@ -3598,12 +3598,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'.,
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] http: Add Accept-Language header if possible
From: Yi EungJun eungjun...@navercorp.com
Add an Accept-Language header which indicates the user's preferred
languages defined by 'LANGUAGE' environment variable if the variable is
not empty.
Example:
LANGUAGE= -
LANGUAGE=ko - Accept-Language: ko; q=1.000, *; q=0.001
LANGUAGE=ko:en - Accept-Language: ko; q=1.000, en; q=0.999, *; q=0.001
This gives git servers a chance to display remote error messages in
the user's preferred language.
---
http.c | 43 +++
t/t5550-http-fetch-dumb.sh | 10 ++
2 files changed, 53 insertions(+)
diff --git a/http.c b/http.c
index 3a28b21..c345616 100644
--- a/http.c
+++ b/http.c
@@ -983,6 +983,47 @@ static void extract_content_type(struct strbuf *raw,
struct strbuf *type,
strbuf_addstr(charset, ISO-8859-1);
}
+/*
+ * Add an Accept-Language header which indicates user's preferred languages
+ * defined by 'LANGUAGE' environment variable if the variable is not empty.
+ *
+ * Example:
+ * LANGUAGE= -
+ * LANGUAGE=ko - Accept-Language: ko; q=1.000, *; q=0.001
+ * LANGUAGE=ko:en - Accept-Language: ko; q=1.000, en; q=0.999, *; q=0.001
+ */
+static void add_accept_language(struct strbuf *buf)
+{
+ const char *p1, *p2;
+ float q = 1.000;
+
+ p1 = getenv(LANGUAGE);
+
+ if (p1 != NULL p1[0] != '\0') {
+ strbuf_reset(buf);
+ strbuf_addstr(buf, Accept-Language: );
+ for (p2 = p1; q 0.001; p2++) {
+ if ((*p2 == ':' || *p2 == '\0') p1 != p2) {
+ if (q 1.0) {
+ strbuf_addstr(buf, , );
+ }
+ strbuf_add(buf, p1, p2 - p1);
+ strbuf_addf(buf, ; q=%.3f, q);
+ q -= 0.001;
+ p1 = p2 + 1;
+
+ if (*p2 == '\0') {
+ break;
+ }
+ }
+ }
+ if (q 1.0) {
+ strbuf_addstr(buf, , );
+ }
+ strbuf_addstr(buf, *; q=0.001\r\n);
+ }
+}
+
/* http_request() targets */
#define HTTP_REQUEST_STRBUF0
#define HTTP_REQUEST_FILE 1
@@ -1020,6 +1061,8 @@ static int http_request(const char *url,
fwrite_buffer);
}
+ add_accept_language(buf);
+
strbuf_addstr(buf, Pragma:);
if (options options-no_cache)
strbuf_addstr(buf, no-cache);
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index ac71418..ea15158 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -196,5 +196,15 @@ test_expect_success 'reencoding is robust to whitespace
oddities' '
grep this is the error message stderr
'
+test_expect_success 'git client sends Accept-Language' '
+ GIT_CURL_VERBOSE=1 LANGUAGE=ko:en git clone
$HTTPD_URL/accept/language 2actual
+ grep ^Accept-Language: ko; q=1.000, en; q=0.999, \*; q=0.001 actual
+'
+
+test_expect_success 'git client does not send Accept-Language' '
+ GIT_CURL_VERBOSE=1 LANGUAGE= git clone $HTTPD_URL/accept/language
2actual
+ test_must_fail grep ^Accept-Language: actual
+'
+
stop_httpd
test_done
--
2.0.1.473.gafdefd9.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] diff-tree: call free_commit_list() instead of duplicating its code
Signed-off-by: Rene Scharfe l@web.de
---
builtin/diff-tree.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index be6417d..ce0e019 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -22,14 +22,10 @@ static int stdin_diff_commit(struct commit *commit, char
*line, int len)
if (isspace(line[40]) !get_sha1_hex(line+41, sha1)) {
/* Graft the fake parents locally to the commit */
int pos = 41;
- struct commit_list **pptr, *parents;
+ struct commit_list **pptr;
/* Free the real parent list */
- for (parents = commit-parents; parents; ) {
- struct commit_list *tmp = parents-next;
- free(parents);
- parents = tmp;
- }
+ free_commit_list(commit-parents);
commit-parents = NULL;
pptr = (commit-parents);
while (line[pos] !get_sha1_hex(line + pos, sha1)) {
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] line-log: use commit_list_append() instead of duplicating its code
Signed-off-by: Rene Scharfe l@web.de --- line-log.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/line-log.c b/line-log.c index 1500101..afcc98d 100644 --- a/line-log.c +++ b/line-log.c @@ -1174,9 +1174,7 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm */ add_line_range(rev, parents[i], cand[i]); clear_commit_line_range(rev, commit); - commit-parents = xmalloc(sizeof(struct commit_list)); - commit-parents-item = parents[i]; - commit-parents-next = NULL; + commit_list_append(parents[i], commit-parents); free(parents); free(cand); free_diffqueues(nparents, diffqueues); -- 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 v20 00/48] Use ref transactions
On 06/20/2014 04:42 PM, Ronnie Sahlberg wrote: This patch series can also be found at https://github.com/rsahlberg/git/tree/ref-transactions This patch series is based on current master and expands on the transaction API. It converts all ref updates, inside refs.c as well as external, to use the transaction API for updates. This makes most of the ref updates to become atomic when there are failures locking or writing to a ref. This version completes the work to convert all ref updates to use transactions. Now that all updates are through transactions I will start working on cleaning up the reading of refs and to create an api for managing reflogs but all that will go in a different patch series. Version 20: - Whitespace and style changes suggested by Jun. I spent most of the day on reviewing this patch series, but now I'm out of time again. Here is a summary from my point of view: Patches 01-19 -- ACK mhagger Patches 20-42 -- I sent various comments, small to large, concerning these patches Patch 43 -- Needs more justification if it is to be acceptable Patch 44 -- Depends on 43 Patches 45-48 -- I didn't quite get to these, but... Perhaps it would be more appropriate for the rules about reference name conflicts to be enforced by the backend, since it is the limitations of the current backend that impose the restrictions. Would that make sense? On the other hand, removing the restrictions isn't simply a matter of picking a different backend, because all Git repositories have to be able to interact with each other. So, I don't yet have a considered opinion on the matter. I think it would be good to try to merge the first part of this patch series to lock in some progress while we continue iterating on the remainder. I'm satisfied that it is all going in the right direction and I am thankful to Ronnie for pushing it forward. But handling 48-patch series is very daunting and I would welcome a split. I'm not sure whether patches 01-19 are necessarily the right split between merge-now/iterate-more; it is more or less an accident that I stopped after patch 19 on an earlier review. Maybe Ronnie could propose a logical subset of the commits as being ready to be merged to next in the nearish term? 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 v4 4/4] cache-tree: Write updated cache-tree after commit
Duy Nguyen pclo...@gmail.com writes: I wonder if we need to update_main_cache_tree() so many times in this function. If I read the code correctly, all roads must lead to update_main_cache_tree(0) in prepare_to_commit(). I think prepare-to-commit is too late; it does not want to know if the index it was given to create a tree out of is the one that the user will keep using after this invocation of git commit or just a temporary one used for partial commit. The cache-tree rebuilt there is what is recorded with commit_tree_extended() in cmd_commit(), but if you are doing a partial commit, these generic code paths are given a temporary index file on the filesystem to work on, and cache-tree in that will not help user's later operation. For three main uses of git commit, prepare_index() does these: - git commit -a and git commit -i $paths update the index with the new contents from the working tree, fully builds cache-tree in-core to write out the tree objects, and writes the index file to the filesystem. Because this index is the one used after this invocation of git commit returns, we have a fully populated cache-tree after this happens. This code path is perfect and there is no need to change. - git commit commits the contents of the index as-is, so technically there is no reason for it to update the index on the filesystem at all, but it refreshes the cached stat data to help the status part of the command, and if it finds that stat data was stale, updates the index on the filesystem because it is wasteful not to do so. As we would be spending I/O cycles to update the index file in that case anyway, we also rebuild the cache-tree and include that in the updated index. When the cached stat data was already up-to-date, however, we do not update the index on the filesystem, so the series under discussion will change the trade-off by doing one more I/O to write out a new index to the filesystem only to update cache-tree. - git commit $paths updates the real index with given $paths and writes it out to the filesystem first. This is the index the user will use after git commit finishes; traditionally our trade-off was populate cache-tree only when we do not have to spend any cycle only to do so (i.e. when we are writing trees anyway, or when we read from a tree), and let it degrade as paths are added, removed and modified and we avoided populating cache-tree in this codepath. The series under discussion will change the trade-off here, too. After it updates this real index, it builds another temporary index that represents the tree state to be committed (starting from HEAD and updates with the given $paths), but that will be discarded and we do not want to spend any extra cycle to do anything only to make its later use more efficient (like writing updated cache-tree to it). If we find out that we have incomplete cache-tree before that call, we could write the index one more time after that call, and that will make an extra I/O only to write out cache-tree to the temporary index that we will discard immediately after for a partial commit. -- 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 v5 3/4] cache-tree: subdirectory tests
David Turner dtur...@twopensource.com writes:
+ printf invalid %s ()\n $@
expect
+ test-dump-cache-tree | \
+ sed -n -e s/$_x40/SHA/ -e s/[0-9]* subtrees//g -e '/#(ref)/d' -e
'/^invalid /p' actual
You only show lines that begin with invalid . Does the first
redact any object name to S H A matter? Also do more than one N
subtrees appear on an output line?
+ test_cmp expect actual
}
test_no_cache_tree () {
@@ -49,6 +50,25 @@ test_expect_success 'git-add invalidates cache-tree' '
test_invalid_cache_tree
'
+test_expect_success 'git-add in subdir invalidates cache-tree' '
+ test_when_finished git reset --hard; git read-tree HEAD
+ mkdir dirx
+ echo I changed this file dirx/foo
+ git add dirx/foo
+ test_invalid_cache_tree
+'
+
+test_expect_success 'git-add in subdir does not invalidate sibling
cache-tree' '
+ git tag no-children
+ test_when_finished git reset --hard no-children; git read-tree HEAD
+ mkdir dir1 dir2
+ test_commit dir1/a
+ test_commit dir2/b
+ echo I changed this file dir1/a
+ git add dir1/a
+ test_invalid_cache_tree dir1/
+'
+
test_expect_success 'update-index invalidates cache-tree' '
test_when_finished git reset --hard; git read-tree HEAD
echo I changed this file foo
--
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 v5 4/4] cache-tree: Write updated cache-tree after commit
David Turner dtur...@twopensource.com writes:
@@ -16,8 +16,26 @@ cmp_cache_tree () {
# We don't bother with actually checking the SHA1:
# test-dump-cache-tree already verifies that all existing data is
# correct.
-test_shallow_cache_tree () {
- printf SHA (%d entries, 0 subtrees)\n $(git ls-files|wc -l) expect
+generate_expected_cache_tree () {
+ dir=$1${1:+/}
+ parent=$2
+ # ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
+ # We want to count only foo because it's the only direct child
+ subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq)
+ subtree_count=$(echo $subtrees|awk '$1 {++c} END {print c}')
+ entries=$(git ls-files|wc -l)
+ printf SHA $dir (%d entries, %d subtrees)\n $entries $subtree_count
+ for subtree in $subtrees
+ do
+ cd $subtree
+ generate_expected_cache_tree $dir$subtree $dir || return 1
+ cd ..
If the || return 1 ever triggers, would the main test process end up
in an unexpected place? A test piece executes test_cache_tree whose
control eventually reaches here and returns failure; the next test
piece will start at a wrong directory, no?
+ done
+ dir=$parent
+}
+
+test_cache_tree () {
+ generate_expected_cache_tree expect
cmp_cache_tree expect
}
@@ -33,14 +51,14 @@ test_no_cache_tree () {
cmp_cache_tree expect
}
-test_expect_failure 'initial commit has cache-tree' '
+test_expect_success 'initial commit has cache-tree' '
test_commit foo
- test_shallow_cache_tree
+ test_cache_tree
'
test_expect_success 'read-tree HEAD establishes cache-tree' '
git read-tree HEAD
- test_shallow_cache_tree
+ test_cache_tree
'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit
Duy Nguyen pclo...@gmail.com writes: ... I know the index_file in prepare_to_commit() is probably index.lock or something, but that does not stop us from locking again (index.lock.lock) if we want to update it. We grabbed the lock on the real index and we have written out the result of update-index --refresh to it (and closed), but we still want to and do keep the lock while add -i works on it. And then after add -i returns, we still have the lock on the real index and the patch wants to write to it again to store the refreshed cache-tree under that lock. It may be the case that the API suite currently lacks a way to allow the caller to reopen the same index.lock file after calling write-locked-index(CLOSE_LOCK), and taking a lock on index.lock to write into index.lock.lock and renaming it to index.lock could be a workaround for it, but doesn't that sound a wrong workaround? -- 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 00/48] Use ref transactions
Michael Haggerty mhag...@alum.mit.edu writes: Patches 01-19 -- ACK mhagger Patches 20-42 -- I sent various comments, small to large, concerning these patches Patch 43 -- Needs more justification if it is to be acceptable Patch 44 -- Depends on 43 Patches 45-48 -- I didn't quite get to these, but... Perhaps it would be more appropriate for the rules about reference name conflicts to be enforced by the backend, since it is the limitations of the current backend that impose the restrictions. Would that make sense? On the other hand, removing the restrictions isn't simply a matter of picking a different backend, because all Git repositories have to be able to interact with each other. I'd say that if you have foo/bar you cannot have foo may have started as an implementation limitation, but the interoperability requirement with existing versions of Git and with existing repositories makes it necessary to enforce it the same way as other rules such as you cannot have double-dots in name, e.g. foo..bar or no branches whose name begins with a dash, neither of which comes from any filesystem issues. That a rule can be loosened with one new backend does not at all mean it is a good idea to loosen it because we can in the first place. I think it would be good to try to merge the first part of this patch series to lock in some progress while we continue iterating on the remainder. I'm satisfied that it is all going in the right direction and I am thankful to Ronnie for pushing it forward. But handling 48-patch series is very daunting and I would welcome a split. I'm not sure whether patches 01-19 are necessarily the right split between merge-now/iterate-more; it is more or less an accident that I stopped after patch 19 on an earlier review. Maybe Ronnie could propose a logical subset of the commits as being ready to be merged to next in the nearish term? Yeah, thanks for going through this, and I agree that we would be better off merging the earlier part first. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit
Duy Nguyen pclo...@gmail.com writes:
Writing cache tree early in prepare_index() does help hooks, but I
would say hooks are uncommon case and we could add an option to
update-index to explicitly rebuild cache-tree, then hooks that do diff
a lot (or other operations that use cache-tree) could rebuild
cache-tree by themselves.
Yes, update-index --update-cache-tree would be a good addition for
completeness; scripts working with plumbing should be able to do
what built-in Porcelains can. They can of course do write-tree in
the meantime so I do not see it as a very high priority, though.
This should apply on top of 'master', and if the series under
discussion turns out to be a good idea, the new call to
update-main-cache-tree I added to this code path should use the
option added by the series that only repairs parts of cache-trees
that can be repaird without writing out new trees, so it is just to
give hints to future people (iow I am not going to apply this patch
myself right now).
builtin/update-index.c | 8
1 file changed, 8 insertions(+)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index ebea285..1ce2274 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -26,6 +26,7 @@ static int allow_remove;
static int allow_replace;
static int info_only;
static int force_remove;
+static int update_cache_tree;
static int verbose;
static int mark_valid_only;
static int mark_skip_worktree_only;
@@ -762,6 +763,8 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
OPT_BIT(0, unmerged, refresh_args.flags,
N_(refresh even if index contains unmerged entries),
REFRESH_UNMERGED),
+ OPT_BOOL(0, update-cache-tree, update_cache_tree,
+N_(update cache-tree before writing the result out)),
{OPTION_CALLBACK, 0, refresh, refresh_args, NULL,
N_(refresh stat information),
PARSE_OPT_NOARG | PARSE_OPT_NONEG,
@@ -918,6 +921,11 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
strbuf_release(buf);
}
+ if (update_cache_tree !unmerged_cache()) {
+ update_main_cache_tree(0);
+ active_cache_changed = 1; /* force write-out */
+ }
+
if (active_cache_changed) {
if (newfd 0) {
if (refresh_args.flags REFRESH_QUIET)
--
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] t/Makefile: always test all lint targets when running tests
Am 07.07.2014 20:13, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Only the two targets test-lint-duplicates and test-lint-executable are currently executed when running the test target. This was done on purpose when the TEST_LINT variable was added in 81127d74. But as this does not include the test-lint-shell-syntax target added the same day in commit c7ce70ac, it is easy to accidentally add non portable shell constructs without noticing that when running the test suite. I not running the lint-shell-syntax that is fundamentally flaky to avoid false positives is very much on purpose. The flakiness is not the fault of the implementor of the lint-shell-syntax, but comes from the approach taken to pretend that simple pattern matching can parse shell scripts. It may not complain on the current set of scripts, but that is not really by design but by accident. So I am not very enthusiastic to see this change myself. Ok, I understand we do not want to lightly risk false positives. I just noticed that I accidentally forgot to sign off this series, so I'd resend just the first patch with a proper SOB, ok? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] Add submodule test harness
Am 07.07.2014 21:40, schrieb Torsten Bögershausen: On 2014-07-07 19.05, Junio C Hamano wrote: Jens Lehmann jens.lehm...@web.de writes: Junio, do you want me to resend 02/14 without the non-portable echo -n or could you just squash the following diff in? Amended locally here already; thanks, both. There seems to be some other trouble under Mac OS, not yet fully tracked down, (may be related to the diff -r) Torsten sees failures of this kind under Mac OS: diff -r .git/modules/sub1/config sub1/.git/config 6d5 worktree = ../../../sub1 8a8 worktree = ../../../sub1 So the config contains the same content, but the worktree setting moved to a different line. This seems to be the result of setting core.worktree in the test_git_directory_is_unchanged function just before the diff -r, but only under Mac OS. And Msysgit complains error: fchmod on c:/xxxt/trash directory.t7613-merge-submodule/submodule_update_repo/.git/modules/sub1/config.lock failed: Function not implemented I'm not sure what this is about, seems to happen during the cp -R of the repo under .git/modules into the submodule. I'm currently investigating both issues (the next steps probably being to install msysgit and to do some Git hacking on a Mac in the family). -- 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 3/4] use new config API for worktree configurations of submodules
Heiko Voigt hvo...@hvoigt.net writes: diff --git a/builtin/checkout.c b/builtin/checkout.c index 07cf555..03ea20d 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -18,6 +18,7 @@ #include xdiff-interface.h #include ll-merge.h #include resolve-undo.h +#include submodule-config.h #include submodule.h #include argv-array.h Hmph. What is this change about? Nobody in checkout.c needs anything new, yet we add a new include? diff --git a/diff.c b/diff.c index f72769a..f692a3c 100644 --- a/diff.c +++ b/diff.c @@ -13,6 +13,7 @@ #include utf8.h #include userdiff.h #include sigchain.h +#include submodule-config.h #include submodule.h #include ll-merge.h #include string-list.h Likewise. It is somewhat unclear to me what real change that improves the life of end-users this series brings to us. The test-submodule-config test program obviously is new but that does not really count until we see real uses. -- 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/14] Add submodule test harness
On 08/07/14 20:34, Jens Lehmann wrote:
Am 07.07.2014 21:40, schrieb Torsten Bögershausen:
On 2014-07-07 19.05, Junio C Hamano wrote:
Jens Lehmann jens.lehm...@web.de writes:
Junio, do you want me to resend 02/14 without the non-portable echo -n
or could you just squash the following diff in?
Amended locally here already; thanks, both.
There seems to be some other trouble under Mac OS, not yet fully tracked
down,
(may be related to the diff -r)
Torsten sees failures of this kind under Mac OS:
diff -r .git/modules/sub1/config sub1/.git/config
6d5
worktree = ../../../sub1
8a8
worktree = ../../../sub1
So the config contains the same content, but the worktree setting moved
to a different line. This seems to be the result of setting core.worktree
in the test_git_directory_is_unchanged function just before the diff -r,
but only under Mac OS.
And Msysgit complains
error: fchmod on c:/xxxt/trash
directory.t7613-merge-submodule/submodule_update_repo/.git/modules/sub1/config.lock
failed: Function not implemented
I'm not sure what this is about, seems to happen during the cp -R of
the repo under .git/modules into the submodule.
I haven't looked into this at all, but from the above message, and
noting that fchmod() is not implemented in mingw (see compat/mingw.h
line 91), and the following:
$ git grep -n fchmod
compat/mingw.h:91:static inline int fchmod(int fildes, mode_t mode)
config.c:1639: if (fchmod(fd, st.st_mode 0) 0) {
config.c:1640: error(fchmod on %s failed: %s,
config.c:1818: if (fchmod(out_fd, st.st_mode 0) 0) {
config.c:1819: ret = error(fchmod on %s failed: %s,
$
[I happen to be on the pu branch at the moment, so YMMV!]
Both calls to fchmod() above are on config lock files, one
in git_config_set_multivar_in_file() and the other in
git_config_rename_section_in_file().
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 01/19] rebase -i: Failed reword prints redundant error message
Fabian Ruch baf...@gmail.com writes:
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. If the edited log
message is empty or is found ill-formatted by one of the commit
hooks, git-rebase--interactive prints three error messages to the
console.
1. The git-commit output, which contains all the output from hook
scripts.
2. A rebase diagnosis saying at which task on the to-do list it
got stuck.
3. Generic presumptions about what could have triggered the
error.
The third message contains redundant information and does not add any
enlightenment either, which makes the output unnecessarily longish
and different from the other command's output. For instance, this is
what the output looks like if the log message is empty (contains
duplicate Signed-off-by lines).
(1.) Aborting commit due to empty commit message. (Duplicate
Signed-off-by lines.)
(2.) Could not amend commit after successfully picking fa1afe1... Some
change
(3.) This is most likely due to an empty commit message, or the
pre-commit hook
failed. If the pre-commit hook failed, you may need to resolve the
issue before
you are able to reword the commit.
Discard the third message.
It is true that a failed hook script might not output any diagnosis...
I think the message originally came from 0becb3e4 (rebase -i:
interrupt rebase when commit --amend failed during reword,
2011-11-30); it seems that the primary point of the change was to
make sure it exits and the warning message may not have been well
thought out, but before discarding the result of somebody else's
work, it may not be a bad idea to ask just in case you may have
overlooked something (Andrew Wong Cc'ed).
but then the generic message is not of much help either. Since this
lack of information affects the built-in git commands for commit,
merge and cherry-pick first, the solution would be to keep track of
the failed hooks in their output so that the user knows which of her
hooks require improvement.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 3 ---
1 file changed, 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 7e1eda0..e733d7f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -506,9 +506,6 @@ do_next () {
do_pick $sha1 $rest
git commit --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
warn Could not amend commit after successfully picking
$sha1... $rest
- warn This is most likely due to an empty commit
message, or the pre-commit hook
- warn failed. If the pre-commit hook failed, you may
need to resolve the issue before
- warn you are able to reword the commit.
exit_with_patch $sha1 1
}
record_in_rewritten $sha1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 02/19] rebase -i: reword complains about empty commit despite --keep-empty
Fabian Ruch baf...@gmail.com writes:
Subject: rebase -i: reword complains about empty commit despite --keep-empty
I had to read the title and then the log twice before understanding
that the above is not change the complaint message (i.e. reword
complaints spelled incorrectly) but is a description of the current
behaviour (i.e. the command complains when 'reword' is used on an
empty commit) that is not accompanied by an evaluation (ok, it
complains; are you saying it is a good thing or a bad thing?) or an
action (if it is a bad thing, what are you doing about it?).
Perhaps
rebase -i: allow rewording an empty commit
or something?
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. If `--keep-empty` is
passed as option to git-rebase--interactive, empty commits ought to
be replayed without complaints. However, if the users chooses to
reword an empty commit by changing the respective to-do list entry
from
pick fa1afe1 Empty commit
to
reword fa1afe1 Empty commit
, then git-rebase--interactive suddenly fails to replay the empty
commit. This is especially counterintuitive because `reword` is
thought of as a `pick` that alters the log message in some way but
nothing more and the unchanged to-do list entry would not fail.
Handle `reword` by cherry-picking the named commit and editing the
log message using
git commit --allow-empty --amend
instead of
git commit --amend.
Add test.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh| 2 +-
t/t3404-rebase-interactive.sh | 8
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index e733d7f..689400e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -504,7 +504,7 @@ do_next () {
mark_action_done
do_pick $sha1 $rest
- git commit --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
+ git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
warn Could not amend commit after successfully picking
$sha1... $rest
exit_with_patch $sha1 1
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8197ed2..9931143 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -75,6 +75,14 @@ test_expect_success 'rebase --keep-empty' '
test_line_count = 6 actual
'
+test_expect_success 'rebase --keep-empty' '
+ git checkout emptybranch
+ set_fake_editor
+ FAKE_LINES=1 reword 2 git rebase --keep-empty -i HEAD~2
+ git log --oneline actual
+ test_line_count = 6 actual
+'
+
test_expect_success 'rebase -i with the exec command' '
git checkout master
(
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 03/19] rebase -i: reword executes pre-commit hook on interim commit
Fabian Ruch baf...@gmail.com writes:
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. This happens in two
steps. Firstly, the named commit is cherry-picked. Secondly, the
commit created in the first step is amended using an unchanged index
to edit the log message. The pre-commit hook is meant to verify the
changes introduced by a commit (for instance, catching inserted
trailing white space). Since the committed tree is not changed
between the two steps, do not execute the pre-commit hook in the
second step.
It is not like the second step is built as a child commit of the
result from the first step, recording the same tree without any
change. Why is it a good thing not to run the pre-commit hook (or
other hooks for that matter)? After all, the result of the second
step is what is recorded in the final history; it just feels wrong
not to test that one, even if it were a good idea to test only once.
Specify the git-commit option `--no-verify` to disable the pre-commit
hook when editing the log message. Because `--no-verify` also skips
the commit-msg hook, execute the hook from within
git-rebase--interactive after the commit is created. Fortunately, the
commit message is still available in `$GIT_DIR/COMMIT_EDITMSG` after
git-commit terminates. Caveat: In case the commit-msg hook finds the
new log message ill-formatted, the user is only notified of the
failed commit-msg hook but the log message is used for the commit
anyway. git-commit ought to offer more fine-grained control over
which hooks are executed.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 689400e..b50770d 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -504,10 +504,19 @@ do_next () {
mark_action_done
do_pick $sha1 $rest
- git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
- warn Could not amend commit after successfully picking
$sha1... $rest
- exit_with_patch $sha1 1
- }
+ # TODO: Work around the fact that git-commit lets us
+ # disable either both the pre-commit and the commit-msg
+ # hook or none. Disable the pre-commit hook because the
+ # tree is left unchanged but run the commit-msg hook
+ # from here because the log message is altered.
+ git commit --allow-empty --amend --no-post-rewrite -n
${gpg_sign_opt:+$gpg_sign_opt}
+ if test -x $GIT_DIR/hooks/commit-msg
+ then
+ $GIT_DIR/hooks/commit-msg
$GIT_DIR/COMMIT_EDITMSG
+ fi || {
+ warn Could not amend commit after successfully
picking $sha1... $rest
+ exit_with_patch $sha1 1
+ }
record_in_rewritten $sha1
;;
edit|e)
--
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] gitk: add keybinding to switch to parent commit
Signed-off-by: Max Kirillov m...@max630.net
---
Hi.
I was missing this one. Actually the most needed is go to first
parent, though the second also may be useful.
gitk | 12
1 file changed, 12 insertions(+)
diff --git a/gitk b/gitk
index 41e5071..de35fe4 100755
--- a/gitk
+++ b/gitk
@@ -2594,6 +2594,9 @@ proc makewindow {} {
bind $ctext $ctxbut {pop_diff_menu %W %X %Y %x %y}
bind $ctext Button-1 {focus %W}
bind $ctext Selection rehighlight_search_results
+for {set i 1} {$i10} {incr i} {
+ bind . $M1B-Key-$i [list go_to_parent $i]
+}
set maincursor [. cget -cursor]
set textcursor [$ctext cget -cursor]
@@ -3016,6 +3019,7 @@ proc keys {} {
[mc Down, n, j Move down one commit]
[mc Left, z, h Go back in history list]
[mc Right, x, l Go forward in history list]
+[mc %s-nGo to n-th parent of current commit in history list $M1T]
[mc PageUp Move up one page in commit list]
[mc PageDownMove down one page in commit list]
[mc %s-Home Scroll to top of commit list $M1T]
@@ -7494,6 +7498,14 @@ proc goforw {} {
}
}
+proc go_to_parent {i} {
+global parents curview targetid
+set ps $parents($curview,$targetid)
+if {[llength $ps] = $i} {
+ selbyid [lindex $ps [expr $i - 1]]
+}
+}
+
proc gettree {id} {
global treefilelist treeidlist diffids diffmergeid treepending
global nullid nullid2
--
2.0.0.526.g5318336
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 00/19] Enable options --signoff, --reset-author for pick, reword
Fabian Ruch baf...@gmail.com writes: Fabian Ruch (19): rebase -i: Failed reword prints redundant error message rebase -i: reword complains about empty commit despite --keep-empty rebase -i: reword executes pre-commit hook on interim commit rebase -i: Teach do_pick the option --edit rebase -i: Implement reword in terms of do_pick rebase -i: Stop on root commits with empty log messages rebase -i: The replay of root commits is not shown with --verbose rebase -i: Root commits are replayed with an unnecessary option rebase -i: Commit only once when rewriting picks rebase -i: Do not die in do_pick rebase -i: Teach do_pick the option --amend rebase -i: Teach do_pick the option --file rebase -i: Prepare for squash in terms of do_pick --amend rebase -i: Implement squash in terms of do_pick rebase -i: Explicitly distinguish replay commands and exec tasks rebase -i: Parse to-do list command line options rebase -i: Teach do_pick the option --reset-author rebase -i: Teach do_pick the option --signoff rebase -i: Enable options --signoff, --reset-author for pick, reword After rebase -i:, some begin with lowercase and many begin with capital, which makes the short-log output look distracting. -- 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/14] Add submodule test harness
On 08/07/14 21:25, Ramsay Jones wrote:
On 08/07/14 20:34, Jens Lehmann wrote:
Am 07.07.2014 21:40, schrieb Torsten Bögershausen:
On 2014-07-07 19.05, Junio C Hamano wrote:
Jens Lehmann jens.lehm...@web.de writes:
Junio, do you want me to resend 02/14 without the non-portable echo -n
or could you just squash the following diff in?
Amended locally here already; thanks, both.
There seems to be some other trouble under Mac OS, not yet fully tracked
down,
(may be related to the diff -r)
Torsten sees failures of this kind under Mac OS:
diff -r .git/modules/sub1/config sub1/.git/config
6d5
worktree = ../../../sub1
8a8
worktree = ../../../sub1
So the config contains the same content, but the worktree setting moved
to a different line. This seems to be the result of setting core.worktree
in the test_git_directory_is_unchanged function just before the diff -r,
but only under Mac OS.
And Msysgit complains
error: fchmod on c:/xxxt/trash
directory.t7613-merge-submodule/submodule_update_repo/.git/modules/sub1/config.lock
failed: Function not implemented
I'm not sure what this is about, seems to happen during the cp -R of
the repo under .git/modules into the submodule.
I haven't looked into this at all, but from the above message, and
noting that fchmod() is not implemented in mingw (see compat/mingw.h
line 91), and the following:
$ git grep -n fchmod
compat/mingw.h:91:static inline int fchmod(int fildes, mode_t mode)
config.c:1639: if (fchmod(fd, st.st_mode 0) 0) {
config.c:1640: error(fchmod on %s failed: %s,
config.c:1818: if (fchmod(out_fd, st.st_mode 0) 0) {
config.c:1819: ret = error(fchmod on %s failed: %s,
$
[I happen to be on the pu branch at the moment, so YMMV!]
Both calls to fchmod() above are on config lock files, one
in git_config_set_multivar_in_file() and the other in
git_config_rename_section_in_file().
See commit daa22c6f8 (config: preserve config file permissions
on edits, 06-05-2014).
ATB,
Ramsay Jones
--
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] log: fix indentation for --graph --show-signature
On Tue, Jul 8, 2014 at 7:12 AM, Zoltan Klinger zoltan.klin...@gmail.com wrote: The git log --graph --show-signature command incorrectly indents the gpg information about signed commits and merged signed tags. It does not follow the level of indentation of the current commit. Reported-by: Jason Pyeron jpye...@pdinc.us Signed-off-by: Zoltan Klinger zoltan.klin...@gmail.com --- diff --git a/t/t4202-log.sh b/t/t4202-log.sh index cb03d28..b429aff 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -3,6 +3,7 @@ test_description='git log' . ./test-lib.sh +. $TEST_DIRECTORY/lib-gpg.sh test_expect_success setup ' @@ -841,4 +842,32 @@ test_expect_success 'dotdot is a parent directory' ' test_cmp expect actual ' +test_expect_success GPG 'log --graph --show-signature' ' + git checkout -b signed master Do you want test_when_finished 'git reset --hard git checkout master' here in case of failure in this test in order to restore sanity for tests which might be added later? + echo foo foo + git add foo + git commit -S -m signed_commit + git log --graph --show-signature -n1 signed actual + grep ^| gpg: Signature made actual + grep ^| gpg: Good signature actual +' + +test_expect_success GPG 'log --graph --show-signature for merged tag' ' + git checkout -b plain master + echo aaa bar + git add bar + git commit -m bar_commit Broken -chain. + git checkout -b tagged master Ditto regarding test_when_finished. + echo bbb baz + git add baz + git commit -m baz_commit Broken -chain. + git tag -s -m signed_tag_msg signed_tag + git checkout plain + git merge --no-ff -m msg signed_tag + git log --graph --show-signature -n1 plain actual + grep ^|\\\ merged tag actual + grep ^| | gpg: Signature made actual + grep ^| | gpg: Good signature actual +' + test_done -- 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, #01; Tue, 8)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * bc/fix-rebase-merge-skip (2014-06-16) 1 commit (merged to 'next' on 2014-06-20 at 01f81f5) + rebase--merge: fix --skip with two conflicts in a row git rebase --skip did not work well when it stopped due to a conflict twice in a row. * dt/refs-check-refname-component-sse (2014-06-18) 1 commit (merged to 'next' on 2014-06-20 at d286027) + refs.c: SSE2 optimizations for check_refname_component Further micro-optimization of a leaf-function. * jk/commit-buffer-length (2014-06-13) 18 commits (merged to 'next' on 2014-06-16 at b2d2d7b) + reuse cached commit buffer when parsing signatures + commit: record buffer length in cache + commit: convert commit-buffer to a slab + commit-slab: provide a static initializer + use get_commit_buffer everywhere + convert logmsg_reencode to get_commit_buffer + use get_commit_buffer to avoid duplicate code + use get_cached_commit_buffer where appropriate + provide helpers to access the commit buffer + provide a helper to set the commit buffer + provide a helper to free commit buffer + sequencer: use logmsg_reencode in get_message + logmsg_reencode: return const buffer + do not create struct commit with xcalloc + commit: push commit_index update into alloc_commit_node + alloc: include any-object allocations in alloc_report + replace dangerous uses of strbuf_attach + commit_tree: take a pointer/len pair rather than a const strbuf Move commit-buffer out of the in-core commit object and keep track of their lengths. Use this to optimize the code paths to validate GPG signatures in commit objects. * ye/http-extract-charset (2014-06-17) 1 commit (merged to 'next' on 2014-06-20 at 9492bae) + http: fix charset detection of extract_content_type() -- [New Topics] * cc/replace-edit (2014-06-25) 3 commits - 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.) Will merge to 'next'. * ep/submodule-code-cleanup (2014-06-30) 1 commit - submodule.c: use the ARRAY_SIZE macro Will merge to 'next'. * jk/replace-edit-raw (2014-06-25) 1 commit - replace: add a --raw mode for --edit (this branch uses cc/replace-edit.) Will merge to 'next'. * jk/strip-suffix (2014-06-30) 9 commits - prepare_packed_git_one: refactor duplicate-pack check - verify-pack: use strbuf_strip_suffix - strbuf: implement strbuf_strip_suffix - index-pack: use strip_suffix to avoid magic numbers - use strip_suffix instead of ends_with in simple cases - replace has_extension with ends_with - implement ends_with via strip_suffix - add strip_suffix function - sha1_file: replace PATH_MAX buffer with strbuf in prepare_packed_git_one() Will merge to 'next'. * jk/tag-contains (2014-06-30) 8 commits - perf: add tests for tag --contains - tag: use commit_contains - commit: provide a fast multi-tip contains function - string-list: add pos to iterator callback - add functions for memory-efficient bitmaps - paint_down_to_common: use prio_queue - tag: factor out decision to stream tags - tag: allow --sort with -n Expecting a reroll. * mg/fix-log-mergetag-color (2014-06-30) 1 commit - log: correctly identify mergetag signature verification status Will merge to 'next'. * mk/merge-incomplete-files (2014-06-30) 2 commits - git-merge-file: do not add LF at EOF while applying unrelated change - t6023-merge-file.sh: fix and mark as broken invalid tests Will merge to 'next'. * rs/status-code-clean-up (2014-06-29) 2 commits (merged to 'next' on 2014-07-08 at db67965) + wt-status: simplify building of summary limit argument + wt-status: use argv_array for environment Will merge to 'master'. * tb/crlf-tests (2014-07-08) 2 commits (merged to 'next' on 2014-07-08 at 40764b7) + t0027: combinations of core.autocrlf, core.eol and text + t0025: rename the test files Will merge to 'master'. * ak/profile-feedback-build (2014-07-08) 4 commits - Fix profile feedback with -jN and add profile-fast - Run the perf test suite for profile feedback too - Don't define away __attribute__ on gcc - Use BASIC_FLAGS for profile feedback Will merge to 'next'. * cb/filter-branch-prune-empty-degenerate-merges (2014-07-01) 1 commit - filter-branch: eliminate duplicate mapped parents Will merge to 'next'. * cc/for-each-mergetag (2014-07-07) 1 commit - commit: add for_each_mergetag() (this branch is used by cc/replace-graft.) Will merge to 'next'. * dt/cache-tree-repair
Re: [PATCH] http: Add Accept-Language header if possible
On Tue, Jul 8, 2014 at 11:54 AM, Yi EungJun semtlen...@gmail.com wrote:
From: Yi EungJun eungjun...@navercorp.com
Add an Accept-Language header which indicates the user's preferred
languages defined by 'LANGUAGE' environment variable if the variable is
not empty.
Example:
LANGUAGE= -
LANGUAGE=ko - Accept-Language: ko; q=1.000, *; q=0.001
LANGUAGE=ko:en - Accept-Language: ko; q=1.000, en; q=0.999, *; q=0.001
This gives git servers a chance to display remote error messages in
the user's preferred language.
---
http.c | 43 +++
t/t5550-http-fetch-dumb.sh | 10 ++
2 files changed, 53 insertions(+)
diff --git a/http.c b/http.c
index 3a28b21..c345616 100644
--- a/http.c
+++ b/http.c
@@ -983,6 +983,47 @@ static void extract_content_type(struct strbuf *raw,
struct strbuf *type,
strbuf_addstr(charset, ISO-8859-1);
}
+/*
+ * Add an Accept-Language header which indicates user's preferred languages
+ * defined by 'LANGUAGE' environment variable if the variable is not empty.
+ *
+ * Example:
+ * LANGUAGE= -
+ * LANGUAGE=ko - Accept-Language: ko; q=1.000, *; q=0.001
+ * LANGUAGE=ko:en - Accept-Language: ko; q=1.000, en; q=0.999, *;
q=0.001
+ */
+static void add_accept_language(struct strbuf *buf)
+{
+ const char *p1, *p2;
+ float q = 1.000;
+
+ p1 = getenv(LANGUAGE);
+
+ if (p1 != NULL p1[0] != '\0') {
+ strbuf_reset(buf);
It seems wrong to clear 'buf' in a function named add_accept_language().
+ strbuf_addstr(buf, Accept-Language: );
+ for (p2 = p1; q 0.001; p2++) {
+ if ((*p2 == ':' || *p2 == '\0') p1 != p2) {
+ if (q 1.0) {
+ strbuf_addstr(buf, , );
+ }
+ strbuf_add(buf, p1, p2 - p1);
+ strbuf_addf(buf, ; q=%.3f, q);
+ q -= 0.001;
+ p1 = p2 + 1;
+
+ if (*p2 == '\0') {
+ break;
+ }
+ }
+ }
+ if (q 1.0) {
+ strbuf_addstr(buf, , );
+ }
+ strbuf_addstr(buf, *; q=0.001\r\n);
Manually adding \r\n is contraindicated. Headers passed to
curl_easy_setopt(c, CURLOPT_HTTPHEADER, headers) must not have \r\n,
since curl will add terminators itself [1].
[1]: http://curl.haxx.se/libcurl/c/CURLOPT_HTTPHEADER.html
+ }
+}
+
/* http_request() targets */
#define HTTP_REQUEST_STRBUF0
#define HTTP_REQUEST_FILE 1
@@ -1020,6 +1061,8 @@ static int http_request(const char *url,
fwrite_buffer);
}
+ add_accept_language(buf);
This is inconsistent with how other headers are handled by this
function. The existing idiom is:
strbuf_add(buf, ...); /* construct header */
headers = curl_slist_apend(headers, buf.buf);
strbuf_reset(buf);
+
strbuf_addstr(buf, Pragma:);
if (options options-no_cache)
strbuf_addstr(buf, no-cache);
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index ac71418..ea15158 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -196,5 +196,15 @@ test_expect_success 'reencoding is robust to whitespace
oddities' '
grep this is the error message stderr
'
+test_expect_success 'git client sends Accept-Language' '
+ GIT_CURL_VERBOSE=1 LANGUAGE=ko:en git clone
$HTTPD_URL/accept/language 2actual
Broken -chain.
+ grep ^Accept-Language: ko; q=1.000, en; q=0.999, \*; q=0.001 actual
Do you want to \-escape the periods? (Or maybe use 'grep -F'?)
+'
+
+test_expect_success 'git client does not send Accept-Language' '
+ GIT_CURL_VERBOSE=1 LANGUAGE= git clone $HTTPD_URL/accept/language
2actual
Broken -chain.
+ test_must_fail grep ^Accept-Language: actual
+'
+
stop_httpd
test_done
--
2.0.1.473.gafdefd9.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 RFC v2 06/19] rebase -i: Stop on root commits with empty log messages
Fabian Ruch baf...@gmail.com writes:
The command line used to recreate root commits specifies the
erroneous option `--allow-empty-message`. If the root commit has an
empty log message, the replay of this commit should fail and the
rebase should be interrupted like for any other commit that is on the
to-do list and has an empty commit message. Remove the option.
The option might have been introduced by copy-and-paste of the first
part of the command line which initializes the authorship of the
sentinel commit. Indeed, the sentinel commit has an empty log message
and this should not trigger a failure, which is why the option
`--allow-empty-message` is correctly specified here.
The first commit --amend uses -C $1 to give the amended result
not just the authorship but also the log message taken from $1.
If we are allowing a commit without any message to be used as $1,
I think --allow-empty-message needs to be there. If $1 requires
the option here, why doesn't the second one, that records the
updated tree with the metainformation taken from the same commit
$1 can successfully commit without the option?
Puzzled...
Add test.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 2 +-
t/t3412-rebase-root.sh | 39 +++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 4c875d5..0af96f2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -510,7 +510,7 @@ do_pick () {
git commit --allow-empty --allow-empty-message --amend \
--no-post-rewrite -n -q -C $1
pick_one -n $1
- git commit --allow-empty --allow-empty-message \
+ git commit --allow-empty \
--amend --no-post-rewrite -n -q -C $1 \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_with_patch $1 Could not apply $1... $2
diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh
index 0b52105..9867705 100755
--- a/t/t3412-rebase-root.sh
+++ b/t/t3412-rebase-root.sh
@@ -278,4 +278,43 @@ test_expect_success 'rebase -i -p --root with conflict
(second part)' '
test_cmp expect-conflict-p out
'
+test_expect_success 'stop rebase --root on empty root log message' '
+ # create a root commit with a non-empty tree so that rebase does
+ # not fail because of an empty commit, and an empty log message
+ echo root-commit file
+ git add file
+ tree=$(git write-tree)
+ root=$(git commit-tree $tree /dev/null)
+ git checkout -b no-message-root-commit $root
+ # do not ff because otherwise neither the patch nor the message
+ # are looked at and checked for emptiness
+ test_when_finished git rebase --abort
+ test_must_fail env EDITOR=true git rebase -i --force-rebase --root
+ echo root-commit file.expected
+ test_cmp file.expected file
+'
+
+test_expect_success 'stop rebase --root on empty child log message' '
+ # create a root commit with a non-empty tree and provide a log
+ # message so that rebase does not fail until the root commit is
+ # successfully replayed
+ echo root-commit file
+ git add file
+ tree=$(git write-tree)
+ root=$(git commit-tree $tree -m root-commit)
+ git checkout -b no-message-child-commit $root
+ # create a child commit with a non-empty patch so that rebase
+ # does not fail because of an empty commit, but an empty log
+ # message
+ echo child-commit file
+ git add file
+ git commit --allow-empty-message --no-edit
+ # do not ff because otherwise neither the patch nor the message
+ # are looked at and checked for emptiness
+ test_when_finished git rebase --abort
+ test_must_fail env EDITOR=true git rebase -i --force-rebase --root
+ echo child-commit file.expected
+ test_cmp file.expected file
+'
+
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 07/19] rebase -i: The replay of root commits is not shown with --verbose
Fabian Ruch baf...@gmail.com writes:
The command line used to recreate root commits specifies the
erroneous option `-q` which suppresses the commit summary message.
However, git-rebase--interactive tends to tell the user about the
commits it creates, if she wishes (cf. command line option
`--verbose`). The code parts handling non-root commits or squash
commits all output commit summary messages. Do not make the replay of
root commits an exception. Remove the option.
It is OK to suppress the commit summary when git-commit is used to
initialize the authorship of the sentinel commit because the
existence of this additional commit is a detail of
git-rebase--interactive's implementation. The option `-q` was
probably introduced as a copy-and-paste error stemming from that part
of the root commit handling code.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
This one I can buy; there is no reason to drop -q from both (which
would give us the same thing twice and the one before the pick_one
-n runs is not the final one anyway) and the later one that records
the updated tree would be the one to report what it did.
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0af96f2..ff04d5d 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -511,7 +511,7 @@ do_pick () {
--no-post-rewrite -n -q -C $1
pick_one -n $1
git commit --allow-empty \
---amend --no-post-rewrite -n -q -C $1 \
+--amend --no-post-rewrite -n -C $1 \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_with_patch $1 Could not apply $1... $2
else
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 08/19] rebase -i: Root commits are replayed with an unnecessary option
Fabian Ruch baf...@gmail.com writes:
The command line used to recreate root commits specifies the
effectless option `-C`. It is used to reuse commit message and
authorship from the named commit but the commit being amended here,
which is the sentinel commit, already carries the authorship and log
message of the processed commit. Note that the committer email and
commit date fields do not match the root commit either way. Remove
the option. However, `-C` (other than `-c`) does not invoke the
editor and the `--amend` option invokes it by default. Disable editor
invocation again by specifying `--no-edit`.
I'd say this is a no-value change, in the sense that it can be
written either way for the same effect and if the original were
written with --amend --no-edit then there would be no reason to
change it to -C $1 because such a change would also be equally a
no-value change.
What exactly are we gaining? Performance? Correctness?
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ff04d5d..17836d5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -511,7 +511,7 @@ do_pick () {
--no-post-rewrite -n -q -C $1
pick_one -n $1
git commit --allow-empty \
---amend --no-post-rewrite -n -C $1 \
+--amend --no-post-rewrite -n --no-edit \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_with_patch $1 Could not apply $1... $2
else
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t5150-request-pull.sh fails on newest master in Debian
On 3 July 2014 23:55, Øyvind A. Holm su...@sunbase.org wrote: When compiling newest master (v2.0.1-472-g6f92e5f) on Debian 7.5 (64-bit), t5150-request-pull.sh fails when compiling with $ make configure $ ./configure --prefix=/usr/local/varprg/git.master.v2.0.1-472-g6f92e5f $ make prefix=/usr/local/varprg/git.master.v2.0.1-472-g6f92e5f $ cd t $ ./t5150-request-pull.sh FYI, t5150-request-pull.sh passes all tests now on newest master (v2.0.1-474-g72c7794) in Debian. There are two new commits on master since I wrote this, and the commit that makes things work again is 4602f1a (diff-tree: call free_commit_list() instead of duplicating its code). Reverting this commit brings the failure back. The whole thing is still a mystery to me, though. I can't see why this should have anything to do with the use of ./configure --prefix. I tested several variants with and without ./configure --prefix, all tests were run several times and were reproducible every time. Was this --prefix thing just a red herring, or is it linked to this in some way? Also, the only file this commit touches is builtin/diff-tree.c, and this file hasn't been modified since 2011. Does anyone know what's going on here? Cheers, Øyvind -- 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: t5150-request-pull.sh fails on newest master in Debian
On Wed, 2014-07-09 at 02:52 +0200, Øyvind A. Holm wrote: On 3 July 2014 23:55, Øyvind A. Holm su...@sunbase.org wrote: When compiling newest master (v2.0.1-472-g6f92e5f) on Debian 7.5 (64-bit), t5150-request-pull.sh fails when compiling with $ make configure $ ./configure --prefix=/usr/local/varprg/git.master.v2.0.1-472-g6f92e5f $ make prefix=/usr/local/varprg/git.master.v2.0.1-472-g6f92e5f $ cd t $ ./t5150-request-pull.sh FYI, t5150-request-pull.sh passes all tests now on newest master (v2.0.1-474-g72c7794) in Debian. There are two new commits on master since I wrote this, and the commit that makes things work again is 4602f1a (diff-tree: call free_commit_list() instead of duplicating its code). Reverting this commit brings the failure back. The whole thing is still a mystery to me, though. I can't see why this should have anything to do with the use of ./configure --prefix. The problem only happens when a ref with an allowed wildcard winds up on a page boundary (with the wildcard before the page boundary). This depends intricately on the details of memory allocation, so pretty much anything could make it come and go. Does the fix I posted work for you? If not, let me know and I'll look into it more. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] cache-tree: Write updated cache-tree after commit
On Wed, Jul 9, 2014 at 12:05 AM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: I wonder if we need to update_main_cache_tree() so many times in this function. If I read the code correctly, all roads must lead to update_main_cache_tree(0) in prepare_to_commit(). I think prepare-to-commit is too late; it does not want to know if the index it was given to create a tree out of is the one that the user will keep using after this invocation of git commit or just a temporary one used for partial commit. The cache-tree rebuilt there is what is recorded with commit_tree_extended() in cmd_commit(), but if you are doing a partial commit, these generic code paths are given a temporary index file on the filesystem to work on, and cache-tree in that will not help user's later operation. Right. Thanks for checking. -- 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
[PATCH v2] log: fix indentation for --graph --show-signature
The git log --graph --show-signature command incorrectly indents the gpg information about signed commits and merged signed tags. It does not follow the level of indentation of the current commit. Example of garbled output: $ git log --show-signature --graph * commit 258e0a237cb69aaa587b0a4fb528bb0316b1b776 |\ gpg: Signature made Mon, Jun 30, 2014 13:22:33 EDT using RSA key ID DA08 gpg: Good signature from Jason Pyeron jpye...@pdinc.us Merge: 727c355 1ca13ed | | Author: Jason Pyeron jpye...@pdinc.us | | Date: Mon Jun 30 13:22:29 2014 -0400 | | | | Merge of 1ca13ed2271d60ba9 branch - rebranding | | | * commit 1ca13ed2271d60ba93d40bcc8db17ced8545f172 | | gpg: Signature made Mon, Jun 23, 2014 9:45:47 EDT using RSA key ID DD37 gpg: Good signature from Stephen Robert Guglielmo s...@guglielmo.us gpg: aka Stephen Robert Guglielmo srguglie...@gmail.com Author: Stephen R Guglielmo s...@guglielmo.us | | Date: Mon Jun 23 09:45:27 2014 -0400 | | | | Minor URL updates In log-tree.c modify show_sig_lines() function to call graph_show_oneline() after each line of gpg information it has printed in order to preserve the level of indentation for the next output line. Reported-by: Jason Pyeron jpye...@pdinc.us Signed-off-by: Zoltan Klinger zoltan.klin...@gmail.com --- Changes since v1: t/t4202-log.sh file: * fix broken -chain in test cases * add test_when_finished scripts to test cases to reset things to master branch log-tree.c | 1 + t/t4202-log.sh | 31 +++ 2 files changed, 32 insertions(+) diff --git a/log-tree.c b/log-tree.c index 10e6844..f13b861 100644 --- a/log-tree.c +++ b/log-tree.c @@ -365,6 +365,7 @@ static void show_sig_lines(struct rev_info *opt, int status, const char *bol) eol = strchrnul(bol, '\n'); printf(%s%.*s%s%s, color, (int)(eol - bol), bol, reset, *eol ? \n : ); + graph_show_oneline(opt-graph); bol = (*eol) ? (eol + 1) : eol; } } diff --git a/t/t4202-log.sh b/t/t4202-log.sh index cb03d28..99ab7ca 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -3,6 +3,7 @@ test_description='git log' . ./test-lib.sh +. $TEST_DIRECTORY/lib-gpg.sh test_expect_success setup ' @@ -841,4 +842,34 @@ test_expect_success 'dotdot is a parent directory' ' test_cmp expect actual ' +test_expect_success GPG 'log --graph --show-signature' ' + test_when_finished git reset --hard git checkout master + git checkout -b signed master + echo foo foo + git add foo + git commit -S -m signed_commit + git log --graph --show-signature -n1 signed actual + grep ^| gpg: Signature made actual + grep ^| gpg: Good signature actual +' + +test_expect_success GPG 'log --graph --show-signature for merged tag' ' + test_when_finished git reset --hard git checkout master + git checkout -b plain master + echo aaa bar + git add bar + git commit -m bar_commit + git checkout -b tagged master + echo bbb baz + git add baz + git commit -m baz_commit + git tag -s -m signed_tag_msg signed_tag + git checkout plain + git merge --no-ff -m msg signed_tag + git log --graph --show-signature -n1 plain actual + grep ^|\\\ merged tag actual + grep ^| | gpg: Signature made actual + grep ^| | gpg: Good signature actual +' + test_done -- 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 v20 00/48] Use ref transactions
On Tue, Jul 08, 2014 at 11:48:06AM -0700, Junio C Hamano wrote: I'd say that if you have foo/bar you cannot have foo may have started as an implementation limitation, but the interoperability requirement with existing versions of Git and with existing repositories makes it necessary to enforce it the same way as other rules such as you cannot have double-dots in name, e.g. foo..bar or no branches whose name begins with a dash, neither of which comes from any filesystem issues. That a rule can be loosened with one new backend does not at all mean it is a good idea to loosen it because we can in the first place. To me it makes sense to to have it as an option, for two reasons: 1. If you want to pay the compatibility cost (e.g., because you work with a defined set of users on a small project and can dictate how they set their git options), you get the extra feature. 2. It provides a migration path if we eventually want to move to a default backend that allows it. I admit that I don't care _too_ much, though. My main desire for it is not to store two live branches that overlap, but to store reflogs for deleted branches without conflicting with live branches. And of course all of this is getting rather ahead of ourselves. We do not have _any_ alternate backends yet, nor even yet the infrastructure to make them. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] http: Add Accept-Language header if possible
On Wed, Jul 09, 2014 at 12:54:06AM +0900, Yi EungJun wrote: From: Yi EungJun eungjun...@navercorp.com Add an Accept-Language header which indicates the user's preferred languages defined by 'LANGUAGE' environment variable if the variable is not empty. Example: LANGUAGE= - LANGUAGE=ko - Accept-Language: ko; q=1.000, *; q=0.001 LANGUAGE=ko:en - Accept-Language: ko; q=1.000, en; q=0.999, *; q=0.001 This gives git servers a chance to display remote error messages in the user's preferred language. Should this also take into account other language-related variables? I'd think $LC_ALL, $LC_MESSAGES, and $LANG would affect it, too. Are colon-separated values a standard in $LANGUAGE? I have never seen them, but I admit I am not very knowledgeable about localization issues. Also, we do we need to do more parsing? My $LANG is set to en_US.UTF-8. The encoding part is presumably uninteresting to the remote server. I also wonder if there are support functions in libc or as part of gettext that can help us get these values. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Branch list by date
On Mon, Jul 07, 2014 at 09:54:55PM -0700, Jeremy Apthorp wrote: I write this missive with dual purpose: firstly to share a potentially useful tool, and secondly to suggest that this feature (with a less mind-wrenchingly disgusting implementation) might be included in mainline git, as for example `git branch [-t] | [--by-time]`. I think what we should aim for is: 1. Teaching git-branch the same sorting as for-each-ref. So first --sort, and then possibly -t as an alias for --sort=committerdate. 2. Teaching git-branch custom output formats. We have -v and -vv, but it should support the full power of for-each-ref's --format atoms. 3. Teach branch and for-each-ref to support readable colors in their formats, like we do for log --format. 4. Optionally add config options to configure defaults for the above, so git branch shows what you want. I'm (slowly) working on a refactor that will unify for-each-ref and branch, which would accomplish (1) and (2). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] t/Makefile: always test all lint targets when running tests
On Mon, Jul 07, 2014 at 11:13:11AM -0700, Junio C Hamano wrote: Jens Lehmann jens.lehm...@web.de writes: Only the two targets test-lint-duplicates and test-lint-executable are currently executed when running the test target. This was done on purpose when the TEST_LINT variable was added in 81127d74. But as this does not include the test-lint-shell-syntax target added the same day in commit c7ce70ac, it is easy to accidentally add non portable shell constructs without noticing that when running the test suite. I not running the lint-shell-syntax that is fundamentally flaky to avoid false positives is very much on purpose. The flakiness is not the fault of the implementor of the lint-shell-syntax, but comes from the approach taken to pretend that simple pattern matching can parse shell scripts. It may not complain on the current set of scripts, but that is not really by design but by accident. So I am not very enthusiastic to see this change myself. Let me play devil's advocate for a moment. Is lint-shell-syntax in fact flaky? I know we discussed false positives when it was originally added, but I think the current implementation tries hard to avoid them. Given that it provides no false positives on the current code base (without many people running it), it seems likely to stay that way. And the cost if we are wrong is either fixing the tool or disabling it (so worst case we are back where we started, modulo a little effort to enable it and then revert). What do we gain? We have an extra line of defense that helps newer shell script writers fix their bugs before they make it to the list. That catches more bugs, and reduces effort for reviewers. And it is exactly these newer shell script writers that need the default flipped; they do not know about portability and the lint target in the first place. I dunno. I am not that enthusiastic about the change, either, but I tend to think it will probably not hurt, and may help. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] dir: remove PATH_MAX limitation
On Sat, Jul 05, 2014 at 12:42:29AM +0200, Karsten Blees wrote: Note: this fix just 'abuses' strbuf as string allocator, len is always 0. prep_exclude() can probably be simplified using more strbuf APIs. Hrm. It looks like you grow it and add some data, but really don't want the length to expand (because the caller depends on it). In other directory-traversal code we follow a pattern like: size_t prefix_len = dir-base.len; strbuf_add(dir-base, cp, stk-baselen - current); /* use full path in dir-base, then pop */ strbuf_setlen(dir-base, stk-baselen); That makes it a little more obvious that the memcpy matches the strbuf_grow (because it all happens inside strbuf_add). Is it possible to do something like that here? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] t/Makefile: always test all lint targets when running tests
On Tue, Jul 8, 2014 at 12:24 PM, Jens Lehmann jens.lehm...@web.de wrote: Am 07.07.2014 20:13, schrieb Junio C Hamano: So I am not very enthusiastic to see this change myself. Ok, I understand we do not want to lightly risk false positives. I just noticed that I accidentally forgot to sign off this series, so I'd resend just the first patch with a proper SOB, ok? Nah, let's do both and how it plays out. My not being very enthusiastic myself does not necessarily mean that it is bad for the project. Maybe most people like it and if I cannot bear with it I can always turn it off myself for my environment. I just have a strange feeling that we may be seeing some twisted shell script updates and when the author gets asked why it was written in such a strange way, the answer might turn out to be just to work around the false positive from the test-lint, which I would not want to see. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] http: Add Accept-Language header if possible
2014-07-09 14:10 GMT+09:00 Jeff King p...@peff.net: On Wed, Jul 09, 2014 at 12:54:06AM +0900, Yi EungJun wrote: From: Yi EungJun eungjun...@navercorp.com Add an Accept-Language header which indicates the user's preferred languages defined by 'LANGUAGE' environment variable if the variable is not empty. Example: LANGUAGE= - LANGUAGE=ko - Accept-Language: ko; q=1.000, *; q=0.001 LANGUAGE=ko:en - Accept-Language: ko; q=1.000, en; q=0.999, *; q=0.001 This gives git servers a chance to display remote error messages in the user's preferred language. Should this also take into account other language-related variables? I'd think $LC_ALL, $LC_MESSAGES, and $LANG would affect it, too. Are colon-separated values a standard in $LANGUAGE? I have never seen them, but I admit I am not very knowledgeable about localization issues. Also, we do we need to do more parsing? My $LANG is set to en_US.UTF-8. The encoding part is presumably uninteresting to the remote server. I also wonder if there are support functions in libc or as part of gettext that can help us get these values. -Peff I agree with you. In fact, I tried to get user's preferred language in the same way as gettext. It has guess_category_value() to do that and the function is good enough because it considers $LANGUAGE, $LC_ALL, $LANG, and also system-dependent preferences. But the function does not seem a public API and I don't know how can I use the function in Git. So I chose to use $LANGUAGE only. -- 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
