* [PATCH v2 14/51] repack_without_ref(): remove temporary
From: mhagger @ 2011-12-12 5:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1323668338-1764-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index ba7a8b0..2e7bc0c 100644
--- a/refs.c
+++ b/refs.c
@@ -1278,12 +1278,10 @@ static struct lock_file packlock;
static int repack_without_ref(const char *refname)
{
struct ref_array *packed;
- struct ref_entry *ref;
int fd, i;
packed = get_packed_refs(get_ref_cache(NULL));
- ref = search_ref_array(packed, refname);
- if (ref == NULL)
+ if (search_ref_array(packed, refname) == NULL)
return 0;
fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
if (fd < 0) {
@@ -1294,8 +1292,7 @@ static int repack_without_ref(const char *refname)
for (i = 0; i < packed->nr; i++) {
char line[PATH_MAX + 100];
int len;
-
- ref = packed->refs[i];
+ struct ref_entry *ref = packed->refs[i];
if (!strcmp(refname, ref->name))
continue;
--
1.7.8
^ permalink raw reply related
* Re: Breakage (?) in configure and git_vsnprintf()
From: Jeff King @ 2011-12-12 6:43 UTC (permalink / raw)
To: Michael Haggerty
Cc: Junio C Hamano, git discussion list, Michal Rokos, Brandon Casey
In-Reply-To: <4EE4F97B.9000202@alum.mit.edu>
On Sun, Dec 11, 2011 at 07:42:03PM +0100, Michael Haggerty wrote:
> 2. The configure problem causes git_vsnprintf() to be wrapped around the
> C library version. This leads to many failures in the test suite. I
> suppose that git_vsnprintf() is broken in some way.
I enabled SNPRINTF_RETURNS_BOGUS manually and was able to see the test
suite failures. Very oddly, I could get them while running the full
suite in parallel, but when I ran individual scripts, the problem went
away. Which makes no sense to me at all.
However, I peeked at the git_vsnprintf function, and one obvious error
is that it calls vsnprintf multiple times on the same va_list.
Fixing that (patch below) makes the test failures go away. I think it's
an Obviously Correct thing to do, anyway, but I'm slightly unnerved by
not understanding why it sometimes caused failures and sometimes not.
Clearly the existing code invokes nasal daemons and anything is allowed
to happen, but it would be nice to understand what triggers the
difference.
I'll leave the issue of "-std=c89" triggering SNPRINTF_RETURNS_BOGUS to
people who know and care about autoconf. My gut is to say "don't do
that". Git is not actually pure c89. We typically target systems that
are _at least_ c89, but it's more important to match and run well on
real-world systems than what was defined in the standard. So we don't
depend on c99, but we do depend on quirks and features that were
prominent in mid-90's Unix variants.
-- >8 --
Subject: [PATCH] compat/snprintf: don't look at va_list twice
If you define SNPRINTF_RETURNS_BOGUS, we use a special
git_vsnprintf wrapper assumes that vsnprintf returns "-1"
instead of the number of characters that you would need to
store the result.
To do this, it invokes vsnprintf multiple times, growing a
heap buffer until we have enough space to hold the result.
However, this means we evaluate the va_list parameter
multiple times, which is generally a bad thing (it may be
modified by calls to vsnprintf, yielding undefined
behavior).
Instead, we must va_copy it and hand the copy to vsnprintf,
so we always have a pristine va_list.
Signed-off-by: Jeff King <peff@peff.net>
---
compat/snprintf.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/compat/snprintf.c b/compat/snprintf.c
index e1e0e75..38fc08d 100644
--- a/compat/snprintf.c
+++ b/compat/snprintf.c
@@ -19,11 +19,13 @@
#undef vsnprintf
int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
{
+ va_list cp;
char *s;
int ret = -1;
if (maxsize > 0) {
- ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
+ va_copy(cp, ap);
+ ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, cp);
if (ret == maxsize-1)
ret = -1;
/* Windows does not NUL-terminate if result fills buffer */
@@ -42,7 +44,8 @@ int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
if (! str)
break;
s = str;
- ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
+ va_copy(cp, ap);
+ ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, cp);
if (ret == maxsize-1)
ret = -1;
}
--
1.7.8.13.g74677
^ permalink raw reply related
* Re: best way to fastforward all tracking branches after a fetch
From: Stefan Haller @ 2011-12-12 7:33 UTC (permalink / raw)
To: Hallvard B Furuseth; +Cc: Gelonida N, git
In-Reply-To: <hbf.20111211x512@bombur.uio.no>
Hallvard B Furuseth <h.b.furuseth@usit.uio.no> wrote:
> Stefan Haller writes:
> >Gelonida N <gelonida@gmail.com> wrote:
> >
> >> What is the best way to fastforward all fastforwardable tracking
> >> branches after a git fetch?
> >
> > Here's a script that does this. It isn't very well tested, I hope I
> > didn't miss any edge cases. Use at your own risk.
>
> Local branches can track each other. So the script needs to toposort
> the branches, or to loop until either nothing was done or an error
> happened. (The latter to prevent an eternal loop on error.)
Is this just theoretical, or are there real use cases for this? What
would be a workflow with such a local tracking branch?
For me personally, the script is good enough, because I only ever have
branches that track an 'origin' branch with the same name.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
^ permalink raw reply
* [PATCH 0/5] mixed bag of minor "git mv" fixes
From: Jeff King @ 2011-12-12 7:45 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
In-Reply-To: <877h226bxe.fsf@picasso.cante.net>
On Sun, Dec 11, 2011 at 11:22:37PM +0200, Jari Aalto wrote:
> Every time I do:
>
> git mv -f FROM TO
>
> Git displays:
>
> warning: destination exists; will overwrite!
>
> Please don't display anything other than errors (no write permission....).
>
> The "-f" is like with mv(1), cp(1); there is nothing than can be done
> afterwards, so the message is redundant and obstructing.
I'm inclined to agree. Outputting a warning just because we did what the
user asked us to is unnecessarily chatty.
When I looked into it, though, it seems that "git mv" is somewhat
neglected, and this trival one-line patch turned into a 5-patch series
of fixes.
[1/5]: docs: mention "-k" for both forms of "git mv"
[2/5]: mv: honor --verbose flag
[3/5]: mv: make non-directory destination error more clear
[4/5]: mv: improve overwrite warning
[5/5]: mv: be quiet about overwriting
-Peff
^ permalink raw reply
* Re: Breakage (?) in configure and git_vsnprintf()
From: Johannes Sixt @ 2011-12-12 7:45 UTC (permalink / raw)
To: Jeff King
Cc: Michael Haggerty, Junio C Hamano, git discussion list,
Michal Rokos, Brandon Casey
In-Reply-To: <20111212064305.GA16511@sigill.intra.peff.net>
Am 12/12/2011 7:43, schrieb Jeff King:
> I'll leave the issue of "-std=c89" triggering SNPRINTF_RETURNS_BOGUS to
> people who know and care about autoconf. My gut is to say "don't do
> that".
Right. But Michael's problem was actually that SNPRINTF_RETURNS_BOGUS was
set incorrectly; his system has a working snprintf (or so I assume). The
reason for the failure is that ./configure's test program produced a
warning, and that warning was turned into an error due to -Werror. Without
-Werror, the test program would have compiled successfully, and the
working snprintf would have been detected.
-- Hannes
^ permalink raw reply
* [PATCH 1/5] docs: mention "-k" for both forms of "git mv"
From: Jeff King @ 2011-12-12 7:50 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
In-Reply-To: <20111212074503.GB16511@sigill.intra.peff.net>
The "git mv" synopsis shows two forms: renaming a file, and
moving files into a directory. They can both make use of the
"-k" flag to ignore errors, so mention it in both places.
Signed-off-by: Jeff King <peff@peff.net>
---
I can kind of see the rationale for the original content. Using "-k" is
a lot more useful if you are actually doing multiple renames, so it
makes more sense in the second form. But it is still useful in the first
form as a shorthand for "git mv 2>/dev/null || true".
I actually would rather just see:
git mv [options] <source> <destination>
git mv [options] <source>... <destination>
but if we are going to go that route, we should probably decide on a
style and convert all of the descriptions at the same time.
Documentation/git-mv.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt
index b8db373..4be7a71 100644
--- a/Documentation/git-mv.txt
+++ b/Documentation/git-mv.txt
@@ -15,7 +15,7 @@ DESCRIPTION
-----------
This script is used to move or rename a file, directory or symlink.
- git mv [-f] [-n] <source> <destination>
+ git mv [-f] [-n] [-k] <source> <destination>
git mv [-f] [-n] [-k] <source> ... <destination directory>
In the first form, it renames <source>, which must exist and be either
--
1.7.8.13.g74677
^ permalink raw reply related
* [PATCH 2/5] mv: honor --verbose flag
From: Jeff King @ 2011-12-12 7:51 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
In-Reply-To: <20111212074503.GB16511@sigill.intra.peff.net>
The code for a verbose flag has been here since "git mv" was
converted to C many years ago, but actually getting the "-v"
flag from the command line was accidentally lost in the
transition.
Signed-off-by: Jeff King <peff@peff.net>
---
This has been broken since 2006, so I guess nobody really cares. But
it's simple to fix.
Documentation/git-mv.txt | 8 ++++++--
builtin/mv.c | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt
index 4be7a71..e3c8448 100644
--- a/Documentation/git-mv.txt
+++ b/Documentation/git-mv.txt
@@ -15,8 +15,8 @@ DESCRIPTION
-----------
This script is used to move or rename a file, directory or symlink.
- git mv [-f] [-n] [-k] <source> <destination>
- git mv [-f] [-n] [-k] <source> ... <destination directory>
+ git mv [-v] [-f] [-n] [-k] <source> <destination>
+ git mv [-v] [-f] [-n] [-k] <source> ... <destination directory>
In the first form, it renames <source>, which must exist and be either
a file, symlink or directory, to <destination>.
@@ -40,6 +40,10 @@ OPTIONS
--dry-run::
Do nothing; only show what would happen
+-v::
+--verbose::
+ Report the names of files as they are moved.
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/builtin/mv.c b/builtin/mv.c
index 5efe6c5..11abaf5 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -59,6 +59,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
int i, newfd;
int verbose = 0, show_only = 0, force = 0, ignore_errors = 0;
struct option builtin_mv_options[] = {
+ OPT__VERBOSE(&verbose, "be verbose"),
OPT__DRY_RUN(&show_only, "dry run"),
OPT__FORCE(&force, "force move/rename even if target exists"),
OPT_BOOLEAN('k', NULL, &ignore_errors, "skip move/rename errors"),
--
1.7.8.13.g74677
^ permalink raw reply related
* [PATCH 3/5] mv: make non-directory destination error more clear
From: Jeff King @ 2011-12-12 7:51 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
In-Reply-To: <20111212074503.GB16511@sigill.intra.peff.net>
If you try to "git mv" multiple files onto another
non-directory file, you confusingly get the "usage" message:
$ touch one two three
$ git add .
$ git mv one two three
usage: git mv [options] <source>... <destination>
[...]
>From the user's perspective, that makes no sense. They just
gave parameters that exactly match that usage!
This behavior dates back to the original C version of "git
mv", which had a usage message like:
usage: git mv (<source> <destination> | <source>... <destination>)
This was slightly less confusing, because it at least
mentions that there are two ways to invoke (but it still
isn't clear why what the user provided doesn't work).
Instead, let's show an error message like:
$ git mv one two three
fatal: destination 'three' is not a directory
We could leave the usage message in place, too, but it
doesn't actually help here. It contains no hints that there
are two forms, nor that multi-file form requires that the
endpoint be a directory. So it just becomes useless noise
that distracts from the real error.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/mv.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index 11abaf5..ae6c30c 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -94,7 +94,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
destination = copy_pathspec(dest_path[0], argv, argc, 1);
} else {
if (argc != 1)
- usage_with_options(builtin_mv_usage, builtin_mv_options);
+ die("destination '%s' is not a directory", dest_path[0]);
destination = dest_path;
}
--
1.7.8.13.g74677
^ permalink raw reply related
* [PATCH 4/5] mv: improve overwrite warning
From: Jeff King @ 2011-12-12 7:52 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
In-Reply-To: <20111212074503.GB16511@sigill.intra.peff.net>
When we try to "git mv" over an existing file, the error
message is fairly informative:
$ git mv one two
fatal: destination exists, source=one, destination=two
When the user forces the overwrite, we give a warning:
$ git mv -f one two
warning: destination exists; will overwrite!
This is less informative, but still sufficient in the simple
rename case, as there is only one rename happening.
But when moving files from one directory to another, it
becomes useless:
$ mkdir three
$ touch one two three/one
$ git add .
$ git mv one two three
fatal: destination exists, source=one, destination=three/one
$ git mv -f one two three
warning: destination exists; will overwrite!
The first message is helpful, but the second one gives us no
clue about what was overwritten. Instead, let's mirror the
first form more closely, with:
$ git mv -f one two three
warning: destination exists (will overwrite), source=one, destination=three/one
Signed-off-by: Jeff King <peff@peff.net>
---
This message looks overly long to me, but I wanted to match the existing
messages. Another option would be just:
warning: overwriting 'three/one'
builtin/mv.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index ae6c30c..c9ecb03 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -177,7 +177,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
* check both source and destination
*/
if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
- warning(_("%s; will overwrite!"), bad);
+ warning(_("%s (will overwrite), source=%s, destination=%s"),
+ bad, src, dst);
bad = NULL;
} else
bad = _("Cannot overwrite");
--
1.7.8.13.g74677
^ permalink raw reply related
* [PATCH 5/5] mv: be quiet about overwriting
From: Jeff King @ 2011-12-12 7:54 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
In-Reply-To: <20111212074503.GB16511@sigill.intra.peff.net>
When a user asks us to force a mv and overwrite the
destination, we print a warning. However, since a typical
use would be:
$ git mv one two
fatal: destination exists, source=one, destination=two
$ git mv -f one two
warning: destination exists (will overwrite), source=one, destination=two
this warning is just noise. We already know we're
overwriting; that's why we gave -f!
This patch silences the warning unless "--verbose" is given.
Signed-off-by: Jeff King <peff@peff.net>
---
You could perhaps argue that it is useful in the case of moving multiple
files into a directory (since it tells you _which_ files were
overwritten). We could turn the warning on in that case, but I'm
inclined to leave it. If the user cares about this information, they can
use "-v" along with "-f".
builtin/mv.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index c9ecb03..b6e7e4f 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -177,8 +177,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
* check both source and destination
*/
if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
- warning(_("%s (will overwrite), source=%s, destination=%s"),
- bad, src, dst);
+ if (verbose)
+ warning(_("%s (will overwrite), source=%s, destination=%s"),
+ bad, src, dst);
bad = NULL;
} else
bad = _("Cannot overwrite");
--
1.7.8.13.g74677
^ permalink raw reply related
* Re: Breakage (?) in configure and git_vsnprintf()
From: Jeff King @ 2011-12-12 8:10 UTC (permalink / raw)
To: Johannes Sixt
Cc: Michael Haggerty, Junio C Hamano, git discussion list,
Michal Rokos, Brandon Casey
In-Reply-To: <4EE5B123.2030708@viscovery.net>
On Mon, Dec 12, 2011 at 08:45:39AM +0100, Johannes Sixt wrote:
> Am 12/12/2011 7:43, schrieb Jeff King:
> > I'll leave the issue of "-std=c89" triggering SNPRINTF_RETURNS_BOGUS to
> > people who know and care about autoconf. My gut is to say "don't do
> > that".
>
> Right. But Michael's problem was actually that SNPRINTF_RETURNS_BOGUS was
> set incorrectly; his system has a working snprintf (or so I assume). The
> reason for the failure is that ./configure's test program produced a
> warning, and that warning was turned into an error due to -Werror. Without
> -Werror, the test program would have compiled successfully, and the
> working snprintf would have been detected.
Right, I understand that. But he has given a set of options that
shouldn't compile git at all (he tells the compiler not to use snprintf
via -std=c89, but we require that it exists, because even our
git_vsnprintf wrapper uses the underlying system vsnprintf).
So yes, the configure script is broken to detect the situation as
SNPRINTF_RETURNS_BOGUS and not "this platform doesn't have snprintf at
all"[1]. But I'm saying that the "we do not have snprintf at all" case
is not all that interesting: git needs it. So I'm not sure compiling
with -std=c89 really makes sense[2].
If somebody wants to make the configure script more accurate, I
certainly don't want to stop them. I'm just not sure it is worth
anybody's time in this case.
-Peff
[1] Yes, obviously we do actually have it, but it is somewhat a fluke
that it works. We tell the compiler during the compile phase that we
don't have it, but then during the link phase it is magically
available in libc.
[2] I can convince git to compile on recent Linux with gcc using
CFLAGS='-std=c89 -Dinline='. Turning on "-Wall -Werror" doesn't
work because all of the inline functions appear to be unused
statics. But if I understand Michael's problem correctly, wouldn't
we be missing the prototype for snprintf, which could cause subtle
errors?
^ permalink raw reply
* Re: [PATCH 1/3] Convert resolve_ref+xstrdup to new resolve_refdup function
From: Junio C Hamano @ 2011-12-12 8:13 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Jonathan Nieder, git, Tony Wang
In-Reply-To: <CACsJy8C0CJyHdtWJ5QVqX9ksHWgdBpm6XekQ+mZP4sxBVA_8vQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> Yeah that slipped in. It should be part of c689332 (Convert many
> resolve_ref() calls to read_ref*() and ref_exists() - 2011-11-13). I
> guess either I missed it or it was a new call site after that patch.
> Split it out as a separate patch?
Yeah, I think it makes sense to split the unrelated part out and place it
early in the series. It seems that you will be updating patch 2 in the
series for __FILE__ anyway so it's not like adding a useless code churn to
do so.
Thanks.
^ permalink raw reply
* Re: [RFC/PATCH 0/7] some sequencer loose ends (Re: Fix revert --abort on Windows)
From: Junio C Hamano @ 2011-12-12 8:15 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Johannes Sixt, Ramkumar Ramachandra, git, Christian Couder,
Martin von Zweigbergk, Jay Soffian
In-Reply-To: <20111211195836.GA25482@elie.hsd1.il.comcast.net>
Jonathan Nieder <jrnieder@gmail.com> writes:
> How did we ever let this in? "git reset" already has well defined
> semantics that have nothing to do with this. Patches 6/7 and 7/7
> would help us forget this UI mistake (and I believe it was a mistake)
> ever happened.
Thanks for catching this. Let's review this quickly and queue it for
maintenance track as necessary.
^ permalink raw reply
* Re: Breakage (?) in configure and git_vsnprintf()
From: Junio C Hamano @ 2011-12-12 8:23 UTC (permalink / raw)
To: Jeff King
Cc: Michael Haggerty, Junio C Hamano, git discussion list,
Michal Rokos, Brandon Casey
In-Reply-To: <20111212064305.GA16511@sigill.intra.peff.net>
Good analysis; thanks.
Back when c4582f9 (Add compat/snprintf.c for systems that return bogus,
2008-03-05) was done we didn't have va_copy emulation available in
git-compat-util.h but now we do, so I agree that this is an Obviously
Correct thing to do.
^ permalink raw reply
* Re: [PATCH v2 00/51] ref-api-C and ref-api-D re-roll
From: Junio C Hamano @ 2011-12-12 8:24 UTC (permalink / raw)
To: mhagger
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <1323668338-1764-1-git-send-email-mhagger@alum.mit.edu>
Thanks for a re-roll. Will take a look early this week.
^ permalink raw reply
* Re: best way to fastforward all tracking branches after a fetch
From: Jeff King @ 2011-12-12 8:25 UTC (permalink / raw)
To: Stefan Haller; +Cc: Hallvard B Furuseth, Gelonida N, git
In-Reply-To: <1kc5m38.m71ik21ytxkhbM%lists@haller-berlin.de>
On Mon, Dec 12, 2011 at 08:33:15AM +0100, Stefan Haller wrote:
> > Local branches can track each other. So the script needs to toposort
> > the branches, or to loop until either nothing was done or an error
> > happened. (The latter to prevent an eternal loop on error.)
>
> Is this just theoretical, or are there real use cases for this? What
> would be a workflow with such a local tracking branch?
I use this all the time.
In git.git, we use a topic branch workflow (i.e., every feature gets its
own topic branch, and topics graduate independently to master as they
are deemed stable). And we use a patch-submission workflow, which means
it's OK for me to rebase my topics locally, because the end-product is a
series of patches sent to the list.
Typically I branch off of "origin/master", so the topic is independent
of anything else. For example, the "jk/credentials" branch in my git
repo is branched from "origin/master" (Junio's master). But sometimes
there is a topic that depends on another topic, but should not be part
of the same series (because the the first topic can graduate to master,
but the second one may still need more time for discussion and cooking).
In that case, I'll set the upstream to the other local topic branch. An
example of this is the "jk/prompt" series, which depends on
"jk/credentials" for infrastructure, but is really a separate issue.
Having the upstream set is convenient, because I can get _just_ the
commits in jk/prompt with "git log @{u}..". Or I can rebase _just_ the
commits in that topic with "git rebase -i". If my upstream were set to
origin, I would accidentally also rebase all of the commits pulled in
from jk/credentials, too.
While my topics are still in development (i.e., before they have even
hit "next"), I tend to rebase them aggressively (so that I keep them up
to date with git development), using a script that is something like[1]:
for i in `topics`; do
git rebase $i@{u} $i
done
And I do topo-sort my topics for exactly the reason mentioned.
-Peff
[1] https://github.com/peff/git/blob/meta/rebase
^ permalink raw reply
* Re: [PATCH 0/4] git-p4: paths for p4
From: Junio C Hamano @ 2011-12-12 5:14 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: git, Gary Gibbons
In-Reply-To: <1323474497-14339-1-git-send-email-pw@padd.com>
Thanks; will queue directly to 'master'.
^ permalink raw reply
* Re: [bug?] checkout -m doesn't work without a base version
From: Junio C Hamano @ 2011-12-12 5:29 UTC (permalink / raw)
To: Pete Harlan; +Cc: git
In-Reply-To: <4EE55D5E.1090908@pcharlan.com>
Thanks, will queue.
^ permalink raw reply
* Re: [PATCH] Update documentation for stripspace
From: Junio C Hamano @ 2011-12-12 6:41 UTC (permalink / raw)
To: Conrad Irwin; +Cc: git
In-Reply-To: <1323655158-5075-1-git-send-email-conrad.irwin@gmail.com>
Conrad Irwin <conrad.irwin@gmail.com> writes:
> Tell the user what this command is intended for, and expand the
> description of what it does.
Thanks.
> Stop referring to the input as <stream>, as this command reads the
> entire input into memory before processing it.
Which can change to stream, but calling it as input would not invalidate
the new wording, so "input" is fine. From the caller's point of view, the
current implementation (or streaming implementation) can read from an
unseekable input stream (i.e. pipe), so the original wording is equally
valid, by the way.
So in that sense, it does not make any difference either way to me (it is
not even worth rerolling this patch to only remove this part of the
change).
> Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>
> ---
> Documentation/git-stripspace.txt | 26 ++++++++++++++++++++------
> builtin/stripspace.c | 2 +-
> 2 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt
> index b78f031..6667d25 100644
> --- a/Documentation/git-stripspace.txt
> +++ b/Documentation/git-stripspace.txt
> @@ -3,26 +3,40 @@ git-stripspace(1)
>
> NAME
> ----
> -git-stripspace - Filter out empty lines
> +git-stripspace - Remove unnecessary whitespace
>
>
> SYNOPSIS
> --------
> [verse]
> -'git stripspace' [-s | --strip-comments] < <stream>
> +'git stripspace' [-s | --strip-comments] < input
>
> DESCRIPTION
> -----------
> -Remove multiple empty lines, and empty lines at beginning and end.
> +
> +Normalizes input in the manner used by 'git' for user-provided metadata such
> +as commit messages, notes, tags and branch descriptions.
The original says "remove" and new one says "normalize*s*". I think we
tend to say things in imperative mood (i.e. without the trailing "s").
I do not think 'user-provided metadata' is a good wording. This is just a
simple text clean-up filter and you can use it to clean your text files
that you mean to store in the repository as well.
> +When run with no arguments this:
> +
> +- removes trailing whitespace from all lines
> +- collapses multiple consecutive empty lines into one empty line
> +- removes blank lines from the beginning and end of the input
> +- ensures the last line ends with exactly one '\n'.
Thanks for a nicely written bulleted list. It clarifies what the command
does quite a bit.
The last one is a bit funny, though.
By definition, you cannot end the last line with more than one '\n' (upon
seeing the second '\n', you would realize immediately that the line you
saw was _not_ the last line). I think you meant the file does not end with
an incomplete line, i.e. "ensures the output does not end with an
incomplete line by adding '\n' at the end if needed".
> +In the case where the input consists entirely of whitespace characters, no
> +output will be produced.
> +
> +*NOTE*: This is intended for cleaning metadata, prefer the `--whitespace=fix`
> +mode of linkgit:git-apply[1] for correcting whitespace of patches or files in
> +the repository.
I can tell that these three lines were the _primary_ thing you wanted to
add with this patch, having never seen anybody got confused between the
whitespace breakage fix and text cleaning, I wonder if this is adding
clarity or giving users an impression that git can do too many things than
they can wrap their mind around and forcing them to wonder if they have to
learn everything git can do for them.
> OPTIONS
> -------
> -s::
> --strip-comments::
> - In addition to empty lines, also strip lines starting with '#'.
> + Also remove all lines starting with '#'.
With the resulting text (with the rules clarified with your above 4-bullet
points) of this manual page, can a user tell what the command does to this
input (I added line numbers, vertical bars and dollar signs to show where
the beginning and the end of lines are):
1 | $
2 |a b c$
3 |$
4 |# comment line$
5 |$
6 |d e f$
The original text at least allows the user to guess correctly, as it hints
that a comment line is treated pretty much like an empty line, and the
"consecutive empty lines are squashed into one" in your bulleted list
would mean that ll 3-5 will become a single blank line.
The new text however gives a wrong hint by saying "Also"; it can be read
as if all the rules in the bullted list are applied first to leave blank
lines at 3 and 5 and then comment line is removed from the result, which
would leave two blank lines in the result.
If I were touching this description, I probably would say something like
"Treat lines starting with a '#' as if they are empty lines".
^ permalink raw reply
* Re: [RFC/PATCH] show tracking branches with their associated branch names
From: Junio C Hamano @ 2011-12-12 7:14 UTC (permalink / raw)
To: Santhosh Kumar Mani; +Cc: Vincent van Ravesteijn, git
In-Reply-To: <1323516226.1698.80.camel@sdesktop>
Santhosh Kumar Mani <santhoshmani@gmail.com> writes:
> Of course, I could get this information in different ways. But it makes
> sense to have this information displayed by default.
It makes sense to make the various information available, but it does not
mean it makes sense to add it by default for everybody at all. Given that
against all common sense, many newbie web-tips and third-party documents
suggest "git branch | sed -ne 's/^\* //p'" as a way to find the current
branch in scripts, I am sure such a change will cause trouble to many
while only helping a few.
I wouldn't mind a new option --verbose-format=... that takes various
formatting letters similar to how --pretty=format:... does, though.
^ permalink raw reply
* Re: best way to fastforward all tracking branches after a fetch
From: Junio C Hamano @ 2011-12-12 8:09 UTC (permalink / raw)
To: Gelonida N; +Cc: git
In-Reply-To: <jbvj5o$skt$1@dough.gmane.org>
Gelonida N <gelonida@gmail.com> writes:
> What is the best way to fastforward all fastforwardable tracking
> branches after a git fetch?
This lacks context and invites too many tangents, so I'll only touch a few
of them.
First of all, why do you want to do this?
You have many local branches that are forked from remote tracking branches
and can be fast-forwarded to their counterparts, iow, these local branches
are often behind their upstream and you do not have your own development
in this repository. Because you can by definition have only one branch
checked out in your working tree, after a fetch from your origin, they
will further fall behind their counterparts.
Coming from that background, I can see you may want these branches that
are not checked out fast-forwarded to their remote counterparts to keep
them stay current, but I first question that background. Why have these
local branches to begin with, if they always are supposed to match their
remote counterpart?
One possible reason (this is one tangent) is that you want to build and
install tips of many branches fetched from the upstream without doing any
local development in this repository (the "upstream" could be your primary
repository and the changes fed to this repository may be your own
development, so this is different from saying that you as a person is only
following other people's work. It is just that nothing is done to the
history in THIS repository). It could be solved by directly checking out
the remote tracking branches into detached head state, e.g.
$ for branch in maint master next
do
git checkout origin/$branch &&
make prefix=$HOME/git-$branch all test install || break
done
and the reason why you want local branches instead may be because your
build infrastructure (i.e. instead of "make" you have a custom script,
just like I use 'Make' script in my 'todo' branch) the does customization
depending on the name of the current branch, and might be more cumbersome
to get the same information for a detached head state (i.e. "the tip of
which remote tracking branch is the current commit?") than asking "git
symbolic-ref" the name of the current branch. But then it is easy to find
out which remote branch was checked out from the reflog for the HEAD (and
it is easier for your script that builds the origin/$branch to use that
information internally when the script calls your 'Make' equivalent). In
any case, it is largely your build customization's problem if this is the
case.
Another tangent. Perhaps the reason why you want these local branches but
they can often be fast-forwarded is because your workflow looks like this:
(1) you fork a topic from origin/master;
(2) you develop a bit;
(3) you push the topic back to origin/master;
(4) time passes, others push to origin/master, while you work on other
branches of yours;
(5) from time to time, you fetch from origin;
(6) you decide to continue working on the topic, so you check it out,
and before continuing, you wish it is already up-to-date.
But then after fast-forwarding the topic in (6), your topic's history
contains commits other than those you made to work toward the goal of your
topic, namely, other commits made by others during (4) for random purposes
that do not have anything to do with achieving the goal of the topic of
yours. Your branch is no longer about what you wanted to accomplish on
your topic. This invites two tangents.
One is a question. If you knew that the topic is not cooked fully and
needs further work after step (6), why did you push it back to the
origin/master in the first place at step (3), contaminating the history
everybody else bases their further work on with the contents of your
"half-done" topic?
Another tangent. Perhaps the fork is not made from origin/master but you
are collaboratively working on the same topic with others, and you handed
off the work up to what you have done at step (3), and others continued to
further the goal of the shared topic during (4). If that is the case,
wouldn't it make more sense to delete the topic after you push it back,
and forking at the point when you actually decide to get back into action?
Yet another. Even if you keep the (stale) topic branch that you already
have pushed out to the remote, because you can work on one topic at a time
in a single working tree anyway, perhaps it makes more sense to delay this
fast-forwarding until you actually check out the topic branch? After all,
your wishing to fast-forward "all branches" imply you have many of them,
and it wouldn't be far-fetched for me to imagine that you will check one
of them out a lot less often than you run "git fetch".
In other words, wouldn't a post-checkout hook be a better place to do
this kind of thing, perhaps like this (completely untested)?
#!/bin/sh
old=$1 new=$2 kind=$3
# did we checkout a branch?
test "$kind" = 1 || exit 0
# what did we check out?
branch=$(git symbolic-ref HEAD 2>/dev/null) || exit 0
# does it track anything? otherwise nothing needs to be done
upstream=$(git for-each-ref --format='%(upstream)' "$branch")
test -z "$upstream" || exit 0
# are we up-to-date? if so no need to do anything
test 0 = $(git rev-list "..$upstream" | wc -l) && exit 0
# do we have something we made? if so no point trying to fast-forward
test 0 = $(git rev-list "$upstream.." | wc -l) || exit 0
# attempt a fast-forward merge with it
git merge --ff-only @{upstream}
That is, of course, assuming that it makes sense to keep these local
branches in the first place.
^ permalink raw reply
* Re: best way to fastforward all tracking branches after a fetch
From: Junio C Hamano @ 2011-12-12 8:28 UTC (permalink / raw)
To: Hallvard B Furuseth; +Cc: Stefan Haller, Gelonida N, git
In-Reply-To: <hbf.20111211x512@bombur.uio.no>
Hallvard B Furuseth <h.b.furuseth@usit.uio.no> writes:
> Local branches can track each other. So the script needs to toposort
> the branches, or to loop until either nothing was done or an error
> happened. (The latter to prevent an eternal loop on error.)
If you have branches A that forked from B that in turn forked from C, and
if after updating C you want to and can successfully update both B and A
by fast-forwarding, that would only mean that neither A nor B had their
own change since they were forked from their upstream, regardless of local
or remote.
What use do these empty branches have in the first place?
^ permalink raw reply
* Re: [PATCH v2 08/51] is_dup_ref(): extract function from sort_ref_array()
From: Jeff King @ 2011-12-12 8:33 UTC (permalink / raw)
To: mhagger
Cc: Junio C Hamano, git, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <1323668338-1764-9-git-send-email-mhagger@alum.mit.edu>
On Mon, Dec 12, 2011 at 06:38:15AM +0100, mhagger@alum.mit.edu wrote:
> +/*
> + * Emit a warning and return true iff ref1 and ref2 have the same name
> + * and the same sha1. Die if they have the same name but different
> + * sha1s.
> + */
> +static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2)
> +{
> + if (!strcmp(ref1->name, ref2->name)) {
> + /* Duplicate name; make sure that the SHA1s match: */
> + if (hashcmp(ref1->sha1, ref2->sha1))
> + die("Duplicated ref, and SHA1s don't match: %s",
> + ref1->name);
> + warning("Duplicated ref: %s", ref1->name);
> + return 1;
> + } else {
> + return 0;
> + }
> +}
As a user, I'm not sure what this message means. If I understand the
context right, this happens when you have duplicate entries in your
packed-refs file?
This would indicate a bug in git, so should this perhaps say "BUG:" in
front, or maybe give some more context so that a user understands it is
not their error, or even a random error on this run, but that git has
accidentally corrupted the packed-refs file (and their best bet is
probably to report the bug to us).
This is obviously not an issue introduced by your patch, as you are
just moving these error messages around. But maybe while the topic is
fresh in your mind it is a good time to improve it. I dunno. AFAICT
nobody has ever actually hit this message, so maybe it doesn't matter.
:)
-Peff
^ permalink raw reply
* Re: Question about commit message wrapping
From: Frans Klaver @ 2011-12-12 8:41 UTC (permalink / raw)
To: Andrew Ardill; +Cc: Jakub Narebski, Sidney San Martín, git
In-Reply-To: <CAH5451kGn72tLAwdvQFBjSyHSL0rUmaPZrbL7Z-KfHWN-HAuCQ@mail.gmail.com>
On Sun, Dec 11, 2011 at 11:00 PM, Andrew Ardill <andrew.ardill@gmail.com> wrote:
>> Additional and the more serious problem with wrapping on output is
>> related to backward compatibility. If you have commit message that is
>> wrapped e.g. to 80 characters, and you wrap on output to 72 characters,
>> you would get ugly and nigh unreadable ragged output
>
> For what it's worth, I do a lot of reading emails on my phone, which
> force wraps line-length to the width of the display (not a set number
> of characters).
> This is always less than 80.
Good point.
>
> Emails on this list are almost exclusively sent pre-wrapped to 80
> character line lengths.
> The result is exactly the kind of ragged output you used in your
> example. Changing this behaviour may break backwards compatibility,
> but it is already broken for 'future' compatibility.
I am starting to think that we need to somehow keep the current
behavior, but override at smaller widths. Maybe even use format=flowed
in format-patch. On the other hand, the fundamental use with git is to
communicate code, and I'm not sure how that [cw]ould be handled.
^ permalink raw reply
* Re: best way to fastforward all tracking branches after a fetch
From: Stefan Haller @ 2011-12-12 9:19 UTC (permalink / raw)
To: Jeff King; +Cc: Hallvard B Furuseth, Gelonida N, git
In-Reply-To: <20111212082526.GC16511@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Mon, Dec 12, 2011 at 08:33:15AM +0100, Stefan Haller wrote:
>
> > > Local branches can track each other. So the script needs to toposort
> > > the branches, or to loop until either nothing was done or an error
> > > happened. (The latter to prevent an eternal loop on error.)
> >
> > Is this just theoretical, or are there real use cases for this? What
> > would be a workflow with such a local tracking branch?
>
> I use this all the time.
>
> In git.git, we use a topic branch workflow (i.e., every feature gets its
> own topic branch, and topics graduate independently to master as they
> are deemed stable). And we use a patch-submission workflow, which means
> it's OK for me to rebase my topics locally, because the end-product is a
> series of patches sent to the list.
>
> Typically I branch off of "origin/master", so the topic is independent
> of anything else. For example, the "jk/credentials" branch in my git
> repo is branched from "origin/master" (Junio's master). But sometimes
> there is a topic that depends on another topic, but should not be part
> of the same series (because the the first topic can graduate to master,
> but the second one may still need more time for discussion and cooking).
> In that case, I'll set the upstream to the other local topic branch. An
> example of this is the "jk/prompt" series, which depends on
> "jk/credentials" for infrastructure, but is really a separate issue.
>
> Having the upstream set is convenient, because I can get _just_ the
> commits in jk/prompt with "git log @{u}..". Or I can rebase _just_ the
> commits in that topic with "git rebase -i". If my upstream were set to
> origin, I would accidentally also rebase all of the commits pulled in
> from jk/credentials, too.
I see, thanks. For my script, I'm wondering then if the most sensible
thing to do is to just skip any branch whose upstream doesn't start with
refs/remotes/.
For a future "git pull --all" feature, it would probably only work on
those branches whose upstream is on the remote being pulled from,
anyway.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
^ 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