Re: [PATCH] Allow custom comment char
2013/1/16 Junio C Hamano gits...@pobox.com:
diff --git a/git-submodule.sh b/git-submodule.sh
index 22ec5b6..1b8d95f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -975,13 +975,19 @@ cmd_summary() {
echo
done |
if test -n $for_status; then
+comment_char=`git config core.commentchar`
+if [ ! -n $comment_char ]; then
+comment_char='#'
+elif [ ${#comment_char} -gt 1 ]; then
Not portable, I think.
+echo $comment_char
+sed -e s|^|$comment_char | -e s|^$comment_char
$|$comment_char|
Can $comment_char be a '|'?
I think it may be the easiest to teach one of the pure-helper
commands, e.g. git stripspace, to do this kind of thing for you
with a new option.
To summarize, along the lines of the attached patch (on top of
jc/custom-comment-char topic).
Very good idea. I'll integrate.
Thanks
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/14] imap-send.c: inline imap_parse_list() in imap_list()
On 01/15/2013 07:51 PM, Matt Kraai wrote: On Tue, Jan 15, 2013 at 09:06:25AM +0100, Michael Haggerty wrote: -static struct imap_list *parse_imap_list(struct imap *imap, char **sp) +static struct imap_list *parse_list(char **sp) The commit subject refers to imap_parse_list and imap_list whereas the code refers to parse_imap_list and parse_list. Yes, you're right. Thanks. 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: [BUG] Possible bug in `remote set-url --add --push`
Junio C Hamano venit, vidit, dixit 15.01.2013 16:53: Michael J Gruber g...@drmicha.warpmail.net writes: Also there is a conceptual confusion: pushurl is meant to push to the same repo using a different url, e.g. something authenticated (https/ssh) for push and something faster/easier for fetch. That is not necessarily true, depending on the definition of your same. Having multiple URLs/PushURLs that refer to physically different locations, as long as git push there immediately followed by git fetch here should work with the repositories that are conceptually equivalent, is a supported mode of operation. In That is my definition of same, in the sense of object-and-ref-same when in-sync (at least regarding all pushed refs; there may be more there). fact, they being physically different _was_ the original motivation of the feature. See 755225d (git builtin push, 2006-04-29). I thought it was about unauthenticated git-protocol vs. git+ssh but was wrong. The definition of the immediate above also depends on your use; it could be tens of minutes (you may be fetching from git.k.org that can be reached from the general public, which may be a cname for multiple machines mirroring a single master.k.org that k.org account holders push to, and there may be propagation delays). In such a scenario, your URL may point at the public git.k.org, pushURL may point at master.k.org, and you may have other pushURLs that point at other places you use as back-up locations (e.g. git.or.cz or github.com). Yes. That is also why we fetch from one fetch URL only, because we assume they point at the same repo and don't need to check. As long as you _mean_ to maintain their contents the same, you can call them conceptually the same repo and your statement becomes true. It never was meant to push to several repos. This is false. It _was_ designed to be used that way from day one. It is very true with me definition of same ;) (I am not saying using it in other ways is an abuse---I am merely saying that pushing to multiple physically different repositories is within its scope). That being said, I don't mind changing the behaviour of set-url. I do not think we want to change the behaviour of set-url. What needs to be fixed is the output from remote -v. It should: * When there is no pushURL but there is a URL, then show it as (fetch/push), and you are done; * When there is one or more pushURLs and a URL, then show the URL as (fetch), and show pushURLs as (push), and you are done; * When there are more than one URLs, and there is no pushURL, then show the first URL as (fetch/push), and the remainder in a notation that says it is used only for push, but it shouldn't be the same (push); the user has to be able to distinguish it from the pushURLs in a repository that also has URLs. Maybe (fetch fallback/push) if we do use it as a fallback? If we don't we probably should? * When there are more than one URLs, and there are one or more pushURLs, then show the first URL as (fetch), the other URLs as (unused), and the pushURLs as (push). Strictly speaking, the last one could be a misconfiguration. If you have: [remote origin] url = one url = two pushurl = three pushurl = four then your git fetch will go to one, and git push will go to three and four, and two is never used. Do we fall back to two if one is unavailable? In any case, people may use a configuration like that to keep track of mirrors and shuffle around the fetch lines (rather than commenting/uncommenting) when one goes offline. It should also be stressed that the third one a supported configuration. With [remote origin] url = one url = two your git fetch goes to one, and your git push will go to one and two. This is the originally intended use case of 755225d. It is to push to and fetch from master.k.org (think of one above) and in addition to push to backup.github.com (two). Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/8 v3] git_remote_helpers: fix input when running under Python 3
On Tue, Jan 15, 2013 at 07:03:16PM -0500, Pete Wyckoff wrote:
j...@keeping.me.uk wrote on Tue, 15 Jan 2013 22:40 +:
This is what keeping the refs as byte strings looks like.
As John knows, it is not possible to interpret text from a byte
string without talking about the character encoding.
Git is (largely) a C program and uses the character set defined
in the C standard, which is a subset of ASCII. But git does
math on strings, like this snippet that takes something from
argv[] and prepends refs/heads/:
strcpy(refname, refs/heads/);
strcpy(refname + strlen(refs/heads/), ret-name);
The result doesn't talk about what character set it is using,
but because it combines a prefix from ASCII with its input,
git makes the assumption that the input is ASCII-compatible.
If you feed a UTF-16 string in argv, e.g.
$ echo master | iconv -f ascii -t utf16 | xargs git branch
xargs: Warning: a NUL character occurred in the input. It cannot be
passed through in the argument list. Did you mean to use the --null option?
fatal: Not a valid object name: ''.
you get an error about NUL, and not the branch you hoped for.
Git assumes that the input character set contains roughly ASCII
in byte positions 0..127.
That's one small reason why the useful character encodings put
ASCII in the 0..127 range, including utf-8, big5 and shift-jis.
ASCII is indeed special due to its legacy, and both C and Python
recognize this.
diff --git a/git_remote_helpers/git/importer.py
b/git_remote_helpers/git/importer.py
@@ -18,13 +18,16 @@ class GitImporter(object):
def get_refs(self, gitdir):
Returns a dictionary with refs.
+
+Note that the keys in the returned dictionary are byte strings as
+read from git.
args = [git, --git-dir= + gitdir, for-each-ref, refs/heads]
-lines = check_output(args).strip().split('\n')
+lines = check_output(args).strip().split('\n'.encode('utf-8'))
refs = {}
for line in lines:
-value, name = line.split(' ')
-name = name.strip('commit\t')
+value, name = line.split(' '.encode('utf-8'))
+name = name.strip('commit\t'.encode('utf-8'))
refs[name] = value
return refs
I'd suggest for this Python conundrum using byte-string literals, e.g.:
lines = check_output(args).strip().split(b'\n')
value, name = line.split(b' ')
name = name.strip(b'commit\t')
Essentially identical to what you have, but avoids naming utf-8 as
the encoding. It instead relies on Python's interpretation of
ASCII characters in string context, which is exactly what C does.
The problem is that AFAICT the byte-string prefix is only available in
Python 2.7 and later (compare [1] and [2]). I think we need this more
convoluted code if we want to keep supporting Python 2.6 (although
perhaps 'ascii' would be a better choice than 'utf-8').
[1] http://docs.python.org/2.6/reference/lexical_analysis.html#literals
[2] http://docs.python.org/2.7/reference/lexical_analysis.html#literals
John
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-remote: distinguish between default and configured URLs
The current output of git remote -v does not distinguish between
explicitly configured push URLs and those coming from fetch lines.
Revise the output so so that URLs are distinguished by their labels:
(fetch): fetch config used for fetching only
(fetch/push): fetch config used for fetching and pushing
(fetch fallback/push): fetch config used for pushing only
(fetch fallback): fetch config which is unused
(push): push config used for pushing
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
Maybe something like this? It even seems to make the code in get_one_entry
clearer.
I yet have to look at the tests, doc and other git-remote invocations.
builtin/remote.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 937484d..ec07109 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1509,25 +1509,28 @@ static int get_one_entry(struct remote *remote, void
*priv)
{
struct string_list *list = priv;
struct strbuf url_buf = STRBUF_INIT;
- const char **url;
- int i, url_nr;
+ char *fetchurl0, *fetchurl1;
+ int i;
+
+ if (remote-pushurl_nr 0) {
+ fetchurl0 = fetch;
+ fetchurl1 = fetch fallback;
+ } else {
+ fetchurl0 = fetch/push;
+ fetchurl1 = fetch fallback/push;
+ }
- if (remote-url_nr 0) {
- strbuf_addf(url_buf, %s (fetch), remote-url[0]);
+ for (i = 0; i remote-url_nr; i++) {
+ strbuf_addf(url_buf, %s (%s), remote-url[0], i ? fetchurl1
: fetchurl0);
string_list_append(list, remote-name)-util =
strbuf_detach(url_buf, NULL);
- } else
+ } /* else */
+ if (remote-url_nr == 0)
string_list_append(list, remote-name)-util = NULL;
- if (remote-pushurl_nr) {
- url = remote-pushurl;
- url_nr = remote-pushurl_nr;
- } else {
- url = remote-url;
- url_nr = remote-url_nr;
- }
- for (i = 0; i url_nr; i++)
+
+ for (i = 0; i remote-pushurl_nr; i++)
{
- strbuf_addf(url_buf, %s (push), url[i]);
+ strbuf_addf(url_buf, %s (push), remote-pushurl[i]);
string_list_append(list, remote-name)-util =
strbuf_detach(url_buf, NULL);
}
--
1.8.1.1.456.g93e7b0a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] test-lib.sh: unfilter GIT_PERF_*
These variables are user parameters to control how to run the perf tests. Allow users to do so. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- PERF - PERF_ t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index f50f834..bf4cf71 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -85,7 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $($PERL_PATH -e ' .*_TEST PROVE VALGRIND - PERF_AGGREGATING_LATER + PERF_ )); my @vars = grep(/^GIT_/ !/^GIT_($ok)/o, @env); print join(\n, @vars); -- 1.8.0.rc2.23.g1fb49df -- 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] git-remote: distinguish between default and configured URLs
Michael J Gruber venit, vidit, dixit 16.01.2013 11:14: The current output of git remote -v does not distinguish between explicitly configured push URLs and those coming from fetch lines. Revise the output so so that URLs are distinguished by their labels: (fetch): fetch config used for fetching only (fetch/push): fetch config used for fetching and pushing (fetch fallback/push): fetch config used for pushing only (fetch fallback): fetch config which is unused (push): push config used for pushing Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- Maybe something like this? It even seems to make the code in get_one_entry clearer. I yet have to look at the tests, doc and other git-remote invocations. Okay, so git remote show remotename copied the logic from git remote -v but neither reused the code nor the output format. I guess we'd have to implement the new logic and keep the old format? Refactoring would require settling on a common format. Both outputs should be ui-as-ui-can, but I'm afraid people are still grepping the output in their scripts :( Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] Avoid non-portable strftime format specifiers in git-cvsimport
On Wed, Jan 16, 2013 at 1:53 AM, Chris Rorvick ch...@rorvick.com wrote: On Tue, Jan 15, 2013 at 5:10 PM, Ben Walton bdwal...@gmail.com wrote: Neither %s or %z are portable strftime format specifiers. There is no need for %s in git-cvsimport as the supplied time is already in seconds since the epoch. For %z, use the function get_tz_offset provided by Git.pm instead. Out of curiosity, which platforms are affected? Assuming DST is a 1 hour shift (patch 2/3) is not necessarily portable either, though this currently appears to only affect a small island off of the coast of Australia. :-) My primary motivation on this change was for solaris. %s isn't supported in 10 (not sure about 11) and %z was only added in 10. The issue affects other older platforms as well. Good point about the 1 hour assumption. Is it worth hacking in additional logic to handle Lord Howe Island? I think it's likely a case of in for a penny, in for a pound but that could lead to madness when it comes to time zones. Either way, the function behaves better now than before. (I wasn't aware of the half hour oddball wrt to DST, so I learned something new here too!) Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens --- -- 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] git-remote: distinguish between default and configured URLs
On Wed, Jan 16, 2013 at 11:14:48AM +0100, Michael J Gruber wrote: The current output of git remote -v does not distinguish between explicitly configured push URLs and those coming from fetch lines. Revise the output so so that URLs are distinguished by their labels: (fetch): fetch config used for fetching only (fetch/push): fetch config used for fetching and pushing (fetch fallback/push): fetch config used for pushing only (fetch fallback): fetch config which is unused (push): push config used for pushing How does this interact with url.base.pushInsteadOf? I have a global rule to convert git:// URLs to ssh:// for pushing: [url g...@example.com:] pushInsteadOf = git://example.com/ With only a URL configured for a remote (no pushURL), I get (with Git 1.8.1): origin git://example.com/repository.git (fetch) origin g...@example.com:repository.git (push) From the original discussion in this thread, I think that if I did git remote set-url --add --push url it would replace my current push URL, and the change to (fetch/push) doesn't help in this case. Should there be special handling for pushInsteadOf here? John -- 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: erratic behavior commit --allow-empty
Jan Engelhardt wrote: On Tuesday 2012-10-02 10:26, Johannes Sixt wrote: Note that git commit -m A --allow-empty *DID* create a commit. Only, that it received the same name (SHA1) as the commit you created before it because it had the exact same contents (files, parents, author, committer, and timestamps). Obviously, your script was executed sufficiently fast that the two commits happend in the same second. What about introducing nanosecond-granular timestamps into Git? Not every platform (supported by git) does have a nanosecond clock resolution Bye, Jojo -- 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] git-remote: distinguish between default and configured URLs
John Keeping venit, vidit, dixit 16.01.2013 11:42: On Wed, Jan 16, 2013 at 11:14:48AM +0100, Michael J Gruber wrote: The current output of git remote -v does not distinguish between explicitly configured push URLs and those coming from fetch lines. Revise the output so so that URLs are distinguished by their labels: (fetch): fetch config used for fetching only (fetch/push): fetch config used for fetching and pushing (fetch fallback/push): fetch config used for pushing only (fetch fallback): fetch config which is unused (push): push config used for pushing How does this interact with url.base.pushInsteadOf? I have a global rule to convert git:// URLs to ssh:// for pushing: [url g...@example.com:] pushInsteadOf = git://example.com/ With only a URL configured for a remote (no pushURL), I get (with Git 1.8.1): origin git://example.com/repository.git (fetch) origin g...@example.com:repository.git (push) From the original discussion in this thread, I think that if I did git remote set-url --add --push url it would replace my current push URL, and the change to (fetch/push) doesn't help in this case. Should there be special handling for pushInsteadOf here? John Thanks for pointing out this case. The new code would still list this as two separate URLs because they really are; whether they come from two config entries or from one being subject to two different insteadof expansions is completely opaque to builtin/remote.c, unless remote.c learns to stick that additional info into struct remote somehow. In short, the separate listing is correct, but in this case there's no improvement in readability. We could still say that (push)InsteadOf is a power feature and we want to help the normal case, but it's a bit half-assed. In the end we might even have to keep track of insteadof-expansions and display those also (i.e. expanded from...)? Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote: distinguish between default and configured URLs
On Wed, Jan 16, 2013 at 01:45:36PM +0100, Michael J Gruber wrote: John Keeping venit, vidit, dixit 16.01.2013 11:42: On Wed, Jan 16, 2013 at 11:14:48AM +0100, Michael J Gruber wrote: The current output of git remote -v does not distinguish between explicitly configured push URLs and those coming from fetch lines. Revise the output so so that URLs are distinguished by their labels: (fetch): fetch config used for fetching only (fetch/push): fetch config used for fetching and pushing (fetch fallback/push): fetch config used for pushing only (fetch fallback): fetch config which is unused (push): push config used for pushing How does this interact with url.base.pushInsteadOf? I have a global rule to convert git:// URLs to ssh:// for pushing: [url g...@example.com:] pushInsteadOf = git://example.com/ With only a URL configured for a remote (no pushURL), I get (with Git 1.8.1): origin git://example.com/repository.git (fetch) origin g...@example.com:repository.git (push) From the original discussion in this thread, I think that if I did git remote set-url --add --push url it would replace my current push URL, and the change to (fetch/push) doesn't help in this case. Should there be special handling for pushInsteadOf here? Thanks for pointing out this case. The new code would still list this as two separate URLs because they really are; whether they come from two config entries or from one being subject to two different insteadof expansions is completely opaque to builtin/remote.c, unless remote.c learns to stick that additional info into struct remote somehow. OK. I like the new format, I was just wondering if it was a simple enhancement to indicate a pushInsteadOf URL specially as well. In short, the separate listing is correct, but in this case there's no improvement in readability. We could still say that (push)InsteadOf is a power feature and we want to help the normal case, but it's a bit half-assed. In the end we might even have to keep track of insteadof-expansions and display those also (i.e. expanded from...)? Given that it's not a trivial enhancement, I'd accept the argument that someone who has configured pushInsteadOf can be expected to understand the underlying git-config semantics of git remote set-url. John -- 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: show file creation/deletion and type change in diffstat
A short string is appended after the path name in diffstat:
- (new) for new 0644 files
- (new +x) for new 0755 files
- (new +l) for new symlinks
- (gone)for deleted files
- (mode +x) for files gaining executable permission
- (mode -x) for files losing executable permission
This shows most of the information from --summary, but in a more compact
format. The only missing information is rewrite percentage. This makes
--stat a replacement for --summary in most cases. In a diff where a
lot of files are added/removed, not displaying --summary shortens the
stat significantly (e.g. a pull)
Another good point is the information is now all in one line. The user
does not have to match two lines from --stat and --summary of the same
file anymore.
The summary piece is short enough that it will not eat too much into
the diffstat estate.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
A resurrection of [1] but without colors.
[1] http://thread.gmane.org/gmane.comp.version-control.git/207749
diff.c | 44 --
t/t3404-rebase-interactive.sh | 2 +-
t/t3406-rebase-message.sh | 4 +-
t/t4012-diff-binary.sh | 4 +-
...diff-tree_--cc_--patch-with-stat_--summary_side | 6 +--
t/t4013/diff.diff-tree_--cc_--stat_--summary_side | 6 +--
...pretty=oneline_--root_--patch-with-stat_initial | 6 +--
.../diff.diff-tree_--pretty_--patch-with-stat_side | 6 +--
...-tree_--pretty_--root_--patch-with-stat_initial | 6 +--
...f-tree_--pretty_--root_--stat_--summary_initial | 6 +--
.../diff.diff-tree_--pretty_--root_--stat_initial | 6 +--
...diff.diff-tree_--root_--patch-with-stat_initial | 6 +--
t/t4013/diff.diff-tree_-c_--stat_--summary_side| 6 +--
.../diff.diff_--patch-with-stat_-r_initial..side | 6 +--
t/t4013/diff.diff_--patch-with-stat_initial..side | 6 +--
t/t4013/diff.diff_--stat_initial..side | 6 +--
t/t4013/diff.diff_-r_--stat_initial..side | 6 +--
..._--attach_--stdout_--suffix=.diff_initial..side | 6 +--
format-patch_--attach_--stdout_initial..master | 16
...format-patch_--attach_--stdout_initial..master^ | 10 ++---
...ff.format-patch_--attach_--stdout_initial..side | 6 +--
...nline_--stdout_--numbered-files_initial..master | 16
...tdout_--subject-prefix=TESTCASE_initial..master | 16
format-patch_--inline_--stdout_initial..master | 16
...format-patch_--inline_--stdout_initial..master^ | 10 ++---
...ormat-patch_--inline_--stdout_initial..master^^ | 6 +--
...ff.format-patch_--inline_--stdout_initial..side | 6 +--
...tch_--stdout_--cover-letter_-n_initial..master^ | 18 -
...at-patch_--stdout_--no-numbered_initial..master | 16
...ormat-patch_--stdout_--numbered_initial..master | 16
t/t4013/diff.format-patch_--stdout_initial..master | 16
.../diff.format-patch_--stdout_initial..master^| 10 ++---
t/t4013/diff.format-patch_--stdout_initial..side | 6 +--
t/t4013/diff.log_--patch-with-stat_master | 16
..._--root_--cc_--patch-with-stat_--summary_master | 22 +--
...f.log_--root_--patch-with-stat_--summary_master | 22 +--
t/t4013/diff.log_--root_--patch-with-stat_master | 22 +--
...og_--root_-c_--patch-with-stat_--summary_master | 22 +--
t/t4013/diff.show_--patch-with-stat_--summary_side | 6 +--
t/t4013/diff.show_--patch-with-stat_side | 6 +--
t/t4013/diff.show_--stat_--summary_side| 6 +--
t/t4013/diff.show_--stat_side | 6 +--
t/t4013/diff.whatchanged_--patch-with-stat_master | 16
..._--root_--cc_--patch-with-stat_--summary_master | 22 +--
...anged_--root_--patch-with-stat_--summary_master | 22 +--
...iff.whatchanged_--root_--patch-with-stat_master | 22 +--
...ed_--root_-c_--patch-with-stat_--summary_master | 22 +--
t/t4045-diff-relative.sh | 2 +-
t/t4049-diff-stat-count.sh | 4 +-
t/t4052-stat-output.sh | 18 -
t/t4202-log.sh | 28 +++---
t/t7602-merge-octopus-many.sh | 12 +++---
52 files changed, 327 insertions(+), 291 deletions(-)
diff --git a/diff.c b/diff.c
index 348f71b..7005628 100644
--- a/diff.c
+++ b/diff.c
@@ -1250,7 +1250,8 @@ static void fn_out_consume(void *priv, char *line,
unsigned long len)
}
}
-static char *pprint_rename(const char *a, const char *b)
+static char *pprint_rename(const char *a, const char *b,
+ const char *summary_word)
{
const char *old = a;
const char *new = b;
@@ -1266,6 +1267,8 @@ static char *pprint_rename(const char *a, const char *b)
quote_c_style(a, name, NULL, 0);
strbuf_addstr(name, = );
[PATCH] clean.c, ls-files.c: respect encapsulation of exclude_list_groups
Consumers of the dir.c traversal API should avoid assuming knowledge
of the internal implementation of exclude_list_groups. Therefore
when adding items to an exclude list, it should be accessed via the
pointer returned from add_exclude_list(), rather than by referencing
a location within dir.exclude_list_groups[EXC_CMDL].
Signed-off-by: Adam Spiers g...@adamspiers.org
---
builtin/clean.c| 6 +++---
builtin/ls-files.c | 15 ++-
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index b098288..b9cb7ad 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -45,6 +45,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
static const char **pathspec;
struct strbuf buf = STRBUF_INIT;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
+ struct exclude_list *el;
const char *qname;
char *seen = NULL;
struct option options[] = {
@@ -97,10 +98,9 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (!ignored)
setup_standard_excludes(dir);
- add_exclude_list(dir, EXC_CMDL, --exclude option);
+ el = add_exclude_list(dir, EXC_CMDL, --exclude option);
for (i = 0; i exclude_list.nr; i++)
- add_exclude(exclude_list.items[i].string, , 0,
- dir.exclude_list_group[EXC_CMDL].el[0], -(i+1));
+ add_exclude(exclude_list.items[i].string, , 0, el, -(i+1));
pathspec = get_pathspec(prefix, argv);
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index fa9ccb8..b4d8b01 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -421,10 +421,10 @@ static int option_parse_z(const struct option *opt,
static int option_parse_exclude(const struct option *opt,
const char *arg, int unset)
{
- struct exclude_list_group *group = opt-value;
+ struct string_list *exclude_list = opt-value;
exc_given = 1;
- add_exclude(arg, , 0, group-el[0], --exclude_args);
+ string_list_append(exclude_list, arg);
return 0;
}
@@ -453,9 +453,11 @@ static int option_parse_exclude_standard(const struct
option *opt,
int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
{
- int require_work_tree = 0, show_tag = 0;
+ int require_work_tree = 0, show_tag = 0, i;
const char *max_prefix;
struct dir_struct dir;
+ struct exclude_list *el;
+ struct string_list exclude_list = STRING_LIST_INIT_NODUP;
struct option builtin_ls_files_options[] = {
{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
paths are separated with NUL character,
@@ -490,7 +492,7 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
OPT_BOOLEAN(0, resolve-undo, show_resolve_undo,
show resolve-undo information),
{ OPTION_CALLBACK, 'x', exclude,
- dir.exclude_list_group[EXC_CMDL], pattern,
+ exclude_list, pattern,
skip files matching pattern,
0, option_parse_exclude },
{ OPTION_CALLBACK, 'X', exclude-from, dir, file,
@@ -525,9 +527,12 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
if (read_cache() 0)
die(index file corrupt);
- add_exclude_list(dir, EXC_CMDL, --exclude option);
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
+ el = add_exclude_list(dir, EXC_CMDL, --exclude option);
+ for (i = 0; i exclude_list.nr; i++) {
+ add_exclude(exclude_list.items[i].string, , 0, el,
--exclude_args);
+ }
if (show_tag || show_valid_bit) {
tag_cached = H ;
tag_unmerged = M ;
--
1.8.1.291.g0730ed6
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] push: update remote tags only with force
Hi there, I was just working on improving git-remote-helper.txt by documenting how remote helper can signal error conditions to git. This lead me to notice a (to me) surprising change in behavior between master and next that I traced back to this patch series. Specifically: On 30.11.2012, at 02:41, Chris Rorvick wrote: This patch series originated in response to the following thread: http://thread.gmane.org/gmane.comp.version-control.git/208354 I made some adjustments based on Junio's last round of feedback including a new patch reworking the push rules comment in remote.c. Also refined some of the log messages--nothing major. Finally, took a stab at putting something together for the release notes, see below. From the discussion in that gmane thread and from the commits in this series, I had the impression that it should mostly affect pushing tags. However, this is not the case: It also changes messages upon regular push conflicts. Consider this test script: #!/bin/sh -ex git init repo_orig cd repo_orig echo a a git add a git commit -m a cd .. git clone repo_orig repo_clone cd repo_orig echo b b git add b git commit -m b cd .. cd repo_clone echo B b git add b git commit -m B git push With git 1.8.1, I get this message: ! [rejected]master - master (non-fast-forward) error: failed to push some refs to '/Users/mhorn/Projekte/foreign/gitifyhg/bugs/git-push-conflict/repo_orig' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Merge the remote changes (e.g. 'git pull') hint: before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. But with next, I get this: ! [rejected]master - master (already exists) error: failed to push some refs to '/Users/mhorn/Projekte/foreign/gitifyhg/bugs/git-push-conflict/repo_orig' hint: Updates were rejected because the destination reference already exists hint: in the remote. This looks like a regression to me. No tags were involve, and the new message is very confusing if not outright wrong -- at least in my mind, but perhaps I am missing a way to interpret it correctly ? What am I missing? Cheers, Max -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fix some clang warnings
Signed-off-by: Max Horn m...@quendi.de --- cache.h | 2 +- git-compat-util.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cache.h b/cache.h index c257953..5c8440b 100644 --- a/cache.h +++ b/cache.h @@ -1148,7 +1148,7 @@ extern int check_repository_format_version(const char *var, const char *value, v extern int git_env_bool(const char *, int); extern int git_config_system(void); extern int config_error_nonbool(const char *); -#ifdef __GNUC__ +#if defined(__GNUC__) ! defined(__clang__) #define config_error_nonbool(s) (config_error_nonbool(s), -1) #endif extern const char *get_log_output_encoding(void); diff --git a/git-compat-util.h b/git-compat-util.h index 4f022a3..cc2abee 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -310,7 +310,7 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2))) * behavior. But since we're only trying to help gcc, anyway, it's OK; other * compilers will fall back to using the function as usual. */ -#ifdef __GNUC__ +#if defined(__GNUC__) ! defined(__clang__) #define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1) #endif -- 1.8.1.1.435.g4e2ebdf -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] guilt patches, including git 1.8 support
On Tue, Jan 15, 2013 at 10:26:06PM -0500, Theodore Ts'o wrote: On Tue, Jan 15, 2013 at 06:26:06PM -0800, Jonathan Nieder wrote: Hi Jeff and other guilty parties, I collected all the guilt patches I could find on-list and added one of my own. Completely untested, except for running the regression tests. These are also available via git protocol from git://repo.or.cz/guilt/mob.git mob Jonathan, thanks for collecting all of the guilt patches! Your repro was also very much really useful since I hadn't grabbed the latest patches from jeffpc's repo before it disappeared after the kernel.org security shutdown. I had repo.or.cz mirroring all along. :) Jeff, do you need some help getting your repro on kernel.org re-established? Yes and no. I was hoping to find some time to restore all the web content on my server, and start using repo.or.cz as the public git repo. With that said, I have only two sigs for my gpg key. (Guilt isn't really related to linux...) Thanks, Jeff. -- Only two things are infinite, the universe and human stupidity, and I'm not sure about the former. - Albert Einstein -- 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 07/14] imap-send.c: inline imap_parse_list() in imap_list()
Michael Haggerty mhag...@alum.mit.edu writes: On 01/15/2013 07:51 PM, Matt Kraai wrote: On Tue, Jan 15, 2013 at 09:06:25AM +0100, Michael Haggerty wrote: -static struct imap_list *parse_imap_list(struct imap *imap, char **sp) +static struct imap_list *parse_list(char **sp) The commit subject refers to imap_parse_list and imap_list whereas the code refers to parse_imap_list and parse_list. Yes, you're right. Thanks. I think I've fixed this (and some other minor points in other patches in the series) while queuing; please check master..3691031c after fetching from me. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Move Git::SVN::get_tz to Git::get_tz_offset
Ben Walton bdwal...@gmail.com writes:
+sub get_tz_offset {
+ # some systmes don't handle or mishandle %z, so be creative.
Hmph. I wonder if we can use %z if it is handled correctly and fall
back to this code only on platforms that are broken?
--
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/7] contrib/subtree: Use %B for Split Subject/Body
gree...@obbligato.org writes: Are you incorporating the other patches? Should I drop them from my list? I actually was planning to accept patches to this subdirectory only through you, hopefully as messages that forward others' changes with your Acked-by: tagline. That frees me from having to keeping track of what goes on there ;-) -- 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: [BUG] Possible bug in `remote set-url --add --push`
Michael J Gruber g...@drmicha.warpmail.net writes: Junio C Hamano venit, vidit, dixit 15.01.2013 16:53: ... * When there are more than one URLs, and there is no pushURL, then show the first URL as (fetch/push), and the remainder in a notation that says it is used only for push, but it shouldn't be the same (push); the user has to be able to distinguish it from the pushURLs in a repository that also has URLs. Maybe (fetch fallback/push) if we do use it as a fallback? If we don't we probably should? I actually think my earlier it shouldn't be the same (push) is not needed and probably is actively wrong. Just like you can tell between (only one .url) (both .url and .pushurl) origin there (fetch/push) origin there (fetch) origin there (push) even when the value of the URL/PushURL, i.e. there, is the same between .url and .pushurl, you should be able to tell between (two .url, no .pushurl) (one .url and one .pushurl) origin there (fetch/push) origin there (fetch) origin another (push) origin another (push) So let's not make it too complex and forget about the different kind of (push). A case that is a potential misconfiguration would look like: (two .url, one .pushurl) origin there (fetch) origin some (unused) origin another (push) I think. -- 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: [BUG] Possible bug in `remote set-url --add --push`
On Tue, Jan 15, 2013 at 10:53 AM, Junio C Hamano gits...@pobox.com wrote: Michael J Gruber g...@drmicha.warpmail.net writes: That being said, I don't mind changing the behaviour of set-url. I do not think we want to change the behaviour of set-url. I agree with Michael that changing the set-url behavior would be appropriate here. If I say --add this pushUrl, don't I mean to create an additional url which is pushed to? I agree that it makes the config situation messy; this is currently a clean sequence, in that it leaves the config unchanged after both steps are completed: git remote set-url --add --push origin /tmp/foo git remote set-url --delete --push origin /tmp/foo If the behavior is changed like Michael suggested, it would not leave the config clean (unless heroic steps were taken to keep track). But I'm not sure that's such a bad thing. In simple command sequences, the results would be clean and the only behavior change is that the initial --add really acts like add and not replace. But more complex sequences could be devised which were affected by this change. I'm curious, Junio. Do you think the set-url behavior is correct as-is, or that changing it will cause breakage for some workflows, or that it complicates the operation too much for people who are already used to the config layout? Phil -- 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: [BUG] Possible bug in `remote set-url --add --push`
Junio C Hamano venit, vidit, dixit 16.01.2013 16:50: Michael J Gruber g...@drmicha.warpmail.net writes: Junio C Hamano venit, vidit, dixit 15.01.2013 16:53: ... * When there are more than one URLs, and there is no pushURL, then show the first URL as (fetch/push), and the remainder in a notation that says it is used only for push, but it shouldn't be the same (push); the user has to be able to distinguish it from the pushURLs in a repository that also has URLs. Maybe (fetch fallback/push) if we do use it as a fallback? If we don't we probably should? I actually think my earlier it shouldn't be the same (push) is not needed and probably is actively wrong. Just like you can tell between (only one .url) (both .url and .pushurl) origin there (fetch/push) origin there (fetch) origin there (push) even when the value of the URL/PushURL, i.e. there, is the same between .url and .pushurl, you should be able to tell between (two .url, no .pushurl) (one .url and one .pushurl) origin there (fetch/push) origin there (fetch) origin another (push) origin another (push) So let's not make it too complex and forget about the different kind of (push). A case that is a potential misconfiguration would look like: (two .url, one .pushurl) origin there (fetch) origin some (unused) origin another (push) I think. I'm sorry but E_NOPARSE. I can't grok the above at all. But I'll try again tomorrow ;) In any case, the issue with (push)instead of that John mentions bothers me: there are two specified URLs but one URL in config only; my patch doesn't make that case clearer at all. My early attempts at amending struct remote produced too many segfaults to continue today... Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Possible bug in `remote set-url --add --push`
Phil Hord venit, vidit, dixit 16.01.2013 17:15: On Tue, Jan 15, 2013 at 10:53 AM, Junio C Hamano gits...@pobox.com wrote: Michael J Gruber g...@drmicha.warpmail.net writes: That being said, I don't mind changing the behaviour of set-url. I do not think we want to change the behaviour of set-url. I agree with Michael that changing the set-url behavior would be appropriate here. If I say --add this pushUrl, don't I mean to create an additional url which is pushed to? I said I wouldn't mind, I didn't vote for it. I agree that it makes the config situation messy; this is currently a clean sequence, in that it leaves the config unchanged after both steps are completed: git remote set-url --add --push origin /tmp/foo git remote set-url --delete --push origin /tmp/foo If the behavior is changed like Michael suggested, it would not leave the config clean (unless heroic steps were taken to keep track). But I'm not sure that's such a bad thing. In simple command sequences, the results would be clean and the only behavior change is that the initial --add really acts like add and not replace. But more complex sequences could be devised which were affected by this change. I'm curious, Junio. Do you think the set-url behavior is correct as-is, or that changing it will cause breakage for some workflows, or that it complicates the operation too much for people who are already used to the config layout? For set url --add --push on top of a push url only being defaulted from a fetch url, both behaviours (replace or add, i.e. current or new) make sense to me. So the questions are: - Is it worth and possible changing? - How to best describe it in remote -v and remote show output? My patch answered to no to the first question and answers the second one in cases where (push)insteadof is not used to transform one fetch config into two different urls for fetch and push. I think :) Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] push: update remote tags only with force
Max Horn m...@quendi.de writes: But with next, I get this: ! [rejected]master - master (already exists) error: failed to push some refs to '/Users/mhorn/Proje...o_orig' hint: Updates were rejected because the destination reference already exists hint: in the remote. This looks like a regression to me. It is an outright bug. The new helper function is_forwrdable() is bogus to assume that both original and updated objects can be locally inspected, but you do not necessarily have the original locally. remote.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/remote.c b/remote.c index aa6b719..4a253ef 100644 --- a/remote.c +++ b/remote.c @@ -1286,9 +1286,12 @@ static inline int is_forwardable(struct ref* ref) if (!prefixcmp(ref-name, refs/tags/)) return 0; - /* old object must be a commit */ + /* +* old object must be a commit, but we may be forcing +* without having it in the first place! +*/ o = parse_object(ref-old_sha1); - if (!o || o-type != OBJ_COMMIT) + if (o o-type != OBJ_COMMIT) return 0; /* new object must be commit-ish */ -- 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] fix some clang warnings
On Wed, Jan 16, 2013 at 03:53:23PM +0100, Max Horn wrote: -#ifdef __GNUC__ +#if defined(__GNUC__) ! defined(__clang__) #define config_error_nonbool(s) (config_error_nonbool(s), -1) #endif You don't say what the warning is, but I'm guessing it's complaining about throwing away the return value from config_error_nonbool? -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 v6 0/8] push: update remote tags only with force
Junio C Hamano gits...@pobox.com writes: Max Horn m...@quendi.de writes: But with next, I get this: ! [rejected]master - master (already exists) error: failed to push some refs to '/Users/mhorn/Proje...o_orig' hint: Updates were rejected because the destination reference already exists hint: in the remote. This looks like a regression to me. It is an outright bug. The new helper function is_forwrdable() is bogus to assume that both original and updated objects can be locally inspected, but you do not necessarily have the original locally. The way the caller uses the result of this function is equally questionable. If this function says we do not want to let this push go through, it translates that unconditionally into we blocked it because the destination already exists. It is fine when pushing into refs/tags/ hierarchy. It is *NOT* OK if the type check does not satisfy this function. In that case, we do not actually see the existence of the destination as a problem, but it is reported as such. We are blocking because we do not like the type of the new object or the type of the old object. If the destination points at a commit, the push can succeed if the user changes what object to push, so saying you cannot push because the destination already exists is just wrong in such a case. -- 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] fix some clang warnings
Jeff King p...@peff.net writes: On Wed, Jan 16, 2013 at 03:53:23PM +0100, Max Horn wrote: -#ifdef __GNUC__ +#if defined(__GNUC__) ! defined(__clang__) #define config_error_nonbool(s) (config_error_nonbool(s), -1) #endif You don't say what the warning is, but I'm guessing it's complaining about throwing away the return value from config_error_nonbool? Yeah, I was wondering about the same thing. The other one looks similar, ignoring the return value of error(). Also, is this some versions of clang do not like this? Or are all versions of clang affected? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] push: update remote tags only with force
On Wed, Jan 16, 2013 at 02:32:03PM +0100, Max Horn wrote:
With git 1.8.1, I get this message:
! [rejected]master - master (non-fast-forward)
[...]
But with next, I get this:
! [rejected]master - master (already exists)
Thanks for the detailed report. I was able to reproduce easily here.
The problem is the logic in is_forwardable:
static inline int is_forwardable(struct ref* ref)
{
struct object *o;
if (!prefixcmp(ref-name, refs/tags/))
return 0;
/* old object must be a commit */
o = parse_object(ref-old_sha1);
if (!o || o-type != OBJ_COMMIT)
return 0;
/* new object must be commit-ish */
o = deref_tag(parse_object(ref-new_sha1), NULL, 0);
if (!o || o-type != OBJ_COMMIT)
return 0;
return 1;
}
The intent is to allow fast-forward only between objects that both point
to commits eventually. But we are doing this check on the client, which
does not necessarily have the object for ref-old_sha1 at all. So it
cannot know the type, and cannot enforce this condition accurately.
I.e., we trigger the !o branch after the parse_object in your example.
-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 v6 0/8] push: update remote tags only with force
Max Horn m...@quendi.de writes: But with next, I get this: ! [rejected]master - master (already exists) error: failed to push some refs to '/Users/mhorn/Projekte/foreign/gitifyhg/bugs/git-push-conflict/repo_orig' hint: Updates were rejected because the destination reference already exists hint: in the remote. This looks like a regression to me. It is in master now X-, and this looks like a bug to me. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] push: update remote tags only with force
Jeff King p...@peff.net writes:
I.e., we trigger the !o branch after the parse_object in your example.
Heh, I didn't see this message until now (gmane seems to be lagging
a bit).
I am very tempted to do this.
* Remove unnecessary not_forwardable from struct ref; it is only
used inside set_ref_status_for_push();
* refs/tags/ is the only hierarchy that cannot be replaced
without --force;
* Remove the misguided attempt to force that everything that
updates an existing ref has to be a commit outside refs/tags/
hierarchy. This code does not know what kind of objects the user
wants to place in refs/frotz/ hierarchy it knows nothing about.
I feel moderately strongly about the last point. Defining special
semantics for one hierarchy (e.g. refs/tags/) and implementing a
policy for enforcement is one thing, but a random policy that
depends on object type that applies globally is simply insane. The
user may want to do refs/tested/ hierarchy that is meant to hold
references to commit, with one annotated tag refs/tested/latest
that points at the latest tested version with some commentary, and
maintain the latter by keep pushing to it. If that is the semantics
the user wanted to ahve in the refs/tested/ hierarchy, it is not
reasonable to require --force for such a workflow. The user knows
better than Git in such a case.
cache.h | 1 -
remote.c | 24 +---
t/t5516-fetch-push.sh | 21 -
3 files changed, 1 insertion(+), 45 deletions(-)
diff --git a/cache.h b/cache.h
index a32a0ea..a942bbd 100644
--- a/cache.h
+++ b/cache.h
@@ -1004,7 +1004,6 @@ struct ref {
requires_force:1,
merge:1,
nonfastforward:1,
- not_forwardable:1,
update:1,
deletion:1;
enum {
diff --git a/remote.c b/remote.c
index aa6b719..2c747c4 100644
--- a/remote.c
+++ b/remote.c
@@ -1279,26 +1279,6 @@ int match_push_refs(struct ref *src, struct ref **dst,
return 0;
}
-static inline int is_forwardable(struct ref* ref)
-{
- struct object *o;
-
- if (!prefixcmp(ref-name, refs/tags/))
- return 0;
-
- /* old object must be a commit */
- o = parse_object(ref-old_sha1);
- if (!o || o-type != OBJ_COMMIT)
- return 0;
-
- /* new object must be commit-ish */
- o = deref_tag(parse_object(ref-new_sha1), NULL, 0);
- if (!o || o-type != OBJ_COMMIT)
- return 0;
-
- return 1;
-}
-
void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
int force_update)
{
@@ -1344,8 +1324,6 @@ void set_ref_status_for_push(struct ref *remote_refs, int
send_mirror,
* passing the --force argument
*/
- ref-not_forwardable = !is_forwardable(ref);
-
ref-update =
!ref-deletion
!is_null_sha1(ref-old_sha1);
@@ -1355,7 +1333,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int
send_mirror,
!has_sha1_file(ref-old_sha1)
|| !ref_newer(ref-new_sha1, ref-old_sha1);
- if (ref-not_forwardable) {
+ if (!prefixcmp(ref-name, refs/tags/)) {
ref-requires_force = 1;
if (!force_ref_update) {
ref-status =
REF_STATUS_REJECT_ALREADY_EXISTS;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 6009372..8f024a0 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -950,27 +950,6 @@ test_expect_success 'push requires --force to update
lightweight tag' '
)
'
-test_expect_success 'push requires --force to update annotated tag' '
- mk_test heads/master
- mk_child child1
- mk_child child2
- (
- cd child1
- git tag -a -m message 1 Tag
- git push ../child2 Tag:refs/tmp/Tag
- git push ../child2 Tag:refs/tmp/Tag
- file1
- git add file1
- git commit -m file1
- git tag -f -a -m message 2 Tag
- test_must_fail git push ../child2 Tag:refs/tmp/Tag
- git push --force ../child2 Tag:refs/tmp/Tag
- git tag -f -a -m message 3 Tag HEAD~
- test_must_fail git push ../child2 Tag:refs/tmp/Tag
- git push --force ../child2 Tag:refs/tmp/Tag
- )
-'
-
test_expect_success 'push --porcelain' '
mk_empty
echo .git/foo To testrepo
--
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] fix some clang warnings
FWIW, I also happen to have the warning:
advice.c:69:2: warning: expression result unused [-Wunused-value]
error('%s' is not possible because you have unmerged files., me);
^~
./git-compat-util.h:314:55: note: expanded from:
#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
^~
with clang: Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final)
(based on LLVM 3.0)
I can't say about other versions.
On Wed, Jan 16, 2013 at 5:53 PM, Junio C Hamano gits...@pobox.com wrote:
Jeff King p...@peff.net writes:
On Wed, Jan 16, 2013 at 03:53:23PM +0100, Max Horn wrote:
-#ifdef __GNUC__
+#if defined(__GNUC__) ! defined(__clang__)
#define config_error_nonbool(s) (config_error_nonbool(s), -1)
#endif
You don't say what the warning is, but I'm guessing it's complaining
about throwing away the return value from config_error_nonbool?
Yeah, I was wondering about the same thing. The other one looks
similar, ignoring the return value of error().
Also, is this some versions of clang do not like this? Or are all
versions of clang affected?
--
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
--
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] fix some clang warnings
On Wed, Jan 16, 2013 at 06:12:57PM +0100, Antoine Pelisse wrote:
FWIW, I also happen to have the warning:
advice.c:69:2: warning: expression result unused [-Wunused-value]
error('%s' is not possible because you have unmerged files., me);
^~
./git-compat-util.h:314:55: note: expanded from:
#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
^~
with clang: Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final)
(based on LLVM 3.0)
I have the same output with:
clang version 3.2 (tags/RELEASE_32/final)
--
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] fix some clang warnings
On 16.01.2013, at 18:18, John Keeping wrote:
On Wed, Jan 16, 2013 at 06:12:57PM +0100, Antoine Pelisse wrote:
FWIW, I also happen to have the warning:
advice.c:69:2: warning: expression result unused [-Wunused-value]
error('%s' is not possible because you have unmerged files., me);
^~
./git-compat-util.h:314:55: note: expanded from:
#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
^~
with clang: Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final)
(based on LLVM 3.0)
I have the same output with:
clang version 3.2 (tags/RELEASE_32/final)
Sorry for not being more specific in my message. I have this with
Apple clang version 4.1 (tags/Apple/clang-421.11.66) (based on LLVM 3.1svn)
Max
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add Auto-Submitted header to post-receive-email
Andy, do you have any thoughts on adding this email header to contrib/hooks/post-receive-email? This patch shouldn't cause problems for anyone with a sanely configured mail delivery agent, and the additional header is very useful in toggling auto responses. This conforms to RFC3834 and is useful in preventing eg vacation auto-responders from replying by default Signed-off-by: Chris Hiestand chiest...@salk.edu --- contrib/hooks/post-receive-email |1 + 1 file changed, 1 insertion(+) diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email index b2171a0..0e5b72d 100755 --- a/contrib/hooks/post-receive-email +++ b/contrib/hooks/post-receive-email @@ -237,6 +237,7 @@ generate_email_header() X-Git-Reftype: $refname_type X-Git-Oldrev: $oldrev X-Git-Newrev: $newrev + Auto-Submitted: auto-generated This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing -- 1.7.10.4 On Sep 21, 2012, at 10:06 AM, Junio C Hamano gits...@pobox.com wrote: Chris Hiestand chiest...@salk.edu writes: My email in April went unanswered so I'm resending it. An Auto-Submitted header would be an improvement to the standard [git] post receive email. Thanks, Chris Begin forwarded message: From: Chris Hiestand chiest...@salk.edu Subject: [PATCH] Add Auto-Submitted header to post-receive-email Date: April 14, 2012 6:15:10 PM PDT To: git@vger.kernel.org, gits...@pobox.com Hi, I think the Auto-Submitted header is a useful hook mail header to include by default. This conforms to RFC3834 and is useful in preventing e.g. vacation auto-responders from replying by default. Perhaps you have already considered this and decided not to include it, but I found no record of such a conversation on this list. I think the lack of response is generally lack of interest, and the primary reason for that was because the To/Cc list did not contain anybody who touched this particular file in the past (and no, I am not among them; as contrib/README says, I am often the wrong person to ask if a patch to contrib/ material makes sense). From 358fc3ae1ebfd7723d54e4033d3e9a9a0322c873 Mon Sep 17 00:00:00 2001 From: Chris Hiestand chiest...@salk.edu Date: Sat, 14 Apr 2012 17:58:39 -0700 Subject: [PATCH] Add Auto-Submitted header to post-receive-email These four lines should not be in the body of the e-mail message (see Documentation/SubmittingPatches). Adds Auto-Submitted: auto-generated to post-receive-email header This conforms to RFC3834 and is useful in preventing eg vacation auto-responders from replying by default --- contrib/hooks/post-receive-email |1 + 1 files changed, 1 insertions(+), 0 deletions(-) Even for contrib/ material, please always sign-off your patch (see Documentation/SubmittingPatches). diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email index 01af9df..282507c 100755 --- a/contrib/hooks/post-receive-email +++ b/contrib/hooks/post-receive-email @@ -237,6 +237,7 @@ generate_email_header() X-Git-Reftype: $refname_type X-Git-Oldrev: $oldrev X-Git-Newrev: $newrev + Auto-Submitted: auto-generated This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing I think the choice of auto-generated is a sensible one, as responding to a 'push' is like triggered by 'cron'. I'd however appreciate comments from people who either worked on this code or list regulars who actually use this code in the production. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/3] config: Introduce diff.algorithm variable
Will replace the one in 'pu' with this round. Looking good. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] push: update remote tags only with force
On Wed, Jan 16, 2013 at 09:10:10AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: I.e., we trigger the !o branch after the parse_object in your example. Heh, I didn't see this message until now (gmane seems to be lagging a bit). I think it is vger lagging, actually. I am very tempted to do this. * Remove unnecessary not_forwardable from struct ref; it is only used inside set_ref_status_for_push(); * refs/tags/ is the only hierarchy that cannot be replaced without --force; Agreed. * Remove the misguided attempt to force that everything that updates an existing ref has to be a commit outside refs/tags/ hierarchy. This code does not know what kind of objects the user wants to place in refs/frotz/ hierarchy it knows nothing about. I agree with what your patch does, but my thinking is a bit different. My original suggestion with respect to object types was that the rule for --force should be do not ever lose any objects without --force. So a fast-forward is OK, as the new objects reference the old. A non-fast forward is not, because objects become unreferenced. Replacing a tag object is not OK, even if it points to the same commit, as you are losing the old tag object (replacing an object with a tag that points to the original object or its descendent is OK in theory, though I doubt it is common enough to worry about). I think that is a reasonable rule that could be applied across all parts of the namespace hierarchy. And it could be applied by the client, because all you need to know is whether ref-old_sha1 is reachable from ref-new_sha1. But it is somewhat orthogonal to the already exists idea, and checking refs/tags/. Those ideas are about enforcing sane rules on the tag hierarchy. My rule is a safety valve that is meant to extend the idea of is fast-forwardable to non-commit object types. If we do it at all, it should be part of the fast-forward check (e.g., as part of ref_newer). The current code conflates the two under the already exists condition, which is just wrong. I think the best thing at this point is to split the two ideas apart, keep the refs/tags check (and translate it to already exists in the UI, as we do), and table the safety valve. I am not even sure if it is something that is useful, and it can come later if we decide it is. I feel moderately strongly about the last point. Defining special semantics for one hierarchy (e.g. refs/tags/) and implementing a policy for enforcement is one thing, but a random policy that depends on object type that applies globally is simply insane. The user may want to do refs/tested/ hierarchy that is meant to hold references to commit, with one annotated tag refs/tested/latest that points at the latest tested version with some commentary, and maintain the latter by keep pushing to it. If that is the semantics the user wanted to ahve in the refs/tested/ hierarchy, it is not reasonable to require --force for such a workflow. The user knows better than Git in such a case. I see what you are saying, but I think the ship has already sailed to some degree. We already implement the non-fast-forward check everywhere, and I cannot have a refs/tested hierarchy that pushes arbitrary commits without regard to their history. If I have such a hierarchy, I have to use --force (or more likely, mark the refspec with +). In my mind, the object-type checking is just making that fast-forward check more thorough (i.e., extending it to non-commit objects). cache.h | 1 - remote.c | 24 +--- t/t5516-fetch-push.sh | 21 - 3 files changed, 1 insertion(+), 45 deletions(-) The patch itself looks fine to me. Whether we agree on the fast-forward object-type checking or not, it is the correct first step to take in either case. -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] clean.c, ls-files.c: respect encapsulation of exclude_list_groups
Adam Spiers g...@adamspiers.org writes: Consumers of the dir.c traversal API should avoid assuming knowledge of the internal implementation of exclude_list_groups. Therefore when adding items to an exclude list, it should be accessed via the pointer returned from add_exclude_list(), rather than by referencing a location within dir.exclude_list_groups[EXC_CMDL]. Sounds sensible. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Question re. git remote repository
Hello, We're just in the process of investigating a versioning tool and are very interesting in git. We have one question we're hoping someone can answer. In regards to the repositories, I think I understand correctly that each developer will have a local repository that they will work from, and that there will also be a remote repository (origin) that will hold the original version of the project. It appears from the limited reading I've done that the remote repository must be hosted at github.com. Is this the case? Ideally we'd prefer to simply create our remote repository on a drive of one of our local network servers. Is this possible? Thanks in advance for the help. David Lang | Application Developer | Tel: 416-340-4800 x.5277 Cardiac IT Dept - Toronto General Hospital The University Health Network 200 Elizabeth St. Toronto, ON M5G 2C4 This e-mail may contain confidential and/or privileged information for the sole use of the intended recipient. Any review or distribution by anyone other than the person for whom it was originally intended is strictly prohibited. If you have received this e-mail in error, please contact the sender and delete all copies. Opinions, conclusions or other information contained in this e-mail may not be that of the organization. -- 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] fix some clang warnings
On Wed, Jan 16, 2013 at 06:26:35PM +0100, Max Horn wrote:
On Wed, Jan 16, 2013 at 06:12:57PM +0100, Antoine Pelisse wrote:
FWIW, I also happen to have the warning:
advice.c:69:2: warning: expression result unused [-Wunused-value]
error('%s' is not possible because you have unmerged files., me);
^~
./git-compat-util.h:314:55: note: expanded from:
#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
^~
with clang: Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final)
(based on LLVM 3.0)
I have the same output with:
clang version 3.2 (tags/RELEASE_32/final)
Sorry for not being more specific in my message. I have this with
Apple clang version 4.1 (tags/Apple/clang-421.11.66) (based on LLVM 3.1svn)
So it seems pretty common, and is just that clang is more concerned
about this than gcc. I think your patch is a reasonable workaround. It
seems a little weird to me that clang defines __GNUC__, but I assume
there are good reasons for it. The commit message should probably be
along the lines of:
Commit a469a10 wraps some error calls in macros to give the compiler a
chance to do more static analysis on their constant -1 return value.
We limit the use of these macros to __GNUC__, since gcc is the primary
beneficiary of the new information, and because we use GNU features
for handling variadic macros.
However, clang also defines __GNUC__, but generates warnings (due to
throwing away the return value from the first half of the macro). We
can squelch the warning by turning off these macros when clang is in
use.
I'm confused, though, why your patch does not have a matching update to
the opterror macro in parse-options.h. It uses exactly the same
technique. Does it not generate a warning?
-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] fix some clang warnings
On Wed, Jan 16, 2013 at 09:50:57AM -0800, Jeff King wrote:
I'm confused, though, why your patch does not have a matching update to
the opterror macro in parse-options.h. It uses exactly the same
technique. Does it not generate a warning?
Ah, I think I see why not.
It is not about the macro itself, but rather the callsites that do not
return error, but call it for its printing side effect. It seems that
clang -Wunused-value is OK with unused values from functions being
discarded, but not with constants. So:
int foo();
void bar()
{
foo(); /* ok */
1; /* not ok */
(foo(), 1); /* not ok */
}
The first one is OK (I think it would fall under -Wunused-result under
either compiler). The middle one is an obvious error, and caught by both
compilers. The last one is OK by gcc, but clang complains.
So opterror does not happen to generate any warnings, because we do not
ever use it in a void context. It should probably be marked the same
way, though, as future-proofing.
The commit message should probably be along the lines of:
[...]
However, clang also defines __GNUC__, but generates warnings (due to
throwing away the return value from the first half of the macro). We
can squelch the warning by turning off these macros when clang is in
use.
So a more accurate description would be:
However, clang also defines __GNUC__, but generates warnings with
-Wunused-value when these macros are used in a void context, because
the constant -1 ends up being useless. Gcc does not complain about
this case (though it is unclear if it is because it is smart enough to
see what we are doing, or too dumb to realize that the -1 is unused).
We can squelch the warning by just disabling these macros when clang
is in use.
-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
[PATCH v2 17/19] fixup! reset $sha1 $pathspec: require $sha1 only to be treeish
---
Sorry, I forgot the documentation updates. I hope this looks ok. Can
you squash this in, Junio? Thanks.
I don't think any documentation update is necessary for the reset on
unborn branch patch. Let me know if you think differently.
Documentation/git-reset.txt | 18 +-
builtin/reset.c | 4 ++--
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 978d8da..a404b47 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -8,20 +8,20 @@ git-reset - Reset current HEAD to the specified state
SYNOPSIS
[verse]
-'git reset' [-q] [commit] [--] paths...
-'git reset' (--patch | -p) [commit] [--] [paths...]
+'git reset' [-q] [tree-ish] [--] paths...
+'git reset' (--patch | -p) [tree-sh] [--] [paths...]
'git reset' [--soft | --mixed | --hard | --merge | --keep] [-q] [commit]
DESCRIPTION
---
-In the first and second form, copy entries from commit to the index.
+In the first and second form, copy entries from tree-ish to the index.
In the third form, set the current branch head (HEAD) to commit, optionally
-modifying index and working tree to match. The commit defaults to HEAD
-in all forms.
+modifying index and working tree to match. The tree-ish/commit defaults
+to HEAD in all forms.
-'git reset' [-q] [commit] [--] paths...::
+'git reset' [-q] [tree-ish] [--] paths...::
This form resets the index entries for all paths to their
- state at commit. (It does not affect the working tree, nor
+ state at tree-ish. (It does not affect the working tree, nor
the current branch.)
+
This means that `git reset paths` is the opposite of `git add
@@ -34,9 +34,9 @@ Alternatively, using linkgit:git-checkout[1] and specifying a
commit, you
can copy the contents of a path out of a commit to the index and to the
working tree in one go.
-'git reset' (--patch | -p) [commit] [--] [paths...]::
+'git reset' (--patch | -p) [tree-ish] [--] [paths...]::
Interactively select hunks in the difference between the index
- and commit (defaults to HEAD). The chosen hunks are applied
+ and tree-ish (defaults to HEAD). The chosen hunks are applied
in reverse to the index.
+
This means that `git reset -p` is the opposite of `git add -p`, i.e.
diff --git a/builtin/reset.c b/builtin/reset.c
index b776867..cb84f1b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -23,8 +23,8 @@
static const char * const git_reset_usage[] = {
N_(git reset [--mixed | --soft | --hard | --merge | --keep] [-q]
[commit]),
- N_(git reset [-q] commit [--] paths...),
- N_(git reset --patch [commit] [--] [paths...]),
+ N_(git reset [-q] tree-ish [--] paths...),
+ N_(git reset --patch [tree-ish] [--] [paths...]),
NULL
};
--
1.8.1.1.454.gce43f05
--
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] fix some clang warnings
On Wed, 16 Jan 2013 09:50:57 -0800, Jeff King p...@peff.net wrote: However, clang also defines __GNUC__, [...] http://sourceforge.net/p/predef/wiki/Compilers/ Notice that the meaning of the __GNUC__ macro has changed subtly over the years, from identifying the GNU C/C++ compiler to identifying any compiler that implements the GNU compiler extensions (...). For example, the Intel C++ on Linux also defines these macros from version 8.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: Question re. git remote repository
On Wed, 16 Jan 2013 17:49:09 + Lang, David david.l...@uhn.ca wrote: We're just in the process of investigating a versioning tool and are very interesting in git. We have one question we're hoping someone can answer. In regards to the repositories, I think I understand correctly that each developer will have a local repository that they will work from, and that there will also be a remote repository (origin) that will hold the original version of the project. The name origin is purely arbitrary: any local repository might have It appears from the limited reading I've done that the remote repository must be hosted at github.com. Is this the case? Of course not. github is just a Git hosting provider. There are plenty of them -- both commercial and not-for-profit (a well-known service bitbucket.org is one example). Ideally we'd prefer to simply create our remote repository on a drive of one of our local network servers. Is this possible? Yes, this is possible, but it's not advised to keep such a reference repository on an exported networked drive for a number of reasons (both performance and bug-free operation). Instead, the canonical way to host reference repositories is to make them accessible via SSH or via HTTP[S]. To do this, a server running some POSIX OS (Linux- or *BSD-based) is the best bet. Both kinds of access require Git itself installed on the server. Obviously, SSH access requires an SSH server software (such as OpenSSH) as well and HTTP[S] access requires a web server (such as Apache). Of course, everything mentioned is available on any sensible OS you might install on your server. Read-only access might be provided by a special tool named Git daemon which is a part of Git. If you have more than a couple of developers you might want to install certain front-end Git software on the server which provides for virtualized Git users and fine-grained control over who can do what. Using gitolite [3] for this is the current trend. Web-browsing for your repositories, if needed, is usually provided by the tool named gitweb [4]. Everything I've just summarised is well explained in [5] and [6] (as an addendum). Another approach is to set up a turn-key solution such as GitLab [1] or gitblit [2]. 1. http://gitlabhq.com/ 2. http://gitblit.com/ 3. https://github.com/sitaramc/gitolite 4. https://git.wiki.kernel.org/index.php/Gitweb 5. http://git-scm.com/book/en/Git-on-the-Server 6. http://git-scm.com/2010/03/04/smart-http.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 17/19] fixup! reset $sha1 $pathspec: require $sha1 only to be treeish
On Wed, Jan 16, 2013 at 10:00 AM, Martin von Zweigbergk martinv...@gmail.com wrote: --- Sorry, I forgot the documentation updates. I hope this looks ok. Can you squash this in, Junio? Thanks. I see the series just entered 'next', so I guess it would have to go on top then. Perhaps with a commit message like as simple as the following. Let me know if you prefer it to be resent as a proper patch. Sorry about the noise. reset: update documentation to require only tree-ish with paths When resetting with paths, we no longer require a commit argument, but only a tree-ish. Update the documentation and synopsis accordingly. -- 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] fix some clang warnings
On Wed, Jan 16, 2013 at 10:00:42AM -0800, Jeff King wrote: So opterror does not happen to generate any warnings, because we do not ever use it in a void context. It should probably be marked the same way, though, as future-proofing. [...] So a more accurate description would be: Here it is all together: -- 8 -- From: Max Horn m...@quendi.de Subject: [PATCH] fix clang -Wunused-value warnings for error functions Commit a469a10 wraps some error calls in macros to give the compiler a chance to do more static analysis on their constant -1 return value. We limit the use of these macros to __GNUC__, since gcc is the primary beneficiary of the new information, and because we use GNU features for handling variadic macros. However, clang also defines __GNUC__, but generates warnings with -Wunused-value when these macros are used in a void context, because the constant -1 ends up being useless. Gcc does not complain about this case (though it is unclear if it is because it is smart enough to see what we are doing, or too dumb to realize that the -1 is unused). We can squelch the warning by just disabling these macros when clang is in use. Signed-off-by: Max Horn m...@quendi.de Signed-off-by: Jeff King p...@peff.net --- cache.h | 2 +- git-compat-util.h | 2 +- parse-options.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cache.h b/cache.h index c257953..5c8440b 100644 --- a/cache.h +++ b/cache.h @@ -1148,7 +1148,7 @@ extern int config_error_nonbool(const char *); extern int git_env_bool(const char *, int); extern int git_config_system(void); extern int config_error_nonbool(const char *); -#ifdef __GNUC__ +#if defined(__GNUC__) ! defined(__clang__) #define config_error_nonbool(s) (config_error_nonbool(s), -1) #endif extern const char *get_log_output_encoding(void); diff --git a/git-compat-util.h b/git-compat-util.h index 2cecf56..2596280 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -297,7 +297,7 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2))) * behavior. But since we're only trying to help gcc, anyway, it's OK; other * compilers will fall back to using the function as usual. */ -#ifdef __GNUC__ +#if defined(__GNUC__) ! defined(__clang__) #define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1) #endif diff --git a/parse-options.h b/parse-options.h index e703853..1c8bd8d 100644 --- a/parse-options.h +++ b/parse-options.h @@ -177,7 +177,7 @@ extern int opterror(const struct option *opt, const char *reason, int flags); extern int optbug(const struct option *opt, const char *reason); extern int opterror(const struct option *opt, const char *reason, int flags); -#ifdef __GNUC__ +#if defined(__GNUC__) ! defined(clang) #define opterror(o,r,f) (opterror((o),(r),(f)), -1) #endif -- 1.8.1.rc1.10.g7d71f7b -- 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] fix some clang warnings
On Wed, Jan 16, 2013 at 10:00:42AM -0800, Jeff King wrote:
It is not about the macro itself, but rather the callsites that do not
return error, but call it for its printing side effect. It seems that
clang -Wunused-value is OK with unused values from functions being
discarded, but not with constants. So:
int foo();
void bar()
{
foo(); /* ok */
1; /* not ok */
(foo(), 1); /* not ok */
}
The first one is OK (I think it would fall under -Wunused-result under
either compiler). The middle one is an obvious error, and caught by both
compilers. The last one is OK by gcc, but clang complains.
I wonder if this would be changed in clang - the change in [1] is
superficially similar.
[1] http://llvm.org/bugs/show_bug.cgi?id=13747
--
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] fix some clang warnings
Jeff King p...@peff.net writes: It seems a little weird to me that clang defines __GNUC__, but I assume there are good reasons for it. The reason is essentially that clang targets compatibility with GCC (implementing the same extensions cie), in the sense drop in replacement that should be able to compile legacy code possibly relying on __GNUC__ and GCC extensions. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix some clang warnings
On Wed, Jan 16, 2013 at 06:12:03PM +, John Keeping wrote:
On Wed, Jan 16, 2013 at 10:00:42AM -0800, Jeff King wrote:
It is not about the macro itself, but rather the callsites that do not
return error, but call it for its printing side effect. It seems that
clang -Wunused-value is OK with unused values from functions being
discarded, but not with constants. So:
int foo();
void bar()
{
foo(); /* ok */
1; /* not ok */
(foo(), 1); /* not ok */
}
The first one is OK (I think it would fall under -Wunused-result under
either compiler). The middle one is an obvious error, and caught by both
compilers. The last one is OK by gcc, but clang complains.
I wonder if this would be changed in clang - the change in [1] is
superficially similar.
[1] http://llvm.org/bugs/show_bug.cgi?id=13747
Yeah, I think it is exactly the same issue, and the fix they mention
there would apply to us, too.
Is it worth applying this at all, then? Or should we apply it but limit
it with a clang version macro (they mention r163034, but I do not know
if it is in a released version yet, nor what macros are available to
inspect the version)?
-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] fix some clang warnings
Is it worth applying this at all, then? Or should we apply it but limit it with a clang version macro (they mention r163034, but I do not know if it is in a released version yet, nor what macros are available to inspect the version)? Please also note that building with clang is not warning-free (though I think it would be nice) -- 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: Question re. git remote repository
On Wed, Jan 16, 2013 at 10:06:15PM +0400, Konstantin Khomoutov wrote: In regards to the repositories, I think I understand correctly that each developer will have a local repository that they will work from, and that there will also be a remote repository (origin) that will hold the original version of the project. Note that this is just one common topology for setting up repos (and it is probably the simplest). Others are described here: http://git-scm.com/book/en/Distributed-Git-Distributed-Workflows Ideally we'd prefer to simply create our remote repository on a drive of one of our local network servers. Is this possible? Yes, this is possible, but it's not advised to keep such a reference repository on an exported networked drive for a number of reasons (both performance and bug-free operation). I agree that performance is not ideal (although if you are on a fast LAN, it probably would not matter much), but I do not recall any specific bugs in that area. Can you elaborate? -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] fix some clang warnings
On Wed, Jan 16, 2013 at 10:15:58AM -0800, Jeff King wrote:
On Wed, Jan 16, 2013 at 06:12:03PM +, John Keeping wrote:
On Wed, Jan 16, 2013 at 10:00:42AM -0800, Jeff King wrote:
It is not about the macro itself, but rather the callsites that do not
return error, but call it for its printing side effect. It seems that
clang -Wunused-value is OK with unused values from functions being
discarded, but not with constants. So:
int foo();
void bar()
{
foo(); /* ok */
1; /* not ok */
(foo(), 1); /* not ok */
}
The first one is OK (I think it would fall under -Wunused-result under
either compiler). The middle one is an obvious error, and caught by both
compilers. The last one is OK by gcc, but clang complains.
I wonder if this would be changed in clang - the change in [1] is
superficially similar.
[1] http://llvm.org/bugs/show_bug.cgi?id=13747
Yeah, I think it is exactly the same issue, and the fix they mention
there would apply to us, too.
Is it worth applying this at all, then? Or should we apply it but limit
it with a clang version macro (they mention r163034, but I do not know
if it is in a released version yet, nor what macros are available to
inspect the version)?
That maps to revision 06b3a06007 in their git repository [1], which is
contained in remotes/origin/release_32 so I think that change should be
in release 3.2, where I still see the warning (although that's not using
a clang built from that source), so I don't think that the fix for that
bug removes the warning in this case.
[1] http://llvm.org/git/clang.git
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git fetch without --recurse-submodules option
Am 16.01.2013 06:45, schrieb 乙酸鋰: With git pull or git fetch without specifying --recurse-submodules, what is the default action? on-demand fetch (unless something else is configured). It seems git fetches submodules wtihout specifying --recurse-submodules. If this is not clear, please update documentation. You are right, the documentation for pull and fetch does not state explicitly that on-demand is the default here when the option is not used. In git pull document --recurse-submodules option, it tells users to see git-config(1) and gitmodules(5), but does not tell users to refer to git fetch --recurse-submodules for the meaning of the switches. In git fetch document --recurse-submodules option, it does not tell users to see git-config(1) or gitmodules(5). Thanks for pointing that out. Unless anyone else wants to improve the documentation I'll look into that in my next git time slot. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] test-lib.sh: unfilter GIT_PERF_*
Nguyễn Thái Ngọc Duy wrote: These variables are user parameters to control how to run the perf tests. Allow users to do so. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com FWIW, of the three versions proposed, this one makes the most sense to me. Looks good. -- 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] fix some clang warnings
On Wed, Jan 16, 2013 at 10:24:49AM -0800, Jeff King wrote: On Wed, Jan 16, 2013 at 06:22:40PM +, John Keeping wrote: [1] http://llvm.org/bugs/show_bug.cgi?id=13747 Yeah, I think it is exactly the same issue, and the fix they mention there would apply to us, too. Is it worth applying this at all, then? Or should we apply it but limit it with a clang version macro (they mention r163034, but I do not know if it is in a released version yet, nor what macros are available to inspect the version)? That maps to revision 06b3a06007 in their git repository [1], which is contained in remotes/origin/release_32 so I think that change should be in release 3.2, where I still see the warning (although that's not using a clang built from that source), so I don't think that the fix for that bug removes the warning in this case. [1] http://llvm.org/git/clang.git Thanks for checking. I'd rather squelch the warning completely (as in my re-post of Max's patch from a few minutes ago), and we can loosen it (possibly with a version check) later when a fix is widely disseminated. I checked again with a trunk build of clang and the warning's still there, so I've created a clang bug [1] to see if they will change the behaviour. I agree that we should squelch the warning for now, it can be changed into a version check if it's accepted as a bug and once we know what version it's fixed in. [1] http://llvm.org/bugs/show_bug.cgi?id=14968 -- 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] attr: fix off-by-one directory component length calculation
Duy Nguyen pclo...@gmail.com writes:
OK I get your point now. Something like this?
-- 8 --
Subject: [PATCH] attr: avoid calling find_basename() twice per path
find_basename() is only used inside collect_all_attrs(), called once
in prepare_attr_stack, then again after prepare_attr_stack()
returns. Both calls return exact same value. Reorder the code to do
the same task once. Also avoid strlen() because we knows the length
after finding basename.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Yeah, I think this is a nice code reduction, readability improvement
and micro optimization rolled into one.
attr.c | 45 ++---
1 file changed, 18 insertions(+), 27 deletions(-)
diff --git a/attr.c b/attr.c
index cfc6748..880f862 100644
--- a/attr.c
+++ b/attr.c
@@ -564,32 +564,12 @@ static void bootstrap_attr_stack(void)
attr_stack = elem;
}
-static const char *find_basename(const char *path)
-{
- const char *cp, *last_slash = NULL;
-
- for (cp = path; *cp; cp++) {
- if (*cp == '/' cp[1])
- last_slash = cp;
- }
- return last_slash ? last_slash + 1 : path;
-}
-
-static void prepare_attr_stack(const char *path)
+static void prepare_attr_stack(const char *path, int dirlen)
{
struct attr_stack *elem, *info;
- int dirlen, len;
+ int len;
const char *cp;
- dirlen = find_basename(path) - path;
-
- /*
- * find_basename() includes the trailing slash, but we do
- * _not_ want it.
- */
- if (dirlen)
- dirlen--;
-
/*
* At the bottom of the attribute stack is the built-in
* set of attribute definitions, followed by the contents
@@ -769,15 +749,26 @@ static int macroexpand_one(int attr_nr, int rem)
static void collect_all_attrs(const char *path)
{
struct attr_stack *stk;
- int i, pathlen, rem;
- const char *basename;
+ int i, pathlen, rem, dirlen;
+ const char *basename, *cp, *last_slash = NULL;
+
+ for (cp = path; *cp; cp++) {
+ if (*cp == '/' cp[1])
+ last_slash = cp;
+ }
+ pathlen = cp - path;
+ if (last_slash) {
+ basename = last_slash + 1;
+ dirlen = last_slash - path;
+ } else {
+ basename = path;
+ dirlen = 0;
+ }
- prepare_attr_stack(path);
+ prepare_attr_stack(path, dirlen);
for (i = 0; i attr_nr; i++)
check_all_attr[i].value = ATTR__UNKNOWN;
- basename = find_basename(path);
- pathlen = strlen(path);
rem = attr_nr;
for (stk = attr_stack; 0 rem stk; stk = stk-prev)
rem = fill(path, pathlen, basename, stk, rem);
--
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] Allow custom comment char
From: Junio C Hamano gits...@pobox.com Some users do want to write a line that begin with a pound sign, #, in their commit log message. Many tracking system recognise a token of #bugid form, for example. The support we offer these use cases is not very friendly to the end users. They have a choice between - Don't do it. Avoid such a line by rewrapping or indenting; and - Use --cleanup=whitespace but remove all the hint lines we add. Give them a way to set a custom comment char, e.g. $ git -c core.commentchar=% commit so that they do not have to do either of the two workarounds. Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- Junio, thanks for the code in your reply to the first version. It works very well and looks nice. I was also unhappy about this \n%c\n thing and pretty unsure with the code in git-submodule.sh. But with this, it looks good to me. Thanks. Changes in v2: - extend git stripspace with an option to make it's input being converted to commented lines - teach git-submodule.sh using this - rename strbuf_commented_addstr to strbuf_add_commented_lines and improve it's design Documentation/config.txt | 6 Documentation/git-stripspace.txt | 8 - Documentation/technical/api-strbuf.txt | 10 ++ builtin/branch.c | 10 +++--- builtin/commit.c | 12 +++ builtin/fmt-merge-msg.c| 2 +- builtin/merge.c| 5 ++- builtin/notes.c| 34 +--- builtin/stripspace.c | 48 +++- builtin/tag.c | 34 ++-- cache.h| 6 config.c | 8 + environment.c | 6 git-submodule.sh | 8 ++--- strbuf.c | 58 -- strbuf.h | 4 +++ t/t0030-stripspace.sh | 35 t/t7502-commit.sh | 7 t/t7508-status.sh | 50 + wt-status.c| 10 +++--- 20 files changed, 283 insertions(+), 78 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index d5809e0..e99b9f2 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -528,6 +528,12 @@ core.editor:: variable when it is set, and the environment variable `GIT_EDITOR` is not set. See linkgit:git-var[1]. +core.commentchar:: + Commands such as `commit` and `tag` that lets you edit + messages consider a line that begins with this character + commented, and removes them after the editor returns + (default '#'). + sequence.editor:: Text editor used by `git rebase -i` for editing the rebase insn file. The value is meant to be interpreted by the shell when it is used. diff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt index a80d946..e6fdfcb 100644 --- a/Documentation/git-stripspace.txt +++ b/Documentation/git-stripspace.txt @@ -35,7 +35,13 @@ OPTIONS --- -s:: --strip-comments:: - Skip and remove all lines starting with '#'. + Skip and remove all lines starting with comment character (default '#'). + +-c:: +--comment-lines:: + Prepend comment character and blank to each line. Lines will automatically + be terminated with a newline. On empty lines, only the comment character + will be prepended. EXAMPLES diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt index 84686b5..2c59cb2 100644 --- a/Documentation/technical/api-strbuf.txt +++ b/Documentation/technical/api-strbuf.txt @@ -156,6 +156,11 @@ then they will free() it. Remove the bytes between `pos..pos+len` and replace it with the given data. +`strbuf_add_commented_lines`:: + + Add a NUL-terminated string to the buffer. Each line will be prepended + by a comment character and a blank. + `strbuf_add`:: Add data of given length to the buffer. @@ -229,6 +234,11 @@ which can be used by the programmer of the callback as she sees fit. Add a formatted string to the buffer. +`strbuf_commented_addf`:: + + Add a formatted string prepended by a comment character and a + blank to the buffer. + `strbuf_fread`:: Read a given size of data from a FILE* pointer to the buffer. diff --git a/builtin/branch.c b/builtin/branch.c index 873f624..3548271 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -706,11 +706,11 @@ static int edit_branch_description(const char *branch_name) read_branch_desc(buf, branch_name); if (!buf.len || buf.buf[buf.len-1] != '\n') strbuf_addch(buf,
Re: [PATCH] git-remote: distinguish between default and configured URLs
Michael J Gruber g...@drmicha.warpmail.net writes:
In short, the separate listing is correct, but in this case there's no
improvement in readability.
Yes, I think the insteadOf rewrite is a related but a separate
issue.
Is remote -v meant for diagnosing remote.origin.{url,pushurl} that
are misconfigured?
If not, the output just should just say the final outcome, i.e. what
destinations we will fetch from and push to, without cluttering the
output.
If on the other hand it is to help users debug their configuration,
the output also needs to explain exactly what made us decide those
destinations to use (e.g. to discover there was a leftover insteadof
in $HOME/.gitconfig the user forgot about).
--
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: [BUG] Possible bug in `remote set-url --add --push`
Junio C Hamano gits...@pobox.com writes: I actually think my earlier it shouldn't be the same (push) is not needed and probably is actively wrong. Just like you can tell between (only one .url) (both .url and .pushurl) origin there (fetch/push) origin there (fetch) origin there (push) What should happen when you have a .pushinsteadof configured that modifies .url for pushing? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- 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: Question re. git remote repository
On Wed, 16 Jan 2013 10:21:56 -0800 Jeff King p...@peff.net wrote: Thanks for elaborating on the origin -- I intended to write up on its special status but got distracted and sent my message missing that bit ;-) [...] Ideally we'd prefer to simply create our remote repository on a drive of one of our local network servers. Is this possible? Yes, this is possible, but it's not advised to keep such a reference repository on an exported networked drive for a number of reasons (both performance and bug-free operation). I agree that performance is not ideal (although if you are on a fast LAN, it probably would not matter much), but I do not recall any specific bugs in that area. Can you elaborate? This one [1] for instance. I also recall seing people having other mystical problems with setups like this so I somehow developed an idea than having a repository on a networked drive is asking for troubles. Of course, if there are happy users of such setups, I would be glad to hear as my precautions might well be unfounded for the recent versions of Git. 1. http://code.google.com/p/msysgit/issues/detail?id=130 -- 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: [BUG] Possible bug in `remote set-url --add --push`
Andreas Schwab sch...@linux-m68k.org writes: Junio C Hamano gits...@pobox.com writes: I actually think my earlier it shouldn't be the same (push) is not needed and probably is actively wrong. Just like you can tell between (only one .url) (both .url and .pushurl) origin there (fetch/push) origin there (fetch) origin there (push) What should happen when you have a .pushinsteadof configured that modifies .url for pushing? I think push should work like this: * the user gives us a nickname; * we look at remote.$nickname.pushurl (and if there isn't, remote.$nickname.url) to decide the logical URLs to push to; * for each logical URL we decided to push, we look at url.*.pushInsteadOf to see if there is one that match the $URL (and if there isn't url.*.insteadOf), and map the logical URL to the final destination. So that we can instruct push to push, when pushing into a repository that logically resides at git://git.k.org/pub/, to instead push into the repository via git-over-ssh, e.g. [remote korg] url = git://git.k.org/pub/scm/git/git.git/ [url git.k.org:/pub/] pushInsteadOf = git://git.k.org/pub/ without affecting the fetching side. As I said in a separate message, the above fetch/push vs fetch and push distinction is not descriptive enough to express the post rewriting that is done with insteadOf; it only helps debugging misconfiguration between .url vs .pushurl, which may be better than the status quo but is not ideal. -- 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: Question re. git remote repository
Hi David, now we are going to have some confusion here, two David Langs on the list :-) On Wed, 16 Jan 2013, Lang, David wrote: We're just in the process of investigating a versioning tool and are very interesting in git. We have one question we're hoping someone can answer. In regards to the repositories, I think I understand correctly that each developer will have a local repository that they will work from, and that there will also be a remote repository (origin) that will hold the original version of the project. It appears from the limited reading I've done that the remote repository must be hosted at github.com. Is this the case? Ideally we'd prefer to simply create our remote repository on a drive of one of our local network servers. Is this possible? Git is peer-to-peer, the 'origin' is just the default to pull from. It can be hosted on any machine that you have access to. A typical case is that you designate one person (or a small group of people) to oversee your master repository, and that person decides when and what to pull there from the developers. This gives you a chance to insert code review, tests, etc between what the developers produce in their local repository and what you then bless as the authoritative 'released' version of the code. However, this master repository is just a matter of convention, it is possible to use any repository as the 'origin', changing it is just a config change away. David Lang -- 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] Make git selectively and conditionally ignore certain stat fields
Junio C Hamano wrote: Robin Rosenberg robin.rosenb...@dewire.com writes: That configurability is a slipperly slope to drag us into giving users more complexity that does not help them very much, I suspect. Earlier somebody mentioned size and mtime is often enough, so I think a single option core.looseStatInfo (substitute loose with short, minimum or whatever adjective that is more appropriate---I am not good at picking phrases, it sounds to me a way to more loosely define stat info cleanliness than we usually do) that makes us ignore all fields (regardless of their zero-ness) other than those two fields might not be a bad way to go. At one point, I used to build (and test) the MSVC version of git on cygwin, which leads to exactly the same problem. So, this is not just an EGit/JGit vs c-git issue, although there can't be many people that will have this problem. (Mixing the MinGW and cygwin versions on the same repo will also have this problem). I had a patch which, essentially, did what you suggest above; ie ignore everything other than size and mtime, *including* ignoring the zero-ness in the index. (I just don't understand why you would think of doing otherwise!! ;-) ). As part of that patch, I also suppressed the empty diff output that used to be shown for stat-dirty files (that's been fixed now right?), otherwise using gitk was a pain. [BTW, given the schizophrenic stat functions on cygwin, you can have this problem with the cygwin version of git - all on it's lonesome!] I can't help with naming, BTW, since I called the config variable core.ramsay-stat. :-P I do not offhand know if such a loose mode is too simple and make it excessively risky, though. I suspect it would be fine ... *however*, I never sent my patch because I didn't think there would be many idiots^H^H^H^H^H^H pioneers like me! :-D 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 v2] Allow custom comment char
Ralf Thielow ralf.thie...@gmail.com writes:
From: Junio C Hamano gits...@pobox.com
Some users do want to write a line that begin with a pound sign, #,
in their commit log message. Many tracking system recognise
a token of #bugid form, for example.
The support we offer these use cases is not very friendly to the end
users. They have a choice between
- Don't do it. Avoid such a line by rewrapping or indenting; and
- Use --cleanup=whitespace but remove all the hint lines we add.
Give them a way to set a custom comment char, e.g.
$ git -c core.commentchar=% commit
so that they do not have to do either of the two workarounds.
Signed-off-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Ralf Thielow ralf.thie...@gmail.com
---
Junio, thanks for the code in your reply to the
first version. It works very well and looks nice.
I was also unhappy about this \n%c\n thing and
pretty unsure with the code in git-submodule.sh.
But with this, it looks good to me. Thanks.
Changes in v2:
- extend git stripspace with an option to make
it's input being converted to commented lines
- teach git-submodule.sh using this
- rename strbuf_commented_addstr to strbuf_add_commented_lines
and improve it's design
Oh, I love it when something like this happens. Throw a perhaps
along these lines patch and then a finished product that fills the
gaps I didn't bother to fill magically appears, even with tests and
updates to comments and documentation.
What good things did I do recently to deserve such a luck? ;-)
@@ -66,21 +67,52 @@ void stripspace(struct strbuf *sb, int skip_comments)
strbuf_setlen(sb, j);
}
+static void comment_lines(struct strbuf *buf)
+{
+ char *msg;
+ size_t len;
+
+ msg = strbuf_detach(buf, len);
+ strbuf_add_commented_lines(buf, msg, len);
+}
This leaks msg (inherited from my perhaps along these lines
patch). I think I can just add free(msg) at the end.
+ if (strip_comments || mode == COMMENT_LINES)
+ git_config(git_default_config, NULL);
Nice spotting. The along these lines patch broke stripspace -s
under custom comment line char; this fixes it.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Move Git::SVN::get_tz to Git::get_tz_offset
Ben Walton bdwal...@gmail.com writes:
On Wed, Jan 16, 2013 at 3:37 PM, Junio C Hamano gits...@pobox.com wrote:
Ben Walton bdwal...@gmail.com writes:
+sub get_tz_offset {
+ # some systmes don't handle or mishandle %z, so be creative.
Hmph. I wonder if we can use %z if it is handled correctly and fall
back to this code only on platforms that are broken?
That would be perfectly acceptable to me. The reason I set it up to
always run through this function here is that when I originally added
this function for git-svn, I'd made it conditional and Eric Wong
preferred that the function be used exclusively[1]. I opted to take
the same approach here to keep things congrous.
If it were to be conditional, I think I'd add a variable to the build
system and have the code leverage that at runtime instead of the
try/except approach I attempted in 2009.
If the code was originally unconditional for a reason (and I think
being bug-to-bug compatible across platforms is actually a good
thing in a tool like importers), I would not object to it. Thanks
for the back-story.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] push: update remote tags only with force
Jeff King p...@peff.net writes:
I see what you are saying, but I think the ship has already sailed to
some degree. We already implement the non-fast-forward check everywhere,
and I cannot have a refs/tested hierarchy that pushes arbitrary
commits without regard to their history. If I have such a hierarchy, I
have to use --force (or more likely, mark the refspec with +).
Yeah, actually in that example, I meant refs/tested/ would have
pointers to bare tree objects. I often rebuild 'pu' and another
private integration branch for testing, reordering the series that
are still not in 'next' and also after rewriting log messages of
some commits. It is not unusual to end up the updated 'pu' having
the identical tree as 'pu' before update, and I want to skip testing
the result (tree equality matters while commit equality does not in
such a use case).
In my mind, the object-type checking is just making that fast-forward
check more thorough (i.e., extending it to non-commit objects).
Yes, I agree with that point of view.
Thanks.
Here is what I am planning to queue (the patch is the same, but the
message is different on the third point).
-- 8 --
Subject: [PATCH] push: fix refs/tags/ hierarchy cannot be updated without
--force
When pushing to update a branch with a commit that is not a
descendant of the commit at the tip, a wrong message already
exists was given, instead of the correct non-fast-forward, if we
do not have the object sitting in the destination repository at the
tip of the ref we are updating.
The primary cause of the bug is that the check in a new helper
function is_forwardable() assumed both old and new objects are
available and can be checked, which is not always the case.
The way the caller uses the result of this function is also wrong.
If the helper says we do not want to let this push go through, the
caller unconditionally translates it into we blocked it because the
destination already exists, which is not true at all in this case.
Fix this by doing these three things:
* Remove unnecessary not_forwardable from struct ref; it is only
used inside set_ref_status_for_push();
* Make refs/tags/ the only hierarchy that cannot be replaced
without --force;
* Remove the misguided attempt to force that everything that
updates an existing ref has to be a commit outside refs/tags/
hierarchy.
The policy last one tried to implement may later be resurrected and
extended to ensure fast-forwardness (defined as not losing
objects, extending from the traditional not losing commits from
the resulting history) when objects that are not commit are
involved (e.g. an annotated tag in hierarchies outside refs/tags),
but such a logic belongs to is this a fast-forward? check that is
done by ref_newer(); is_forwardable(), which is now removed, was not
the right place to do so.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
cache.h | 1 -
remote.c | 43 +++
t/t5516-fetch-push.sh | 21 -
3 files changed, 7 insertions(+), 58 deletions(-)
diff --git a/cache.h b/cache.h
index a32a0ea..a942bbd 100644
--- a/cache.h
+++ b/cache.h
@@ -1004,7 +1004,6 @@ struct ref {
requires_force:1,
merge:1,
nonfastforward:1,
- not_forwardable:1,
update:1,
deletion:1;
enum {
diff --git a/remote.c b/remote.c
index aa6b719..d3a1ca2 100644
--- a/remote.c
+++ b/remote.c
@@ -1279,26 +1279,6 @@ int match_push_refs(struct ref *src, struct ref **dst,
return 0;
}
-static inline int is_forwardable(struct ref* ref)
-{
- struct object *o;
-
- if (!prefixcmp(ref-name, refs/tags/))
- return 0;
-
- /* old object must be a commit */
- o = parse_object(ref-old_sha1);
- if (!o || o-type != OBJ_COMMIT)
- return 0;
-
- /* new object must be commit-ish */
- o = deref_tag(parse_object(ref-new_sha1), NULL, 0);
- if (!o || o-type != OBJ_COMMIT)
- return 0;
-
- return 1;
-}
-
void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
int force_update)
{
@@ -1320,32 +1300,23 @@ void set_ref_status_for_push(struct ref *remote_refs,
int send_mirror,
}
/*
-* The below logic determines whether an individual
-* refspec A:B can be pushed. The push will succeed
-* if any of the following are true:
+* Decide whether an individual refspec A:B can be
+* pushed. The push will succeed if any of the
+* following are true:
*
* (1) the remote reference B does not exist
*
* (2) the remote reference B is being removed (i.e.,
* pushing :B where no source is specified)
*
-* (3) the
Re: [PATCH v2] Allow custom comment char
Am 16.01.2013 20:18, schrieb Ralf Thielow: From: Junio C Hamano gits...@pobox.com Some users do want to write a line that begin with a pound sign, #, in their commit log message. Many tracking system recognise a token of #bugid form, for example. The support we offer these use cases is not very friendly to the end users. They have a choice between - Don't do it. Avoid such a line by rewrapping or indenting; and - Use --cleanup=whitespace but remove all the hint lines we add. Give them a way to set a custom comment char, e.g. $ git -c core.commentchar=% commit so that they do not have to do either of the two workarounds. Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- Junio, thanks for the code in your reply to the first version. It works very well and looks nice. I was also unhappy about this \n%c\n thing and pretty unsure with the code in git-submodule.sh. I can't see anything wrong with it (but didn't have the time to test it). On my todo list (but *way* down) is the task to replace the call to git submodule summary --for-status ... in wt_status_print_submodule_summary() with a call to git diff --submodule (and - at least in the long term - rip out the --for-status option from the submodule script). Maybe now is a good time for someone else to tackle that? (especially as the new strbuf_commented_add*() functions should make that rather easy) -- 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: Question re. git remote repository
Konstantin Khomoutov kostix+...@007spb.ru wrote in message news:20130116233744.7d0775eaec98ce154a9de...@domain007.com... On Wed, 16 Jan 2013 10:21:56 -0800 Jeff King p...@peff.net wrote: I agree that performance is not ideal (although if you are on a fast LAN, it probably would not matter much), but I do not recall any specific bugs in that area. Can you elaborate? Of course, if there are happy users of such setups, I would be glad to hear as my precautions might well be unfounded for the recent versions of Git. I'm a happy user of git on network file systems (NFS and CIFS/SMB), although not a heavy user. 1. http://code.google.com/p/msysgit/issues/detail?id=130 I wouldn't be surprised if there are some subtle POSIX-Win32 compatibility issues here between msysgit and Samba (POSIX GIT, ported to use Win32 file system functions, sending those Win32 requests to a Samba server, and the Samba server translating those Win32 requests back into POSIX functions). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/3] config: Introduce diff.algorithm variable
Junio C Hamano gits...@pobox.com writes: Will replace the one in 'pu' with this round. Looking good. Thanks. By the way, wouldn't we want some tests to protect this feature from future breakages? -- 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] clean.c, ls-files.c: respect encapsulation of exclude_list_groups
Adam Spiers g...@adamspiers.org writes: Consumers of the dir.c traversal API should avoid assuming knowledge of the internal implementation of exclude_list_groups. Therefore when adding items to an exclude list, it should be accessed via the pointer returned from add_exclude_list(), rather than by referencing a location within dir.exclude_list_groups[EXC_CMDL]. Signed-off-by: Adam Spiers g...@adamspiers.org --- builtin/clean.c| 6 +++--- builtin/ls-files.c | 15 ++- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index b098288..b9cb7ad 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -45,6 +45,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) static const char **pathspec; struct strbuf buf = STRBUF_INIT; struct string_list exclude_list = STRING_LIST_INIT_NODUP; + struct exclude_list *el; When a type exclude_list exists and used in the same function, having a local variable of the same name but of a different type becomes a bit awkward. builtin/ls-files.c shares the same structure. Does the file-scope exclude_args variable need to be a file-scope static over there? It seems that it is closely tied to the elements of the string list, so it may make sense to: * remove the file-scope static exclude_args; * rename exclude_list string list variable to exclude_args; and * replace --exclude_args in the loop that iterates over exclude_list (now exclude_args) with -(i+1) or something, just like you do in builtin/clean.c below. - add_exclude_list(dir, EXC_CMDL, --exclude option); + el = add_exclude_list(dir, EXC_CMDL, --exclude option); for (i = 0; i exclude_list.nr; i++) - add_exclude(exclude_list.items[i].string, , 0, - dir.exclude_list_group[EXC_CMDL].el[0], -(i+1)); + add_exclude(exclude_list.items[i].string, , 0, el, -(i+1)); We may want to use for_each_string_list_item() here and in the corresponding loop in builtin/ls-files.c, but because we do need to give the -(i + 1) label to each element, I think the code is OK as-is. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] fix clang -Wconstant-conversion with bit fields
clang incorrectly reports a constant conversion warning (implicit
truncation to bit field) when using the flag = ~FLAG form, because
~FLAG needs to be truncated.
Convert this form to flag = flag ~FLAG fixes the issue as
the right operand now fits into the bit field.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
I'm sorry about this fix, it really seems bad, yet it's one step closer
to warning-free clang compilation.
It seems quite clear to me that it's a bug in clang.
bisect.c | 2 +-
builtin/checkout.c | 2 +-
builtin/reflog.c | 4 ++--
commit.c | 4 ++--
revision.c | 8
upload-pack.c | 4 ++--
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/bisect.c b/bisect.c
index bd1b7b5..34ac01d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -63,7 +63,7 @@ static void clear_distance(struct commit_list *list)
{
while (list) {
struct commit *commit = list-item;
- commit-object.flags = ~COUNTED;
+ commit-object.flags = commit-object.flags ~COUNTED;
list = list-next;
}
}
diff --git a/builtin/checkout.c b/builtin/checkout.c
index a9c1b5a..2c83234 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -717,7 +717,7 @@ static void orphaned_commit_warning(struct commit *old,
struct commit *new)
init_revisions(revs, NULL);
setup_revisions(0, NULL, revs, NULL);
- object-flags = ~UNINTERESTING;
+ object-flags = object-flags ~UNINTERESTING;
add_pending_object(revs, object, sha1_to_hex(object-sha1));
for_each_ref(add_pending_uninteresting_ref, revs);
diff --git a/builtin/reflog.c b/builtin/reflog.c
index b3c9e27..3079c81 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -170,7 +170,7 @@ static int commit_is_complete(struct commit *commit)
}
/* clear flags from the objects we traversed */
for (i = 0; i found.nr; i++)
- found.objects[i].item-flags = ~STUDYING;
+ found.objects[i].item-flags = found.objects[i].item-flags
~STUDYING;
if (is_incomplete)
commit-object.flags |= INCOMPLETE;
else {
@@ -229,7 +229,7 @@ static void mark_reachable(struct expire_reflog_cb *cb)
struct commit_list *leftover = NULL;
for (pending = cb-mark_list; pending; pending = pending-next)
- pending-item-object.flags = ~REACHABLE;
+ pending-item-object.flags = pending-item-object.flags
~REACHABLE;
pending = cb-mark_list;
while (pending) {
diff --git a/commit.c b/commit.c
index e8eb0ae..800779d 100644
--- a/commit.c
+++ b/commit.c
@@ -883,7 +883,7 @@ struct commit_list *reduce_heads(struct commit_list *heads)
/* Uniquify */
for (p = heads; p; p = p-next)
- p-item-object.flags = ~STALE;
+ p-item-object.flags = p-item-object.flags ~STALE;
for (p = heads, num_head = 0; p; p = p-next) {
if (p-item-object.flags STALE)
continue;
@@ -894,7 +894,7 @@ struct commit_list *reduce_heads(struct commit_list *heads)
for (p = heads, i = 0; p; p = p-next) {
if (p-item-object.flags STALE) {
array[i++] = p-item;
- p-item-object.flags = ~STALE;
+ p-item-object.flags = p-item-object.flags ~STALE;
}
}
num_head = remove_redundant(array, num_head);
diff --git a/revision.c b/revision.c
index d7562ee..ed1c16d 100644
--- a/revision.c
+++ b/revision.c
@@ -787,9 +787,9 @@ static void limit_to_ancestry(struct commit_list *bottom,
struct commit_list *li
/* We are done with the TMP_MARK */
for (p = list; p; p = p-next)
- p-item-object.flags = ~TMP_MARK;
+ p-item-object.flags = p-item-object.flags ~TMP_MARK;
for (p = bottom; p; p = p-next)
- p-item-object.flags = ~TMP_MARK;
+ p-item-object.flags = p-item-object.flags ~TMP_MARK;
free_commit_list(rlist);
}
@@ -1948,7 +1948,7 @@ static int remove_duplicate_parents(struct commit *commit)
/* count them while clearing the temporary mark */
surviving_parents = 0;
for (p = commit-parents; p; p = p-next) {
- p-item-object.flags = ~TMP_MARK;
+ p-item-object.flags = p-item-object.flags ~TMP_MARK;
surviving_parents++;
}
return surviving_parents;
@@ -2378,7 +2378,7 @@ static struct commit *get_revision_1(struct rev_info
*revs)
if (revs-reflog_info) {
fake_reflog_parent(revs-reflog_info, commit);
- commit-object.flags = ~(ADDED | SEEN | SHOWN);
+ commit-object.flags = commit-object.flags ~(ADDED |
SEEN | SHOWN);
}
/*
diff --git a/upload-pack.c b/upload-pack.c
index 7c05b15..74d8f0e 100644
---
[PATCH 2/2] fix clang -Wtautological-compare with unsigned enum
Create a GREP_HEADER_FIELD_MIN so we can check that the field value is
sane and silent the clang warning.
Clang warning happens because the enum is unsigned (this is
implementation-defined, and there is no negative fields) and the check
is then tautological.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
I tried to consider discussion [1] and this [2] discussion on clang's list
With these two patches and the patch from Max Horne, I'm finally able to
compile with CC=clang CFLAGS=-Werror.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/184908
[2]:
http://clang-developers.42468.n3.nabble.com/Possibly-invalid-enum-tautology-warning-td3233140.html
grep.c | 3 ++-
grep.h | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/grep.c b/grep.c
index 4bd1b8b..bb548ca 100644
--- a/grep.c
+++ b/grep.c
@@ -625,7 +625,8 @@ static struct grep_expr *prep_header_patterns(struct
grep_opt *opt)
for (p = opt-header_list; p; p = p-next) {
if (p-token != GREP_PATTERN_HEAD)
die(bug: a non-header pattern in grep header list.);
- if (p-field 0 || GREP_HEADER_FIELD_MAX = p-field)
+ if (p-field GREP_HEADER_FIELD_MIN ||
+ GREP_HEADER_FIELD_MAX = p-field)
die(bug: unknown header field %d, p-field);
compile_regexp(p, opt);
}
diff --git a/grep.h b/grep.h
index 8fc854f..e4a1df5 100644
--- a/grep.h
+++ b/grep.h
@@ -28,7 +28,8 @@ enum grep_context {
};
enum grep_header_field {
- GREP_HEADER_AUTHOR = 0,
+ GREP_HEADER_FIELD_MIN = 0,
+ GREP_HEADER_AUTHOR = GREP_HEADER_FIELD_MIN,
GREP_HEADER_COMMITTER,
GREP_HEADER_REFLOG,
--
1.8.1.1.435.g20d29be.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: Question re. git remote repository
Ideally we'd prefer to simply create our remote repository on a drive of one of our local network servers. Is this possible? Yes, this is possible, but it's not advised to keep such a reference repository on an exported networked drive for a number of reasons (both performance and bug-free operation). I agree that performance is not ideal (although if you are on a fast LAN, it probably would not matter much), but I do not recall any specific bugs in that area. Can you elaborate? This one [1] for instance. I also recall seing people having other mystical problems with setups like this so I somehow developed an idea than having a repository on a networked drive is asking for troubles. Of course, if there are happy users of such setups, I would be glad to hear as my precautions might well be unfounded for the recent versions of Git. 1. http://code.google.com/p/msysgit/issues/detail?id=130 A group I was with used a master repository on a windows share for quite some time without a database corruption being seen. -- 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: Question re. git remote repository
On Wed, 16 Jan 2013, Stephen Smith wrote: Ideally we'd prefer to simply create our remote repository on a drive of one of our local network servers. Is this possible? Yes, this is possible, but it's not advised to keep such a reference repository on an exported networked drive for a number of reasons (both performance and bug-free operation). I agree that performance is not ideal (although if you are on a fast LAN, it probably would not matter much), but I do not recall any specific bugs in that area. Can you elaborate? This one [1] for instance. I also recall seing people having other mystical problems with setups like this so I somehow developed an idea than having a repository on a networked drive is asking for troubles. Of course, if there are happy users of such setups, I would be glad to hear as my precautions might well be unfounded for the recent versions of Git. 1. http://code.google.com/p/msysgit/issues/detail?id=130 A group I was with used a master repository on a windows share for quite some time without a database corruption being seen. -- I think the risk is that if you have multiple people doing actions on the shared filesystem you can run into trouble. As long as only one copy of git is ever running against the repository, I don't see any reason for there to be a problem. But if you try to have one filesystem, with multiple people running git on their machines against that shared filesystem, I would expect you to have all sorts of problems. David Lang -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
On Wed, Jan 16, 2013 at 11:47:22PM +0100, Antoine Pelisse wrote: clang incorrectly reports a constant conversion warning (implicit truncation to bit field) when using the flag = ~FLAG form, because ~FLAG needs to be truncated. Convert this form to flag = flag ~FLAG fixes the issue as the right operand now fits into the bit field. Signed-off-by: Antoine Pelisse apeli...@gmail.com --- I'm sorry about this fix, it really seems bad, yet it's one step closer to warning-free clang compilation. It seems quite clear to me that it's a bug in clang. Which version of clang did you see this with? I don't get these warnings with clang 3.2. John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
On Thu, Jan 17, 2013 at 12:08 AM, John Keeping j...@keeping.me.uk wrote: On Wed, Jan 16, 2013 at 11:47:22PM +0100, Antoine Pelisse wrote: clang incorrectly reports a constant conversion warning (implicit truncation to bit field) when using the flag = ~FLAG form, because ~FLAG needs to be truncated. Which version of clang did you see this with? I don't get these warnings with clang 3.2. Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0) It's good to know it's been fixed ! -- 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] fix clang -Wtautological-compare with unsigned enum
With these two patches and the patch from Max Horne, I'm deeply sorry for this typo Max -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
So I guess we should drop this patch, it's probably not worth it, especially if it's been fixed already by clang. On Thu, Jan 17, 2013 at 12:09 AM, Antoine Pelisse apeli...@gmail.com wrote: On Thu, Jan 17, 2013 at 12:08 AM, John Keeping j...@keeping.me.uk wrote: On Wed, Jan 16, 2013 at 11:47:22PM +0100, Antoine Pelisse wrote: clang incorrectly reports a constant conversion warning (implicit truncation to bit field) when using the flag = ~FLAG form, because ~FLAG needs to be truncated. Which version of clang did you see this with? I don't get these warnings with clang 3.2. Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0) It's good to know it's been fixed ! -- 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: Question re. git remote repository
David Lang da...@lang.hm wrote in message news:alpine.deb.2.02.1301161459060.21...@nftneq.ynat.uz... But if you try to have one filesystem, with multiple people running git on their machines against that shared filesystem, I would expect you to have all sorts of problems. What leads you to think you will have problems? Why would there be more of a problem on a network file system as opposed to local file system that can be accessed by multiple users? Linus seemed to think it should work: http://permalink.gmane.org/gmane.comp.version-control.git/122670 And git init specifically has a shared option: --shared[=(false|true|umask|group|all|world|everybody|0xxx)] Specify that the git repository is to be shared amongst several users. This allows users belonging to the same group to push into that repository. When specified, the config variable core.sharedRepository is set so that files and directories under $GIT_DIR are created with the requested permissions. When not specified, git will use permissions reported by umask(2). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
Antoine Pelisse apeli...@gmail.com writes:
clang incorrectly reports a constant conversion warning (implicit
truncation to bit field) when using the flag = ~FLAG form, because
~FLAG needs to be truncated.
Convert this form to flag = flag ~FLAG fixes the issue as
the right operand now fits into the bit field.
If the clang incorrectly reports is already recognised by clang
folks as a bug to be fixed in clang, I'd rather not to take this
patch.
I do not think it is reasonable to expect people to remember that
they have to write flags = ~TO_DROP in a longhand whenever they
are adding new code that needs to do bit-fields, so even if this
patch makes clang silent for the _current_ code, it will not stay
that way. Something like
#define FLIP_BIT_CLR(fld,bit) do { \
typeof(fld) *x = (fld); \
*x = *x (~(bit)); \
} while (0)
may be more palapable but not by a large margin.
Yuck.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
Junio C Hamano gits...@pobox.com writes:
Antoine Pelisse apeli...@gmail.com writes:
clang incorrectly reports a constant conversion warning (implicit
truncation to bit field) when using the flag = ~FLAG form, because
~FLAG needs to be truncated.
Convert this form to flag = flag ~FLAG fixes the issue as
the right operand now fits into the bit field.
If the clang incorrectly reports is already recognised by clang
folks as a bug to be fixed in clang, I'd rather not to take this
patch.
I do not think it is reasonable to expect people to remember that
they have to write flags = ~TO_DROP in a longhand whenever they
are adding new code that needs to do bit-fields, so even if this
patch makes clang silent for the _current_ code, it will not stay
that way. Something like
#define FLIP_BIT_CLR(fld,bit) do { \
typeof(fld) *x = (fld); \
*x = *x (~(bit)); \
} while (0)
may be more palapable but not by a large margin.
Yuck.
Double yuck. I meant palatable.
In any case, I see somebody reports that more recent clang does not
have this bug in the near-by message, so let's forget about this
issue.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question re. git remote repository
On Wed, 16 Jan 2013, Matt Seitz (matseitz) wrote: David Lang da...@lang.hm wrote in message news:alpine.deb.2.02.1301161459060.21...@nftneq.ynat.uz... But if you try to have one filesystem, with multiple people running git on their machines against that shared filesystem, I would expect you to have all sorts of problems. What leads you to think you will have problems? Why would there be more of a problem on a network file system as opposed to local file system that can be accessed by multiple users? There are safety checks and synchronization primitives that work between mulitiple users on one machine (where you can see what other processes are running for example) that don't work with separate machines using a filesystem Linus seemed to think it should work: http://permalink.gmane.org/gmane.comp.version-control.git/122670 well, he knows git better than I do, but using git over NFS/CIFS is not the same as saying that you have multiple users on different systems making changes. In the link you point at, he says that you can have problems with some types of actions. He points out things like git prune, but I would also say that there are probably race conditions if you have two git processes that try to change the HEAD to different things at the same time. And git init specifically has a shared option: --shared[=(false|true|umask|group|all|world|everybody|0xxx)] Specify that the git repository is to be shared amongst several users. This allows users belonging to the same group to push into that repository. When specified, the config variable core.sharedRepository is set so that files and directories under $GIT_DIR are created with the requested permissions. When not specified, git will use permissions reported by umask(2). I think this is dealing with multiple users _reading_ a repository, not making updates to it at the same time. David Lang -- 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 3/8] git_remote_helpers: Force rebuild if python version changes
j...@keeping.me.uk wrote on Tue, 15 Jan 2013 22:58 +:
For reference, putting the version check in setup.py looks like this:
-- 8 --
diff --git a/git_remote_helpers/setup.py b/git_remote_helpers/setup.py
index 6de41de..2c21eb5 100644
--- a/git_remote_helpers/setup.py
+++ b/git_remote_helpers/setup.py
@@ -3,6 +3,7 @@
Distutils build/install script for the git_remote_helpers package.
from distutils.core import setup
+import sys
# If building under Python3 we need to run 2to3 on the code, do this by
# trying to import distutils' 2to3 builder, which is only available in
@@ -13,6 +14,24 @@ except ImportError:
# 2.x
from distutils.command.build_py import build_py
+
+current_version = '%d.%d' % sys.version_info[:2]
+try:
+f = open('GIT-PYTHON_VERSION', 'r')
+latest_version = f.read().strip()
+f.close()
+
+if latest_version != current_version:
+if not '--force' in sys.argv:
+sys.argv.insert(0, '--force')
+except IOError:
+pass
+
+f = open('GIT-PYTHON_VERSION', 'w')
+f.write(current_version)
+f.close()
+
+
setup(
name = 'git_remote_helpers',
version = '0.1.0',
That's about the same overhead as doing it in the Makefile,
and a bit more obscure. I don't mind your initial version
so much anymore. Thanks for thinking about it.
-- Pete
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/8 v3] git_remote_helpers: fix input when running under Python 3
j...@keeping.me.uk wrote on Wed, 16 Jan 2013 09:45 +: On Tue, Jan 15, 2013 at 07:03:16PM -0500, Pete Wyckoff wrote: I'd suggest for this Python conundrum using byte-string literals, e.g.: lines = check_output(args).strip().split(b'\n') value, name = line.split(b' ') name = name.strip(b'commit\t') Essentially identical to what you have, but avoids naming utf-8 as the encoding. It instead relies on Python's interpretation of ASCII characters in string context, which is exactly what C does. The problem is that AFAICT the byte-string prefix is only available in Python 2.7 and later (compare [1] and [2]). I think we need this more convoluted code if we want to keep supporting Python 2.6 (although perhaps 'ascii' would be a better choice than 'utf-8'). [1] http://docs.python.org/2.6/reference/lexical_analysis.html#literals [2] http://docs.python.org/2.7/reference/lexical_analysis.html#literals Drat. The b'' syntax seems to work on 2.6.8, in spite of the docs, but certainly isn't in 2.5. I think you had hit on the best compromise with encoding, but maybe ascii is a little less presumptuous than utf-8, and more indicative of the encoding assumption. -- Pete -- 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] Add Auto-Submitted header to post-receive-email
Hi Chris, Chris Hiestand wrote: Andy, do you have any thoughts on adding this email header to contrib/hooks/post-receive-email? This patch shouldn't cause problems for anyone with a sanely configured mail delivery agent, and the additional header is very useful in toggling auto responses. I'm not Andy, but it sounds very sane to me. So for what it's worth, Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks, Jonathan (patch left unsnipped for reference) This conforms to RFC3834 and is useful in preventing eg vacation auto-responders from replying by default Signed-off-by: Chris Hiestand chiest...@salk.edu --- contrib/hooks/post-receive-email |1 + 1 file changed, 1 insertion(+) diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email index b2171a0..0e5b72d 100755 --- a/contrib/hooks/post-receive-email +++ b/contrib/hooks/post-receive-email @@ -237,6 +237,7 @@ generate_email_header() X-Git-Reftype: $refname_type X-Git-Oldrev: $oldrev X-Git-Newrev: $newrev + Auto-Submitted: auto-generated This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing -- 1.7.10.4 -- 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: Question re. git remote repository
From: David Lang [mailto:da...@lang.hm] On Wed, 16 Jan 2013, Matt Seitz (matseitz) wrote: Linus seemed to think it should work: http://permalink.gmane.org/gmane.comp.version-control.git/122670 In the link you point at, he says that you can have problems with some types of actions. He points out things like git prune, Linus wrote: You do need to be a bit careful if you do maintenance operations concurrently (I would suggest avoiding doing concurrent git gc --prune, for example), but any normal git workflow should be fine. but I would also say that there are probably race conditions if you have two git processes that try to change the HEAD to different things at the same time. What makes you think there are race conditions? Linus wrote: And git doesn't have proper locking, because it doesn't need it for database ops: git objects are stable. For refs, git should be using the proper NFS-safe create and atomic rename ops. And git init specifically has a shared option: --shared[=(false|true|umask|group|all|world|everybody|0xxx)] I think this is dealing with multiple users _reading_ a repository, not making updates to it at the same time. The description of shared says This allows users belonging to the same group to push into that repository. The push command is about making updates. -- 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: Question re. git remote repository
On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote: From: David Lang [mailto:da...@lang.hm] On Wed, 16 Jan 2013, Matt Seitz (matseitz) wrote: Linus seemed to think it should work: http://permalink.gmane.org/gmane.comp.version-control.git/122670 In the link you point at, he says that you can have problems with some types of actions. He points out things like git prune, Linus wrote: You do need to be a bit careful if you do maintenance operations concurrently (I would suggest avoiding doing concurrent git gc --prune, for example), but any normal git workflow should be fine. but I would also say that there are probably race conditions if you have two git processes that try to change the HEAD to different things at the same time. What makes you think there are race conditions? Linus wrote: And git doesn't have proper locking, because it doesn't need it for database ops: git objects are stable. For refs, git should be using the proper NFS-safe create and atomic rename ops. As Linus points out, objects are stable, so when you create objects you don't have to worry about locking, if two things write an object at the same time, the same contents are being written so races don't matter. However, if you have two people doing a commit or merge, you will get different results based on the order they are happening in. This seems to be exactly the type of thing that falls into the 'maintinance operations' category. Linus says that git does not have proper locking, so think about it, what do you think will happen if person A does git add a/b; git commit and person B does git add c/d; git commit? Since the tree will look different depending on what order these four commands execute in, the resulting HEAD could have multiple different values depending on the order. The individual commits may even be different. David Lang And git init specifically has a shared option: --shared[=(false|true|umask|group|all|world|everybody|0xxx)] I think this is dealing with multiple users _reading_ a repository, not making updates to it at the same time. The description of shared says This allows users belonging to the same group to push into that repository. The push command is about making updates. -- 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: Question re. git remote repository
From: David Lang [mailto:da...@lang.hm] Linus says that git does not have proper locking, so think about it, what do you think will happen if person A does git add a/b; git commit and person B does git add c/d; git commit? Sorry, I wasn't clear. My assumption is that a shared repository on a network file system will either be: 1. a bare repository that is normally accessed only by git push and git pull (or git fetch), the central repository model. 2. a repository where only one user does git add and git commit, while other users will do git pull, the peer-to-peer model (you pull changes from me, I pull changes from you). -- 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: Question re. git remote repository
On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote: From: David Lang [mailto:da...@lang.hm] Linus says that git does not have proper locking, so think about it, what do you think will happen if person A does git add a/b; git commit and person B does git add c/d; git commit? Sorry, I wasn't clear. My assumption is that a shared repository on a network file system will either be: 1. a bare repository that is normally accessed only by git push and git pull (or git fetch), the central repository model. pulling from it would not be a problem, I could see issues with multiple pushes taking place (the underlying repository would not get corrupted, but you will very quickly hit conflicts where the push is not a fast forward and you need to merge, not just push) 2. a repository where only one user does git add and git commit, while other users will do git pull, the peer-to-peer model (you pull changes from me, I pull changes from you). At this point only one system is writing to the repository and it doesn't matter that it's on network storage vs local storage. pulling from a shared repository is probably safe, but I wouldn't bet against there being any conditions where a pull at the same time someone is doing an update being able to cause problems. The normal thing is to do the pulls through git-daemon, and that does make sure that what you are pulling is consistant. David Lang -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] push: update remote tags only with force
On Wed, Jan 16, 2013 at 11:43 AM, Jeff King p...@peff.net wrote: I think that is a reasonable rule that could be applied across all parts of the namespace hierarchy. And it could be applied by the client, because all you need to know is whether ref-old_sha1 is reachable from ref-new_sha1. is_forwardable() did solve a UI issue. Previously all instances where old is not reachable by new were assumed to be addressable with a merge. is_forwardable() attempted to determine if the concept of forwarding made sense given the inputs. For example, if old is a blob it is useless to suggest merging it. Chris -- 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: Question re. git remote repository
From: David Lang [mailto:da...@lang.hm] On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote: 1. a bare repository that is normally accessed only by git push and git pull (or git fetch), the central repository model. pulling from it would not be a problem, I could see issues with multiple pushes taking place (the underlying repository would not get corrupted, but you will very quickly hit conflicts where the push is not a fast forward and you need to merge, not just push) How is that different on a network file system, as opposed to using http, ssh, or git-daemon? Don't you get a not a fast-forward error, regardless of the protocol? 2. a repository where only one user does git add and git commit, while other users will do git pull, the peer-to-peer model (you pull changes from me, I pull changes from you). pulling from a shared repository is probably safe, but I wouldn't bet against there being any conditions where a pull at the same time someone is doing an update being able to cause problems. Why do you think there would be a problem? The normal thing is to do the pulls through git-daemon, and that does make sure that what you are pulling is consistant. What does git pull via git-daemon do to ensure consistency that is different from git pull on a network file system? -- 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: Question re. git remote repository
On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote: From: David Lang [mailto:da...@lang.hm] On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote: 1. a bare repository that is normally accessed only by git push and git pull (or git fetch), the central repository model. pulling from it would not be a problem, I could see issues with multiple pushes taking place (the underlying repository would not get corrupted, but you will very quickly hit conflicts where the push is not a fast forward and you need to merge, not just push) How is that different on a network file system, as opposed to using http, ssh, or git-daemon? Don't you get a not a fast-forward error, regardless of the protocol? true. 2. a repository where only one user does git add and git commit, while other users will do git pull, the peer-to-peer model (you pull changes from me, I pull changes from you). pulling from a shared repository is probably safe, but I wouldn't bet against there being any conditions where a pull at the same time someone is doing an update being able to cause problems. Why do you think there would be a problem? The normal thing is to do the pulls through git-daemon, and that does make sure that what you are pulling is consistant. What does git pull via git-daemon do to ensure consistency that is different from git pull on a network file system? git pull via the daemon looks at what tree items you have on each end, and then it sends you the items needed to make you match the server. If there are partial updates on the server (due to some update in process) the daemon should not see that, but if you are grabbing the files directly, I would be less confident that you are always safe. you may _be_ safe, and if others who really know the internals speak up, take their word on it. But, absent assurances that we know that everything is done in the right order in the face of a networked filesystem (which may break visibility of changes due to caching), I would not trust such raw access for updates at all, and only somewhat trust it for read-only use. David Lang -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] push: update remote tags only with force
On Wed, Jan 16, 2013 at 08:19:28PM -0600, Chris Rorvick wrote: On Wed, Jan 16, 2013 at 11:43 AM, Jeff King p...@peff.net wrote: I think that is a reasonable rule that could be applied across all parts of the namespace hierarchy. And it could be applied by the client, because all you need to know is whether ref-old_sha1 is reachable from ref-new_sha1. is_forwardable() did solve a UI issue. Previously all instances where old is not reachable by new were assumed to be addressable with a merge. is_forwardable() attempted to determine if the concept of forwarding made sense given the inputs. For example, if old is a blob it is useless to suggest merging it. I think it makes sense to mark such a case as different from a regular non-fast-forward (because git pull is not the right advice), but: 1. is_forwardable should assume a missing object is a commit not to regress the common case; otherwise we do not show the pull advice when we probably should, and most of the time it is going to be a commit 2. When we know that we are not working with commits, I am not sure that already exists is the right advice to give for such a case. It is neither this tag already exists, so we do not update it, nor is it strictly cannot fast forward this commit, but rather something else. The expanded definition of what is a fast forward that I suggested would let this fall naturally between the two. -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 v6 0/8] push: update remote tags only with force
On Wed, Jan 16, 2013 at 9:11 PM, Jeff King p...@peff.net wrote: is_forwardable() did solve a UI issue. Previously all instances where old is not reachable by new were assumed to be addressable with a merge. is_forwardable() attempted to determine if the concept of forwarding made sense given the inputs. For example, if old is a blob it is useless to suggest merging it. I think it makes sense to mark such a case as different from a regular non-fast-forward (because git pull is not the right advice), but: 1. is_forwardable should assume a missing object is a commit not to regress the common case; otherwise we do not show the pull advice when we probably should, and most of the time it is going to be a commit Yes, obviously this was a bug, thus the use of attempted above. It would have been better to assume a missing 'old' was potentially forwardable to present the user with the most helpful advice. 2. When we know that we are not working with commits, I am not sure that already exists is the right advice to give for such a case. It is neither this tag already exists, so we do not update it, nor is it strictly cannot fast forward this commit, but rather something else. But the reference already existing in the remote is a substantial reason for not allowing the push in all of these cases. You can break this out further if you like to explain why the specific reference shouldn't be moved on the remote, but this is even more complicated a simple is old reachable from new? test. Chris -- 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 07/14] imap-send.c: inline imap_parse_list() in imap_list()
On 01/16/2013 04:34 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: On 01/15/2013 07:51 PM, Matt Kraai wrote: On Tue, Jan 15, 2013 at 09:06:25AM +0100, Michael Haggerty wrote: -static struct imap_list *parse_imap_list(struct imap *imap, char **sp) +static struct imap_list *parse_list(char **sp) The commit subject refers to imap_parse_list and imap_list whereas the code refers to parse_imap_list and parse_list. Yes, you're right. Thanks. I think I've fixed this (and some other minor points in other patches in the series) while queuing; please check master..3691031c after fetching from me. Looks good. Thanks. 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: Question re. git remote repository
David Lang da...@lang.hm wrote in message news:alpine.deb.2.02.1301161843390.21...@nftneq.ynat.uz... On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote: 2. a repository where only one user does git add and git commit, while other users will do git pull, the peer-to-peer model (you pull changes from me, I pull changes from you). you may _be_ safe, and if others who really know the internals speak up, take their word on it. Well, we already have Linus's article. I guess the question is whether the use case I describe above falls under: A. maintenance operations B. normal git workflow Personally, I would consider this use case to be a normal git workflow. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] push: update remote tags only with force
On Wed, Jan 16, 2013 at 10:48 AM, Junio C Hamano gits...@pobox.com wrote: It is fine when pushing into refs/tags/ hierarchy. It is *NOT* OK if the type check does not satisfy this function. In that case, we do not actually see the existence of the destination as a problem, but it is reported as such. We are blocking because we do not like the type of the new object or the type of the old object. If the destination points at a commit, the push can succeed if the user changes what object to push, so saying you cannot push because the destination already exists is just wrong in such a case. So the solution is to revert back to recommending a merge? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] push: update remote tags only with force
Chris Rorvick ch...@rorvick.com writes: On Wed, Jan 16, 2013 at 10:48 AM, Junio C Hamano gits...@pobox.com wrote: It is fine when pushing into refs/tags/ hierarchy. It is *NOT* OK if the type check does not satisfy this function. In that case, we do not actually see the existence of the destination as a problem, but it is reported as such. We are blocking because we do not like the type of the new object or the type of the old object. If the destination points at a commit, the push can succeed if the user changes what object to push, so saying you cannot push because the destination already exists is just wrong in such a case. So the solution is to revert back to recommending a merge? Of course not, because at that point you may not even have what you were attempting to overwrite. Nobody says it is even something you could merge. The recommended solution certainly will involve a fetch (not pull or pull --rebase). You fetch from over there to check what you were about to overwrite, examine the situation to decide what the appropriate action is. The point is that Git in general, and the codepath that was touched by the patch in particular, does not have enough information to decide what the appropriate action is for the user, especially when the ref is outside the ones we know what the conventional uses of them are. We can make policy decisions like tags are meant to be unmoving anchor points, so it is unusual to overwrite any old with any new, heads are meant to be branch tips, and because rewinding them while more than one repositories are working with them will cause issues to other repositories, it is unusual to push a non-fast-forward and enforcement mechanism for such policy decisions will help users, but that is only because we know what their uses are. The immediate action we should take is to get closer to the original behaviour of not complaining with ref already exists, which is nonsensical. That does not mean that we will forbid improving the codepath by giving different advices depending on the case. One of the new advices could tell them to fetch it and inspect the situation, if old is not something we do not even have (hence we cannot check its type, let alone the ancestry relationship of it with new), for example. -- 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
