* [PATCH] config: display key_delim for config --bool --get-regexp
From: Matthieu Moy @ 2011-10-10 12:54 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <201110101220.21890.brian.foster@maxim-ic.com>
The previous logic in show_config was to print the delimiter when the
value was set, but Boolean variables have an implicit value "true" when
they appear with no value in the config file. As a result, we got:
git_Config --get-regexp '.*\.Boolean' #1. Ok: example.boolean
git_Config --bool --get-regexp '.*\.Boolean' #2. NO: example.booleantrue
Fix this by defering the display of the separator until after the value
to display has been computed.
Reported-by: Brian Foster <brian.foster@maxim-ic.com>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
builtin/config.c | 20 +++++++++++++-------
t/t1300-repo-config.sh | 6 ++++++
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 0b4ecac..0315ad7 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -99,6 +99,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
const char *vptr = value;
int must_free_vptr = 0;
int dup_error = 0;
+ int must_print_delim = 0;
if (!use_key_regexp && strcmp(key_, key))
return 0;
@@ -109,10 +110,8 @@ static int show_config(const char *key_, const char *value_, void *cb)
return 0;
if (show_keys) {
- if (value_)
- printf("%s%c", key_, key_delim);
- else
- printf("%s", key_);
+ printf("%s", key_);
+ must_print_delim = 1;
}
if (seen && !do_all)
dup_error = 1;
@@ -130,16 +129,23 @@ static int show_config(const char *key_, const char *value_, void *cb)
} else if (types == TYPE_PATH) {
git_config_pathname(&vptr, key_, value_);
must_free_vptr = 1;
+ } else if (value_) {
+ vptr = value_;
+ } else {
+ /* Just show the key name */
+ vptr = "";
+ must_print_delim = 0;
}
- else
- vptr = value_?value_:"";
seen++;
if (dup_error) {
error("More than one value for the key %s: %s",
key_, vptr);
}
- else
+ else {
+ if (must_print_delim)
+ printf("%c", key_delim);
printf("%s%c", vptr, term);
+ }
if (must_free_vptr)
/* If vptr must be freed, it's a pointer to a
* dynamically allocated buffer, it's safe to cast to
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3e140c1..dffccf8 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -333,6 +333,12 @@ test_expect_success 'get-regexp variable with no value' \
'git config --get-regexp novalue > output &&
cmp output expect'
+echo 'novalue.variable true' > expect
+
+test_expect_success 'get-regexp --bool variable with no value' \
+ 'git config --bool --get-regexp novalue > output &&
+ cmp output expect'
+
echo 'emptyvalue.variable ' > expect
test_expect_success 'get-regexp variable with empty value' \
--
1.7.7.140.ge3099
^ permalink raw reply related
* Re: [BUG 1.7.6.1] `git config --bool --get-regexp' omits separating space... sometimes!
From: Matthieu Moy @ 2011-10-10 12:44 UTC (permalink / raw)
To: Brian Foster; +Cc: git mailing list
In-Reply-To: <201110101220.21890.brian.foster@maxim-ic.com>
Brian Foster <brian.foster@maxim-ic.com> writes:
> example.boolean
> example.booleantrue
There's a problem in
static int show_config(const char *key_, const char *value_, void *cb)
in builtin/config.c. Patch follows.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* [BUG 1.7.6.1] `git config --bool --get-regexp' omits separating space... sometimes!
From: Brian Foster @ 2011-10-10 10:20 UTC (permalink / raw)
To: git mailing list
Hello,
# Script to illustrate the problem:
rm -f Config
cat <<\EOF >Config
[Example]
Boolean
Other = yes
EOF
git_Config() { git config --file Config "$@"; }
git version
git_Config --get-regexp '.*\.Boolean' #1. ✓ Ok: example.boolean
git_Config --bool --get-regexp '.*\.Boolean' #2. ✗ NO: example.booleantrue
git_Config --get-regexp '.*\.Other' #3. ✓ Ok: example.other yes
git_Config --bool --get-regexp '.*\.Other' #4. ✓ Ok: example.other true
exit
# Output:
git version 1.7.6.1
example.boolean
example.booleantrue
example.other yes
example.other true
Case 2 omits the space between the key-name and (generated) value,
making the output difficult to parse/process. Without checking,
I assume --int (and friends) have a similar bug?
cheers!
-blf-
--
Brian FOSTER
Principal MTS, Software
Maxim Integrated Products (Microcontroller BU), formerly Innova Card
Web : http://www.maxim-ic.com/
^ permalink raw reply
* [PATCH v2 3/7] invalidate_ref_cache(): expose this function in refs API
From: Michael Haggerty @ 2011-10-10 8:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>
Make invalidate_ref_cache() an official part of the refs API. It is
currently a fact of life that code outside of refs.c mucks about with
references. This change gives such code a way of informing the refs
module that it should no longer trust its cache.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 2 +-
refs.h | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index 89b2a0e..fb46cf5 100644
--- a/refs.c
+++ b/refs.c
@@ -223,7 +223,7 @@ static struct cached_refs *get_cached_refs(const char *submodule)
return refs;
}
-static void invalidate_ref_cache(const char *submodule)
+void invalidate_ref_cache(const char *submodule)
{
clear_cached_refs(get_cached_refs(submodule));
}
diff --git a/refs.h b/refs.h
index 5de06e5..3ddc4e4 100644
--- a/refs.h
+++ b/refs.h
@@ -77,6 +77,14 @@ extern void unlock_ref(struct ref_lock *lock);
/** Writes sha1 into the ref specified by the lock. **/
extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);
+/*
+ * Invalidate the reference cache for the specified submodule. Use
+ * submodule=NULL to invalidate the cache for the main module. This
+ * function must be called if references are changed via a mechanism
+ * other than the refs API.
+ */
+extern void invalidate_ref_cache(const char *submodule);
+
/** Setup reflog before using. **/
int log_ref_setup(const char *ref_name, char *logfile, int bufsize);
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v2 7/7] clear_cached_refs(): inline function
From: Michael Haggerty @ 2011-10-10 8:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>
clear_cached_refs() was only called from one place, so inline it
there.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 01a5958..bf53189 100644
--- a/refs.c
+++ b/refs.c
@@ -191,12 +191,6 @@ static void clear_cached_loose_refs(struct cached_refs *refs)
refs->did_loose = 0;
}
-static void clear_cached_refs(struct cached_refs *refs)
-{
- clear_cached_packed_refs(refs);
- clear_cached_loose_refs(refs);
-}
-
static struct cached_refs *create_cached_refs(const char *submodule)
{
int len;
@@ -237,7 +231,9 @@ static struct cached_refs *get_cached_refs(const char *submodule)
void invalidate_ref_cache(const char *submodule)
{
- clear_cached_refs(get_cached_refs(submodule));
+ struct cached_refs *refs = get_cached_refs(submodule);
+ clear_cached_packed_refs(refs);
+ clear_cached_loose_refs(refs);
}
static struct ref_list *read_packed_refs(FILE *f)
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v2 6/7] write_ref_sha1(): only invalidate the loose ref cache
From: Michael Haggerty @ 2011-10-10 8:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>
Since write_ref_sha1() can only write loose refs and cannot write
symbolic refs, there is no need for it to invalidate the packed ref
cache.
Suggested by: Martin Fick <mfick@codeaurora.org>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index 6e480ad..01a5958 100644
--- a/refs.c
+++ b/refs.c
@@ -1519,7 +1519,7 @@ int write_ref_sha1(struct ref_lock *lock,
unlock_ref(lock);
return -1;
}
- invalidate_ref_cache(NULL);
+ clear_cached_loose_refs(get_cached_refs(NULL));
if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v2 5/7] clear_cached_refs(): extract two new functions
From: Michael Haggerty @ 2011-10-10 8:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>
Extract two new functions from clear_cached_refs():
clear_loose_ref_cache() and clear_packed_ref_cache().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 22 +++++++++++++++++-----
1 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 9f004ce..6e480ad 100644
--- a/refs.c
+++ b/refs.c
@@ -175,14 +175,26 @@ static void free_ref_list(struct ref_list *list)
}
}
-static void clear_cached_refs(struct cached_refs *refs)
+static void clear_cached_packed_refs(struct cached_refs *refs)
{
- if (refs->did_loose && refs->loose)
- free_ref_list(refs->loose);
if (refs->did_packed && refs->packed)
free_ref_list(refs->packed);
- refs->loose = refs->packed = NULL;
- refs->did_loose = refs->did_packed = 0;
+ refs->packed = NULL;
+ refs->did_packed = 0;
+}
+
+static void clear_cached_loose_refs(struct cached_refs *refs)
+{
+ if (refs->did_loose && refs->loose)
+ free_ref_list(refs->loose);
+ refs->loose = NULL;
+ refs->did_loose = 0;
+}
+
+static void clear_cached_refs(struct cached_refs *refs)
+{
+ clear_cached_packed_refs(refs);
+ clear_cached_loose_refs(refs);
}
static struct cached_refs *create_cached_refs(const char *submodule)
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v2 2/7] invalidate_ref_cache(): take the submodule as parameter
From: Michael Haggerty @ 2011-10-10 8:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>
Instead of invalidating the ref cache on an all-or-nothing basis,
allow the cache for individual submodules to be invalidated.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 56e4254..89b2a0e 100644
--- a/refs.c
+++ b/refs.c
@@ -223,13 +223,9 @@ static struct cached_refs *get_cached_refs(const char *submodule)
return refs;
}
-static void invalidate_ref_cache(void)
+static void invalidate_ref_cache(const char *submodule)
{
- struct cached_refs *refs = cached_refs;
- while (refs) {
- clear_cached_refs(refs);
- refs = refs->next;
- }
+ clear_cached_refs(get_cached_refs(submodule));
}
static struct ref_list *read_packed_refs(FILE *f)
@@ -1212,7 +1208,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
ret |= repack_without_ref(refname);
unlink_or_warn(git_path("logs/%s", lock->ref_name));
- invalidate_ref_cache();
+ invalidate_ref_cache(NULL);
unlock_ref(lock);
return ret;
}
@@ -1511,7 +1507,7 @@ int write_ref_sha1(struct ref_lock *lock,
unlock_ref(lock);
return -1;
}
- invalidate_ref_cache();
+ invalidate_ref_cache(NULL);
if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v2 4/7] clear_cached_refs(): rename parameter
From: Michael Haggerty @ 2011-10-10 8:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>
...for consistency with the rest of this module.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index fb46cf5..9f004ce 100644
--- a/refs.c
+++ b/refs.c
@@ -175,14 +175,14 @@ static void free_ref_list(struct ref_list *list)
}
}
-static void clear_cached_refs(struct cached_refs *ca)
+static void clear_cached_refs(struct cached_refs *refs)
{
- if (ca->did_loose && ca->loose)
- free_ref_list(ca->loose);
- if (ca->did_packed && ca->packed)
- free_ref_list(ca->packed);
- ca->loose = ca->packed = NULL;
- ca->did_loose = ca->did_packed = 0;
+ if (refs->did_loose && refs->loose)
+ free_ref_list(refs->loose);
+ if (refs->did_packed && refs->packed)
+ free_ref_list(refs->packed);
+ refs->loose = refs->packed = NULL;
+ refs->did_loose = refs->did_packed = 0;
}
static struct cached_refs *create_cached_refs(const char *submodule)
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v2 1/7] invalidate_ref_cache(): rename function from invalidate_cached_refs()
From: Michael Haggerty @ 2011-10-10 8:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>
It is the cache that is being invalidated, not the references.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 2cb93e2..56e4254 100644
--- a/refs.c
+++ b/refs.c
@@ -223,7 +223,7 @@ static struct cached_refs *get_cached_refs(const char *submodule)
return refs;
}
-static void invalidate_cached_refs(void)
+static void invalidate_ref_cache(void)
{
struct cached_refs *refs = cached_refs;
while (refs) {
@@ -1212,7 +1212,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
ret |= repack_without_ref(refname);
unlink_or_warn(git_path("logs/%s", lock->ref_name));
- invalidate_cached_refs();
+ invalidate_ref_cache();
unlock_ref(lock);
return ret;
}
@@ -1511,7 +1511,7 @@ int write_ref_sha1(struct ref_lock *lock,
unlock_ref(lock);
return -1;
}
- invalidate_cached_refs();
+ invalidate_ref_cache();
if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH v2 0/7] Provide API to invalidate refs cache
From: Michael Haggerty @ 2011-10-10 8:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <1318225574-18785-1-git-send-email-mhagger@alum.mit.edu>
Sorry for superseding my own patch series, but I prefer this patch
series to the one that I submitted earlier today:
1. An API function deserves a more carefully-selected name:
invalidate_ref_cache().
2. It gives me a chance to submit the first "bite" of my scalable-refs
changes.
These patches apply on top of mh/iterate-refs, which is in next but
not in master.
This patch series provides an API for external code to invalidate the
ref cache that is used internally to refs.c. It also allows code
*within* refs.c to invalidate only the packed or only the loose refs
for a module/submodule.
IMPORTANT:
I won't myself have time to figure out who, outside of refs.c, has to
*call* invalidate_ref_cache(). The candidates that I know off the top
of my head are git-clone, git-submodule, and git-pack-refs. It would
be great if experts in those areas would insert calls to
invalidate_ref_cache() where needed.
Even better would be if the meddlesome code were changed to use the
refs API. I'd be happy to help expanding the refs API if needed to
accommodate your needs.
This is why the API for invalidating only packed or loose refs is
private. After code outside refs.c is changed to use the refs API, it
will get the optimal behavior for free (and at that time
invalidate_ref_cache() can be removed again).
Michael Haggerty (7):
invalidate_ref_cache(): rename function from invalidate_cached_refs()
invalidate_ref_cache(): take the submodule as parameter
invalidate_ref_cache(): expose this function in refs API
clear_cached_refs(): rename parameter
clear_cached_refs(): extract two new functions
write_ref_sha1(): only invalidate the loose ref cache
clear_cached_refs(): inline function
refs.c | 34 +++++++++++++++++++---------------
refs.h | 8 ++++++++
2 files changed, 27 insertions(+), 15 deletions(-)
--
1.7.7.rc2
^ permalink raw reply
* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Matthieu Moy @ 2011-10-10 7:50 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, git, Jay Soffian, Ramkumar Ramachandra
In-Reply-To: <m3ehyl1g5v.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> Yet another issue is if we should blindly trust automatic merge resolution.
> It is considered a good practice by some to always check (e.g. by compiling
> and possibly also running tests) the result of merge, whether it required
> merge conflict resolution or not.
I agree that trusting merge blindly is bad, but still, if there are no
merge conflicts, and if the merge is broken, I'd prefer commiting a
fixup patch right after the merge than fixing it before committing.
Because if the merge needs a fix, it usually means something tricky that
deserves its own patch and commit message. At worse, one can still reset
--merge HEAD^.
One other issue with not committing automatically is for beginners. I
see that all the time when the merge has conflicts. newbies fix the
conflicts, and when they're done: "fine, conflicts solved, let's
continue hacking" without committing. The resulting history is totally
messy because it mixes merges and actual edits. For these users, not
committing automatically in the absence of conflict would make the
situation even worse.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Jakub Narebski @ 2011-10-10 7:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jay Soffian, Ramkumar Ramachandra
In-Reply-To: <7vr52lo1m3.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> By the way, on the other side of this same coin lies another use case
> (different from the one in the footnote in the previous message) for
> "merge --no-commit". When you know that a particular merge _will_ need
> semantic adjustments, even if it were to textually merge cleanly, you
> would want the command to ask you for help to come up with the final tree,
> instead of trusting the clean automerge result. This often happens when
> the topic branch you are about to merge has changed the semantics of an
> existing function (e.g. adding a new parameter) while the branch you are
> on has added new callsite to the function (or the other way around). In
> such a merge, you would need to adjust the new callsite that does not know
> about the additional parameter to the new function signature. For exactly
> the same reason, it is not a kosher advice to give to users of modern Git
> to "interfere with the merge with 'merge --no-commit', and then conclude
> with 'commit'", as 'commit' has less information than 'merge' itself what
> 'merge' wants to do in addition to recording the result as a 'commit'.
Yet another issue is if we should blindly trust automatic merge resolution.
It is considered a good practice by some to always check (e.g. by compiling
and possibly also running tests) the result of merge, whether it required
merge conflict resolution or not.
IIRC Linus lately said that making "git merge" automatically commit
was one of bad design decisions of git, for the above reason...
--
Jakub Narębski
^ permalink raw reply
* Git bug. gitattributes' pattern does not respect spaces in the filenames
From: Alexey Shumkin @ 2011-10-10 7:02 UTC (permalink / raw)
To: git
Hello everyone!
There's a description for the understanding of a
situation.
I have a project on Windows. I use Git under Cygwin.
There are some *.xml in the project. But some of them are in cp1251
encoding, another ones are in UTF-8. For the first ones there is no
need of any conversion to see the git-diff, but for the *.xml's in UTF-8
I set
*.xml diff=utf8-to-cp1251
And according to this I have
$ git config diff.utf8-to-cp1251.textconv 'iconv -f utf-8 -t cp1251'
Unfortunately, *.xml's in cp1251 DOES match this pattern, too.
As far as cp1251 and UTF-8 files are in different folders,
it is logically enough to set pattern like
<folder with a UTF-8-xmls>/*.xml diff=utf8-to-cp1251
for the UTF-8 files.
BUT!
Unfortunately, <folder with a UTF-8-xmls> have spaces in its name,
so textconv filter does not work because of error of
parsing .gitattributes
I have no enough skills to patch Git to fix this error.
Is anybody interested in to do?
Thanks
^ permalink raw reply
* Re: [PATCH] commit: teach --gpg-sign option
From: Michael J Gruber @ 2011-10-10 6:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4nzhrebp.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 10.10.2011 00:27:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> BTW: commit --amend --gpg-sign strips an existing signature rather than
>> adding one. We might want the user to have a say here.
>
> I think it deserves a separate command (commit --add-gpg-sign) that is
> used _only_ to add an additional signature by another person without
> affecting anything else in the commit (i.e. the tree, the parents and the
> author and committership information) from the viewpoint of the workflow,
Agreed, as I asked "the user to have a say".
> Obviously that "add-signature" mode needs to be aware of the existing
> signature. It is a deliberate design decision to strip existing signature
> when anything in the commit changes, which is the norm for --amend.
What norm? --amend keeps some header fields and discards others. In
fact, signing a commit "without changing it" (i.e. keeping tree, parents
etc., alias "--amend -C HEAD") should be the normal use case for signing
the tip of an existing branch. I mean, I have no problems adding to this:
git help fixup
`git fixup' is aliased to `commit --amend -C HEAD'
But what is the best default for the workflows that we encourage (commit
early, ...)? You answer a pull-request which happens to be a
fast-forward, sign the tip and suddenly you've taken over ownership (and
changed dates)??? Signing a commit should not do this.
Michael
^ permalink raw reply
* Re: [PATCH] commit: teach --gpg-sign option
From: Michael J Gruber @ 2011-10-10 6:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8votrhbr.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 09.10.2011 23:22:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> I just noticed that this format differs from the one of signed
>> tags. What special reason is there for the "sig " indentation?
>
> Read the part of the message you are quoting.
I certainly did, and certainly did not find any mention. Do you think I
would have asked otherwise? I'm trying to be helpful by testing out a
patch in flight. That is: *I* am trying to be helpful.
This
> The lines of GPG detached signature are placed in new header lines, after
> the standard tree/parent/author/committer headers, instead of tucking the
> signature block at the end of the commit log message text (similar to how
> signed tag is done), for multiple reasons:
gave me the impression that commit signatures are done "similar to how
signed tag is done".
*After* doing several "cat-file" and after your insisting that you had
described the "sig " indent I come to the conclusion that you
implemented it this way "instead of... [doing it] similar to how signed
tag is done".
Before that, I misread those paragraphs (togetheter with the
non-existing object format doc) to mean that have a section in the
object which is ignored automatically, which is where tag signatures are
(while in fact they are not) and where commit signatures will go.
I have to say I dislike the fact that we would have different signature
formats. But I have spent too much time on this unnecessarily already.
Michael
^ permalink raw reply
* [PATCH 2/2] invalidate_cached_refs(): expose this function in refs API
From: Michael Haggerty @ 2011-10-10 5:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <1318225574-18785-1-git-send-email-mhagger@alum.mit.edu>
Make invalidate_cached_refs() an official part of the refs API. It is
currently a fact of life that code outside of refs.c mucks about with
references. This change gives such code a way of informing the refs
module that it should no longer trust its cache.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 2 +-
refs.h | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index 49b73c4..0483ecc 100644
--- a/refs.c
+++ b/refs.c
@@ -223,7 +223,7 @@ static struct cached_refs *get_cached_refs(const char *submodule)
return refs;
}
-static void invalidate_cached_refs(const char *submodule)
+void invalidate_cached_refs(const char *submodule)
{
clear_cached_refs(get_cached_refs(submodule));
}
diff --git a/refs.h b/refs.h
index 5de06e5..63dc68c 100644
--- a/refs.h
+++ b/refs.h
@@ -77,6 +77,14 @@ extern void unlock_ref(struct ref_lock *lock);
/** Writes sha1 into the ref specified by the lock. **/
extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);
+/*
+ * Invalidate the reference cache for the specified submodule. Use
+ * submodule=NULL to invalidate the cache for the main module. This
+ * function must be called if references are changed via a mechanism
+ * other than the refs API.
+ */
+extern void invalidate_cached_refs(const char *submodule);
+
/** Setup reflog before using. **/
int log_ref_setup(const char *ref_name, char *logfile, int bufsize);
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 1/2] invalidate_cached_refs(): take the submodule as parameter
From: Michael Haggerty @ 2011-10-10 5:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <1318225574-18785-1-git-send-email-mhagger@alum.mit.edu>
Instead of invalidating the refs cache on an all-or-nothing basis,
allow the cache for individual submodules to be invalidated.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 2cb93e2..49b73c4 100644
--- a/refs.c
+++ b/refs.c
@@ -223,13 +223,9 @@ static struct cached_refs *get_cached_refs(const char *submodule)
return refs;
}
-static void invalidate_cached_refs(void)
+static void invalidate_cached_refs(const char *submodule)
{
- struct cached_refs *refs = cached_refs;
- while (refs) {
- clear_cached_refs(refs);
- refs = refs->next;
- }
+ clear_cached_refs(get_cached_refs(submodule));
}
static struct ref_list *read_packed_refs(FILE *f)
@@ -1212,7 +1208,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
ret |= repack_without_ref(refname);
unlink_or_warn(git_path("logs/%s", lock->ref_name));
- invalidate_cached_refs();
+ invalidate_cached_refs(NULL);
unlock_ref(lock);
return ret;
}
@@ -1511,7 +1507,7 @@ int write_ref_sha1(struct ref_lock *lock,
unlock_ref(lock);
return -1;
}
- invalidate_cached_refs();
+ invalidate_cached_refs(NULL);
if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 0/2] Provide API to invalidate refs cache
From: Michael Haggerty @ 2011-10-10 5:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Michael Haggerty
In-Reply-To: <7v39f2rko5.fsf@alter.siamese.dyndns.org>
I am happy to provide the API; see the following patches.
But I won't have time to figure out who, outside of refs.c, has to
*call* invalidate_cached_refs(). The candidates that I know off the
top of my head are git-clone, git-submodule, and git-pack-refs. It
would be great if experts in those areas would insert calls to
invalidate_cached_refs() where needed.
Even better would be if the meddlesome code were changed to use the
refs API. I'd be happy to help expanding the refs API if needed to
accommodate your needs.
Michael Haggerty (2):
invalidate_cached_refs(): take the submodule as parameter
invalidate_cached_refs(): expose this function in refs API
refs.c | 12 ++++--------
refs.h | 8 ++++++++
2 files changed, 12 insertions(+), 8 deletions(-)
--
1.7.7.rc2
^ permalink raw reply
* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-10 5:29 UTC (permalink / raw)
To: git; +Cc: Jay Soffian, Ramkumar Ramachandra
In-Reply-To: <7v8votpx4n.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> To understand why it is a wrong mental model, you need to imagine a world
> where the logic to resolve conflicts in "git merge" is improved so that it
> needs less help from the users. rerere.autoupdate is half-way there---the
> user allows the merge machinery to take advantage of conflict resolutions
> that the user has performed previously. Even though we currently do not
> let "git merge" proceed to commit the result, it is entirely plausible to
> go one step further and treat the resulting tree from applying the rerere
> information as the result of the automerge. When that happens, it is very
> natural for the user to expect that the rest of what "git merge" does for
> a clean automerge to be carried out. After all, from the end user's point
> of view, it _is_ a clean auto-merge. The only difference is how the user
> helped the automerge machinery.
Addendum.
I am not suggesting that we should change rerere.autoupdate to go all the
way and record a merge commit by default automatically when rerere applies
cleanly.
I personally think that it is a sensible default to set rerere.autoupdate
to false (or not to set the variable at all) to ensure that a merge that
conflicts is always inspected by the end user, given that rerere is merely
a heuristic (even though it is a damn good one) and produces a surprising
result.
But that is a policy preference; some people want to trust rerere more
than I do and that is a valid choice for them to make. To support such a
policy preference, I am perfectly fine with introducing a third value to
rerere.autoupdate in addition to yes/no to allow commands (e.g. "merge",
"am", etc.) to continue when rerere resolved conflicts cleanly in a
situation where they would have stopped and asked user to help resolving.
By the way, on the other side of this same coin lies another use case
(different from the one in the footnote in the previous message) for
"merge --no-commit". When you know that a particular merge _will_ need
semantic adjustments, even if it were to textually merge cleanly, you
would want the command to ask you for help to come up with the final tree,
instead of trusting the clean automerge result. This often happens when
the topic branch you are about to merge has changed the semantics of an
existing function (e.g. adding a new parameter) while the branch you are
on has added new callsite to the function (or the other way around). In
such a merge, you would need to adjust the new callsite that does not know
about the additional parameter to the new function signature. For exactly
the same reason, it is not a kosher advice to give to users of modern Git
to "interfere with the merge with 'merge --no-commit', and then conclude
with 'commit'", as 'commit' has less information than 'merge' itself what
'merge' wants to do in addition to recording the result as a 'commit'.
Either the 'commit' command needs to detect that it is conclusing the
merge and trigger the merge hooks the same way as 'merge' itself does,
(which is a bad design, as 'commit' will need to know about the clean-up
operations of all the other commands that may ask users to help and let
'commit' conclude it), or the end user instruction needs to be updated so
that 'merge --continue' is used in such a situation to give 'merge' a
chance to finish up. Again we could have "git continue" wrapper that knows
how to tell what operation was in progress and invokes "merge --continue"
when it detects that it was a 'merge' that was in progress, but that is a
mere fluff.
^ permalink raw reply
* Re: [PATCH] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-10 4:58 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git
In-Reply-To: <CACBZZX6xsnAv4S8zAqi08bcqrghZ8nKdzFP=UNCqZOqrEeLFnA@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> A nit: no other commit header is an abbreviation, it would be more
> consistent to use "signature" instead of "sig".
You are probably right.
^ permalink raw reply
* Re: [PATCH 1/3] traverse_trees(): allow pruning with pathspec
From: Michael Haggerty @ 2011-10-10 4:42 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git, Linus Torvalds
In-Reply-To: <CACsJy8D9sJOtXj_jkVSyoAJ9TC4wmKNAD5YwsFXTYkpvM4e13w@mail.gmail.com>
On 10/09/2011 11:35 PM, Nguyen Thai Ngoc Duy wrote:
> On Mon, Oct 10, 2011 at 2:39 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> On 08/29/2011 11:33 PM, Junio C Hamano wrote:
>>> diff --git a/tree-walk.c b/tree-walk.c
>>> index 33f749e..808bb55 100644
>>> --- a/tree-walk.c
>>> +++ b/tree-walk.c
>>> [...]
>>> @@ -376,16 +396,22 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
>>> mask |= 1ul << i;
>>> if (S_ISDIR(entry[i].mode))
>>> dirmask |= 1ul << i;
>>> + e = &entry[i];
>>> }
>>> if (!mask)
>>> break;
>>> - ret = info->fn(n, mask, dirmask, entry, info);
>>> - if (ret < 0) {
>>> - error = ret;
>>> - if (!info->show_all_errors)
>>> - break;
>>> + interesting = prune_traversal(e, info, &base, interesting);
>>
>> According to gcc 4.2.4 (though, strangely, not gcc 4.4.3):
I checked this a bit more carefully. gcc 4.2.4 emits a warning when the
-O1 or -O2 optimization levels are used, but not with -O0. gcc 4.4.3
does not emit a warning regardless of optimization level.
>> tree-walk.c: In function ‘traverse_trees’:
>> tree-walk.c:347: warning: ‘e’ may be used uninitialized in this function
>
> False alarm. If e is not initialized in the for loop, mask would be
> zero and therefore prune_traversal(e, info, &base, interesting), which
> would use uninitialized "e", would never be called.
That's good to know. Still, it might be worthwhile to initialize the
variable explicitly to avoid future confusion.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: Recovering Committed Changes in a Detached Head?
From: Miles Bader @ 2011-10-10 1:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin Fick, SZEDER Gábor, Daly Gutierrez, git
In-Reply-To: <7vfwj1px6h.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> First, maybe git could create refs for these automatically, perhaps
>> with a name like orphans/1? Maybe these refs would only be visible
>> via git branch --orphans.
>
> Instead of spelling them orphans/$n, you already have @{$n}.
Hmm, shouldn't that be "HEAD@{$n}" (as the reflog output suggests)?
[Well, I dunno, I'm generally kind of confused by the @{...} notation,
but I just tried it out, and just @{$n} seems to refer to the current
branch, which presumably won't include the orphaned bit once one it's
been orphaned...]
Thanks,
-Miles
--
`The suburb is an obsolete and contradictory form of human settlement'
^ permalink raw reply
* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-09 23:23 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Ramkumar Ramachandra
In-Reply-To: <CAG+J_Dzrk5x0+JRC8EbrAxjZE+hD+-5mp+H=F=M8Su2WosPfmg@mail.gmail.com>
[Adding Ram to the Cc: list as this topic has much to do with resuming a
sequencer-driven workflow, and dropping Todd who does not seem to have
much to say on this topic]
Jay Soffian <jaysoffian@gmail.com> writes:
> I am complaining that git-merge implements commits internally, which
> gives it unique behavior from git-{commit/cherry-pick/revert} (the
> latter two of which just run external git-commit). I'm saying merge is
> fundamentally broken to do it this way. And maybe that's something
> that should be fixed in 2.0 -- that git-merge should just call out to
> git-commit, just like cherry-pick/revert do.
The purpose of the plumbing commands, e.g. commit-tree, is to implement a
unit of logical step at the mechanical level well without enforcing policy
decisions that may not apply to all situations.
On the other hand, the purpose of the Porcelain commands is to support a
concrete workflow element. "git commit" should support what users want to
happen when advancing the history by one commit, "git pull" should support
what users want to happen when integrating work done in another repository
to the history you are currently growing, etc. It is where we should and
do allow users to implement their own policy with hooks and configuration
variables when they want to, and it is even fine if we implemented sane
default policies with ways to override them (e.g. "commit --allow-empty",
"merge --no-ff").
Some Porcelain commands cannot complete their workflow element by
themselves in certain situations without getting help from users, and they
give control back to the user when they need such help. "git rebase", "git
am", "git merge", etc. can and do stop and ask the user to help resolving
conflicts.
The unfortunate historical accident that we may want to correct is that
some of these "we stopped in the middle and asked the user to help before
continuing" situation were presented as if "we stopped and aborted in the
middle, leaving the user to fix up the mess", which is a completely wrong
mental model. "Upon conflicts, 'git merge' stops in the middle, and you
complete it with 'git commit'" is a prime example of this. We even wrongly
label such a situation as "failed merge". It is not failed---it merely is
not auto-completed and waiting to be completed with user's help.
To understand why it is a wrong mental model, you need to imagine a world
where the logic to resolve conflicts in "git merge" is improved so that it
needs less help from the users. rerere.autoupdate is half-way there---the
user allows the merge machinery to take advantage of conflict resolutions
that the user has performed previously. Even though we currently do not
let "git merge" proceed to commit the result, it is entirely plausible to
go one step further and treat the resulting tree from applying the rerere
information as the result of the automerge. When that happens, it is very
natural for the user to expect that the rest of what "git merge" does for
a clean automerge to be carried out. After all, from the end user's point
of view, it _is_ a clean auto-merge. The only difference is how the user
helped the automerge machinery.
The root cause of the inconsistencies you are bringing up (which I agree
are annoying and I further agree that it is a worthy thing to address) is
that even though we tell the users "after helping the 'git merge', you
conclude it with 'git commit'", the concluding 'git commit' does _not_
perform what the user configured 'git merge' to do before a merge is
concluded, unlike a cleanly resolved 'git merge'.
This is merely an unfortunate historical accident. Because "git merge" did
not have any user configurable policy decisions (read: hooks) when this
"conclude with commit" was coded, "conclude with commit" was sufficient to
emulate the case where the merge did not need any help from the user.
But it no longer is true with modern Git.
With more recent changes, e.g. the sequencer work and "git cherry-pick"
that takes multiple commits, "conclude with commit" is becoming less and
less correct thing to say. The workflow elements these commands implement
do have "create a commit" as one essential part, but that is not the only
thing they do. If anything, I think the right way forward is to update the
UI with this rule for consistency:
Some tools can stop in the middle when they cannot automatically compute
the outcome, and give control back to the user asking for help. After
helping these tool, the way to resume what was being done is to invoke
the tool with the "--continue" option. All user level policy decisions
implemented by hooks and configurations the tool normally obey when it
does not need such help from the end user are obeyed when continuing.
I wouldn't mind if that is "invoke 'git continue' command", even though I
suspect that may make the implementation slightly more complex (I haven't
thought things through). "git commit" as a way to conclude a merge that
was stopped in the middle due to a conflict should be deprecated in the
longer term, like say in Git 2.0 someday.
[Footnote]
*1* By the way, "git merge --no-commit" is an oddball. It primarily is
used when the user does _not_ want the resulting commit but wants to
further modify the tree state (e.g. cherry picking a part of what was done
in the side branch). At the philosophical level, the user should be using
merge machinery at the "plumbing" level (e.g. merge-recursive backend),
but the interface to invoke the plumbing level merge machinery is so
arcane (they are after all designed for scripts not for humans) that
nobody does so in practice. And for that purpose, I think it is Ok for the
user to do anything after "git merge --no-commit" finishes (either leaving
conflicts or leaving a cleanly merged state), including "git commit".
Because that "git commit" is very different from the "conclude conflicted
merge with commit" which is a poor substitute for "git merge --continue"
in modern Git, I think it is perfectly fine and even preferable if it does
not obey any "git merge" semantics (i.e. user defined policy that pertains
to "merge" operations).
^ permalink raw reply
* Re: Recovering Committed Changes in a Detached Head?
From: Junio C Hamano @ 2011-10-09 23:22 UTC (permalink / raw)
To: Martin Fick; +Cc: SZEDER Gábor, Daly Gutierrez, git
In-Reply-To: <ab706826-75df-4410-941e-6b40ec92713c@email.android.com>
Martin Fick <mfick@codeaurora.org> writes:
> While rflog is cool,...
>
> First, maybe git could create refs for these automatically, perhaps with a name like orphans/1? Maybe these refs would only be visible via git branch --orphans.
Instead of spelling them orphans/$n, you already have @{$n}.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox