Re: [PATCH] am: terminate state files with a newline
On Sun, Aug 23, 2015 at 11:48:44PM -0700, Junio C Hamano wrote: > > I think the "if" here is redundant; strbuf_complete_line already handles > > it. > > True. And I like your write_state_bool() wrapper (which should be > "static void" to the builtin/am.c) very much. > > On top of that, I think the right thing to do to write_file() would > be to first clean-up the second parameter "fatal" to an "unsigned > flags" whose (1<<0) bit is "fatal", (1<<1) bit is "binary", and make > this new call to "strbuf_complete_line()" only when "binary" bit is > not set. > > The new comment I added before write_file() function needs to be > adjusted if we were to do this, obviously. Yup, I agree with all of that. I'm about to go to bed, so I'll assume you or Paul will cook up a patch. :) -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] am: terminate state files with a newline
Jeff King writes:
> FWIW, I had a similar thought when reading the original thread. I also
> noted that all of the callers here pass "1" for the "fatal" parameter,
> and that they are either bools or single strings. I wonder if:
>
> void write_state_bool(struct am_state *state, const char *name, int v)
> {
> write_file(am_path(state, name), 1, "%s\n", v ? "t" : "f");
> }
>
> would make the call-sites even easier to read (and of course the "\n"
> would be dropped here if it does migrate up to write_file()).
>
>> @@ -634,6 +641,9 @@ int write_file(const char *path, int fatal, const char
>> *fmt, ...)
>> va_start(params, fmt);
>> strbuf_vaddf(&sb, fmt, params);
>> va_end(params);
>> +if (sb.len)
>> +strbuf_complete_line(&sb);
>> +
>
> I think the "if" here is redundant; strbuf_complete_line already handles
> it.
True. And I like your write_state_bool() wrapper (which should be
"static void" to the builtin/am.c) very much.
On top of that, I think the right thing to do to write_file() would
be to first clean-up the second parameter "fatal" to an "unsigned
flags" whose (1<<0) bit is "fatal", (1<<1) bit is "binary", and make
this new call to "strbuf_complete_line()" only when "binary" bit is
not set.
The new comment I added before write_file() function needs to be
adjusted if we were to do this, obviously.
--
To unsubscribe from this list: send the line "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: Minor bug with help.autocorrect.
Jeff King writes: > Yeah, I agree these are the only two sane fixes. Plumbing the quiet flag > through does seem really invasive; every "git_config_get_foo" variant > would have to learn it. Probably it's too gross to have a global like: > > config_lax_mode = 1; > git_config_get_string(key.buf, &v); > config_lax_mode = 0; > > That makes a nice tidy patch, but I have a feeling we would regret it > later. :) Yeah, I do think the double-checking the patch in your follow-up does is not so bad. Thanks for following it through (now I must remember not to drop these patches ;-). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] git-p4: fix faulty paths for case insensitive systems
Lars Schneider writes: >> - Have you checked "git log" on our history and notice how nobody >> says "PROBLEM:" and "SOLUTION:" in capital letters? Don't try to >> be original in the form; your contributions are already original >> and valuable in the substance ;-) > haha ok. I will make them lower case :-) I cannot tell if you are joking or not, but just in case you are serious, please check "git log" for recent history again. We do not mark our paragraphs with noisy labels like "PROBLEM" and "SOLUTION", regardless of case. Typically, our description outlines the current status (which prepares the reader's mind to understand what you are going to talk about), highlight what is problematic in that current status, and then explains what change the patch does and justifies why it is the right change, in this order. So those who read your description can tell PROBLEM and SOLUTION apart without being told with labels. >> - I think I saw v3 yesterday. It would be nice to see a brief >> description of what has been updated in this version. > I discovered an optimization. In v3 I fixed the paths of *all* files > that are underneath of a given P4 clone path. In v4 I fix only the > paths that are visible on the client via client-spec (P4 can perform > partial checkouts via “client-views”). I was wondering how to convey > this change. Would have been a cover letter for v4 the correct way or > should I have made another commit on top of my v3 change? Often people do this with either (1) a cover letter for v4, that shows the "git diff" output to go from the result of applying v3 to the result of applying v4 to the same initial state; or (2) a textual description after three-dash line of v4 that explains what has changed relative to v3. The latter is often done when the change between v3 and v4 is small enough. > Yes, that is PEP-8 style and I will change it > accordingly. Unfortunately git-p4.py does not follow a style guide. > e.g. line 2369: def commit(self, details, files, branch, parent = ""): OK, just as I suspected. Then do not worry too much about it for now, as fixes to existing style violations should be done outside of this change, perhaps after the dust settles (or if you prefer, you can do so as a preliminary clean-up patch, that does not change anything but style, and then build your fix on top of it). > More annoyingly (to me at least) is that git-p4 mixes CamelCase with > snake_case even within classes/functions. I think I read somewhere > that these kind of refactorings are discouraged. I assume that applies > here, too? If you are doing something other than style fixes (call that "meaningful work"), it is strongly discouraged to fix existing style violations in the same commit. If you are going to do meaningful work on an otherwise dormant part of the system (you can judge it by checking the recent history of the files you are going to touch, e.g. "git log --no-merges pu -- git-p4.py"), you are encouraged to first do the style fixes in separate patches as preliminary clean-ups without changing anything else and then build your meaningful work on top of it. What is discouraged is a change that tries to only do style fixes etc. to parts of the system that are actively being modified by other people for their meaningful work. >> You are verifying what the set of "canonical" paths should be by >> checking ls-files output. I think that is what you intended to do >> (i.e. I am saying "I think the code is more correct than the earlier >> round that used find"), but I just am double checking. > I agree that “ls-files” is better because it reflects what ends up > in the Git repository and how it ends up there. Thanks. I wanted to double-check that the problem you saw was not about what is left in the filesystem but more about what is recorded in the Git history. "ls-files" check is absolutely the right approach in that 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: Minor bug with help.autocorrect.
On Mon, Aug 24, 2015 at 01:43:27AM -0400, Jeff King wrote:
> > I wonder if alias_lookup() and check_pager_config(), two functions
> > that *know* that the string they have, cmd, could be invalid and
> > unusable key to give to the config API, should be doing an extra
> > effort (e.g. call parse_key() with QUIET option and refrain from
> > calling git_config_get_value()). It feels that for existing callers
> > of parse_key(), not passing QUIET would be the right thing to do.
> >
> > Of course, I am OK if git_config_get_value() and friends took the
> > QUIET flag and and passed it all the way down to parse_key(); that
> > would be a much more correct approach to address this issue (these
> > two callers do not have to effectively call parse_key() twice that
> > way), but at the same time, that would be a lot more involved
> > change.
>
> Yeah, I agree these are the only two sane fixes. Plumbing the quiet flag
> through does seem really invasive; every "git_config_get_foo" variant
> would have to learn it. [...]
Here is a patch to do the first. While it seems a little gross to have
to call parse_key twice, I think it does make sense. The alias.* and
pager.* config trees are mis-designed, and we are papering over the
problem for historical reasons.
-- >8 --
Subject: config: silence warnings for command names with invalid keys
When we are running the git command "foo", we may have to
look up the config keys "pager.foo" and "alias.foo". These
config schemes are mis-designed, as the command names can be
anything, but the config syntax has some restrictions. For
example:
$ git foo_bar
error: invalid key: pager.foo_bar
error: invalid key: alias.foo_bar
git: 'foo_bar' is not a git command. See 'git --help'.
You cannot name an alias with an underscore. And if you have
an external command with one, you cannot configure its
pager.
In the long run, we may develop a different config scheme
for these features. But in the near term (and because we'll
need to support the existing scheme indefinitely), we should
at least squelch the error messages shown above.
These errors come from git_config_parse_key. Ideally we
would pass a "quiet" flag to the config machinery, but there
are many layers between the pager code and the key parsing.
Passing a flag through all of those would be an invasive
change.
Instead, let's provide a config function to report on
whether a key is syntactically valid, and have the pager and
alias code skip lookup for bogus keys. We can build this
easily around the existing git_config_parse_key, with two
minor modifications:
1. We now handle a NULL store_key, to validate but not
write out the normalized key.
2. We accept a "quiet" flag to avoid writing to stderr.
This doesn't need to be a full-blown public "flags"
field, because we can make the existing implementation
a static helper function, keeping the mess contained
inside config.c.
Signed-off-by: Jeff King
---
alias.c | 3 ++-
cache.h | 1 +
config.c | 39 +--
pager.c | 3 ++-
t/t7006-pager.sh | 9 +
5 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/alias.c b/alias.c
index 6aa164a..a11229d 100644
--- a/alias.c
+++ b/alias.c
@@ -5,7 +5,8 @@ char *alias_lookup(const char *alias)
char *v = NULL;
struct strbuf key = STRBUF_INIT;
strbuf_addf(&key, "alias.%s", alias);
- git_config_get_string(key.buf, &v);
+ if (git_config_key_is_valid(key.buf))
+ git_config_get_string(key.buf, &v);
strbuf_release(&key);
return v;
}
diff --git a/cache.h b/cache.h
index 4e25271..8de519a 100644
--- a/cache.h
+++ b/cache.h
@@ -1440,6 +1440,7 @@ extern int git_config_pathname(const char **, const char
*, const char *);
extern int git_config_set_in_file(const char *, const char *, const char *);
extern int git_config_set(const char *, const char *);
extern int git_config_parse_key(const char *, char **, int *);
+extern int git_config_key_is_valid(const char *key);
extern int git_config_set_multivar(const char *, const char *, const char *,
int);
extern int git_config_set_multivar_in_file(const char *, const char *, const
char *, const char *, int);
extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index 9fd275f..8adc15a 100644
--- a/config.c
+++ b/config.c
@@ -1848,7 +1848,7 @@ int git_config_set(const char *key, const char *value)
* baselen - pointer to int which will hold the length of the
* section + subsection part, can be NULL
*/
-int git_config_parse_key(const char *key, char **store_key, int *baselen_)
+static int git_config_parse_key_1(const char *key, char **store_key, int
*baselen_, int quiet)
{
int i, dot, baselen;
const char *last_dot = strrchr(key, '.');
@@ -1859,12 +1859,14 @@ int git_config_parse_key(const char *key, char
**store_key, int *baselen_)
Re: Minor bug with help.autocorrect.
On Fri, Aug 21, 2015 at 03:13:38PM -0700, Junio C Hamano wrote: > > diff --git a/config.c b/config.c > > index 9fd275f..dd0cb52 100644 > > --- a/config.c > > +++ b/config.c > > @@ -1314,7 +1314,7 @@ static struct config_set_element > > *configset_find_element(struct config_set *cs, > > * `key` may come from the user, so normalize it before using it > > * for querying entries from the hashmap. > > */ > > - ret = git_config_parse_key(key, &normalized_key, NULL); > > + ret = git_config_parse_key(key, &normalized_key, NULL, > > CONFIG_ERROR_QUIET); > > Hmm, I am not sure if this is correct, or it is trying to do things > at too low a level. > > configset_add_value() calls configset_find_element(). > > A NULL return from find_element() could be because parse_key() > errored out due to bad name, or the key genuinely did not exist in > the hashmap, and the caller cannot tell. So add_value() can end up > adding a new pair under a bogus key, which is not a new > problem, but what makes me cautious is that it happens silently with > the updated code. > > In fact, git_configset_add_file() uses git_config_from_file() with > configset_add_value() as its callback function, and the error that > is squelched with this CONFIG_ERROR_QUIET would be the only thing > that tells the user that the config file being read is malformed. I assumed that we would only get well-formed keys here. That is, the config parser should be enforcing the rules already in config.c:get_parse_source and friends. In that sense, the parse_key in the configset_add_value path _should_ be a noop (and this patch does make it worse by quieting a warning we would get for a potential bug). For example: $ echo "utter_crap = true" >.git/config $ git config foo.bar fatal: bad config file line 6 in .git/config It looks like the "-c" code is more forgiving, though, and does rely on this check: $ git -c utter_crap=true log >/dev/null error: key does not contain a section: utter_crap So the patch is a regression there. > Right now, "git config" does not seem to use the full configset API > so nobody would notice, but still... Hmm. I don't think that matters for bad config files. Even after we move to configset there, it will still have to run that same parsing code. But when I say: $ git config utter_crap error: key does not contain a section: utter_crap I think we would end up relying on this code to tell me that my request was bogus. And that needs to keep being noisy, to tell the user what's going on (as opposed to check_pager_config(), which really wants to say "I'm _aware_ I might be feeding you junk"). > I wonder if alias_lookup() and check_pager_config(), two functions > that *know* that the string they have, cmd, could be invalid and > unusable key to give to the config API, should be doing an extra > effort (e.g. call parse_key() with QUIET option and refrain from > calling git_config_get_value()). It feels that for existing callers > of parse_key(), not passing QUIET would be the right thing to do. > > Of course, I am OK if git_config_get_value() and friends took the > QUIET flag and and passed it all the way down to parse_key(); that > would be a much more correct approach to address this issue (these > two callers do not have to effectively call parse_key() twice that > way), but at the same time, that would be a lot more involved > change. Yeah, I agree these are the only two sane fixes. Plumbing the quiet flag through does seem really invasive; every "git_config_get_foo" variant would have to learn it. Probably it's too gross to have a global like: config_lax_mode = 1; git_config_get_string(key.buf, &v); config_lax_mode = 0; That makes a nice tidy patch, but I have a feeling we would regret it later. :) -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: [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
On Fri, Aug 21, 2015 at 12:47:30PM -0400, Anders Kaseorg wrote: > On Fri, 21 Aug 2015, Jeff King wrote: > > - we may still have the opposite problem with renames. That is, a > > rename is _also_ a deletion, but will go to the end. So I would > > expect renaming the symlink "foo" to "bar" and then adding > > "foo/world" would end up with: > > > >M 100644 :3 foo/world > >R foo bar > > > > (because we push renames to the end in our sort). And indeed, > > importing that does seem to get it wrong (we end up with "bar/world" > > and no symlink). > > > > We can't fix the ordering in the second case without breaking the first > > case. So I'm not sure it's fixable on the fast-export end. > > Hmm, renames have a more fundamental ordering problem: swapping two > (normal) files and using fast-export -C -B results in > > R foo bar > R bar foo > > which cannot be reimported correctly without fast-import fixes. Yeah, you're right. Fast-export's view of the world comes from diff, which is that the "source" side is immutable. Whereas fast-import seems to mutate the tree in-place as it reads the set of operations. I wonder what would break if we simply fixed that. I.e., is anybody else depending on: R foo bar M bar ... to modify "foo" and not "bar". I kind of wonder if it is insane to turn on renames at all in fast-export. -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] am: terminate state files with a newline
On Sun, Aug 23, 2015 at 12:05:32PM -0700, Junio C Hamano wrote:
> > - write_file(am_path(state, "threeway"), 1, state->threeway ? "t" : "f");
> > + write_file(am_path(state, "threeway"), 1, "%s\n", state->threeway ? "t"
> > : "f");
>
> Stepping back a bit, after realizing that "write_file()" is a
> short-hand for "I have all information necessary to produce the full
> contents of a file, now go ahead and create and write that and
> close", I have to wonder what caller even wants to create a file
> with an incomplete line at the end.
FWIW, I had a similar thought when reading the original thread. I also
noted that all of the callers here pass "1" for the "fatal" parameter,
and that they are either bools or single strings. I wonder if:
void write_state_bool(struct am_state *state, const char *name, int v)
{
write_file(am_path(state, name), 1, "%s\n", v ? "t" : "f");
}
would make the call-sites even easier to read (and of course the "\n"
would be dropped here if it does migrate up to write_file()).
> @@ -634,6 +641,9 @@ int write_file(const char *path, int fatal, const char
> *fmt, ...)
> va_start(params, fmt);
> strbuf_vaddf(&sb, fmt, params);
> va_end(params);
> + if (sb.len)
> + strbuf_complete_line(&sb);
> +
I think the "if" here is redundant; strbuf_complete_line already handles
it.
-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: Where to report security vulnerabilities in git?
On 08/22/2015 04:25 AM, Guido Vranken wrote: > List, > > I would like to report security vulnerabilities in git. Due to the > sensitive nature of security-impacting bugs I would like to know if > there's a dedicated e-mail address for this, so that the issues at > play can be patched prior to a coordinated public disclosure of the > germane exploitation details. I did find an older thread in the > archive addressing this question ( > http://thread.gmane.org/gmane.comp.version-control.git/260328/ ), but > because I'm unsure if those e-mail addresses are still relevant, I'm > asking again. If it has anything to do with remote access (via ssh or http) please copy me also. I wrote/write/maintain gitolite, which is a reasonably successful access control system for git servers. regards sitaram signature.asc Description: OpenPGP digital signature
Doc, git-svn, added mention of config key: svn-remote..include-paths
I send this small doc-patch back in June[1], but it may not have come through properly, or may have been lost, so I'm resending it. Thanks Brett [1] http://marc.info/?l=git&m=143313445425214&w=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
[PATCH] Doc, git-svn, added mention of config key: svn-remote..include-paths
Signed-off-by: Brett Randall --- Documentation/git-svn.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt index 11d1e2f..0c0f60b 100644 --- a/Documentation/git-svn.txt +++ b/Documentation/git-svn.txt @@ -174,6 +174,9 @@ Skip "branches" and "tags" of first level directories;; (including automatic fetches due to 'clone', 'dcommit', 'rebase', etc) on a given repository. '--ignore-paths' takes precedence over '--include-paths'. ++ +[verse] +config key: svn-remote..include-paths --log-window-size=;; Fetch log entries per request when scanning Subversion history. -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] generate-cmdlist: re-implement as shell script
527ec39 (generate-cmdlist: parse common group commands, 2015-05-21)
replaced generate-cmdlist.sh with a more functional Perl version,
generate-cmdlist.perl. The Perl version gleans named tags from a new
"common groups" section in command-list.txt and recognizes those tags in
"command list" section entries in place of the old 'common' tag. This
allows git-help to, not only recognize, but also group common commands.
Although the tests require Perl, 527ec39 creates an unconditional
dependence upon Perl in the build system itself, which can not be
overridden with NO_PERL. Such a dependency may be undesirable; for
instance, the 'git-lite' package in the FreeBSD ports tree is intended
as a minimal Git installation (which may, for example, be useful on
servers needing only local clone and update capability), which,
historically, has not depended upon Perl[1].
Therefore, revive generate-cmdlist.sh and extend it to recognize "common
groups" and its named tags. Retire generate-cmdlist.perl.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/275905/focus=276132
Signed-off-by: Eric Sunshine
---
Changes since v1 [2]:
* Avoid literal newline inside ${var:+val} when composing grep match
patterns, lest some shells might trip over it. Instead, pass match
patterns to grep via -f temporary file.
* Don't assume it's portable to feed commands-list.txt to script as
stdin and expect one command (sed) to consume only part of it, leaving
the rest for consumption by a subsequent command (grep). While this
works on some platforms, in practice, the first command (sed) may
buffer and eat more of the input than expected. Instead, pass
commands-list.txt as argument and have each command independently open
and consume it.
* Replace errant 'd' sed command with 'b' to skip comment lines. The end
result is the same, but the other "skipping" actions use 'b', and 'b'
was intended all along.
v2 is still a single patch since I couldn't quite figure out[3] whether
Junio was asking for the patch to be submitted as a reversion plus a
cleanup plus an enhancement or not.
[2]: http://thread.gmane.org/gmane.comp.version-control.git/276226
[3]: http://thread.gmane.org/gmane.comp.version-control.git/276226/focus=276256
Makefile | 4 ++--
generate-cmdlist.perl | 50 --
generate-cmdlist.sh | 51 +++
3 files changed, 53 insertions(+), 52 deletions(-)
delete mode 100755 generate-cmdlist.perl
create mode 100755 generate-cmdlist.sh
diff --git a/Makefile b/Makefile
index e39ca6c..5d3e3b9 100644
--- a/Makefile
+++ b/Makefile
@@ -1697,10 +1697,10 @@ $(BUILT_INS): git$X
ln -s $< $@ 2>/dev/null || \
cp $< $@
-common-cmds.h: generate-cmdlist.perl command-list.txt
+common-cmds.h: generate-cmdlist.sh command-list.txt
common-cmds.h: $(wildcard Documentation/git-*.txt)
- $(QUIET_GEN)$(PERL_PATH) generate-cmdlist.perl command-list.txt > $@+
&& mv $@+ $@
+ $(QUIET_GEN)./generate-cmdlist.sh command-list.txt >$@+ && mv $@+ $@
SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
diff --git a/generate-cmdlist.perl b/generate-cmdlist.perl
deleted file mode 100755
index 31516e3..000
--- a/generate-cmdlist.perl
+++ /dev/null
@@ -1,50 +0,0 @@
-#!/usr/bin/perl
-use strict;
-use warnings;
-
-print <<"EOT";
-/* Automatically generated by $0 */
-
-struct cmdname_help {
- char name[16];
- char help[80];
- unsigned char group;
-};
-
-static char *common_cmd_groups[] = {
-EOT
-
-my $n = 0;
-my %grp;
-while (<>) {
- last if /^### command list/;
- next if (1../^### common groups/) || /^#/ || /^\s*$/;
- chop;
- my ($k, $v) = split ' ', $_, 2;
- $grp{$k} = $n++;
- print "\tN_(\"$v\"),\n";
-}
-
-print "};\n\nstatic struct cmdname_help common_cmds[] = {\n";
-
-while (<>) {
- next if /^#/ || /^\s*$/;
- my @tags = split;
- my $cmd = shift @tags;
- for my $t (@tags) {
- if (exists $grp{$t}) {
- my $s;
- open my $f, '<', "Documentation/$cmd.txt" or die;
- while (<$f>) {
- ($s) = /^$cmd - (.+)$/;
- last if $s;
- }
- close $f;
- $cmd =~ s/^git-//;
- print "\t{\"$cmd\", N_(\"$s\"), $grp{$t}},\n";
- last;
- }
- }
-}
-
-print "};\n";
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
new file mode 100755
index 000..6b39c62
--- /dev/null
+++ b/generate-cmdlist.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+
+echo "/* Automatically generated by $0 */
+struct cmdname_help {
+ char name[16];
+ char help[80];
+ unsigned char group;
+};
+
+static const char *common_cmd_gro
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
Karthik Nayak writes: > diff --git a/Documentation/git-for-each-ref.txt > b/Documentation/git-for-each-ref.txt > index 1997657..06d468e 100644 > --- a/Documentation/git-for-each-ref.txt > +++ b/Documentation/git-for-each-ref.txt > @@ -133,7 +133,8 @@ align:: > `` is either left, right or middle and `` is > the total length of the content with alignment. If the > contents length is more than the width then no alignment is > - performed. > + performed. If used with '--quote' everything in between %(align:..) > + and %(end) is quoted. There's no --quote, there are --shell, --python, ... (well, actually, I would have prefered to have a single --quote=language option, but that's not how it is now). I had already commented on a preliminary version of this series off-list. I think all my previous comments have been taken into account. Thanks, -- 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 v13 11/12] tag.c: implement '--format' option
Karthik Nayak writes: > --- a/Documentation/git-tag.txt > +++ b/Documentation/git-tag.txt > @@ -13,7 +13,8 @@ SYNOPSIS >[ | ] > 'git tag' -d ... > 'git tag' [-n[]] -l [--contains ] [--points-at ] > - [--column[=] | --no-column] [--create-reflog] [--sort=] > [...] > + [--column[=] | --no-column] [--create-reflog] [--sort=] > + [--format=] [...] > 'git tag' -v ... > > DESCRIPTION > @@ -158,6 +159,11 @@ This option is only applicable when listing tags without > annotation lines. > The object that the new tag will refer to, usually a commit. > Defaults to HEAD. > > +:: Shouldn't this be --format , not just ? We usually use the named argument in the SYNOPSIS for positional arguments, but not for arguments following an option. -- 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] am: terminate state files with a newline
Paul Tan writes:
> Did we ever explictly allow external programs to poke around the
> contents of the .git/rebase-apply directory?
We tell users to take a peek into it when "am" fails, don't we, by
naming $GIT_DIR/rebase-apply/patch?
> - write_file(am_path(state, "threeway"), 1, state->threeway ? "t" : "f");
> + write_file(am_path(state, "threeway"), 1, "%s\n", state->threeway ? "t"
> : "f");
Stepping back a bit, after realizing that "write_file()" is a
short-hand for "I have all information necessary to produce the full
contents of a file, now go ahead and create and write that and
close", I have to wonder what caller even wants to create a file
with an incomplete line at the end.
All callers outside builtin/am.c except one caller uses it to
produce a single line file. The oddball is "git branch" that uses
it to prepare a temporary file used to edit branch description.
builtin/branch.c: if (write_file(git_path(edit_description), 0, "%s",
buf.buf)) {
The payload it prepares in buf.buf ends with a canned comment that
ends with LF. So in that sense it is not even an oddball.
The above analysis makes me wonder if this is a simpler and more
future proof approach.
Or did I miss any caller or a reasonable potential future use case
that wants to create a binary file or a text file that ends with an
incomplete line?
wrapper.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/wrapper.c b/wrapper.c
index e451463..7a92298 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -621,6 +621,13 @@ char *xgetcwd(void)
return strbuf_detach(&sb, NULL);
}
+/*
+ * Create a TEXT file by specifying its full contents via fmt and the
+ * remainder of args that are used like "printf". A terminating LF is
+ * added at the end of the file if it is missing (it is simpler for
+ * the callers because the function is often used to create a
+ * single-liner file).
+ */
int write_file(const char *path, int fatal, const char *fmt, ...)
{
struct strbuf sb = STRBUF_INIT;
@@ -634,6 +641,9 @@ int write_file(const char *path, int fatal, const char
*fmt, ...)
va_start(params, fmt);
strbuf_vaddf(&sb, fmt, params);
va_end(params);
+ if (sb.len)
+ strbuf_complete_line(&sb);
+
if (write_in_full(fd, sb.buf, sb.len) != sb.len) {
int err = errno;
close(fd);
--
To unsubscribe from this list: send the line "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: List tags for a certain branch
On Sun, Aug 23, 2015 at 08:06:39PM +0200, CoDEmanX wrote: > > in a future version of git you should be able to do "git tag > --merged" to get the tags that are "merged" to a particular branch. > > Would it return every tag in the branch, even if it was created in > that branch, and not merged from somewhere else? If no, than it's not > what I am looking for. > > (Or more precisely: would it give a list of all tags, that point to > commits present in the specified branch, regardless of whether these > commits were originally done in that branch or merged from another > branch? I hope my understanding of git is correct here...) Yes, it would show all tags that are present in the history leading up to that branch tip[1]. Git tracks a DAG of the history; it does not remember "which branch" a particular action happened on. -Peff [1] Actually the argument to "--merged" does not need to be a branch at all. It is really a commit, so you could specify a tag, an extended sha-1 expression, etc. -- To unsubscribe from this list: send the line "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: Eric Sunshine mail delivery failure
On Sun, Aug 23, 2015 at 2:48 PM, Eric Sunshine wrote: > On Sun, Aug 23, 2015 at 2:36 PM, Elia Pinto wrote: >> Il 23/Ago/2015 20:26, "Eric Sunshine" ha scritto: >>> I did change the CNAME to an A just in case, though who knows how long >>> it will take for the change to propagate over to web.de's server. >> Anyone can check Here https://dnschecker.org/#CNAME/Mail.sunshineco.com >> It would fail with your change > > Interesting service; thanks for the pointer. However, since it's just > querying a random set of DNS servers, it's not necessarily indicative > of whether the change has actually propagated to the DNS server(s) > answering web.de's mail server's queries. Local configuration (TTL's, > etc.) on those servers or anywhere in between, as well as network > conditions, could impact propagation to an unknown degree. Also, the propagation time of the A record can be quite different from the point at which the CNAME record finally expires (based upon its TTL, which may differ dramatically from server to server), so the above CNAME query may continue to succeed long after the A record has propagated. -- To unsubscribe from this list: send the line "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: Eric Sunshine mail delivery failure
On Sun, Aug 23, 2015 at 2:36 PM, Elia Pinto wrote: > Il 23/Ago/2015 20:26, "Eric Sunshine" ha scritto: >> On Sun, Aug 23, 2015 at 1:16 PM, Johannes Löthberg >> wrote: >> > Just an A record would be enough. The issue is that mail.sunshineco.com >> > has >> > neither an A nor an record, it is a CNAME to sunshineco.com, which >> > is >> > invalid according to RFC2181. >> >> Interestingly, the default configuration for all domains managed by >> this service provider is for the mailhost to be a CNAME. While the >> restriction in section 10.3 of RFC2181 makes sense as a way to avoid >> extra "network burden", in practice, email services seem to be pretty >> relaxed about it, and follow the CNAME indirection as needed. >> >> I suppose it's possible that web.de is being extra strict (although it >> seems that such strictness would be painful for its users), or this >> could just be a temporary DNS lookup failure. It's hard to tell based >> upon the errors René reported. >> >> I did change the CNAME to an A just in case, though who knows how long >> it will take for the change to propagate over to web.de's server. > Anyone can check Here https://dnschecker.org/#CNAME/Mail.sunshineco.com > It would fail with your change Interesting service; thanks for the pointer. However, since it's just querying a random set of DNS servers, it's not necessarily indicative of whether the change has actually propagated to the DNS server(s) answering web.de's mail server's queries. Local configuration (TTL's, etc.) on those servers or anywhere in between, as well as network conditions, could impact propagation to an unknown degree. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git-remote-helper behavior on Windows, not recognizing blank line as terminator
Hello, I'm having some issues with git remote helper behavior on Windows. According to the protocol (https://www.kernel.org/pub/software/scm/git/docs/gitremote-helpers.html), when doing things like listing capabilities, git expects the remote helper to send back a blank line when it's done. I'm having trouble having git recognize the blank line (see https://github.com/anishathalye/git-remote-dropbox/issues/13#issuecomment-133894730 for details). Has anyone come across this behavior before? Am I doing something wrong, or could there be a bug in git? What's the best way to proceed? Any help or suggestions would be greatly appreciated! Regards, Anish -- To unsubscribe from this list: send the line "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: Eric Sunshine mail delivery failure
On Sun, Aug 23, 2015 at 1:16 PM, Johannes Löthberg wrote: > On 23/08, René Scharfe wrote: >> Eric, hope you see this reply on the list. Direct replies to >> sunsh...@sunshineco.com are rejected by my mail provider on submit in >> Thunderbird with the following message: >> >>Requested action not taken: mailbox unavailable >>invalid DNS MX or A/ resource record. >> >> And with this one when using their web interface: >> >>A message that you sent could not be delivered to one or more of >>its recipients. This is a permanent error. The following address >>failed: >> >>"sunsh...@sunshineco.com": >>no valid MX hosts found >> >> It seems web.de wants you to get an record before I'm allowed to send >> mails to you. > > Just an A record would be enough. The issue is that mail.sunshineco.com has > neither an A nor an record, it is a CNAME to sunshineco.com, which is > invalid according to RFC2181. Interestingly, the default configuration for all domains managed by this service provider is for the mailhost to be a CNAME. While the restriction in section 10.3 of RFC2181 makes sense as a way to avoid extra "network burden", in practice, email services seem to be pretty relaxed about it, and follow the CNAME indirection as needed. I suppose it's possible that web.de is being extra strict (although it seems that such strictness would be painful for its users), or this could just be a temporary DNS lookup failure. It's hard to tell based upon the errors René reported. I did change the CNAME to an A just in case, though who knows how long it will take for the change to propagate over to web.de's server. -- To unsubscribe from this list: send the line "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: List tags for a certain branch
> in a future version of git you should be able to do "git tag --merged" to get the tags that are "merged" to a particular branch. Would it return every tag in the branch, even if it was created in that branch, and not merged from somewhere else? If no, than it's not what I am looking for. (Or more precisely: would it give a list of all tags, that point to commits present in the specified branch, regardless of whether these commits were originally done in that branch or merged from another branch? I hope my understanding of git is correct here...) I was able to achieve my goal with a Python script, that - gets a list of all commit hashes for a given branch and creates a set - creates a dictionary / hashmap from refs/tags/*, using commit hashes as keys and tag names as values - creates a set from dictionary keys - does an intersection operation using both sets - and maps the remaining hashes back to tag names. See the updated SO question: http://stackoverflow.com/a/32169239/2044940 Thank you Peff!
[PATCH] rev-list: make it obvious that we do not support notes
On Sun, Aug 23, 2015 at 01:43:09PM -0400, Jeff King wrote:
> I don't know how deeply anybody _cares_ about showing notes via
> rev-list. It has clearly never worked. But rather than silently
> accepting --show-notes (and not showing any notes!), perhaps we should
> tell the user that it does not work. Likewise, the "%N" --format
> specifier never expands in rev-list, and should probably be removed from
> the rev-list documentation.
I think that would look like this. IMHO this is a strict improvement on
the current state. I would be happier still if somebody wanted to plumb
the notes options through for rev-list, but I am not personally
interested in spending the time on that.
-- >8 --
Subject: rev-list: make it obvious that we do not support notes
The rev-list command does not have the internal
infrastructure to display notes. Running:
git rev-list --notes HEAD
will silently ignore the "--notes" option. Running:
git rev-list --notes --grep=. HEAD
will crash on an assert. Running:
git rev-list --format=%N HEAD
will place a literal "%N" in the output (it does not even
expand to an empty string).
Let's have rev-list tell the user that it cannot fill the
user's request, rather than silently producing wrong data.
Likewise, let's remove mention of the notes options from the
rev-list documentation.
Signed-off-by: Jeff King
---
Documentation/pretty-formats.txt | 2 ++
Documentation/pretty-options.txt | 2 ++
Documentation/rev-list-options.txt | 2 ++
builtin/rev-list.c | 3 +++
4 files changed, 9 insertions(+)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index dc865cb..671cebd 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -139,7 +139,9 @@ The placeholders are:
- '%f': sanitized subject line, suitable for a filename
- '%b': body
- '%B': raw body (unwrapped subject and body)
+ifndef::git-rev-list[]
- '%N': commit notes
+endif::git-rev-list[]
- '%GG': raw verification message from GPG for a signed commit
- '%G?': show "G" for a Good signature, "B" for a Bad signature, "U" for a
good,
untrusted signature and "N" for no signature
diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 642af6e..8d6c5ce 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -42,6 +42,7 @@ people using 80-column terminals.
verbatim; this means that invalid sequences in the original
commit may be copied to the output.
+ifndef::git-rev-list[]
--notes[=]::
Show the notes (see linkgit:git-notes[1]) that annotate the
commit, when showing the commit log message. This is the default
@@ -73,6 +74,7 @@ being displayed. Examples: "--notes=foo" will show only notes
from
--[no-]standard-notes::
These options are deprecated. Use the above --notes/--no-notes
options instead.
+endif::git-rev-list[]
--show-signature::
Check the validity of a signed commit object by passing the signature
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index a9b808f..f1c5220 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -58,9 +58,11 @@ endif::git-rev-list[]
more than one `--grep=`, commits whose message
matches any of the given patterns are chosen (but see
`--all-match`).
+ifndef::git-rev-list[]
+
When `--show-notes` is in effect, the message from the notes is
matched as if it were part of the log message.
+endif::git-rev-list[]
--all-match::
Limit the commits output to ones that match all given `--grep`,
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index c0b4b53..d80d1ed 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -350,6 +350,9 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
revs.diff)
usage(rev_list_usage);
+ if (revs.show_notes)
+ die(_("rev-list does not support display of notes"));
+
save_commit_buffer = (revs.verbose_header ||
revs.grep_filter.pattern_list ||
revs.grep_filter.header_list);
--
2.5.0.685.g0ca4974
--
To unsubscribe from this list: send the line "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] t5004: test ZIP archives with many entries
On Sun, Aug 23, 2015 at 5:29 AM, "René Scharfe" wrote:
> Am 23.08.2015 um 07:54 schrieb Eric Sunshine:
>> On Sat, Aug 22, 2015 at 3:06 PM, René Scharfe wrote:
>>> +test_lazy_prereq ZIPINFO '
>>> + n=$("$ZIPINFO" "$TEST_DIRECTORY"/t5004/empty.zip | sed -n "2s/.*
>>> //p")
>>> + test "x$n" = "x0"
>>> +'
>>
>> Unfortunately, this sed expression isn't portable due to dissimilar
>> output of various zipinfo implementations. On Linux, the output of
>> zipinfo is:
>>
>> $ zipinfo t/t5004/empty.zip
>> Archive: t/t5004/empty.zip
>> Zip file size: 62 bytes, number of entries: 0
>> Empty zipfile.
>> $
>>
>> however, on Mac OS X:
>>
>> $ zipinfo t/t5004/empty.zip
>> Archive: t/t5004/empty.zip 62 bytes 0 files
>> Empty zipfile.
>> $
>>
>> and on FreeBSD, the zipinfo command seems to have been removed
>> altogether in favor of "unzip -Z" (emulate zipinfo).
>
> I suspected that zipinfo's output might be formatted differently on
> different platforms and tried to guard against it by checking for the
> number zero there. Git's ZIP file creation is platform independent
> (modulo bugs), so having a test run at least somewhere should
> suffice. In theory.
>
> We could add support for the one-line-summary variant on OS X easily,
> though.
Probably, although it's looking like testing on Mac OS X won't be
fruitful (see below).
>> One might hope that "unzip -Z" would be a reasonable replacement for
>> zipinfo, however, it is apparently only partially implemented on
>> FreeBSD, and requires that -1 be passed, as well. Even with "unzip -Z
>> -1", there are issues. The output on Linux and Mac OS X is:
>>
>> $ unzip -Z -1 t/t5004/empty.zip
>> Empty zipfile.
>> $
>>
>> but FreeBSD differs:
>>
>> $ unzip -Z -1 t/t5004/empty.zip
>> $
>>
>> With a non-empty zip file, the output is identical on all platforms:
>>
>> $ unzip -Z -1 twofiles.zip
>> file1
>> file2
>> $
>>
>> So, if you combine that with "wc -l" or test_line_count, you may have
>> a portable and reliable entry counter.
>
> Counting all entries is slow, and more importantly it's not what we
> want. In this test we need the number of entries recorded in the ZIP
> directory, not the actual number of entries found by scanning the
> archive, or the directory.
Ah, right. The commit message did state this clearly enough...
> On Linux "unzip -Z -1 many.zip | wc -l" reports 65792 even before
> adding ZIP64 support; only without -1 we get the interesting numbers
> (specifically with "unzip -Z many.zip | sed -n '2p;$p'"):
>
> Zip file size: 6841366 bytes, number of entries: 256
> 65792 files, 0 bytes uncompressed, 0 bytes compressed: 0.0%
>
>> With these three patches applied, Mac OS X has trouble with 'many.zip':
>>
>> $ unzip -Z -1 many.zip
>> warning [many.zip]: 76 extra bytes at beginning or within zipfile
>>(attempting to process anyway)
>> error [many.zip]: reported length of central directory is
>>-76 bytes too long (Atari STZip zipfile? J.H.Holm ZIPSPLIT 1.1
>>zipfile?). Compensating...
>> 00/
>> 00/00
>> ...
>> ff/ff
>> error: expected central file header signature not found (file
>>#65793). (please check that you have transferred or created the
>>zipfile in the appropriate BINARY mode and that you have compiled
>>UnZip properly)
>>
>> And FreeBSD doesn't like it either:
>>
>> $ unzip -Z -1 many.zip
>> unzip: Invalid central directory signature
>> $
>>
>
> Looks like they don't support ZIP64. Or I got some of the fields wrong
> after all.
A >65536 file zip created on Mac OS X with Mac's "zip" command given
to "unzip" or "zipinfo" results in exactly the same warnings/errors as
above (including the bit about "76 extra bytes" and "-76 bytes too
long"), so it doesn't seem to be a problem with your implementation.
> https://en.wikipedia.org/wiki/Zip_%28file_format%29#ZIP64 says: "OS X
> Yosemite does support the creation of ZIP64 archives, but does not
> support unzipping these archives using the shipped unzip command-line
> utility or graphical Archive Utility.[citation needed]".
>
> How does unzip react to a ZIP file with more than 65535 entries that
> was created natively on these platforms? And what does zipinfo (a real
> one, without -1) report at the top for such files?
On Mac OS X, unzip does extract all the files (although complains as
noted above). zipinfo caps out at reporting 65535 for the number of
files (although it lists them all fine). With the warnings/errors
filtered out for clarity:
$ zipinfo biggy.zip
Archive: biggy.zip 9642874 bytes 65535 files
...
--
To unsubscribe from this list: send the line "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 `git rev-list --show-notes HEAD` when there are no notes
On Sat, Aug 22, 2015 at 05:14:39PM +0200, Johannes Schindelin wrote:
> The `format_display_notes()` function clearly assumes that the data
> structure holding the notes has been initialized already, i.e. that the
> `display_notes_trees` variable is no longer `NULL`.
>
> However, when there are no notes whatsoever, this variable is still
> `NULL`, even after initialization.
>
> So let's be graceful and just return if that data structure is `NULL`.
Hrm. This is supposed to be made non-NULL by calling init_display_notes.
The "git-log" code does this properly, and can show notes. The
"rev-list" code does not, and hits this assert. But that also means that
it cannot actually _show_ notes, and your patch is papering over the
problem.
I would think we would need something more like this:
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index ff84a82..fc73a6f 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -279,6 +279,7 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
int bisect_show_vars = 0;
int bisect_find_all = 0;
int use_bitmap_index = 0;
+ struct userformat_want w;
git_config(git_default_config, NULL);
init_revisions(&revs, prefix);
@@ -349,6 +350,14 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
revs.diff)
usage(rev_list_usage);
+ memset(&w, 0, sizeof(w));
+ userformat_find_requirements(NULL, &w);
+
+ if (!revs.show_notes_given && w.notes)
+ revs.show_notes = 1;
+ if (revs.show_notes)
+ init_display_notes(&revs.notes_opt);
+
save_commit_buffer = (revs.verbose_header ||
revs.grep_filter.pattern_list ||
revs.grep_filter.header_list);
but it looks like that is not enough to convince the pretty-printer to
actually show notes (I suspect we need something like the
pretty_ctx->notes_message setup that is in show_log()).
I don't know how deeply anybody _cares_ about showing notes via
rev-list. It has clearly never worked. But rather than silently
accepting --show-notes (and not showing any notes!), perhaps we should
tell the user that it does not work. Likewise, the "%N" --format
specifier never expands in rev-list, and should probably be removed from
the rev-list documentation.
Although...
> Reported in https://github.com/msysgit/git/issues/363.
After reading your subject, I wondered why "git rev-list --show-notes
HEAD" did not crash for me (whether or not I had notes). But the key
element from that issue is the addition of "--grep", which is what
triggers us to actually look at the notes (since rev-list otherwise does
not support displaying them). And that _does_ work with my patch above.
So perhaps that is useful, though again, it has never worked in the
past (and with your patch, it would silently return no results, whether
the grep matched or not).
Of course it's a terrible interface to make "--show-notes --grep" grep
the notes, but not actually _show_ them. :-/
> diff --git a/notes.c b/notes.c
> index df08209..24a335a 100644
> --- a/notes.c
> +++ b/notes.c
> @@ -1266,7 +1266,10 @@ void format_display_notes(const unsigned char
> *object_sha1,
> struct strbuf *sb, const char *output_encoding, int
> raw)
> {
> int i;
> - assert(display_notes_trees);
> +
> + if (!display_notes_trees)
> + return;
> +
So I'm not really in favor of this approach, but if we do go that route,
the comment above the declaration of format_display_notes needs to be
updated.
-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: Eric Sunshine mail delivery failure
On 23/08, René Scharfe wrote: Eric, hope you see this reply on the list. Direct replies to sunsh...@sunshineco.com are rejected by my mail provider on submit in Thunderbird with the following message: Requested action not taken: mailbox unavailable invalid DNS MX or A/ resource record. And with this one when using their web interface: A message that you sent could not be delivered to one or more of its recipients. This is a permanent error. The following address failed: "sunsh...@sunshineco.com": no valid MX hosts found It seems web.de wants you to get an record before I'm allowed to send mails to you. Sounds crazy. Sorry about that. Time to find a better provider, I guess. :-( Just an A record would be enough. The issue is that mail.sunshineco.com has neither an A nor an record, it is a CNAME to sunshineco.com, which is invalid according to RFC2181. -- Sincerely, Johannes Löthberg PGP Key ID: 0x50FB9B273A9D0BB5 https://theos.kyriasis.com/~kyrias/ signature.asc Description: PGP signature
Re: [PATCH v4] git-p4: fix faulty paths for case insensitive systems
On 21 Aug 2015, at 20:01, Junio C Hamano wrote: > larsxschnei...@gmail.com writes: > >> From: Lars Schneider >> >> PROBLEM: >> We run P4 servers on Linux and P4 clients on Windows. For an unknown >> reason the file path for a number of files in P4 does not match the >> directory path with respect to case sensitivity. >> >> E.g. `p4 files` might return >> //depot/path/to/file1 >> //depot/PATH/to/file2 >> >> If you use P4/P4V then these files end up in the same directory, e.g. >> //depot/path/to/file1 >> //depot/path/to/file2 >> >> If you use git-p4 then all files not matching the correct file path >> (e.g. `file2`) will be ignored. >> >> SOLUTION: >> Identify path names that are different with respect to case sensitivity. >> If there are any then run `p4 dirs` to build up a dictionary >> containing the "correct" cases for each path. It looks like P4 >> interprets "correct" here as the existing path of the first file in a >> directory. The path dictionary is used later on to fix all paths. >> >> This is only applied if the parameter "--ignore-path-case" is passed to >> the git-p4 clone command. >> >> >> Signed-off-by: Lars Schneider >> --- > > Thanks. A few comments. > > - Have you checked "git log" on our history and notice how nobody > says "PROBLEM:" and "SOLUTION:" in capital letters? Don't try to > be original in the form; your contributions are already original > and valuable in the substance ;-) haha ok. I will make them lower case :-) > > - I think I saw v3 yesterday. It would be nice to see a brief > description of what has been updated in this version. I discovered an optimization. In v3 I fixed the paths of *all* files that are underneath of a given P4 clone path. In v4 I fix only the paths that are visible on the client via client-spec (P4 can perform partial checkouts via “client-views”). I was wondering how to convey this change. Would have been a cover letter for v4 the correct way or should I have made another commit on top of my v3 change? > > I do not do Perforce and am not familiar enough with this code to > judge myself. Will wait for area experts (you have them CC'ed, which > is good) to comment. > >> diff --git a/git-p4.py b/git-p4.py >> index 073f87b..61a587b 100755 >> --- a/git-p4.py >> +++ b/git-p4.py >> @@ -1931,7 +1931,7 @@ class View(object): >> (self.client_prefix, clientFile)) >> return clientFile[len(self.client_prefix):] >> >> -def update_client_spec_path_cache(self, files): >> +def update_client_spec_path_cache(self, files, fixPathCase = None): > > I didn't check the remainder of the file, but I thought it is > customery *not* to have spaces around '=' when the parameter is > written with its default value in Python? Yes, that is PEP-8 style and I will change it accordingly. Unfortunately git-p4.py does not follow a style guide. e.g. line 2369: def commit(self, details, files, branch, parent = ""): More annoyingly (to me at least) is that git-p4 mixes CamelCase with snake_case even within classes/functions. I think I read somewhere that these kind of refactorings are discouraged. I assume that applies here, too? > >> diff --git a/t/t9821-git-p4-path-variations.sh >> b/t/t9821-git-p4-path-variations.sh >> ... >> +test_expect_success 'Clone the repo and WITHOUT path fixing' ' >> +client_view "//depot/One/... //client/..." && >> +git p4 clone --use-client-spec --destination="$git" //depot && >> +test_when_finished cleanup_git && >> +( >> +cd "$git" && >> +# This method is used instead of "test -f" to ensure the case is >> +# checked even if the test is executed on case-insensitive file >> systems. >> +cat >expect <<-\EOF && >> +two/File2.txt >> +EOF > > I think we usually write the body of the indented here text > (i.e. "<<-") indented to the same level as the command and > delimiter, i.e. > > cat >expect <<-\EOF && >body of the here document line 1 >body of the here document line 2 >... >EOF ok > >> +git ls-files >actual && >> +test_cmp expect actual >> +) >> +' > > I think you used to check the result with "find .", i.e. what the > filesystem actually recorded. "ls-files" gives what the index > records (i.e. to be committed if you were to make a new commit from > that index). They can be different in case on case-insensitive > filesystems, which I think are the ones that are most problematic > with the issue your patch is trying to address. > > You are verifying what the set of "canonical" paths should be by > checking ls-files output. I think that is what you intended to do > (i.e. I am saying "I think the code is more correct than the earlier > round that used find"), but I just am double checking. I agree that “ls-files” is better because it reflects what ends up in the Git repository and how it ends up there. > >> +test_e
Re: List tags for a certain branch
On Sun, Aug 23, 2015 at 05:27:46PM +0200, CoDEmanX wrote: > the question how to list tags, that point to commits contained in a certain > branch came up on StackOverflow couple times, and this appears to be the > only fast solution (example for local devel branch): > > git log --simplify-by-decoration --decorate --pretty=%d > "refs/heads/devel" | fgrep 'tag: ' > > It would be much much simpler, if the tag command supporter an optional > parameter to specify a branch: > > git tag --list --branch devel > > It should result in something like: > > Test-Tag1 > Test-Tag2 > Test-Tag3 > Another-Tag > And-Another I think the option you are looking for is "--merged", which currently only the "branch" command nows about. So right now you can do: git branch --merged devel to get a list of branches that are contained in "devel". There is work underway to unify the selection/filter code for git-branch and git-tag, so in a future version of git you should be able to do "git tag --merged" to get the tags that are "merged" to a particular branch. -Peff PS I'm not sure if we should pick a more generic name than "--merged" when git-tag learns this feature. For branches, it makes sense to ask "which branches are merged to this other branch". But the operation is really "which items are ancestors of the commit I gave". It is the opposite of "--contains" in that sense. Sort of a "--contained-in". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
List tags for a certain branch
Hi everyone, the question how to list tags, that point to commits contained in a certain branch came up on StackOverflow couple times, and this appears to be the only fast solution (example for local devel branch): git log --simplify-by-decoration --decorate --pretty=%d "refs/heads/devel" | fgrep 'tag: ' It would be much much simpler, if the tag command supporter an optional parameter to specify a branch: git tag --list --branch devel It should result in something like: Test-Tag1 Test-Tag2 Test-Tag3 Another-Tag And-Another ... even if all three Test-Tag* tags point to the same commit! What above mentioned git log-solution does in this situtation is: (HEAD, tag: Test-Tag1, tag: Test-Tag2, Test-Tag3, fork/devel, devel) (tag: Another-Tag) (tag: And-Another) BTW: git describe FULL_HASH can't be used to return all tags that point to the given hash. The only workaround seems to be: git for-each-rev ref/tags | grep FULL_HASH I hope you consider to add the proposed --branch option to a future git version. Thanks! SO question that started the discussion: http://stackoverflow.com/questions/32166548/how-to-list-all-tags-within-a-certain-git-branch Best, Cody -- To unsubscribe from this list: send the line "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: Submodule, subtree, or something else?
On Pk, 2015-08-21 at 17:07 -0700, Stefan Beller wrote: > On Fri, Aug 21, 2015 at 3:47 PM, Jānis Rukšāns > wrote: > > > > A major drawback of submodules in my opinion is the > > inability to make a full clone from an existing one without having > > access to the central repository, which is something I have to do from > > time to time. > > Can you elaborate on that a bit more? > git clone --recurse-submodules should do that no matter which remote > you contact? I mean that if I have cloned a repository with submodules, cloning that repository with --recurse-submodules will either access the "central server" if absolute URLs are used, or requires additional clones for each submodule. For example git clone --recursive http://somewhere/projectA.git git clone --recursive file://$(pwd)/projectA projectA.tmp The second command will cause the submodules to be downloaded again, or expect them to be found in $(pwd). Or am I mistaken, or doing something wrong? -- To unsubscribe from this list: send the line "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] dir.c: make last_exclude_matching_from_list() run til the end
Signed-off-by: Nguyễn Thái Ngọc Duy
---
dir.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/dir.c b/dir.c
index c00c7e2..3a7630a 100644
--- a/dir.c
+++ b/dir.c
@@ -901,6 +901,7 @@ static struct exclude
*last_exclude_matching_from_list(const char *pathname,
int *dtype,
struct exclude_list *el)
{
+ struct exclude *exc = NULL; /* undecided */
int i;
if (!el->nr)
@@ -922,18 +923,22 @@ static struct exclude
*last_exclude_matching_from_list(const char *pathname,
if (match_basename(basename,
pathlen - (basename - pathname),
exclude, prefix, x->patternlen,
- x->flags))
- return x;
+ x->flags)) {
+ exc = x;
+ break;
+ }
continue;
}
assert(x->baselen == 0 || x->base[x->baselen - 1] == '/');
if (match_pathname(pathname, pathlen,
x->base, x->baselen ? x->baselen - 1 : 0,
- exclude, prefix, x->patternlen, x->flags))
- return x;
+ exclude, prefix, x->patternlen, x->flags)) {
+ exc = x;
+ break;
+ }
}
- return NULL; /* undecided */
+ return exc;
}
/*
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] dir.c: don't exclude whole dir prematurely if neg pattern may match
If there is a pattern "!foo/bar", this patch makes it not exclude "foo"
right away. This gives us a chance to examine "foo" and re-include
"foo/bar".
In order for it to detect that the directory under examination should
not be excluded right away, in other words it is a parent directory of a
negative pattern, the "directory path" of the negative pattern must be
literal. Patterns like "!f?o/bar" can't stop "foo" from being excluded.
Basename matching (i.e. "no slashes in the pattern") or must-be-dir
matching (i.e. "trailing slash in the pattern") does not work well with
this. For example, if we descend in "foo" and are examining "foo/abc",
current code for "foo/" pattern will check if path "foo/abc", not "foo",
is a directory. The same problem with basename matching. These may need
big code reorg to make it work.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/gitignore.txt| 21 ---
dir.c | 76 +-
t/t3001-ls-files-others-exclude.sh | 20 ++
3 files changed, 109 insertions(+), 8 deletions(-)
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 473623d..889a72a 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -82,12 +82,9 @@ PATTERN FORMAT
- An optional prefix "`!`" which negates the pattern; any
matching file excluded by a previous pattern will become
- included again. It is not possible to re-include a file if a parent
- directory of that file is excluded. Git doesn't list excluded
- directories for performance reasons, so any patterns on contained
- files have no effect, no matter where they are defined.
- Put a backslash ("`\`") in front of the first "`!`" for patterns
- that begin with a literal "`!`", for example, "`\!important!.txt`".
+ included again. It is possible to re-include a file if a parent
+ directory of that file is excluded, with restrictions. See section
+ NOTES for detail.
- If the pattern ends with a slash, it is removed for the
purpose of the following description, but it would only find
@@ -141,6 +138,18 @@ not tracked by Git remain untracked.
To stop tracking a file that is currently tracked, use
'git rm --cached'.
+To re-include a file when its parent directory is excluded, the
+following conditions must match:
+
+ - The directory part in the re-include rules must be literal (i.e. no
+ wildcards)
+
+ - The rules to exclude the parent directory must not end with a
+ trailing slash.
+
+ - The rules to exclude the parent directory must have at least one
+ slash.
+
EXAMPLES
diff --git a/dir.c b/dir.c
index 3a7630a..a1f711c 100644
--- a/dir.c
+++ b/dir.c
@@ -882,6 +882,25 @@ int match_pathname(const char *pathname, int pathlen,
*/
if (!patternlen && !namelen)
return 1;
+ /*
+* This can happen when we ignore some exclude rules
+* on directories in other to see if negative rules
+* may match. E.g.
+*
+* /abc
+* !/abc/def/ghi
+*
+* The pattern of interest is "/abc". On the first
+* try, we should match path "abc" with this pattern
+* in the "if" statement right above, but the caller
+* ignores it.
+*
+* On the second try with paths within "abc",
+* e.g. "abc/xyz", we come here and try to match it
+* with "/abc".
+*/
+ if (!patternlen && namelen && *name == '/')
+ return 1;
}
return fnmatch_icase_mem(pattern, patternlen,
@@ -890,6 +909,48 @@ int match_pathname(const char *pathname, int pathlen,
}
/*
+ * Return non-zero if pathname is a directory and an ancestor of the
+ * literal path in a (negative) pattern. This is used to keep
+ * descending in "foo" and "foo/bar" when the pattern is
+ * "!foo/bar/.gitignore". "foo/notbar" will not be descended however.
+ */
+static int match_neg_path(const char *pathname, int pathlen, int *dtype,
+ const char *base, int baselen,
+ const char *pattern, int prefix, int patternlen,
+ int flags)
+{
+ assert((flags & EXC_FLAG_NEGATIVE) && !(flags & EXC_FLAG_NODIR));
+
+ if (*dtype == DT_UNKNOWN)
+ *dtype = get_dtype(NULL, pathname, pathlen);
+ if (*dtype != DT_DIR)
+ return 0;
+
+ if (*pattern == '/') {
+ pattern++;
+ patternlen--;
+ prefix--;
+ }
+
+ if (baselen) {
+ if (((pathlen < baselen && base[pathlen] == '/') ||
+pathlen == baselen) &&
+ !strncmp_icase(pathname, base, pathlen))
+ return 1;
+ pathname += base
[PATCH 0/2] gitignore, re-inclusion fix
This is an old problem. I attempted once [1] and then was reminded [2] with some more comments on the original patch. Let's try again. The problem is this .gitignore currently does not work, but it should: /abc !/abc/def/ghi This patch fixes that by realizing that the last rule may re-include something in abc/def so it does not exclude "abc" and "abc/def" outright to give the last rule a chance to match. [1] http://article.gmane.org/gmane.comp.version-control.git/259870 [2] http://thread.gmane.org/gmane.comp.version-control.git/265901/focus=266227 Nguyễn Thái Ngọc Duy (2): dir.c: make last_exclude_matching_from_list() run til the end dir.c: don't exclude whole dir prematurely if neg pattern may match Documentation/gitignore.txt| 21 ++--- dir.c | 89 +++--- t/t3001-ls-files-others-exclude.sh | 20 + 3 files changed, 118 insertions(+), 12 deletions(-) -- 2.3.0.rc1.137.g477eb31 -- To unsubscribe from this list: send the line "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] am: terminate state files with a newline
Hi, Quoting Paul Tan : Did we ever explictly allow external programs to poke around the contents of the .git/rebase-apply directory? I think it may not be so good, as it means that it may not be possible to switch the storage format in the future (e.g. to allow atomic modifications, maybe?) :-/ . Think of e.g. libgit2, JGit/EGit and all the other git implementations. They should be able to look everywhere in .git, shouldn't they? I don't think we will just "switch" the storage format of any parts of the repo. Whatever new formats may come (ref backends, index v5, pack v4), they will be an opt-in feature for a long time before becoming default, and there must be an even longer deprecation period before the old format gets phased out, if ever. Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Eric Sunshine mail delivery failure
Eric, hope you see this reply on the list. Direct replies to sunsh...@sunshineco.com are rejected by my mail provider on submit in Thunderbird with the following message: Requested action not taken: mailbox unavailable invalid DNS MX or A/ resource record. And with this one when using their web interface: A message that you sent could not be delivered to one or more of its recipients. This is a permanent error. The following address failed: "sunsh...@sunshineco.com": no valid MX hosts found It seems web.de wants you to get an record before I'm allowed to send mails to you. Sounds crazy. Sorry about that. Time to find a better provider, I guess. :-( René -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t5004: test ZIP archives with many entries
Am 23.08.2015 um 07:54 schrieb Eric Sunshine:
> On Sat, Aug 22, 2015 at 3:06 PM, René Scharfe wrote:
>> diff --git a/t/t5004-archive-corner-cases.sh
>> b/t/t5004-archive-corner-cases.sh
>> index 654adda..c6bd729 100755
>> --- a/t/t5004-archive-corner-cases.sh
>> +++ b/t/t5004-archive-corner-cases.sh
>> @@ -115,4 +115,44 @@ test_expect_success 'archive empty subtree by direct
>> pathspec' '
>> check_dir extract sub
>> '
>>
>> +ZIPINFO=zipinfo
>> +
>> +test_lazy_prereq ZIPINFO '
>> + n=$("$ZIPINFO" "$TEST_DIRECTORY"/t5004/empty.zip | sed -n "2s/.*
>> //p")
>> + test "x$n" = "x0"
>> +'
>
> Unfortunately, this sed expression isn't portable due to dissimilar
> output of various zipinfo implementations. On Linux, the output of
> zipinfo is:
>
> $ zipinfo t/t5004/empty.zip
> Archive: t/t5004/empty.zip
> Zip file size: 62 bytes, number of entries: 0
> Empty zipfile.
> $
>
> however, on Mac OS X:
>
> $ zipinfo t/t5004/empty.zip
> Archive: t/t5004/empty.zip 62 bytes 0 files
> Empty zipfile.
> $
>
> and on FreeBSD, the zipinfo command seems to have been removed
> altogether in favor of "unzip -Z" (emulate zipinfo).
Thanks for your thorough checks!
I suspected that zipinfo's output might be formatted differently on
different platforms and tried to guard against it by checking for the
number zero there. Git's ZIP file creation is platform independent
(modulo bugs), so having a test run at least somewhere should
suffice. In theory.
We could add support for the one-line-summary variant on OS X easily,
though.
> One might hope that "unzip -Z" would be a reasonable replacement for
> zipinfo, however, it is apparently only partially implemented on
> FreeBSD, and requires that -1 be passed, as well. Even with "unzip -Z
> -1", there are issues. The output on Linux and Mac OS X is:
>
> $ unzip -Z -1 t/t5004/empty.zip
> Empty zipfile.
> $
>
> but FreeBSD differs:
>
> $ unzip -Z -1 t/t5004/empty.zip
> $
>
> With a non-empty zip file, the output is identical on all platforms:
>
> $ unzip -Z -1 twofiles.zip
> file1
> file2
> $
>
> So, if you combine that with "wc -l" or test_line_count, you may have
> a portable and reliable entry counter.
Counting all entries is slow, and more importantly it's not what we
want. In this test we need the number of entries recorded in the ZIP
directory, not the actual number of entries found by scanning the
archive, or the directory.
On Linux "unzip -Z -1 many.zip | wc -l" reports 65792 even before
adding ZIP64 support; only without -1 we get the interesting numbers
(specifically with "unzip -Z many.zip | sed -n '2p;$p'"):
Zip file size: 6841366 bytes, number of entries: 256
65792 files, 0 bytes uncompressed, 0 bytes compressed: 0.0%
> With these three patches applied, Mac OS X has trouble with 'many.zip':
>
> $ unzip -Z -1 many.zip
> warning [many.zip]: 76 extra bytes at beginning or within zipfile
>(attempting to process anyway)
> error [many.zip]: reported length of central directory is
>-76 bytes too long (Atari STZip zipfile? J.H.Holm ZIPSPLIT 1.1
>zipfile?). Compensating...
> 00/
> 00/00
> ...
> ff/ff
> error: expected central file header signature not found (file
>#65793). (please check that you have transferred or created the
>zipfile in the appropriate BINARY mode and that you have compiled
>UnZip properly)
>
> And FreeBSD doesn't like it either:
>
> $ unzip -Z -1 many.zip
> unzip: Invalid central directory signature
> $
>
Looks like they don't support ZIP64. Or I got some of the fields wrong
after all.
https://en.wikipedia.org/wiki/Zip_%28file_format%29#ZIP64 says: "OS X
Yosemite does support the creation of ZIP64 archives, but does not
support unzipping these archives using the shipped unzip command-line
utility or graphical Archive Utility.[citation needed]".
How does unzip react to a ZIP file with more than 65535 entries that
was created natively on these platforms? And what does zipinfo (a real
one, without -1) report at the top for such files?
Thanks,
René
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
