* [PATCH] Reword -M, when in `git log`s documention, to suggest --follow
From: Alex Vandiver @ 2009-12-21 20:40 UTC (permalink / raw)
To: git
The documentation for `git log` is sadly misleading when it comes
to tracking renames. By far the most common option that users
new to git want is the ability to view the history of a file
across renames. Unfortunately, `git log --help` shows:
NAME
git-log - Show commit logs
[...]
OPTIONS
[...]
-M
Detect renames.
...and most users stop reading there. Unfortunately, what
they're generally looking for comes significantly later:
[...]
--follow
Continue listing the history of a file beyond renames.
Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
Documentation/diff-options.txt | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 8707d0e..bcbad88 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -175,7 +175,13 @@ endif::git-format-patch[]
Break complete rewrite changes into pairs of delete and create.
-M::
+ifdef::git-log[]
+ Show renames in diff output. See `--follow` to track history
+ across renames.
+endif::git-log[]
+ifndef::git-log[]
Detect renames.
+endif::git-log[]
-C::
Detect copies as well as renames. See also `--find-copies-harder`.
--
1.6.6.rc0.363.g69d13.dirty
^ permalink raw reply related
* git's fascination with absolute paths
From: J Chapman Flack @ 2009-12-21 18:42 UTC (permalink / raw)
To: git
Hi list,
I have a requirement involving the reasonably common Unix-y design
pattern where a directory owned by a particular user lives in a
directory that user can't access, with a setuid gate that chdirs to
the right place and then drops to the real user's id to do certain
allowed things.
I wanted to use git for some of those allowed things and I can't,
because the code seems to call make_absolute_path on approximately
everything, and this is one of the situations that illustrates why it
isn't safe to assume you can get an absolute path that's even
usable (let alone race-free) corresponding to a relative path
in general.
git init tries to do this on the db path (even if
it's specified explicitly in GIT_DIR), and even if I do the git init
as root in advance, all the other git subcommands I've tried also
try to do it and fail when they chdir up out of the
working directory and try to chdir (not fchdir) back in.
In general it seems best for a program to stay free of assumptions
about absolute paths except when there is a specific functional
requirement that needs them. I assume there is something git does
that requires it to have this limitation, but it's not intuitive
to me if I just think about what I expect an scm system to do.
I've searched on 'absolute' in the list archive to see if there
was a past discussion like "we've decided we need absolute paths
everywhere because X" but I didn't find any. Can someone
describe what the reasoning was? A security concern perhaps?
(And one more serious than the race condition built into
make_absolute_path?)
Or, perhaps I should be asking, what is there in git that will
break if I recompile it with make_absolute_path(p){return p;}?
Does it store absolute paths in the db? Would a recompiled
version produce a db other gits couldn't read?
(Or less drastic perhaps, what if there were a version of m_a_p
that still returned an absolute path when possible and safe, and
just returned p otherwise so the program could still be usable?)
Thanks,
Chapman Flack
mathematics, purdue university
^ permalink raw reply
* Re: Delete a commit
From: Andreas Schwab @ 2009-12-21 18:17 UTC (permalink / raw)
To: Johan 't Hart; +Cc: git
In-Reply-To: <hgocfi$pge$1@ger.gmane.org>
Johan 't Hart <johanthart@gmail.com> writes:
> Bertram Scharpf schreef:
>> Hi,
>
>> % git fsck --lost-found
>> dangling commit 6abc221327e896c850c56dafae92277bcfe68e2b
>>
>> It is still there. This is the one I want to delete.
>
> It is not in the history of any head anymore, so you could consider it
> deleted. ('git log' does not show this commit)
Except through the reflog, most likely.
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
* Re: Delete a commit
From: Johan 't Hart @ 2009-12-21 17:49 UTC (permalink / raw)
To: git
In-Reply-To: <20091220004340.GA30440@marge.bs.l>
Bertram Scharpf schreef:
> Hi,
> % git fsck --lost-found
> dangling commit 6abc221327e896c850c56dafae92277bcfe68e2b
>
> It is still there. This is the one I want to delete.
It is not in the history of any head anymore, so you could consider it
deleted. ('git log' does not show this commit)
If you want to prune unreferenced objects, try:
git prune
('git help prune' for options)
> Bertram
>
>
^ permalink raw reply
* Re: [PATCH] Introduce the GIT_HOME environment variable
From: Michael J Gruber @ 2009-12-21 16:54 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Jeff King, Junio C Hamano, Miklos Vajna, Moe, git
In-Reply-To: <vpqskb49tuq.fsf@bauges.imag.fr>
Matthieu Moy venit, vidit, dixit 21.12.2009 17:26:
> Jeff King <peff@peff.net> writes:
>
>> Are we even close to having this sort of universal support for
>> ~/.config?
>
> Definitely not universal as of now. Probably precisely because each
> application thinks "I'll take care of $XDG_CONFIG_HOME after others
> do" ;-).
>
>> Traditionally, the standard for Unix
>> has been for config files to be $HOME/.something. You can argue that
>> ~/.config is a better standard, but I don't think git is failing to
>> use a standard; it is simply following a different one.
>
> I've probably been unclear about this. I'm not arguing about moving
> away from $HOME/.gitconfig as the default (IOW, we're in agreement
> here ;-) ). It's there, and the migration path would be much more
> painfull than the benefit.
>
> What I'm saying is that _if_ we introduce a variable to point to an
> alternate .gitconfig, then we should use something like
> $XDG_CONFIG_HOME/git/config and not $GIT_HOME/.gitconfig
>
> I don't have a strong opinion on whether we should introduce such
> variable (it seems the only use-case is the one which started this
> thread, and it is already solved without it, so ...).
>
>> But we do have such a variable: $HOME. The concept of $GIT_HOME was
>> proposed to provide a way to divert _just_ git to a different config
>> directory, something that would not be any easier with
>> $XDG_CONFIG_HOME.
>
> Right, but I don't see any use-case for this.
>
> The use-case which started this thread was to have several physical
> users using the same Unix account, with the desire that each physical
> user should be able to use his own editor setups. In this case, you
> want your editor and your other applications to follow the schema.
>
>> Anyway, as far as the future of git goes, even if we did want to switch
>> to $XDG_CONFIG_HOME, we could not do so suddenly without breaking
>> everybody's current setup. Which would mean any implementation of it
>> would have to handle both the current and the new proposed locations.
>> You can obviously just read from both, but there are a lot of open
>> questions, like "which should take precedence?" and "what does git
>> config --global --edit do?". I am not opposed to hearing a clever
>> proposal that handles all such issues, but I am not going to think too
>> hard about it myself. :)
>
> Right, the thing I had in mind was to use $XDG_CONFIG_HOME just like
> $GIT_HOME (i.e. use it if it's set), but doing so would suddenly break
> the setup of people having already set $XDG_CONFIG_HOME, and having a
> $HOME/.gitconfig.
>
> Well, then, I don't know, maybe my proposal wasn't as clever as I
> thought ;-).
Well, I'd say the usual approach would be "use the first one found out
of $XYZ/config and $HOME/.gitconfig in this order", whether XYZ equals
$GIT_HOME or $XDG_CONFIG_HOME/git or what not. And that applies both to
reading as well writing the config. We should only merge config from
different types of sources (system/global/local), not from alternate
locations within the same type.
That way, nobody's setup gets broken, and having "git_custom_home()"
factorized out there is no real maintenance burden. I have no opinion
about the choice of XYZ.
Michael
^ permalink raw reply
* Re: [PATCH] Introduce the GIT_HOME environment variable
From: Matthieu Moy @ 2009-12-21 16:26 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Miklos Vajna, Moe, git
In-Reply-To: <20091221155902.GA22665@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Are we even close to having this sort of universal support for
> ~/.config?
Definitely not universal as of now. Probably precisely because each
application thinks "I'll take care of $XDG_CONFIG_HOME after others
do" ;-).
> Traditionally, the standard for Unix
> has been for config files to be $HOME/.something. You can argue that
> ~/.config is a better standard, but I don't think git is failing to
> use a standard; it is simply following a different one.
I've probably been unclear about this. I'm not arguing about moving
away from $HOME/.gitconfig as the default (IOW, we're in agreement
here ;-) ). It's there, and the migration path would be much more
painfull than the benefit.
What I'm saying is that _if_ we introduce a variable to point to an
alternate .gitconfig, then we should use something like
$XDG_CONFIG_HOME/git/config and not $GIT_HOME/.gitconfig
I don't have a strong opinion on whether we should introduce such
variable (it seems the only use-case is the one which started this
thread, and it is already solved without it, so ...).
> But we do have such a variable: $HOME. The concept of $GIT_HOME was
> proposed to provide a way to divert _just_ git to a different config
> directory, something that would not be any easier with
> $XDG_CONFIG_HOME.
Right, but I don't see any use-case for this.
The use-case which started this thread was to have several physical
users using the same Unix account, with the desire that each physical
user should be able to use his own editor setups. In this case, you
want your editor and your other applications to follow the schema.
> Anyway, as far as the future of git goes, even if we did want to switch
> to $XDG_CONFIG_HOME, we could not do so suddenly without breaking
> everybody's current setup. Which would mean any implementation of it
> would have to handle both the current and the new proposed locations.
> You can obviously just read from both, but there are a lot of open
> questions, like "which should take precedence?" and "what does git
> config --global --edit do?". I am not opposed to hearing a clever
> proposal that handles all such issues, but I am not going to think too
> hard about it myself. :)
Right, the thing I had in mind was to use $XDG_CONFIG_HOME just like
$GIT_HOME (i.e. use it if it's set), but doing so would suddenly break
the setup of people having already set $XDG_CONFIG_HOME, and having a
$HOME/.gitconfig.
Well, then, I don't know, maybe my proposal wasn't as clever as I
thought ;-).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] Introduce the GIT_HOME environment variable
From: Jeff King @ 2009-12-21 15:59 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Junio C Hamano, Miklos Vajna, Moe, git
In-Reply-To: <vpqhbrkd3o6.fsf@bauges.imag.fr>
On Mon, Dec 21, 2009 at 11:25:45AM +0100, Matthieu Moy wrote:
> You may not care about consistancy between applications, but I do.
> Currently, to version-control my user's configuration, I have
> $HOME/etc containing my user's config files, and the actual config
> files are symlinks to it. If applications were agreeing on a directory
> where configuration files would be stored (is it is the case on
> systems like MS Windows, and I think Mac OS), I would just had done
> "cd this-config-directory; git init".
Are we even close to having this sort of universal support for
~/.config? I also keep my dot-files in a git repository. I don't have a
single one in ~/.config[1], but I do have ~/.profile, ~/.vimrc,
~/.netrc, ~/.gitconfig, and others[2]. Traditionally, the standard for
Unix has been for config files to be $HOME/.something. You can argue
that ~/.config is a better standard, but I don't think git is failing to
use a standard; it is simply following a different one.
[1] I'll grant that is probably because I am a curmudgeon, and spend 99%
of my computing time in xterm+bash+vim.
[2] Don't even get me started on ~/.mozilla/firefox/$RAND_HEX.default/user.js.
> With the proposed $GIT_HOME, I have a way to specify _Git_'s path to
> config files. Another application may propose $WHATEVER_ELSE_HOME, and
> yet another would say $HOME_YET_ANOTHER_ONE, and so on. There's a
> proposal to have a single environment variable for all this, why
> reject it?
But we do have such a variable: $HOME. The concept of $GIT_HOME was
proposed to provide a way to divert _just_ git to a different config
directory, something that would not be any easier with $XDG_CONFIG_HOME.
Anyway, as far as the future of git goes, even if we did want to switch
to $XDG_CONFIG_HOME, we could not do so suddenly without breaking
everybody's current setup. Which would mean any implementation of it
would have to handle both the current and the new proposed locations.
You can obviously just read from both, but there are a lot of open
questions, like "which should take precedence?" and "what does git
config --global --edit do?". I am not opposed to hearing a clever
proposal that handles all such issues, but I am not going to think too
hard about it myself. :)
-Peff
^ permalink raw reply
* Re: [RFC PATCH] Record a single transaction for conflicting push operations
From: Gustav Hållberg @ 2009-12-21 14:31 UTC (permalink / raw)
To: Karl Wiberg; +Cc: Catalin Marinas, git
In-Reply-To: <b8197bcb0912210548q67c1da4bhe023bed2811394d4@mail.gmail.com>
On 2009-12-21 14:48, Karl Wiberg wrote:
> I've seen more than one complaint that the current behavior is
> confusing even if we don't count the bug, so I thought this was part
> of the motivation.
I don't know if this would be better than the other suggested solutions,
but if "stg log" would clearly identify multi-stage entries as such, the
current confusion would probably mostly go away.
Currently this is done reasonably well for make_temp_patch(), which says
"refresh (create temporary patch)" in the log, but I think this could be
taken further.
For example, if such annotations said "foo: stage N" or similar,
indicating that this was the Nth step in the "foo" command (think
"rebase" or whatever), it would be good enough for me least.
- Gustav
^ permalink raw reply
* Re: [RFC PATCH] Record a single transaction for conflicting push operations
From: Karl Wiberg @ 2009-12-21 13:48 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git, Gustav Hållberg
In-Reply-To: <b0943d9e0912210348o37b71935x5fad4f1a4be4b70@mail.gmail.com>
On Mon, Dec 21, 2009 at 12:48 PM, Catalin Marinas
<catalin.marinas@gmail.com> wrote:
> 2009/12/21 Karl Wiberg <kha@treskal.com>:
>
>> By the way, you do realize there's another command that requires
>> two steps to undo completely: refresh? And that one is harder to
>> get out of---undoing it all in one step would mean throwing away
>> the updates to the patch.
>
> But it looks to me like refresh does this by running separate
> transactions.
Yes. So it won't be affected by whatever you do here. (Unless you
consider that refresh -p needs to reorder patches, which can result in
conflicts---right now, refresh -p can result in three log entries.)
> The push command does this in a single transaction, so the quickest
> fix for the HEAD != top undo problem was to only record one log per
> transaction.
I've seen more than one complaint that the current behavior is
confusing even if we don't count the bug, so I thought this was part
of the motivation.
> If we keep the current behaviour with two logs per transaction, we
> need to preserve the HEAD prior to the conflict so that logging
> doesn't get the wrong HEAD (which is the new conflicting HEAD
> currently). The patch below appears to fix this problem and still
> generate two logs per transaction. While I'm more in favour of a
> single log per transaction, if people find it useful I'm happy to
> keep the current behaviour.
I haven't seen anyone but me defent the current design, and it's not a
big deal for me either, so I'd say go with just one transaction.
--
Karl Wiberg, kha@treskal.com
subrabbit.wordpress.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [RFC PATCH] Record a single transaction for conflicting push operations
From: Catalin Marinas @ 2009-12-21 11:48 UTC (permalink / raw)
To: Karl Wiberg; +Cc: git, Gustav Hållberg
In-Reply-To: <b8197bcb0912202308p296207av416cd5590a11251b@mail.gmail.com>
2009/12/21 Karl Wiberg <kha@treskal.com>:
> By the way, you do realize there's another command that requires two
> steps to undo completely: refresh? And that one is harder to get out
> of---undoing it all in one step would mean throwing away the updates
> to the patch.
But it looks to me like refresh does this by running separate
transactions. The push command does this in a single transaction, so
the quickest fix for the HEAD != top undo problem was to only record
one log per transaction.
If we keep the current behaviour with two logs per transaction, we
need to preserve the HEAD prior to the conflict so that logging
doesn't get the wrong HEAD (which is the new conflicting HEAD
currently). The patch below appears to fix this problem and still
generate two logs per transaction. While I'm more in favour of a
single log per transaction, if people find it useful I'm happy to keep
the current behaviour.
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 30a153b..ba97c4f 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -197,18 +197,14 @@ class StackTransaction(object):
exception) and do nothing."""
self.__check_consistency()
log.log_external_mods(self.__stack)
- new_head = self.head
-
- # Set branch head.
- if set_head:
- if iw:
- try:
- self.__checkout(new_head.data.tree, iw, allow_bad_head)
- except git.CheckoutException:
- # We have to abort the transaction.
- self.abort(iw)
- self.__abort()
- self.__stack.set_head(new_head, self.__msg)
+
+ if set_head and iw:
+ try:
+ self.__checkout(self.head.data.tree, iw, allow_bad_head)
+ except git.CheckoutException:
+ # We have to abort the transaction.
+ self.abort(iw)
+ self.__abort()
if self.__error:
if self.__conflicts:
@@ -216,8 +212,11 @@ class StackTransaction(object):
else:
out.error(self.__error)
- # Write patches.
- def write(msg):
+ # Write patches and update the branch head.
+ def write(msg, new_head):
+ # Set branch head.
+ if new_head:
+ self.__stack.set_head(new_head, self.__msg)
for pn, commit in self.__patches.iteritems():
if self.__stack.patches.exists(pn):
p = self.__stack.patches.get(pn)
@@ -231,12 +230,16 @@ class StackTransaction(object):
self.__stack.patchorder.unapplied = self.__unapplied
self.__stack.patchorder.hidden = self.__hidden
log.log_entry(self.__stack, msg)
+
old_applied = self.__stack.patchorder.applied
- write(self.__msg)
if self.__conflicting_push != None:
+ write(self.__msg, set_head and self.head)
self.__patches = _TransPatchMap(self.__stack)
self.__conflicting_push()
- write(self.__msg + ' (CONFLICT)')
+ write(self.__msg + ' (CONFLICT)', set_head and self.head)
+ else:
+ write(self.__msg, set_head and self.head)
+
if print_current_patch:
_print_current_patch(old_applied, self.__applied)
@@ -346,10 +349,10 @@ class StackTransaction(object):
if merge_conflict:
# When we produce a conflict, we'll run the update()
# function defined below _after_ having done the
- # checkout in run(). To make sure that we check out
- # the real stack top (as it will look after update()
- # has been run), set it hard here.
- self.head = comm
+ # checkout in run(). Make sure that we have a consistent
+ # HEAD before the update function is called below (which
+ # sets the real HEAD).
+ self.head = self.top
else:
comm = None
s = 'unmodified'
@@ -367,6 +370,8 @@ class StackTransaction(object):
x = self.unapplied
del x[x.index(pn)]
self.applied.append(pn)
+ # Set the real conflicting HEAD.
+ self.head = comm
if merge_conflict:
# We've just caused conflicts, so we must allow them in
# the final checkout.
diff --git a/t/t3103-undo-hard.sh b/t/t3103-undo-hard.sh
index 2d0f382..a71cd32 100755
--- a/t/t3103-undo-hard.sh
+++ b/t/t3103-undo-hard.sh
@@ -46,7 +46,7 @@ test_expect_success 'Try to undo without --hard' '
cat > expected.txt <<EOF
EOF
-test_expect_failure 'Try to undo with --hard' '
+test_expect_success 'Try to undo with --hard' '
stg undo --hard &&
stg status a > actual.txt &&
test_cmp expected.txt actual.txt &&
--
Catalin
^ permalink raw reply related
* Re: [PATCH 2/5] git-svn: memoize conversion of SVN merge ticket info to git commit ranges
From: Eric Wong @ 2009-12-21 10:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Andrew Myrick, Sam Vilain
In-Reply-To: <1261344246.20752.24.camel@denix>
Sam Vilain <sam@vilain.net> wrote:
> On Sun, 2009-12-20 at 05:33 +1300, Sam Vilain wrote:
> > +sub lookup_svn_merge {
> > + my $uuid = shift;
> > + my $url = shift;
> > + my $merge = shift;
> > +
> > + my ($source, $revs) = split ":", $merge;
> > + my $path = $source;
> > + $path =~ s{^/}{};
> > + my $gs = Git::SVN->find_by_url($url.$source, $url, $path);
> > + if ( !$gs ) {
> > + warn "Couldn't find revmap for $url$source\n";
> > + next;
> > + }
>
> As mentioned in the other thread, that 'next' should now be 'return'.
Thanks Sam and Andrew. I've acked this series and pushed them out along
with a release notes update as well as another small fix (inline below).
Eric Wong (2):
git svn: fix --revision when fetching deleted paths
update release notes for git svn in 1.6.6
Sam Vilain (5):
git-svn: expand the svn mergeinfo test suite, highlighting some failures
git-svn: memoize conversion of SVN merge ticket info to git commit ranges
git-svn: fix some mistakes with interpreting SVN mergeinfo commit ranges
git-svn: exclude already merged tips using one rev-list call
git-svn: detect cherry-picks correctly.
>From 577e9fcad2c8968846b365226b89778050496a78 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 21 Dec 2009 02:06:04 -0800
Subject: [PATCH] git svn: fix --revision when fetching deleted paths
When using the -r/--revision argument to fetch deleted history,
calling SVN::Ra::get_log() from an SVN::Ra object initialized
to track the deleted URL will fail.
This regression was introduced in:
commit 4aacaeb3dc82bb6479e70e120053dc27a399460e
"fix shallow clone when upstream revision is too new"
We now ignore errors from SVN::Ra::get_log() here because using
--revision will always override the value of $head here if
(and only if) we're tracking deleted directories.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
git-svn.perl | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index d362de7..a6f5061 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1741,7 +1741,11 @@ sub fetch_all {
my $ra = Git::SVN::Ra->new($url);
my $uuid = $ra->get_uuid;
my $head = $ra->get_latest_revnum;
- $ra->get_log("", $head, 0, 1, 0, 1, sub { $head = $_[1] });
+
+ # ignore errors, $head revision may not even exist anymore
+ eval { $ra->get_log("", $head, 0, 1, 0, 1, sub { $head = $_[1] }) };
+ warn "W: $@\n" if $@;
+
my $base = defined $fetch ? $head : 0;
# read the max revs for wildcard expansion (branches/*, tags/*)
--
Eric Wong
^ permalink raw reply related
* Re: [PATCH] Introduce the GIT_HOME environment variable
From: Matthieu Moy @ 2009-12-21 10:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, Moe, git
In-Reply-To: <7vskb6bwvu.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> http://standards.freedesktop.org/basedir-spec/basedir-spec-0.6.html
>>
>> It solves the same problem ("set on environment variable, and change
>> my whole Git config"), but
>>
>> * It's a standard. It's really nice to be able to ...
>> * It avoids hidden files. With $GIT_CONFIG, a user doing
>
> I think the above are actually three bullet points (i.e. you lack line
> break and bullet before "It's really nice").
No, I don't.
You can do
| cd ~/.config
| ls
|
| to see a user's configuration for many applications at a time,
_because_ it's a standard, and because it's followed by several
applications.
> And the third bullet is more or less a small subset of the second
> one, since you need "ls -a" without making them non-dot,
The standard may not write black-on-white
$XDG_CONFIG_HOME/subdir/filename _with filename being non-hidden_, but
in practice, this is what's happening.
> And I personally don't care very much about that second "It's really
> nice to be able to" point.
You may not care about consistancy between applications, but I do.
Currently, to version-control my user's configuration, I have
$HOME/etc containing my user's config files, and the actual config
files are symlinks to it. If applications were agreeing on a directory
where configuration files would be stored (is it is the case on
systems like MS Windows, and I think Mac OS), I would just had done
"cd this-config-directory; git init".
With the proposed $GIT_HOME, I have a way to specify _Git_'s path to
config files. Another application may propose $WHATEVER_ELSE_HOME, and
yet another would say $HOME_YET_ANOTHER_ONE, and so on. There's a
proposal to have a single environment variable for all this, why
reject it?
> As to the particular "standard" cited, I don't know how relevant it is to
> us at this moment, or in this topic. Judging from the fact that it
> doesn't even define the scope of the standard (e.g. what classes of
> applications are expected to follow it, for what benefit do they follow
> it, how are they expected to handle differences between their historical
> practice and the new world order it introduces, etc. etc....), I suspect
> it is a very early draft that will be heavily copyedited before final,
> once professional standard writers start looking at it.
I mostly agree on the critics, but do you have any better "standard"
(actually, not necessarily an official standard, but "something that
various applications can agree on") to propose?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: possible code error at run_command.c
From: Frank Li @ 2009-12-21 9:13 UTC (permalink / raw)
To: Johannes Sixt; +Cc: kusmabite, git
In-Reply-To: <4B2F3732.6030903@viscovery.net>
> This line will trigger the check. It initializes failed_errno with itself,
> which is still uninitialized at this time.
>
I see.
I always use release version at finial production.
This is not important. I read code again. There are not path.
Thank you take care.
^ permalink raw reply
* Re: possible code error at run_command.c
From: Erik Faye-Lund @ 2009-12-21 9:08 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Frank Li, git
In-Reply-To: <4B2F3732.6030903@viscovery.net>
On Mon, Dec 21, 2009 at 9:52 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Erik Faye-Lund schrieb:
>> On Mon, Dec 21, 2009 at 9:30 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Frank Li schrieb:
>>>> Maybe some excute path miss initialized it. Otherwise compiler will
>>>> not report warning.
>>> LOOK AT THE CODE. There is no such code path.
>>>
>>
>> That's odd.
>
> Only if Frank did the homework and removed the initialization from
>
> int failed_errno = failed_errno;
>
>> I agree, there isn't such a code path. But this is the
>> first time I've ever seen this MSVC-feature turn up false positives,
>> which puzzles me.
>
> This line will trigger the check. It initializes failed_errno with itself,
> which is still uninitialized at this time.
>
> Note that we have more definitions of this kind:
>
> $ git grep -E ' ([a-zA-Z_][a-zA-Z_0-9]*) = \1[;,]' *.c
> builtin-rev-list.c: int reaches = reaches, all = all;
> match-trees.c: unsigned mode1 = mode1;
> match-trees.c: unsigned mode2 = mode2;
> run-command.c: int failed_errno = failed_errno;
> transport.c: int cmp = cmp, len;
> wt-status.c: int status = status;
>
> -- Hannes
>
>
Right, I didn't think of that. Since that is the case, I'd say
disabling run-time checks is perfectly sane.
That being said, it might make sense for MSVC people to be able to
have this feature turned on so they can easier find REAL
programmer-errors. But I guess removing the self-assignments makes
trouble for non-MSVC people.
Perhaps some of these warnings could be avoided in a "safer" way? IMO,
assigning variables to themselves like this is a bit fishy, as it
effectively hides uninitialized-use warnings for the entire lifetime
of that variable. Sure, the programmer who did that probably knows
what he or she does -- but they can't possibly know what people will
do in the future. If we could somehow turn these (really few)
occasions into something that produces reliable warnings on
uninitialized variables, I think we might be able to catch some bugs
earlier.
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: possible code error at run_command.c
From: Johannes Sixt @ 2009-12-21 8:52 UTC (permalink / raw)
To: kusmabite; +Cc: Frank Li, git
In-Reply-To: <40aa078e0912210041g1fd94c77g6cf9f1b236b6ecd7@mail.gmail.com>
Erik Faye-Lund schrieb:
> On Mon, Dec 21, 2009 at 9:30 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Frank Li schrieb:
>>> Maybe some excute path miss initialized it. Otherwise compiler will
>>> not report warning.
>> LOOK AT THE CODE. There is no such code path.
>>
>
> That's odd.
Only if Frank did the homework and removed the initialization from
int failed_errno = failed_errno;
> I agree, there isn't such a code path. But this is the
> first time I've ever seen this MSVC-feature turn up false positives,
> which puzzles me.
This line will trigger the check. It initializes failed_errno with itself,
which is still uninitialized at this time.
Note that we have more definitions of this kind:
$ git grep -E ' ([a-zA-Z_][a-zA-Z_0-9]*) = \1[;,]' *.c
builtin-rev-list.c: int reaches = reaches, all = all;
match-trees.c: unsigned mode1 = mode1;
match-trees.c: unsigned mode2 = mode2;
run-command.c: int failed_errno = failed_errno;
transport.c: int cmp = cmp, len;
wt-status.c: int status = status;
-- Hannes
^ permalink raw reply
* Re: possible code error at run_command.c
From: Erik Faye-Lund @ 2009-12-21 8:41 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Frank Li, git
In-Reply-To: <4B2F320A.6010908@viscovery.net>
On Mon, Dec 21, 2009 at 9:30 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Frank Li schrieb:
>> Maybe some excute path miss initialized it. Otherwise compiler will
>> not report warning.
>
> LOOK AT THE CODE. There is no such code path.
>
That's odd. I agree, there isn't such a code path. But this is the
first time I've ever seen this MSVC-feature turn up false positives,
which puzzles me.
Perhaps Frank has had to modify start_command for MSVC, and somehow
introduced such an error? I dunno.
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: possible code error at run_command.c
From: Johannes Sixt @ 2009-12-21 8:30 UTC (permalink / raw)
To: Frank Li; +Cc: git
In-Reply-To: <1976ea660912210018y15acfe32o78841d5e0968f793@mail.gmail.com>
Frank Li schrieb:
> Maybe some excute path miss initialized it. Otherwise compiler will
> not report warning.
LOOK AT THE CODE. There is no such code path.
-- Hannes
^ permalink raw reply
* Re: possible code error at run_command.c
From: Frank Li @ 2009-12-21 8:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4B2F2727.4060405@viscovery.net>
>
> Disable this check - it just takes away performance. :-)
Release version will disable this.
Debug version enable this by default is just for find out code error.
>
> (If you don't disable the check, then keep the required changes private.)
>
>> I prefer change it to
>> int failed_errno = errno;
>
> You don't need to initialize the variable at all because it is always
> initialized elsewhere before it is used. Perhaps MSVC is clever enough to
> see it?
>
Maybe some excute path miss initialized it. Otherwise compiler will
not report warning.
^ permalink raw reply
* Re: [PATCH] rebase -i: print the editor name if it fails to launch
From: Björn Gustavsson @ 2009-12-21 7:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4onlbi3q.fsf@alter.siamese.dyndns.org>
2009/12/20 Junio C Hamano <gitster@pobox.com>:
> Isn't this too elaborate? git_editor() has already run and when it
> attempted to launch the editor it assigned to GIT_EDITOR in order to use
> it as an eval string.
I wanted to hide the details about the inner working of git_editor(),
but since it is only called in one place, yes, it is too elaborate.
>
> git_editor "$TODO" ||
> die_abort "Failed to run '${GIT_EDITOR:-your editor}'"
>
> would suffice, no?
Yes. Much better. I will rewrite my patch when I'll get home from
work later today.
--
Björn Gustavsson, Erlang/OTP, Ericsson AB
^ permalink raw reply
* Re: possible code error at run_command.c
From: Johannes Sixt @ 2009-12-21 7:43 UTC (permalink / raw)
To: Frank Li; +Cc: git
In-Reply-To: <1976ea660912202329x42f6add3j9175867e8723a4bd@mail.gmail.com>
Frank Li schrieb:
>> This is a commonly used idiom to avoid an (incorrect) compiler warning
>> about an uninitialized variable.
>>
>> Strictly speaking, I think that you are right by saying "means nothing"
>> because the use of the uninitialized variable invokes undefined behavior
>> (and for this reason, I dislike this construct), but in practice it will
>> not make a difference.
>>
>
> This error is captured at MSVC environment by run time varible check.
Disable this check - it just takes away performance. :-)
(If you don't disable the check, then keep the required changes private.)
> I prefer change it to
> int failed_errno = errno;
You don't need to initialize the variable at all because it is always
initialized elsewhere before it is used. Perhaps MSVC is clever enough to
see it?
-- Hannes
^ permalink raw reply
* Re: possible code error at run_command.c
From: Frank Li @ 2009-12-21 7:29 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4B2F214D.2020905@viscovery.net>
> This is a commonly used idiom to avoid an (incorrect) compiler warning
> about an uninitialized variable.
>
> Strictly speaking, I think that you are right by saying "means nothing"
> because the use of the uninitialized variable invokes undefined behavior
> (and for this reason, I dislike this construct), but in practice it will
> not make a difference.
>
This error is captured at MSVC environment by run time varible check.
I prefer change it to
int failed_errno = errno;
^ permalink raw reply
* Re: possible code error at run_command.c
From: Wincent Colaiuta @ 2009-12-21 7:09 UTC (permalink / raw)
To: Frank Li; +Cc: git
In-Reply-To: <1976ea660912202246k45732bf2p111bbeb78047693e@mail.gmail.com>
El 21/12/2009, a las 07:46, Frank Li escribió:
> int start_command(struct child_process *cmd)
> {
> int need_in, need_out, need_err;
> int fdin[2], fdout[2], fderr[2];
> int failed_errno = failed_errno;
>
> I have not found failed_errno as global variable.
> failed_errno = failed_errno means nothing.
>
> It is possible coding error and should be
> int failed_errno= 0 or
> failed_errno=errno.
Via "git blame":
commit 5a7a3671b74c043216549b94a718da04cc3ffcd6
Author: David Soria Parra <dsp@php.net>
Date: Tue Aug 4 11:28:40 2009 +0200
run-command.c: squelch a "use before assignment" warning
i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5490)
compiler
(and probably others) mistakenly thinks variable failed_errno is
used
before assigned. Work it around by giving it a fake
initialization.
Signed-off-by: David Soria Parra <dsp@php.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Cheers,
Wincent
^ permalink raw reply
* Re: possible code error at run_command.c
From: Johannes Sixt @ 2009-12-21 7:18 UTC (permalink / raw)
To: Frank Li; +Cc: git
In-Reply-To: <1976ea660912202246k45732bf2p111bbeb78047693e@mail.gmail.com>
Frank Li schrieb:
> int start_command(struct child_process *cmd)
> {
> int need_in, need_out, need_err;
> int fdin[2], fdout[2], fderr[2];
> int failed_errno = failed_errno;
>
> I have not found failed_errno as global variable.
> failed_errno = failed_errno means nothing.
This is a commonly used idiom to avoid an (incorrect) compiler warning
about an uninitialized variable.
Strictly speaking, I think that you are right by saying "means nothing"
because the use of the uninitialized variable invokes undefined behavior
(and for this reason, I dislike this construct), but in practice it will
not make a difference.
-- Hannes
^ permalink raw reply
* Re: [RFC PATCH] Record a single transaction for conflicting push operations
From: Karl Wiberg @ 2009-12-21 7:08 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git, Gustav Hållberg
In-Reply-To: <b0943d9e0912201521k73bdcb5fl333e845028954050@mail.gmail.com>
On Mon, Dec 21, 2009 at 12:21 AM, Catalin Marinas
<catalin.marinas@gmail.com> wrote:
> 2009/12/19 Karl Wiberg <kha@treskal.com>:
>
>> Better. But couldn't you remove the update function completely and
>> just inline the code in it, since it's called immediately?
>
> Of course, I tried, but couldn't get it to work. I get HEAD and top
> not equal unless I call update() between _TransPatchMap and
> self.__halt(). For the non-conflicting case we need to call update
> before or after this "if merge_conflict".
>
> One solution is to split the "if merge_conflict" in two but maybe
> you have a better idea.
Yes, duplicating the conditional was what I had in mind. But if you
don't find it to improve the readability of the code (as compared to
having a function), I certainly won't insist.
Thanks for working on this.
By the way, you do realize there's another command that requires two
steps to undo completely: refresh? And that one is harder to get out
of---undoing it all in one step would mean throwing away the updates
to the patch.
--
Karl Wiberg, kha@treskal.com
subrabbit.wordpress.com
www.treskal.com/kalle
^ permalink raw reply
* possible code error at run_command.c
From: Frank Li @ 2009-12-21 6:46 UTC (permalink / raw)
To: git
int start_command(struct child_process *cmd)
{
int need_in, need_out, need_err;
int fdin[2], fdout[2], fderr[2];
int failed_errno = failed_errno;
I have not found failed_errno as global variable.
failed_errno = failed_errno means nothing.
It is possible coding error and should be
int failed_errno= 0 or
failed_errno=errno.
^ 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