* Re: [PATCH 3/8] Teach git var about GIT_EDITOR
From: Johannes Sixt @ 2009-10-30 20:51 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Ben Walton, David Roundy, GIT List
In-Reply-To: <20091030102658.GD1610@progeny.tock>
Jonathan Nieder schrieb:
> From: Johannes Sixt <j.sixt@viscovery.net>
>
> Expose the command used by launch_editor() for scripts to use.
> This should allow one to avoid searching for a proper editor
> separately in each command.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks for cleaning up behind me. I don't mind if you take authorship, but
if you do keep my name, please make it:
From: Johannes Sixt <j6t@kdbg.org>
> -int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
> +const char *git_editor(void)
> {
> const char *editor, *terminal;
>
> ...
> terminal = getenv("TERM");
> - if (!editor && (!terminal || !strcmp(terminal, "dumb")))
> + if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
> /* Terminal is dumb but no VISUAL nor EDITOR defined. */
> - return error(
> + error(
> "No editor specified in GIT_EDITOR, core.editor, VISUAL,\n"
> "or EDITOR. Tried to fall back to vi but terminal is dumb.\n"
> "Please set one of these variables to an appropriate\n"
> "editor or run again with options that will not cause an\n"
> "editor to be invoked (e.g., -m or -F for git commit).");
> + return NULL;
> + }
I somehow dislike that this huge error message is in git_editor(). The
return value, NULL, should be indication enough for the callers to handle
the situation suitable. In particular, launch_editor() wants to write this
big warning, but 'git var -l' can avoid the error message and write only a
short notice:
GIT_EDITOR=terminal is dumb, but VISUAL and EDITOR unset
> +static const char *editor(int flag)
> +{
> + const char *pgm = git_editor();
> +
> + if (!pgm && (flag & IDENT_ERROR_ON_NO_NAME))
> + die("cannot find a suitable editor");
> + return pgm;
This should be
return pgm ? pgm : "terminal is dumb, but VISUAL and EDITOR unset";
otherwise, 'git var -l' later trips over printf("%s", NULL).
-- Hannes
^ permalink raw reply
* git-daemon in IPv4 and IPv6 mode
From: Sebastian Pipping @ 2009-10-30 21:10 UTC (permalink / raw)
To: git; +Cc: Robert Buchholz
hello!
it seems like the git daemon cannot bind to both IPv4 and IPv6 at the
same time, yet. if that's true either ipv4 or ipv6 people are standing
in the rain unless i run two instances of the daemon in parallel.
is supporting both in a single instance planned or possible already? if
so: how?
thanks in advance,
sebastian
^ permalink raw reply
* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Jeff King @ 2009-10-30 20:26 UTC (permalink / raw)
To: Erick Mattos; +Cc: Junio C Hamano, git
In-Reply-To: <1256931394-9338-1-git-send-email-erick.mattos@gmail.com>
On Fri, Oct 30, 2009 at 05:36:34PM -0200, Erick Mattos wrote:
> Anyway this update creates new options for choosing the source timestamp
> or a new one. And set as default for -c option (editing one) to take a
> new timestamp and for -C option the source timestamp. That is because
> we are normally using the source as template when we we are editing and
> as a correction when we are just copying it.
>
> Those options are also useful for --amend option which is by default
> behaving the same.
Thanks, this is something I have been wanting. I have always thought
that --amend should give a new timestamp, so that while I'm fixing up
commits via "rebase -i" the commits end up in correct date order.
Your patch seems to always use the old timestamp for -C, the new one for
-c, and the old one for --amend. I would want it always for --amend.
I talked with Shawn about this at the GitTogether; his counter-argument
was that many people in maintainer roles will be amending or rebasing
just to do little things, like marking Signed-off-by, and that the date
should remain the same.
So my suspicion is that there are some people who almost always want
--new-timestamp, and some people who almost always want --old-timestamp,
depending on their usual workflow. In which case a config option
probably makes the most sense (but keeping the command-line to override
the config, of course).
> ---
> Documentation/git-commit.txt | 10 ++++++++--
> builtin-commit.c | 15 ++++++++++++---
> 2 files changed, 20 insertions(+), 5 deletions(-)
Tests?
> Take an existing commit object, and reuse the log message
> - and the authorship information (including the timestamp)
> - when creating the commit.
> + and the authorship information when creating the commit.
> + By default, timestamp is taken from specified commit unless
> + option --new-timestamp is included.
We should clarify that this is the _author_ timestamp. The committer
timestamp is always updated. Yes, it is talking about authorship
information in the previous sentence, but at least I had to read it
a few times to figure that out. Plus there are some minor English
tweaks needed, so maybe:
and the authorship information when creating the commit.
By default, the author timestamp is taken from the specified commit
unless the option --new-timestamp is used.
> -c <commit>::
> --reedit-message=<commit>::
> Like '-C', but with '-c' the editor is invoked, so that
> the user can further edit the commit message.
> + By default, timestamp is recalculated and not taken from
> + specified commit unless option --old-timestamp is included.
Ditto:
By default, the author timestamp is recalculated and not taken from
the specified commit unless the option --old-timestamp is used.
> @@ -134,6 +138,8 @@ OPTIONS
> current tip -- if it was a merge, it will have the parents of
> the current tip as parents -- so the current top commit is
> discarded.
> + By default, timestamp is taken from latest commit unless option
> + --new-timestamp is included.
And:
By default, the author timestamp is taken from the latest commit
unless the option --new-timestamp is included.
> + if (old_timestamp && new_timestamp)
> + die("You can not use --old-timesamp and --new-timestamp together.");
Typo: s/samp/stamp/
But this mutual exclusivity implies to me that the option should
probably be "--timestamp=old|new".
-Peff
^ permalink raw reply
* Re: [PATCH] More precise description of 'git describe --abbrev'
From: Andreas Schwab @ 2009-10-30 20:18 UTC (permalink / raw)
To: Gisle Aas; +Cc: git, gitster
In-Reply-To: <b48ea8a00910291438r8b66a0fq9e821393ecfff0bf@mail.gmail.com>
Gisle Aas <gisle@aas.no> writes:
> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
> index b231dbb..743eb95 100644
> --- a/Documentation/git-describe.txt
> +++ b/Documentation/git-describe.txt
> @@ -44,7 +44,9 @@ OPTIONS
>
> --abbrev=<n>::
> Instead of using the default 7 hexadecimal digits as the
> - abbreviated object name, use <n> digits.
> + abbreviated object name, use <n> digits or as many digits
> + are needed to form a unique object name. An <n> of 0
"as many digits as needed"?
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* [PATCH] t915{0,1}: use $TEST_DIRECTORY
From: Jeff King @ 2009-10-30 20:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sam Vilain, git
Because --root can put our trash directories elsewhere,
using ".." may not always work.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t9150-svk-mergetickets.sh | 3 ++-
t/t9151-svn-mergeinfo.sh | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t9150-svk-mergetickets.sh b/t/t9150-svk-mergetickets.sh
index 8000c34..dd0c2ba 100644
--- a/t/t9150-svk-mergetickets.sh
+++ b/t/t9150-svk-mergetickets.sh
@@ -8,7 +8,8 @@ test_description='git-svn svk merge tickets'
. ./lib-git-svn.sh
test_expect_success 'load svk depot' "
- svnadmin load -q '$rawsvnrepo' < '../t9150/svk-merge.dump' &&
+ svnadmin load -q '$rawsvnrepo' \
+ < '$TEST_DIRECTORY/t9150/svk-merge.dump' &&
git svn init --minimize-url -R svkmerge \
-T trunk -b branches '$svnrepo' &&
git svn fetch --all
diff --git a/t/t9151-svn-mergeinfo.sh b/t/t9151-svn-mergeinfo.sh
index 7eb36e5..9bee516 100644
--- a/t/t9151-svn-mergeinfo.sh
+++ b/t/t9151-svn-mergeinfo.sh
@@ -8,7 +8,8 @@ test_description='git-svn svn mergeinfo properties'
. ./lib-git-svn.sh
test_expect_success 'load svn dump' "
- svnadmin load -q '$rawsvnrepo' < '../t9151/svn-mergeinfo.dump' &&
+ svnadmin load -q '$rawsvnrepo' \
+ < '$TEST_DIRECTORY/t9151/svn-mergeinfo.dump' &&
git svn init --minimize-url -R svnmerge \
-T trunk -b branches '$svnrepo' &&
git svn fetch --all
--
1.6.5.2.224.g22719.dirty
^ permalink raw reply related
* Re: Bug#553296: gitignore broken completely
From: Klaus Ethgen @ 2009-10-30 20:08 UTC (permalink / raw)
To: Jeff King; +Cc: 553296, Junio C Hamano, git
In-Reply-To: <20091030200500.GA24831@coredump.intra.peff.net>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
Am Fr den 30. Okt 2009 um 21:05 schrieb Jeff King:
> > > + test_cmp expect output
[...]
> No, because it is building on the previous tests. The point of the test
> is:
>
> - we already have 'file' tracked and in gitignore (from previous
> tests in the series)
> - we add other-file to have some other file which is tracked but not
> in gitignore
> - we check the output of "ls-files -i" to make sure that "file" is
> there, but "other-file" is not
Aha, now I get it, the "test_cmp" do check the _content_ of the file
expect and not the file expect itself.
Regards
Klaus
Ps. Uh, oh, maybe I should truncate the Cc ;-) Ok, hoping that is ok so
too.
- --
Klaus Ethgen http://www.ethgen.de/
pub 2048R/D1A4EDE5 2000-02-26 Klaus Ethgen <Klaus@Ethgen.de>
Fingerprint: D7 67 71 C4 99 A6 D4 FE EA 40 30 57 3C 88 26 2B
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iQEVAwUBSutHuJ+OKpjRpO3lAQoL2gf8DixaPjnEkLhOZUK4VVsXU7zJ/bB3qWnP
71yq3sPA3Fi9Bq51d06YnxAQe/WqJWnOtSzgw8V5QquTaEM5Lqp6bgY49jynj6mf
wRSL+L4y+jpuerIQ/SK3yVcNw837URfQNDGFPNVSJAwGYfeMWiYh52fqwPVxO37p
wsNryc2CqpviaiqK12XwyEn7NHxPROdIRm0K8uezsw1fzWUcx4hN8IVbl8i8z0ss
brgyuMsT+R0GnazT/IndLkGniuBFBF2pdyD5uYv/lqF+q8TOC+iQzhxuA0cj/EC6
y5FuVCOTLA938bmclvhnl1hfzg4+6c5xoqqtWI2DzJoXkm2zJpvlLQ==
=VEYC
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [RFC/PATCH v6 2/2] gitweb: Smarter snapshot names
From: Mark Rada @ 2009-10-30 20:07 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1256854062-25496-3-git-send-email-jnareb@gmail.com>
Hmm, I guess I was taking too long? Fair enough.
I've had more to do this academic term than I thought.
Thank you for getting this done.
--
Mark Rada (ferrous26)
marada@uwaterloo.ca
On 09-10-29 6:07 PM, Jakub Narebski wrote:
> From: Mark Rada <marada@uwaterloo.ca>
>
> Teach gitweb how to produce nicer snapshot names by only using the
> short hash id. If clients make requests using a tree-ish that is not
> a partial or full SHA-1 hash, then the short hash will also be appended
> to whatever they asked for. If clients request snapshot of a tag
> (which means that $hash ('h') parameter has 'refs/tags/' prefix),
> use only tag name.
>
> This also includes tests cases for t9502-gitweb-standalone-parse-output.
>
> Signed-off-by: Mark Rada <marada@uwaterloo.ca>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This is currently an RFC because commit message does not describe
> changes completely. Among others it doesn't describe current rules.
> Also tests could be better described.
>
> Changes since v5 (by Mark Rada):
> - put common parts of git_get_full_hash and git_get_short_hash
> into git_get_hash, do not do double verification
> - separate finding snapshot name and prefix in snapshot_name()
> subroutine, more smarts for snapshot name
> - improved tests (use 'tar' snapshot format, testing new smarts,
> testing prefix)
>
> gitweb/gitweb.perl | 76 +++++++++++++++----
> t/t9502-gitweb-standalone-parse-output.sh | 112 +++++++++++++++++++++++++++++
> 2 files changed, 172 insertions(+), 16 deletions(-)
> create mode 100755 t/t9502-gitweb-standalone-parse-output.sh
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8d4a2ae..d8dfd95 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1983,16 +1983,27 @@ sub quote_command {
>
> # get HEAD ref of given project as hash
> sub git_get_head_hash {
> - my $project = shift;
> + return git_get_full_hash(shift, 'HEAD');
> +}
> +
> +sub git_get_full_hash {
> + return git_get_hash(@_);
> +}
> +
> +sub git_get_short_hash {
> + return git_get_hash(@_, '--short=7');
> +}
> +
> +sub git_get_hash {
> + my ($project, $hash, @options) = @_;
> my $o_git_dir = $git_dir;
> my $retval = undef;
> $git_dir = "$projectroot/$project";
> - if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
> - my $head = <$fd>;
> + if (open my $fd, '-|', git_cmd(), 'rev-parse',
> + '--verify', '-q', @options, $hash) {
> + $retval = <$fd>;
> + chomp $retval if defined $retval;
> close $fd;
> - if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
> - $retval = $1;
> - }
> }
> if (defined $o_git_dir) {
> $git_dir = $o_git_dir;
> @@ -5179,6 +5190,43 @@ sub git_tree {
> git_footer_html();
> }
>
> +sub snapshot_name {
> + my ($project, $hash) = @_;
> +
> + # path/to/project.git -> project
> + # path/to/project/.git -> project
> + my $name = to_utf8($project);
> + $name =~ s,([^/])/*\.git$,$1,;
> + $name = basename($name);
> + # sanitize name
> + $name =~ s/[[:cntrl:]]/?/g;
> +
> + my $ver = $hash;
> + if ($hash =~ /^[0-9a-fA-F]+$/) {
> + # shorten SHA-1 hash
> + my $full_hash = git_get_full_hash($project, $hash);
> + if ($full_hash =~ /^$hash/ && length($hash) > 7) {
> + $ver = git_get_short_hash($project, $hash);
> + }
> + } elsif ($hash =~ m!^refs/tags/(.*)$!) {
> + # tags don't need shortened SHA-1 hash
> + $ver = $1;
> + } else {
> + # branches and other need shortened SHA-1 hash
> + if ($hash =~ m!^refs/(?:heads|remotes)/(.*)$!) {
> + $ver = $1;
> + }
> + $ver .= '-' . git_get_short_hash($project, $hash);
> + }
> + # in case of hierarchical branch names
> + $ver =~ s!/!.!g;
> +
> + # name = project-version_string
> + $name = "$name-$ver";
> +
> + return wantarray ? ($name, $name) : $name;
> +}
> +
> sub git_snapshot {
> my $format = $input_params{'snapshot_format'};
> if (!@snapshot_fmts) {
> @@ -5203,24 +5251,20 @@ sub git_snapshot {
> die_error(400, 'Object is not a tree-ish');
> }
>
> - my $name = $project;
> - $name =~ s,([^/])/*\.git$,$1,;
> - $name = basename($name);
> - my $filename = to_utf8($name);
> - $name =~ s/\047/\047\\\047\047/g;
> - my $cmd;
> - $filename .= "-$hash$known_snapshot_formats{$format}{'suffix'}";
> - $cmd = quote_command(
> + my ($name, $prefix) = snapshot_name($project, $hash);
> + my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
> + my $cmd = quote_command(
> git_cmd(), 'archive',
> "--format=$known_snapshot_formats{$format}{'format'}",
> - "--prefix=$name/", $hash);
> + "--prefix=$prefix/", $hash);
> if (exists $known_snapshot_formats{$format}{'compressor'}) {
> $cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
> }
>
> + $filename =~ s/(["\\])/\\$1/g;
> print $cgi->header(
> -type => $known_snapshot_formats{$format}{'type'},
> - -content_disposition => 'inline; filename="' . "$filename" . '"',
> + -content_disposition => 'inline; filename="' . $filename . '"',
> -status => '200 OK');
>
> open my $fd, "-|", $cmd
> diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
> new file mode 100755
> index 0000000..fbf4e26
> --- /dev/null
> +++ b/t/t9502-gitweb-standalone-parse-output.sh
> @@ -0,0 +1,112 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2009 Mark Rada
> +#
> +
> +test_description='gitweb as standalone script (parsing script output).
> +
> +This test runs gitweb (git web interface) as a CGI script from the
> +commandline, and checks that it produces the correct output, either
> +in the HTTP header or the actual script output.'
> +
> +
> +. ./gitweb-lib.sh
> +
> +# ----------------------------------------------------------------------
> +# snapshot file name
> +
> +cat >>gitweb_config.perl <<\EOF
> +
> +$known_snapshot_formats{'tar'} = {
> + 'display' => 'tar',
> + 'type' => 'application/x-tar',
> + 'suffix' => '.tar',
> + 'format' => 'tar',
> +};
> +
> +$feature{'snapshot'}{'default'} = ['tar'];
> +EOF
> +
> +test_expect_success setup '
> + test_commit first foo &&
> + git branch xx/test &&
> + FULL_ID=$(git rev-parse --verify HEAD) &&
> + SHORT_ID=$(git rev-parse --verify --short=7 HEAD)
> +'
> +test_debug '
> + echo "FULL_ID = $FULL_ID"
> + echo "SHORT_ID = $SHORT_ID"
> +'
> +
> +test_expect_success 'snapshots: give full hash' '
> + gitweb_run "p=.git;a=snapshot;h=$FULL_ID;sf=tar" &&
> + grep ".git-$SHORT_ID.tar" gitweb.headers
> +'
> +test_debug 'cat gitweb.headers'
> +test_debug 'cat gitweb.log'
> +
> +test_expect_success 'snapshots: give short hash' '
> + gitweb_run "p=.git;a=snapshot;h=$SHORT_ID;sf=tar" &&
> + grep ".git-$SHORT_ID.tar" gitweb.headers
> +'
> +test_debug 'cat gitweb.headers'
> +
> +test_expect_success 'snapshots: give almost full hash' '
> + ID=$(git rev-parse --short=30 HEAD) &&
> + gitweb_run "p=.git;a=snapshot;h=$ID;sf=tar" &&
> + grep ".git-$SHORT_ID.tar" gitweb.headers
> +'
> +test_debug 'cat gitweb.headers'
> +
> +test_expect_success 'snapshots: give HEAD' '
> + gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tar" &&
> + grep ".git-HEAD-$SHORT_ID.tar" gitweb.headers
> +'
> +test_debug 'cat gitweb.headers'
> +
> +test_expect_success 'snapshots: give short branch name' '
> + gitweb_run "p=.git;a=snapshot;h=master;sf=tar" &&
> + ID=$(git rev-parse --verify --short=7 master) &&
> + grep ".git-master-$ID.tar" gitweb.headers
> +'
> +test_debug 'cat gitweb.headers'
> +
> +test_expect_success 'snapshots: give short tag name' '
> + gitweb_run "p=.git;a=snapshot;h=first;sf=tar" &&
> + ID=$(git rev-parse --verify --short=7 first) &&
> + grep ".git-first-$ID.tar" gitweb.headers
> +'
> +test_debug 'cat gitweb.headers'
> +
> +test_expect_success 'snapshots: give full branch name' '
> + gitweb_run "p=.git;a=snapshot;h=refs/heads/master;sf=tar" &&
> + ID=$(git rev-parse --verify --short=7 master) &&
> + grep ".git-master-$ID.tar" gitweb.headers
> +'
> +test_debug 'cat gitweb.headers'
> +
> +test_expect_success 'snapshots: give full tag name' '
> + gitweb_run "p=.git;a=snapshot;h=refs/tags/first;sf=tar" &&
> + grep ".git-first.tar" gitweb.headers
> +'
> +test_debug 'cat gitweb.headers'
> +
> +test_expect_success 'snapshots: give hierarchical branch name' '
> + gitweb_run "p=.git;a=snapshot;h=xx/test;sf=tar" &&
> + grep -v "filename=.*/" gitweb.headers
> +'
> +test_debug 'cat gitweb.headers'
> +
> +cat >expected <<EOF
> +.git-HEAD-$SHORT_ID/
> +.git-HEAD-$SHORT_ID/foo
> +EOF
> +test_expect_success 'snapshots: check prefix' '
> + gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tar" &&
> + grep ".git-HEAD-$SHORT_ID.tar" gitweb.headers &&
> + "$TAR" tf gitweb.body >actual &&
> + test_cmp expected actual
> +'
> +test_debug 'cat gitweb.headers'
> +
> +test_done
^ permalink raw reply
* Re: Bug#553296: gitignore broken completely
From: Klaus Ethgen @ 2009-10-30 20:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, 553296, git
In-Reply-To: <7vvdhwk6dq.fsf@alter.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
Hi Junio,
Am Fr den 30. Okt 2009 um 20:51 schrieb Junio C Hamano:
> I am not sure; my head spins when I see "tracked but ignored" (you have
> a new one in the test) which is quite a bogus concept.
>
> Does it mean "tracked but would be ignored if it weren't---perhaps it was
> added by mistake?"?
Right. Exact. Sorry if my arguing was a bit misunderstandable. (My
spellchecker do not know "misunderstandable", so I hope it is ok; I am
from Germany, we like long words. ;-)
Regards
Klaus
- --
Klaus Ethgen http://www.ethgen.de/
pub 2048R/D1A4EDE5 2000-02-26 Klaus Ethgen <Klaus@Ethgen.de>
Fingerprint: D7 67 71 C4 99 A6 D4 FE EA 40 30 57 3C 88 26 2B
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iQEVAwUBSutG9J+OKpjRpO3lAQqBCQf+MWXaVdCeD7bT/dTCEQ8WfVKwISR6qNCP
wttV5oHnMp/f1mDWqhwYc4OUeOcLtkcV6pNf4oCMLY5v6zuvnLDgGB/8XW3l6JgZ
Y7m0UoSRu0s5Hux+VX/+CLwJ1cylstkwx3yOxcZ5VSXAgDlRIrt7LpwCsh/7lMSO
n5KUHw3/RodTwomxXUSRTQqU+SQMOUJrUvYR1EE9zR0YQkDKYqhulhUH+uirLaBQ
0rfJB7sTRqFfv2kHMLzO8t+jfco39cOZWTxlE1QBN1+k7aTZIJD2d79atfY7NLo0
2kCfcJXRurQ2rSpjUAbDVBo8JvkclnEnVLZP+s5kuwzej0hy4D2ocg==
=sN6Y
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: Bug#553296: gitignore broken completely
From: Jeff King @ 2009-10-30 20:05 UTC (permalink / raw)
To: Klaus Ethgen; +Cc: 553296, Junio C Hamano, git
In-Reply-To: <20091030200148.GD10671@ikki.ethgen.de>
On Fri, Oct 30, 2009 at 09:01:48PM +0100, Klaus Ethgen wrote:
> > +test_expect_success 'ls-files -i lists only tracked-but-ignored files' '
> > + echo content >other-file &&
> > + git add other-file &&
> > + echo file >expect &&
> > + git ls-files -i --exclude-standard >output &&
> > + test_cmp expect output
> > +'
> > +
> > test_done
>
> Do that fit? shouldn't it be "test_cmp other-file output"? "git ls-files
> -i --exclude-standard" should show the files in the index that are also
> in the exclude list. And you only add other-file to the index. And
> shouldn't there be also a "echo other-file > .gitignore"?
No, because it is building on the previous tests. The point of the test
is:
- we already have 'file' tracked and in gitignore (from previous
tests in the series)
- we add other-file to have some other file which is tracked but not
in gitignore
- we check the output of "ls-files -i" to make sure that "file" is
there, but "other-file" is not
-Peff
^ permalink raw reply
* Re: Bug#553296: gitignore broken completely
From: Klaus Ethgen @ 2009-10-30 20:01 UTC (permalink / raw)
To: Jeff King; +Cc: 553296, Junio C Hamano, git
In-Reply-To: <20091030190552.GA3528@coredump.intra.peff.net>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
Hi Jeff,
Am Fr den 30. Okt 2009 um 20:05 schrieb Jeff King:
> On Fri, Oct 30, 2009 at 02:41:55PM -0400, Jeff King wrote:
>
> > > 6. Revert the patch and rework it so that it will only have effect if
> > > there is no -i option on the command line. (That is similiar to a
> > > mix of 3 and 4.)
> >
> > Yeah, that would actually be the least invasive change, and would keep
> > "-i" just as it is. If we do anything except a simple, I think your (6)
> > makes the most sense.
Ok.
> > Let me see if I can make a patch.
Fine, thanks.
> - Show ignored files in the output.
> - Note that this also reverses any exclude list present.
> + Show only ignored files in the output. When showing files in the
> + index, print only those matched by an exclude pattern. When
> + showing "other" files, show only those matched by an exclude
> + pattern.
Hmmm... I do not see that much difference. But that might be justified
by my bad English.
> --- a/t/t3003-ls-files-exclude.sh
> +++ b/t/t3003-ls-files-exclude.sh
> @@ -29,4 +29,12 @@ test_expect_success 'add file to gitignore' '
> '
> check_all_output
>
> +test_expect_success 'ls-files -i lists only tracked-but-ignored files' '
> + echo content >other-file &&
> + git add other-file &&
> + echo file >expect &&
> + git ls-files -i --exclude-standard >output &&
> + test_cmp expect output
> +'
> +
> test_done
Do that fit? shouldn't it be "test_cmp other-file output"? "git ls-files
- -i --exclude-standard" should show the files in the index that are also
in the exclude list. And you only add other-file to the index. And
shouldn't there be also a "echo other-file > .gitignore"?
Regards
Klaus
- --
Klaus Ethgen http://www.ethgen.de/
pub 2048R/D1A4EDE5 2000-02-26 Klaus Ethgen <Klaus@Ethgen.de>
Fingerprint: D7 67 71 C4 99 A6 D4 FE EA 40 30 57 3C 88 26 2B
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iQEVAwUBSutGLJ+OKpjRpO3lAQqmZwgAi7DqPorAdp0dZtgMpgzCsrWTJx5xb/dv
voQ4K5pYmxG1PgLb8zY7ywIvcG36uakCGvwgFQxsLzSgg0hRO1UsQ8XFVqSPp62X
evsZ/On5LYHY3xz+Fl0cPM5oomtqY+ZmHAd5syj4oh6hSHM8J93RuQzGEfMshtrQ
NUfAbAVmjuA5d4Cl/SLNUvoLE6M6O3YGkCGKXA9aPQcker6W+nODExJgTqyh4RHK
ATfbPC6+VJGUPfjVGmrqHVW8LOb/wP7grEBMaHvGpP/ysh4FHjy6HtXLhyPSRES/
KpsMG5g0dF0dbS0qBgrIck+6cifiHNlAbxc5LF7tydFBMnRAmjDyeA==
=7JTn
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: Bug#553296: gitignore broken completely
From: Jeff King @ 2009-10-30 19:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Klaus Ethgen, 553296, git
In-Reply-To: <7vvdhwk6dq.fsf@alter.siamese.dyndns.org>
On Fri, Oct 30, 2009 at 12:51:13PM -0700, Junio C Hamano wrote:
> > Here it is. I think this is the right thing to do. Junio?
>
> I am not sure; my head spins when I see "tracked but ignored" (you have
> a new one in the test) which is quite a bogus concept.
>
> Does it mean "tracked but would be ignored if it weren't---perhaps it was
> added by mistake?"?
Yes, that is exactly what it means.
-Peff
^ permalink raw reply
* Re: Bug#553296: gitignore broken completely
From: Junio C Hamano @ 2009-10-30 19:51 UTC (permalink / raw)
To: Jeff King; +Cc: Klaus Ethgen, 553296, Junio C Hamano, git
In-Reply-To: <20091030190552.GA3528@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Oct 30, 2009 at 02:41:55PM -0400, Jeff King wrote:
>
>> > 6. Revert the patch and rework it so that it will only have effect if
>> > there is no -i option on the command line. (That is similiar to a
>> > mix of 3 and 4.)
>>
>> Yeah, that would actually be the least invasive change, and would keep
>> "-i" just as it is. If we do anything except a simple, I think your (6)
>> makes the most sense.
>>
>> Let me see if I can make a patch.
>
> Here it is. I think this is the right thing to do. Junio?
I am not sure; my head spins when I see "tracked but ignored" (you have
a new one in the test) which is quite a bogus concept.
Does it mean "tracked but would be ignored if it weren't---perhaps it was
added by mistake?"?
^ permalink raw reply
* [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Erick Mattos @ 2009-10-30 19:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Erick Mattos
The code herein changes commit timestamp recording from a source in a
more intuitive way.
Description:
When we use one of the options above we are normally trying to do mainly
two things: one is using the source as a template and second is to
recreate a commit with corrections.
In the first case timestamp should by default be taken by the time we
are doing the commit, not by the source. On the second case the actual
behavior is acceptable.
Anyway this update creates new options for choosing the source timestamp
or a new one. And set as default for -c option (editing one) to take a
new timestamp and for -C option the source timestamp. That is because
we are normally using the source as template when we we are editing and
as a correction when we are just copying it.
Those options are also useful for --amend option which is by default
behaving the same.
Signed-off-by: Erick Mattos <erick.mattos@gmail.com>
---
Documentation/git-commit.txt | 10 ++++++++--
builtin-commit.c | 15 ++++++++++++---
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0578a40..27c61d2 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
[(-c | -C) <commit>] [-F <file> | -m <msg>]
+ [(--old-timestamp | --new-timestamp)]
[--allow-empty] [--no-verify] [-e] [--author=<author>]
[--cleanup=<mode>] [--] [[-i | -o ]<file>...]
@@ -61,13 +62,16 @@ OPTIONS
-C <commit>::
--reuse-message=<commit>::
Take an existing commit object, and reuse the log message
- and the authorship information (including the timestamp)
- when creating the commit.
+ and the authorship information when creating the commit.
+ By default, timestamp is taken from specified commit unless
+ option --new-timestamp is included.
-c <commit>::
--reedit-message=<commit>::
Like '-C', but with '-c' the editor is invoked, so that
the user can further edit the commit message.
+ By default, timestamp is recalculated and not taken from
+ specified commit unless option --old-timestamp is included.
-F <file>::
--file=<file>::
@@ -134,6 +138,8 @@ OPTIONS
current tip -- if it was a merge, it will have the parents of
the current tip as parents -- so the current top commit is
discarded.
+ By default, timestamp is taken from latest commit unless option
+ --new-timestamp is included.
+
--
It is a rough equivalent for:
diff --git a/builtin-commit.c b/builtin-commit.c
index c395cbf..a924584 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -52,6 +52,7 @@ static char *edit_message, *use_message;
static char *author_name, *author_email, *author_date;
static int all, edit_flag, also, interactive, only, amend, signoff;
static int quiet, verbose, no_verify, allow_empty, dry_run;
+static int old_timestamp, new_timestamp;
static char *untracked_files_arg;
/*
* The default commit message cleanup mode will remove the lines
@@ -91,8 +92,10 @@ static struct option builtin_commit_options[] = {
OPT_FILENAME('F', "file", &logfile, "read log from file"),
OPT_STRING(0, "author", &force_author, "AUTHOR", "override author for commit"),
OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
- OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit "),
+ OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
+ OPT_BOOLEAN(0, "old-timestamp", &old_timestamp, "reuse previous commit's timestamp"),
+ OPT_BOOLEAN(0, "new-timestamp", &new_timestamp, "regenerate timestamp, ignoring previous one"),
OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
OPT_FILENAME('t', "template", &template_file, "use specified template file"),
OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
@@ -396,7 +399,8 @@ static void determine_author_info(void)
name = xstrndup(a + 8, lb - (a + 8));
email = xstrndup(lb + 2, rb - (lb + 2));
- date = xstrndup(rb + 2, eol - (rb + 2));
+ if (!new_timestamp)
+ date = xstrndup(rb + 2, eol - (rb + 2));
}
if (force_author) {
@@ -763,11 +767,16 @@ static int parse_and_validate_options(int argc, const char *argv[],
die("You have nothing to amend.");
if (amend && in_merge)
die("You are in the middle of a merge -- cannot amend.");
+ if (old_timestamp && new_timestamp)
+ die("You can not use --old-timesamp and --new-timestamp together.");
if (use_message)
f++;
- if (edit_message)
+ if (edit_message) {
+ if (!old_timestamp)
+ new_timestamp = 1;
f++;
+ }
if (logfile)
f++;
if (f > 1)
--
1.6.5.2.102.g3ad0
^ permalink raw reply related
* Re: Bug#553296: gitignore broken completely
From: Jeff King @ 2009-10-30 19:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Klaus Ethgen, 553296, git
In-Reply-To: <7vaaz8lleg.fsf@alter.siamese.dyndns.org>
On Fri, Oct 30, 2009 at 12:41:27PM -0700, Junio C Hamano wrote:
> I've never understood the use of "ls-files -i" without -o, so in that
> sense, I have done 2. myself already long time ago.
>
> In other words, I do not really care that much, and the choice would be
> between "0. do not do anything---the patch in question was a bugfix for
> longstanding insanity" and your "4. -i without -o didn't make much sense
> but now it does and here is the new meaning".
OK, I think the patch I sent elsewhere in the thread should be applied,
then, as it should make you, me, and Klaus happy.
-Peff
^ permalink raw reply
* Re: Bug#553296: gitignore broken completely
From: Junio C Hamano @ 2009-10-30 19:41 UTC (permalink / raw)
To: Jeff King; +Cc: Klaus Ethgen, 553296, Junio C Hamano, git
In-Reply-To: <20091030173838.GB18583@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I am not sure simply reverting is the best choice; the patch does do
> something useful. And while it strictly breaks backwards compatibility
> on the output without "-i", the old behavior was considered a bug. But
> the "-i" behavior is useless now, so we need to figure out how to
> proceed. We can:
>
> 1. Revert and accept that the behavior is historical. Callers need to
> work around it by dropping --exclude* when invoking ls-files.
>
> 2. Declare "-i" useless, deprecate and/or remove. Obviously this is
> also breaking existing behavior, but again, I don't think that
> using "-i" actually accomplishes anything.
>
> 3. Revert for now, and then reinstate the patch during a major version
> bump when we are declaring some compatibility breakages.
>
> 4. Re-work "-i" to show tracked but ignored files, but still show all
> files when "-i" is not given at all.
>
> I think (4) preserves the benefit of the patch in question, but still
> allows your usage ("git ls-files -i --exclude-standard"). I do question
> whether that usage is worth supporting. Certainly I wouldn't implement
> it if I were writing git-ls-files from scratch today, but we do in
> general try to maintain backwards compatibility, especially for plumbing
> like ls-files.
>
> Junio, what do you want to do?
I've never understood the use of "ls-files -i" without -o, so in that
sense, I have done 2. myself already long time ago.
In other words, I do not really care that much, and the choice would be
between "0. do not do anything---the patch in question was a bugfix for
longstanding insanity" and your "4. -i without -o didn't make much sense
but now it does and here is the new meaning".
^ permalink raw reply
* Re: [RFC PATCH v4 03/26] pkt-line: Make packet_read_line easier to debug
From: Junio C Hamano @ 2009-10-30 19:35 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, Junio C Hamano, git
In-Reply-To: <20091030175741.GC18583@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Oct 29, 2009 at 02:58:29PM -0700, Shawn O. Pearce wrote:
>
>> Junio C Hamano <gitster@pobox.com> wrote:
>> > > The NUL assignment isn't about safe_read(), its about making the
>> > > block of 4 bytes read a proper C-style string so we can safely pass
>> > > it down into the snprintf that is within die().
>> >
>> > I knew and understood all of what you just said. static linelen[] is not
>> > about stack allocation. It's about letting the compiler initialize it to
>> > five NULs so that you do not have to assign NUL to its fifth place before
>> > you die. This removes one added line from your patch.
>>
>> Thanks, I get it now. Patch amended.
>
> I am just a bystander, so maybe my opinion is not worth anything, but
> personally I think you are obfuscating the code to save a single line.
Well, the comment was not a serious "you should do it this way" to begin
with. An extra assignment in the error path is not a big deal.
^ permalink raw reply
* Re: [RFC PATCH v4 03/26] pkt-line: Make packet_read_line easier to debug
From: David Brown @ 2009-10-30 19:31 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Jeff King, Junio C Hamano, git@vger.kernel.org
In-Reply-To: <20091030191239.GF10505@spearce.org>
On Fri, Oct 30, 2009 at 12:12:40PM -0700, Shawn O. Pearce wrote:
> > If you really just want to initialize to zero, using
> >
> > char linelen[5] = { 0 };
>
> Bleh, I find that has hard to grok as what we have now. Perhaps my
> understanding of the relevant standards is incomplete, but I'd read
> that as linelen[0] = 0, but the other 4 positions are undefined
> and may be not be initialized.
C isn't that into undefined things. In this particular case, all
remaining values will be initialized to zero.
> I actually considered this one, but again I wasn't clear what would
> happen in the standard C library when we fed a string that wasn't
> actually NUL terminated. Is the library permitted to call strlen()
> before formatting? If so strlen() could SIGSEGV if we are unlucky
> and no NUL is present between our string and the end of the assigned
> memory region.
The linux manpage says "If a precision is given, no null byte
need be present". This text is copied verbatim out of the Posix
specification, so a C library that failed to handle this would be
non-compliant.
I think the %.4s is clearest, and is, in fact, somewhat
idiomatic.
David
^ permalink raw reply
* Re: [RFC PATCH v4 03/26] pkt-line: Make packet_read_line easier to debug
From: Jeff King @ 2009-10-30 19:30 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20091030191239.GF10505@spearce.org>
On Fri, Oct 30, 2009 at 12:12:40PM -0700, Shawn O. Pearce wrote:
> > I am just a bystander, so maybe my opinion is not worth anything, but
> > personally I think you are obfuscating the code to save a single line.
>
> Yup, me too. But I'm also willing to do what I need to get my
> patches included in git.git. Smart HTTP is something a lot of people
> have been waiting for. If the maintainer wants the bikeshed to be
> a particular shade of red, I'll paint it that way.
Yes, I too want it merged ASAP, so please Junio if you disagree with my
comments, ignore them. But I did want to point out that the proposed
"improvement" was making things worse, IMHO. So I feel like I was at
least bikeshedding his bikeshed, instead of starting my own. ;)
> > If you really just want to initialize to zero, using
> >
> > char linelen[5] = { 0 };
>
> Bleh, I find that has hard to grok as what we have now. Perhaps my
> understanding of the relevant standards is incomplete, but I'd read
> that as linelen[0] = 0, but the other 4 positions are undefined
> and may be not be initialized.
Your understanding is incomplete. :) C99, 6.7.8, paragraph 19:
... all subobjects that are not initialized explicitly shall be
initialized implicitly the same as objects that have static storage
duration.
I don't recall offhand whether that was the case in C89 or not.
But that being said, it was an attempt to make the meaning clear. If
it's not to you (who I consider at least reasonably competent ;) ), then
it failed.
> > die("protocol error: bad line length character: %.4s", linelen);
>
> I actually considered this one, but again I wasn't clear what would
> happen in the standard C library when we fed a string that wasn't
> actually NUL terminated. Is the library permitted to call strlen()
> before formatting? If so strlen() could SIGSEGV if we are unlucky
> and no NUL is present between our string and the end of the assigned
> memory region.
No, using a precision specifier for a string with no NUL is explicitly
allowed by the standard (and we use it elsewhere in git, IIRC).
> To me, my original version was the most clear, to me and anyone
> else who could ever possibly come by to read it. The "one extra
> line of code" is also only in an error condition which never occurs
> (but did once due to a bug in the HTTP code, which is why I added
> this patch to my series, to help debug it). Its not like this is
> a performance sensitive section of git that Linus is going to come
> back and overhaul.
Heh. You never know what performance item Linus will complain about. ;)
But yes, I am fine with your initial one, too. The drawback to it (and
my "%.4s") is not that one line of code is so unbearable, but that it is
one extra thing anyone using it as a string must do, and if they forget,
we get a horrible segfault.
Anyway, my complaint has been lodged. Junio is aware of the alternatives
and can pick whichever he wants.
-Peff
^ permalink raw reply
* Re: [PATCH] More precise description of 'git describe --abbrev'
From: Gisle Aas @ 2009-10-30 19:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7viqdwlnam.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 685 bytes --]
On Fri, Oct 30, 2009 at 20:00, Junio C Hamano <gitster@pobox.com> wrote:
> Adding an explanation like this so that nobody will be tempted to "fix"
> the example would be the best, I think.
Updated patch attached.
>
> [torvalds@g5 git]$ git describe --all --abbrev=4 v1.0.5^2
> tags/v1.0.0-21-g975b
>
> Note that the suffix you get if you type this command today may be
> longer than what Linus saw above when he ran this command, as your
> git repository may have new commits whose object names begin with
> 975b that did not exist back then, and "-g975b" suffix alone is not
> sufficient to disambiguate these commits.
>
>
[-- Attachment #2: 0001-More-precise-description-of-git-describe-abbrev.patch --]
[-- Type: application/octet-stream, Size: 2430 bytes --]
From 5f6b405fa24861ae89207d334ea2e11f383a7e4e Mon Sep 17 00:00:00 2001
From: Gisle Aas <gisle@aas.no>
Date: Thu, 29 Oct 2009 22:29:35 +0100
Subject: [PATCH] More precise description of 'git describe --abbrev'
Also adds a note about why the output in the examples might give
different output today.
Signed-off-by: Gisle Aas <gisle@aas.no>
---
Documentation/git-describe.txt | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index b231dbb..f8d91b1 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -44,7 +44,9 @@ OPTIONS
--abbrev=<n>::
Instead of using the default 7 hexadecimal digits as the
- abbreviated object name, use <n> digits.
+ abbreviated object name, use <n> digits or as many digits
+ are needed to form a unique object name. An <n> of 0
+ will suppress long format, only showing the closest tag.
--candidates=<n>::
Instead of considering only the 10 most recent tags as
@@ -68,8 +70,8 @@ OPTIONS
This is useful when you want to see parts of the commit object name
in "describe" output, even when the commit in question happens to be
a tagged version. Instead of just emitting the tag name, it will
- describe such a commit as v1.2-0-deadbeef (0th commit since tag v1.2
- that points at object deadbeef....).
+ describe such a commit as v1.2-0-gdeadbee (0th commit since tag v1.2
+ that points at object deadbee....).
--match <pattern>::
Only consider tags matching the given pattern (can be used to avoid
@@ -108,7 +110,7 @@ the output shows the reference path as well:
[torvalds@g5 git]$ git describe --all --abbrev=4 v1.0.5^2
tags/v1.0.0-21-g975b
- [torvalds@g5 git]$ git describe --all HEAD^
+ [torvalds@g5 git]$ git describe --all --abbrev=4 HEAD^
heads/lt/describe-7-g975b
With --abbrev set to 0, the command can be used to find the
@@ -117,6 +119,13 @@ closest tagname without any suffix:
[torvalds@g5 git]$ git describe --abbrev=0 v1.0.5^2
tags/v1.0.0
+Note that the suffix you get if you type these commands today may be
+longer than what Linus saw above when he ran this command, as your
+git repository may have new commits whose object names begin with
+975b that did not exist back then, and "-g975b" suffix alone is not
+sufficient to disambiguate these commits.
+
+
SEARCH STRATEGY
---------------
--
1.6.2.95.g934f7
^ permalink raw reply related
* Re: [RFC PATCH v4 03/26] pkt-line: Make packet_read_line easier to debug
From: Shawn O. Pearce @ 2009-10-30 19:12 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20091030175741.GC18583@coredump.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Thu, Oct 29, 2009 at 02:58:29PM -0700, Shawn O. Pearce wrote:
> > Junio C Hamano <gitster@pobox.com> wrote:
> > > I knew and understood all of what you just said. static linelen[] is not
> > > about stack allocation. It's about letting the compiler initialize it to
> > > five NULs so that you do not have to assign NUL to its fifth place before
> > > you die. This removes one added line from your patch.
>
> I am just a bystander, so maybe my opinion is not worth anything, but
> personally I think you are obfuscating the code to save a single line.
Yup, me too. But I'm also willing to do what I need to get my
patches included in git.git. Smart HTTP is something a lot of people
have been waiting for. If the maintainer wants the bikeshed to be
a particular shade of red, I'll paint it that way.
> When I see a static variable, I assume it is because the value should be
> saved from invocation to invocation, and now I will spend time wondering
> why that would be the case here.
Me too. I also can't grok the code I just wrote, for the same
reason, it just reads wrong. But who am I to argue with the guy
who is most likely going to be the one who needs to deal with it
in the future?
> If you really just want to initialize to zero, using
>
> char linelen[5] = { 0 };
Bleh, I find that has hard to grok as what we have now. Perhaps my
understanding of the relevant standards is incomplete, but I'd read
that as linelen[0] = 0, but the other 4 positions are undefined
and may be not be initialized.
> I think it would be even more clear to leave it as
>
> die("protocol error: bad line length character: %.4s", linelen);
I actually considered this one, but again I wasn't clear what would
happen in the standard C library when we fed a string that wasn't
actually NUL terminated. Is the library permitted to call strlen()
before formatting? If so strlen() could SIGSEGV if we are unlucky
and no NUL is present between our string and the end of the assigned
memory region.
To me, my original version was the most clear, to me and anyone
else who could ever possibly come by to read it. The "one extra
line of code" is also only in an error condition which never occurs
(but did once due to a bug in the HTTP code, which is why I added
this patch to my series, to help debug it). Its not like this is
a performance sensitive section of git that Linus is going to come
back and overhaul.
--
Shawn.
^ permalink raw reply
* Re: [RFC PATCH v4 11/26] Move WebDAV HTTP push under remote-curl
From: Clemens Buchacher @ 2009-10-30 19:06 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: git, Shawn O. Pearce, Daniel Barkalow, Mike Hommey
In-Reply-To: <be6fef0d0910300910me43c77fue6dcb6034dd0ea5b@mail.gmail.com>
On Sat, Oct 31, 2009 at 12:10:29AM +0800, Tay Ray Chuan wrote:
> Clemens, the following addresses your non-desire to remove the
> "unpacked refs" test in your earlier email
> <20091025161630.GB8532@localhost>.
> Now that the first push in "push to remote repository with packed
> refs" succeeds, the "unpacked refs" test is redundant.
How can the changed result of one test suddenly make another test redundant?
The two are testing different things.
> Since http-push
> in that first test already updated /refs/heads/master and /info/refs,
> 'git update-ref' incorrectly reverts the earlier push, and 'git
> update-server-info' is redundant.
No, 'git update-ref' correctly reverts the earlier push, so we can push again
and 'git update-server-info' is therefore necessary for the test to work
independently of its predecessors result.
Clemens
^ permalink raw reply
* Re: Bug#553296: gitignore broken completely
From: Jeff King @ 2009-10-30 19:05 UTC (permalink / raw)
To: Klaus Ethgen; +Cc: 553296, Junio C Hamano, git
In-Reply-To: <20091030184155.GC19901@coredump.intra.peff.net>
On Fri, Oct 30, 2009 at 02:41:55PM -0400, Jeff King wrote:
> > 6. Revert the patch and rework it so that it will only have effect if
> > there is no -i option on the command line. (That is similiar to a
> > mix of 3 and 4.)
>
> Yeah, that would actually be the least invasive change, and would keep
> "-i" just as it is. If we do anything except a simple, I think your (6)
> makes the most sense.
>
> Let me see if I can make a patch.
Here it is. I think this is the right thing to do. Junio?
-- >8 --
Subject: [PATCH] ls-files: unbreak "ls-files -i"
Commit b5227d8 changed the behavior of "ls-files" with
respect to includes, but accidentally broke the "-i" option
The original behavior was:
1. if no "-i" is given, cull all results according to --exclude*
2. if "-i" is given, show the inverse of (1)
The broken behavior was:
1. if no "-i" is given:
a. for "-o", cull results according to --exclude*
b. for index files, always show all
2. if "-i" is given:
a. for "-o", shows the inverse of (1a)
b. for index files, always show all
The fixed behavior keeps the new (1b) behavior introduced
by b5227d8, but fixes the (2b) behavior to show only ignored
files, not all files.
This patch also tweaks the documentation. The original text
was somewhat obscure in the first place, but it is also now
inaccurate (the relationship between (1b) and (2b) is not
quite a "reverse").
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-ls-files.txt | 6 ++++--
builtin-ls-files.c | 8 ++++++++
t/t3003-ls-files-exclude.sh | 8 ++++++++
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 021066e..625723e 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -48,8 +48,10 @@ OPTIONS
-i::
--ignored::
- Show ignored files in the output.
- Note that this also reverses any exclude list present.
+ Show only ignored files in the output. When showing files in the
+ index, print only those matched by an exclude pattern. When
+ showing "other" files, show only those matched by an exclude
+ pattern.
-s::
--stage::
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index c5c0407..c9a03e5 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -170,6 +170,10 @@ static void show_files(struct dir_struct *dir, const char *prefix)
if (show_cached | show_stage) {
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
+ int dtype = ce_to_dtype(ce);
+ if (dir->flags & DIR_SHOW_IGNORED &&
+ !excluded(dir, ce->name, &dtype))
+ continue;
if (show_unmerged && !ce_stage(ce))
continue;
if (ce->ce_flags & CE_UPDATE)
@@ -182,6 +186,10 @@ static void show_files(struct dir_struct *dir, const char *prefix)
struct cache_entry *ce = active_cache[i];
struct stat st;
int err;
+ int dtype = ce_to_dtype(ce);
+ if (dir->flags & DIR_SHOW_IGNORED &&
+ !excluded(dir, ce->name, &dtype))
+ continue;
if (ce->ce_flags & CE_UPDATE)
continue;
err = lstat(ce->name, &st);
diff --git a/t/t3003-ls-files-exclude.sh b/t/t3003-ls-files-exclude.sh
index fc1e379..d5ec333 100755
--- a/t/t3003-ls-files-exclude.sh
+++ b/t/t3003-ls-files-exclude.sh
@@ -29,4 +29,12 @@ test_expect_success 'add file to gitignore' '
'
check_all_output
+test_expect_success 'ls-files -i lists only tracked-but-ignored files' '
+ echo content >other-file &&
+ git add other-file &&
+ echo file >expect &&
+ git ls-files -i --exclude-standard >output &&
+ test_cmp expect output
+'
+
test_done
--
1.6.5.2.224.g22719.dirty
^ permalink raw reply related
* Re: [PATCH] More precise description of 'git describe --abbrev'
From: Junio C Hamano @ 2009-10-30 19:00 UTC (permalink / raw)
To: Gisle Aas; +Cc: git
In-Reply-To: <b48ea8a00910300101i38f2e4a8q2c2e014d2df1a150@mail.gmail.com>
Gisle Aas <gisle@aas.no> writes:
> On Thu, Oct 29, 2009 at 23:47, Junio C Hamano <gitster@pobox.com> wrote:
>> Gisle Aas <gisle@aas.no> writes:
>>
>>> Also make the examples show what 'git describe' actually outputs
>>> currently. I guess the default --abbrev value has been changed from 4
>>> to 7 at some point.
>>
>> Some are good changes, but I do not think the example with --abbrev=4 is.
>>
>> $ git describe 975bf9cf5ad5d440f98f464ae8124609a4835ce1
>> v1.3.2-216-g975bf9c
>> $ git describe 975b31dc6e12fba8f7b067ddbe32230995e05400
>> v1.0.0-21-g975b31d
>>
>> Next time somebody adds a new object whose name happens to begin with
>> 975b3 you would need to update the example output.
>
> Yeah, I know, but I don't think that's a big deal. So do you want an
> updated patch for that? We could either simply remove this example or
> make it use --abbrev=10 or something like that.
Not touching the example would be the simplest.
Adding an explanation like this so that nobody will be tempted to "fix"
the example would be the best, I think.
[torvalds@g5 git]$ git describe --all --abbrev=4 v1.0.5^2
tags/v1.0.0-21-g975b
Note that the suffix you get if you type this command today may be
longer than what Linus saw above when he ran this command, as your
git repository may have new commits whose object names begin with
975b that did not exist back then, and "-g975b" suffix alone is not
sufficient to disambiguate these commits.
^ permalink raw reply
* Re: [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool option
From: Junio C Hamano @ 2009-10-30 18:54 UTC (permalink / raw)
To: Charles Bailey
Cc: Scott Chacon, Junio C Hamano, Jay Soffian, git list,
David Aguilar
In-Reply-To: <20091030174421.GA21486@hashpling.org>
Charles Bailey <charles@hashpling.org> writes:
> On Wed, Oct 28, 2009 at 02:39:32PM -0700, Scott Chacon wrote:
>> p4merge is now a built-in diff/merge tool.
>> This adds p4merge to git-completion and updates
>> the documentation to mention p4merge.
>>
>> Signed-Off-By: Scott Chacon <schacon@gmail.com>
>> ---
>
> Acked-by: Charles Bailey <charles@hashpling.org>
>
> I'm aware that we haven't reached full agreement on the best way to
> make p4merge + git as Mac OS X friendly as possible, but Jay said that
> this patch is 'good enough' and I agree. If we go with this for now,
> we're not closing the door to further improvements.
Will queue; the peculiarity of MacOS X may be annoying, but the annoyance
is not limited to the topic of adding p4merge to this codepath.
^ permalink raw reply
* Bug#553296: gitignore broken completely
From: Klaus Ethgen @ 2009-10-30 18:23 UTC (permalink / raw)
To: Jeff King; +Cc: 553296, Junio C Hamano, git
In-Reply-To: <20091030173838.GB18583@coredump.intra.peff.net>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
Am Fr den 30. Okt 2009 um 18:38 schrieb Jeff King:
> It was not exactly on purpose. The point of that change was that "git
> ls-files" should also show things mentioned in the .gitignore, because
> .gitignore has nothing to do whatsoever with tracked files.
(Mostly) true. It has when sending such patches that adds a ignored
file, to some other.
> But I simply forgot about "git ls-files -i", so changing it was an
> unintended side effect (and sadly, we don't seem to have any regression
> tests for it).
Could happen. No Problem with that as such.
> > That makes the whole sense of -i ad absurdum (I do not know how to tell
> > "ad absurdum" in english). With that patch there is no way anymore to
> > see if some ignored files are accidentally committed.
>
> Yes, with the current code "-i" serves no purpose unless you are using
> "-o".
But this is an other use case.
> > I have to use git often as frontend for that broken design (svn). So to
> > hold the ignores up2date I use "git svn show-ignore > .gitignore" But
> > many, many repositories have broken ignores so I have to check it with
> > "git ls-files -i --exclude-standard" to see if there is accidentally
> > ignored files. Well, that patch makes that impossible at all!
>
> Just to be clear: nothing is actually broken about ignores that cover
> tracked files. Ignores are (and have been since long before this patch)
> purely about untracked files. So there is no problem at all with:
See my comment above.
> The _only_ thing that respected such ignores was "git ls-files", and the
> point of the patch in question was to fix that.
Well ls-files is used to see such broken files. (Another example is if
you accidentally add a file which you (later) decide to be ignored. You
will have no change to find that files at all anymore.) With the patch
that use case of ls-files has been gone without a replacement.
> > So I think, this patch is a bug at all!
> >
> > I add Jeff (and Junio as he did the commit) to the Cc. @Jeff and or
> > Junio: could you please revoke that patch?
>
> I am not sure simply reverting is the best choice; the patch does do
> something useful. And while it strictly breaks backwards compatibility
> on the output without "-i", the old behavior was considered a bug. But
> the "-i" behavior is useless now, so we need to figure out how to
> proceed. We can:
>
> 1. Revert and accept that the behavior is historical. Callers need to
> work around it by dropping --exclude* when invoking ls-files.
>
> 2. Declare "-i" useless, deprecate and/or remove. Obviously this is
> also breaking existing behavior, but again, I don't think that
> using "-i" actually accomplishes anything.
>
> 3. Revert for now, and then reinstate the patch during a major version
> bump when we are declaring some compatibility breakages.
>
> 4. Re-work "-i" to show tracked but ignored files, but still show all
> files when "-i" is not given at all.
I have two more options:
5. Revert the patch and rework it to have a option to ignore or
respect the excluded files (Such as --use-excludes for example) or
respect them anyway if a --exclude* option is given on command
line.
6. Revert the patch and rework it so that it will only have effect if
there is no -i option on the command line. (That is similiar to a
mix of 3 and 4.)
I have nothing against the patch as such. But in the current form it
breaks at least one frequent used use case of ls-files.
> I think (4) preserves the benefit of the patch in question, but still
> allows your usage ("git ls-files -i --exclude-standard"). I do question
> whether that usage is worth supporting. Certainly I wouldn't implement
> it if I were writing git-ls-files from scratch today,
Well I had to search explicit for this use case as I had several
problems with ignored files in combination with svn. So I would. (if I
know the code good enough. And this problem to list such files did made
enough pain to me that I would go into the code to get it implemented.)
Regards
Klaus
- --
Klaus Ethgen http://www.ethgen.de/
pub 2048R/D1A4EDE5 2000-02-26 Klaus Ethgen <Klaus@Ethgen.de>
Fingerprint: D7 67 71 C4 99 A6 D4 FE EA 40 30 57 3C 88 26 2B
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iQEVAwUBSusvI5+OKpjRpO3lAQqMhQgAnM7w+VqvUB+zFCJj8sCqO6QgcI+oup1z
dMwsv+5QU+S5UH7xdm/L6AhFJEsWsbpHzytVg7Rv3wCp8OzFiXmnjfUw+3JEvuLJ
+ggWHvFeKkTReDaRY00dafAQCP/8h0Yar6hVmXfdqGeiOnK0LeN5OXx0T3K51U/2
r8YOeNPZOzbaITcRaeIi5ghAEpyAgdqEw++f8h1xCRGo6DyncUIoexmFSG0pZS8q
tsyksW7q02LscTEp6PinQa7jhUN0xWJFTvpuCBWlfNgNTTffWt1xDjXTebwRKsYR
cT1ygEiI2+aZfrE47Fn91G9I+JjF5KYo7jt5UFNFlck9YsEKGPf22g==
=2ZcU
-----END PGP SIGNATURE-----
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox