* [PATCH 4/4] Bomb out when --ack and --sign are both passed to "refresh".
From: Yann Dirson @ 2006-11-23 23:17 UTC (permalink / raw)
To: Catalin Marinas; +Cc: GIT list
In-Reply-To: <20061123230721.9769.38403.stgit@gandelf.nowhere.earth>
Old behaviour was silently ignoring --ack, which could be confusing.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
stgit/commands/refresh.py | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py
index ea0fe6f..aa5ff78 100644
--- a/stgit/commands/refresh.py
+++ b/stgit/commands/refresh.py
@@ -107,6 +107,8 @@ def func(parser, options, args):
if options.sign:
sign_str = 'Signed-off-by'
+ if options.ack:
+ raise CmdException, '--ack and --sign were both specified'
elif options.ack:
sign_str = 'Acked-by'
^ permalink raw reply related
* [PATCH 1/4] Make Series::patch_applied public.
From: Yann Dirson @ 2006-11-23 23:16 UTC (permalink / raw)
To: Catalin Marinas; +Cc: GIT list
In-Reply-To: <20061123230721.9769.38403.stgit@gandelf.nowhere.earth>
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
stgit/stack.py | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/stgit/stack.py b/stgit/stack.py
index a477e7d..49cfebb 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -417,7 +417,7 @@ class Series:
def __patch_is_current(self, patch):
return patch.get_name() == read_string(self.__current_file)
- def __patch_applied(self, name):
+ def patch_applied(self, name):
"""Return true if the patch exists in the applied list
"""
return name in self.get_applied()
@@ -430,7 +430,7 @@ class Series:
def patch_exists(self, name):
"""Return true if there is a patch with the given name, false
otherwise."""
- return self.__patch_applied(name) or self.__patch_unapplied(name)
+ return self.patch_applied(name) or self.__patch_unapplied(name)
def __begin_stack_check(self):
"""Save the current HEAD into .git/refs/heads/base if the stack
@@ -713,7 +713,7 @@ class Series:
before_existing = False, refresh = True):
"""Creates a new patch
"""
- if self.__patch_applied(name) or self.__patch_unapplied(name):
+ if self.patch_applied(name) or self.__patch_unapplied(name):
raise StackException, 'Patch "%s" already exists' % name
if not message and can_edit:
@@ -773,7 +773,7 @@ class Series:
if self.__patch_is_current(patch):
self.pop_patch(name)
- elif self.__patch_applied(name):
+ elif self.patch_applied(name):
raise StackException, 'Cannot remove an applied patch, "%s", ' \
'which is not current' % name
^ permalink raw reply related
* [PATCH 2/4] Allows to refresh a non-top applied patch.
From: Yann Dirson @ 2006-11-23 23:16 UTC (permalink / raw)
To: Catalin Marinas; +Cc: GIT list
In-Reply-To: <20061123230721.9769.38403.stgit@gandelf.nowhere.earth>
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
stgit/commands/refresh.py | 25 ++++++++++++++++++++++---
1 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py
index 610d18a..ea0fe6f 100644
--- a/stgit/commands/refresh.py
+++ b/stgit/commands/refresh.py
@@ -66,6 +66,8 @@ options = [make_option('-f', '--force',
make_option('--commemail',
help = 'use COMMEMAIL as the committer ' \
'e-mail'),
+ make_option('-p', '--patch',
+ help = 'refresh named (applied) PATCH instead of the top one'),
make_option('--sign',
help = 'add Signed-off-by line',
action = 'store_true'),
@@ -80,9 +82,15 @@ def func(parser, options, args):
if autoresolved != 'yes':
check_conflicts()
- patch = crt_series.get_current()
- if not patch:
- raise CmdException, 'No patches applied'
+ if options.patch:
+ patch = options.patch
+ if not crt_series.patch_applied(patch):
+ raise CmdException, 'Patches "%s" not applied' % patch
+ origpatch = crt_series.get_current()
+ else:
+ patch = crt_series.get_current()
+ if not patch:
+ raise CmdException, 'No patches applied'
if not options.force:
check_head_top_equal()
@@ -110,6 +118,13 @@ def func(parser, options, args):
or options.authname or options.authemail or options.authdate \
or options.commname or options.commemail \
or options.sign or options.ack:
+
+ if options.patch:
+ applied = crt_series.get_applied()
+ between = applied[applied.index(patch)+1:]
+ between.reverse()
+ pop_patches(between)
+
print 'Refreshing patch "%s"...' % patch,
sys.stdout.flush()
@@ -126,6 +141,10 @@ def func(parser, options, args):
committer_email = options.commemail,
backup = True, sign_str = sign_str)
+ if options.patch:
+ between.reverse()
+ push_patches(between)
+
print 'done'
else:
^ permalink raw reply related
* [PATCH 0/4] Series short description
From: Yann Dirson @ 2006-11-23 23:16 UTC (permalink / raw)
To: Catalin Marinas; +Cc: GIT list
The following adds a --patch flag to "stg refresh". I mainly needed it to
edit the log message in patches down the stack, but with some work it could
be used to record changes in another file and come back.
That would require to pop patches with --keep, and if we ask to only record
a subset of the changes, we would need to implement "push --keep" first.
This patch is not perfect, since it reverses a list twice, but I did not
find an elegant way to make it better. Python gurus, feel free to improve
this :)
While I was at it, I noticed that "stg goto" need not revert the applied list
if we're going to an unapplied patch (sorta compensate my extraneous revert above ;),
and I noticed a DWIM behaviour on the (should-be)-erroneous "refresh --ack --sign".
--
Yann Dirson <ydirson@altern.org> |
Debian-related: <dirson@debian.org> | Support Debian GNU/Linux:
| Freedom, Power, Stability, Gratis
^ permalink raw reply
* Re: [PATCH] Make git-clone --use-separate-remote the default
From: Junio C Hamano @ 2006-11-23 23:12 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20061123225835.30071.99265.stgit@machine.or.cz>
Petr Baudis <pasky@suse.cz> writes:
> and --use-immingled-remote can be used to get the original behaviour;
> it is also implied by --bare.
What's immingled?
> We get confused, frustrated and data-losing users *daily* on #git now
> because git-clone still produces the crippled repositories having the
> remote and local heads freely mixed together.
>
> Signed-off-by: Petr Baudis <pasky@suse.cz>
Being strongly opinionated, not giving enough credit for the
evolutionary process behind the history and venting frustration
in the proposed commit log message is never a good strategy to
get the patch applied.
Even though I fully agree that use-separate-remotes should be
the default, to the point that I do not think we do not even
need a backward compatibility option. People who want to use
traditional layout for simple one-remote-branch-only project
would not suffer anyway because 'origin' still means origin in
the new layout (refs/remotes/origin/HEAD).
We would need to update the tutorials to match this,though. I
think it talks about the traditional layout and say 'See, now
you can run "ls .git/refs/heads/{master,origin}"' or something
like that.
^ permalink raw reply
* Re: [PATCH] git-cvsimport: add suport for CVS pserver method HTTP/1.x proxying
From: Junio C Hamano @ 2006-11-23 23:02 UTC (permalink / raw)
To: Martin Langhoff (CatalystIT); +Cc: git
In-Reply-To: <45661C36.9010101@catalyst.net.nz>
"Martin Langhoff (CatalystIT)" <martin@catalyst.net.nz> writes:
> Junio C Hamano wrote:
>> Except that this statement made me go "huh?" wondering what it
>> would do to the $filehandle to evaluate <$filehandle> in a void
>> context:
>>
>> + # Skip the empty line of the proxy server output
>> + <$s>;
>
> It's a perl idiom that will discard one line of the $filehandle. If we
> are 200% certain that it is empty, then it's fine. OTOH, it may well
> be a bug in the particular proxy implementation Iñaki is using -- I
> don't know enough about CVS proxying to tell.
It is more about HTTP proxying and it is my understanding that
response to CONNECT method request has that empty line after the
successful (2xx) response line and zero or more response
headers. The code is still wrong; it does not have a loop to
discard the potential response headers that come before the
empty line the code we are discussing discards.
By the way does anybody know where this behaviour is officially
specified? Luotonen's draft-luotonen-web-proxy-tunneling-01.txt
is pretty clear about the empty line that comes after the
response headers, but that document being an internet-draft has
expired long time ago, but still seem to be quoted by others.
>> The "I/O Operators" section talks about evaluating <$s> in a
>> scalar context (i.e. "$rep = <$s>"), which we all know would
>> return a single line, and in list context, which swallows
>
> This is in scalar context, and that's safe to rely on.
If it were in scalar context I would agree fully. That
behaviour is documented. But my point is that it is in void
context, and I did not find document specifying what should
happen.
^ permalink raw reply
* [PATCH] Make git-clone --use-separate-remote the default
From: Petr Baudis @ 2006-11-23 22:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
and --use-immingled-remote can be used to get the original behaviour;
it is also implied by --bare.
We get confused, frustrated and data-losing users *daily* on #git now
because git-clone still produces the crippled repositories having the
remote and local heads freely mixed together.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
Documentation/git-clone.txt | 20 ++++++++++++++------
git-clone.sh | 14 +++++++-------
2 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 8606047..b1ad79f 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -11,7 +11,8 @@ SYNOPSIS
[verse]
'git-clone' [--template=<template_directory>] [-l [-s]] [-q] [-n] [--bare]
[-o <name>] [-u <upload-pack>] [--reference <repository>]
- [--use-separate-remote] <repository> [<directory>]
+ [--use-separate-remote | --use-immingled-remote] <repository>
+ [<directory>]
DESCRIPTION
-----------
@@ -71,9 +72,10 @@ OPTIONS
Make a 'bare' GIT repository. That is, instead of
creating `<directory>` and placing the administrative
files in `<directory>/.git`, make the `<directory>`
- itself the `$GIT_DIR`. This implies `-n` option. When
- this option is used, neither the `origin` branch nor the
- default `remotes/origin` file is created.
+ itself the `$GIT_DIR`. This implies the `-n` and
+ `--use-immingled-remote' option. When this option is used,
+ neither the `origin` branch nor the default `remotes/origin`
+ file is created.
--origin <name>::
-o <name>::
@@ -97,8 +99,14 @@ OPTIONS
--use-separate-remote::
Save remotes heads under `$GIT_DIR/remotes/origin/` instead
- of `$GIT_DIR/refs/heads/`. Only the master branch is saved
- in the latter.
+ of `$GIT_DIR/refs/heads/`. Only the local master branch is
+ saved in the latter. This is the default.
+
+--use-immingled-remote::
+ Save remotes heads in the same namespace as the local heads,
+ `$GIT_DIR/refs/heads/'. In regular repositories, this is
+ a legacy setup git-clone created by default in older Git
+ versions. It is also still implied by `--bare'.
<repository>::
The (possibly remote) repository to clone from. It can
diff --git a/git-clone.sh b/git-clone.sh
index 3f006d1..9ed4135 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -14,7 +14,7 @@ die() {
}
usage() {
- die "Usage: $0 [--template=<template_directory>] [--use-separate-remote] [--reference <reference-repo>] [--bare] [-l [-s]] [-q] [-u <upload-pack>] [--origin <name>] [-n] <repo> [<dir>]"
+ die "Usage: $0 [--template=<template_directory>] [--use-immingled-remote] [--reference <reference-repo>] [--bare] [-l [-s]] [-q] [-u <upload-pack>] [--origin <name>] [-n] <repo> [<dir>]"
}
get_repo_base() {
@@ -115,7 +115,7 @@ bare=
reference=
origin=
origin_override=
-use_separate_remote=
+use_separate_remote=t
while
case "$#,$1" in
0,*) break ;;
@@ -134,7 +134,10 @@ while
template="$1" ;;
*,-q|*,--quiet) quiet=-q ;;
*,--use-separate-remote)
+ # default
use_separate_remote=t ;;
+ *,--use-immingled-remote)
+ use_separate_remote= ;;
1,--reference) usage ;;
*,--reference)
shift; reference="$1" ;;
@@ -169,18 +172,15 @@ repo="$1"
test -n "$repo" ||
die 'you must specify a repository to clone.'
-# --bare implies --no-checkout
+# --bare implies --no-checkout and --use-immingled-remote
if test yes = "$bare"
then
if test yes = "$origin_override"
then
die '--bare and --origin $origin options are incompatible.'
fi
- if test t = "$use_separate_remote"
- then
- die '--bare and --use-separate-remote options are incompatible.'
- fi
no_checkout=yes
+ use_separate_remote=
fi
^ permalink raw reply related
* [PATCH 3/3] git-svn: preserve uncommitted changes after dcommit
From: Eric Wong @ 2006-11-23 22:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong
In-Reply-To: <1164322445180-git-send-email-normalperson@yhbt.net>
Using dcommit could cause the user to lose uncommitted changes
during the reset --hard operation, so change it to reset --mixed.
If dcommit chooses the rebase path, then git-rebase will already
error out when local changes are made.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
git-svn.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 6feae56..bb8935a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -623,7 +623,7 @@ sub dcommit {
} else {
print "No changes between current HEAD and $gs\n",
"Hard resetting to the latest $gs\n";
- @finish = qw/reset --hard/;
+ @finish = qw/reset --mixed/;
}
sys('git', @finish, $gs);
}
--
1.4.4.1.g22a08
^ permalink raw reply related
* [PATCH 1/3] git-svn: error out from dcommit on a parent-less commit
From: Eric Wong @ 2006-11-23 22:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong
dcommit would unconditionally append "~1" to a commit in order
to generate a diff. Now we generate a meaningful error message
if we try to generate an impossible diff.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
git-svn.perl | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 80b7b87..f0db4af 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -589,6 +589,13 @@ sub dcommit {
chomp(my @refs = safe_qx(qw/git-rev-list --no-merges/, "$gs..HEAD"));
my $last_rev;
foreach my $d (reverse @refs) {
+ if (quiet_run('git-rev-parse','--verify',"$d~1") != 0) {
+ die "Commit $d\n",
+ "has no parent commit, and therefore ",
+ "nothing to diff against.\n",
+ "You should be working from a repository ",
+ "originally created by git-svn\n";
+ }
unless (defined $last_rev) {
(undef, $last_rev, undef) = cmt_metadata("$d~1");
unless (defined $last_rev) {
--
1.4.4.1.g22a08
^ permalink raw reply related
* [PATCH 2/3] git-svn: correctly handle revision 0 in SVN repositories
From: Eric Wong @ 2006-11-23 22:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong
In-Reply-To: <1164322445180-git-send-email-normalperson@yhbt.net>
some SVN repositories have a revision 0 (committed by no author
and no date) when created; so when we need to ensure that we
check any revision variables are defined, and not just
non-zero.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
git-svn.perl | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index f0db4af..6feae56 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -232,7 +232,7 @@ sub rebuild {
my @commit = grep(/^git-svn-id: /,`git-cat-file commit $c`);
next if (!@commit); # skip merges
my ($url, $rev, $uuid) = extract_metadata($commit[$#commit]);
- if (!$rev || !$uuid) {
+ if (!defined $rev || !$uuid) {
croak "Unable to extract revision or UUID from ",
"$c, $commit[$#commit]\n";
}
@@ -832,8 +832,14 @@ sub commit_diff {
print STDERR "Needed URL or usable git-svn id command-line\n";
commit_diff_usage();
}
- my $r = shift || $_revision;
- die "-r|--revision is a required argument\n" unless (defined $r);
+ my $r = shift;
+ unless (defined $r) {
+ if (defined $_revision) {
+ $r = $_revision
+ } else {
+ die "-r|--revision is a required argument\n";
+ }
+ }
if (defined $_message && defined $_file) {
print STDERR "Both --message/-m and --file/-F specified ",
"for the commit message.\n",
@@ -2493,7 +2499,7 @@ sub extract_metadata {
my $id = shift or return (undef, undef, undef);
my ($url, $rev, $uuid) = ($id =~ /^git-svn-id:\s(\S+?)\@(\d+)
\s([a-f\d\-]+)$/x);
- if (!$rev || !$uuid || !$url) {
+ if (!defined $rev || !$uuid || !$url) {
# some of the original repositories I made had
# identifiers like this:
($rev, $uuid) = ($id =~/^git-svn-id:\s(\d+)\@([a-f\d\-]+)/);
--
1.4.4.1.g22a08
^ permalink raw reply related
* Re: [PATCH] git-cvsimport: add suport for CVS pserver method HTTP/1.x proxying
From: Martin Langhoff (CatalystIT) @ 2006-11-23 22:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Inaki Arenaza, git
In-Reply-To: <7v64d5keke.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Except that this statement made me go "huh?" wondering what it
> would do to the $filehandle to evaluate <$filehandle> in a void
> context:
>
> + # Skip the empty line of the proxy server output
> + <$s>;
It's a perl idiom that will discard one line of the $filehandle. If we
are 200% certain that it is empty, then it's fine. OTOH, it may well be
a bug in the particular proxy implementation Iñaki is using -- I don't
know enough about CVS proxying to tell.
> The "I/O Operators" section talks about evaluating <$s> in a
> scalar context (i.e. "$rep = <$s>"), which we all know would
> return a single line, and in list context, which swallows
This is in scalar context, and that's safe to rely on. Whether it is
clear enough in this non-Perl-native project... is a good flamewar
waiting to happen :-)
cheers,
martin
--
-----------------------------------------------------------------------
Martin @ Catalyst .Net .NZ Ltd, PO Box 11-053, Manners St, Wellington
WEB: http://catalyst.net.nz/ PHYS: Level 2, 150-154 Willis St
OFFICE: +64(4)916-7224 MOB: +64(21)364-017
Make things as simple as possible, but no simpler - Einstein
^ permalink raw reply
* [PATCH] archive-zip: don't use sizeof(struct ...)
From: René Scharfe @ 2006-11-23 22:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Gerrit Pape
In-Reply-To: <7vpsbdkhzc.fsf@assigned-by-dhcp.cox.net>
We can't rely on sizeof(struct zip_*) returning the sum of
all struct members. At least on ARM padding is added at the
end, as Gerrit Pape reported. This fixes the problem but
still lets the compiler do the summing by introducing
explicit padding at the end of the structs and then taking
its offset as the combined size of the preceding members.
As Junio correctly notes, the _end[] marker array's size
must be greater than zero for compatibility with compilers
other than gcc. The space wasted by the markers can safely
be neglected because we only have one instance of each
struct, i.e. in sum 3 wasted bytes on i386, and 0 on ARM. :)
We still rely on the compiler to not add padding between the
struct members, but that's reasonable given that all of them
are unsigned char arrays.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
archive-zip.c | 24 ++++++++++++++++++------
1 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/archive-zip.c b/archive-zip.c
index ae5572a..36e922a 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -35,6 +35,7 @@ struct zip_local_header {
unsigned char size[4];
unsigned char filename_length[2];
unsigned char extra_length[2];
+ unsigned char _end[1];
};
struct zip_dir_header {
@@ -55,6 +56,7 @@ struct zip_dir_header {
unsigned char attr1[2];
unsigned char attr2[4];
unsigned char offset[4];
+ unsigned char _end[1];
};
struct zip_dir_trailer {
@@ -66,8 +68,18 @@ struct zip_dir_trailer {
unsigned char size[4];
unsigned char offset[4];
unsigned char comment_length[2];
+ unsigned char _end[1];
};
+/*
+ * On ARM, padding is added at the end of the struct, so a simple
+ * sizeof(struct ...) reports two bytes more than the payload size
+ * we're interested in.
+ */
+#define ZIP_LOCAL_HEADER_SIZE offsetof(struct zip_local_header, _end)
+#define ZIP_DIR_HEADER_SIZE offsetof(struct zip_dir_header, _end)
+#define ZIP_DIR_TRAILER_SIZE offsetof(struct zip_dir_trailer, _end)
+
static void copy_le16(unsigned char *dest, unsigned int n)
{
dest[0] = 0xff & n;
@@ -211,7 +223,7 @@ static int write_zip_entry(const unsigne
}
/* make sure we have enough free space in the dictionary */
- direntsize = sizeof(struct zip_dir_header) + pathlen;
+ direntsize = ZIP_DIR_HEADER_SIZE + pathlen;
while (zip_dir_size < zip_dir_offset + direntsize) {
zip_dir_size += ZIP_DIRECTORY_MIN_SIZE;
zip_dir = xrealloc(zip_dir, zip_dir_size);
@@ -234,8 +246,8 @@ static int write_zip_entry(const unsigne
copy_le16(dirent.attr1, 0);
copy_le32(dirent.attr2, attr2);
copy_le32(dirent.offset, zip_offset);
- memcpy(zip_dir + zip_dir_offset, &dirent, sizeof(struct zip_dir_header));
- zip_dir_offset += sizeof(struct zip_dir_header);
+ memcpy(zip_dir + zip_dir_offset, &dirent, ZIP_DIR_HEADER_SIZE);
+ zip_dir_offset += ZIP_DIR_HEADER_SIZE;
memcpy(zip_dir + zip_dir_offset, path, pathlen);
zip_dir_offset += pathlen;
zip_dir_entries++;
@@ -251,8 +263,8 @@ static int write_zip_entry(const unsigne
copy_le32(header.size, uncompressed_size);
copy_le16(header.filename_length, pathlen);
copy_le16(header.extra_length, 0);
- write_or_die(1, &header, sizeof(struct zip_local_header));
- zip_offset += sizeof(struct zip_local_header);
+ write_or_die(1, &header, ZIP_LOCAL_HEADER_SIZE);
+ zip_offset += ZIP_LOCAL_HEADER_SIZE;
write_or_die(1, path, pathlen);
zip_offset += pathlen;
if (compressed_size > 0) {
@@ -282,7 +294,7 @@ static void write_zip_trailer(const unsi
copy_le16(trailer.comment_length, sha1 ? 40 : 0);
write_or_die(1, zip_dir, zip_dir_offset);
- write_or_die(1, &trailer, sizeof(struct zip_dir_trailer));
+ write_or_die(1, &trailer, ZIP_DIR_TRAILER_SIZE);
if (sha1)
write_or_die(1, sha1_to_hex(sha1), 40);
}
^ permalink raw reply related
* Re: [PATCH] git-cvsimport: add suport for CVS pserver method HTTP/1.x proxying
From: Junio C Hamano @ 2006-11-23 22:01 UTC (permalink / raw)
To: Inaki Arenaza; +Cc: git, Martin Langhoff
In-Reply-To: <11642344172790-git-send-email-iarenuno@eteo.mondragon.edu>
Thanks. The patch looks very sane, isolated to be safe enough,
and useful.
Except that this statement made me go "huh?" wondering what it
would do to the $filehandle to evaluate <$filehandle> in a void
context:
+ # Skip the empty line of the proxy server output
+ <$s>;
and I ended up looking in perlop.pod and came up empty.
The "I/O Operators" section talks about evaluating <$s> in a
scalar context (i.e. "$rep = <$s>"), which we all know would
return a single line, and in list context, which swallows
everything up to EOF, an obvious disaster for this particular
use. I couldn't find how it is defined to behave in a void
context. By experiments I know this returns only one line, but
it leaves me feeling somewhat uneasy.
Also it has a style inconsistency between "if(expression) {" and
"if(expression){", and I do not like either of them, but fixing
that should be left to a separate patch.
I'll apply this unless Martin or other people on the list who
have stake in cvsimport objects.
^ permalink raw reply
* Re: [PATCH] Increase length of function name buffer
From: Junio C Hamano @ 2006-11-23 20:53 UTC (permalink / raw)
To: Andy Parkins; +Cc: git
In-Reply-To: <200611231005.17859.andyparkins@gmail.com>
Andy Parkins <andyparkins@gmail.com> writes:
> In xemit.c:xdl_emit_diff() a buffer for showing the function name as
> commentary is allocated; this buffer was 40 characters. This is a bit
> small; particularly for C++ function names where there is often an
> identical prefix (like void LongNamespace::LongClassName) on multiple
> functions, which makes the context the same everywhere. In other words
> the context is useless. This patch increases that buffer to 80
> characters - which may still not be enough, but is better.
>
> Signed-off-by: Andy Parkins <andyparkins@gmail.com>
I agree this is a good problem to address.
I wonder however which is easier to read, a loooong heading line
as you do in this patch, or "...TailOfVeryLongClassName::method"
that still fits on a single line without terminal line-wrapping.
Especially with our default settings that runs "less" with -S to
truncate the right hand side of a long line, truncating at the
top (instead of at the tail as we currently do) might be easier
to read.
Not rejecting your implementation and asking to change the
patch; asking for opinions from the list.
^ permalink raw reply
* Re: [PATCH] config option core.showroot to enable showing the diff of the root commit
From: Junio C Hamano @ 2006-11-23 20:52 UTC (permalink / raw)
To: Peter Baumann; +Cc: git
In-Reply-To: <slrnemaqt1.esn.Peter.B.Baumann@xp.machine.xx>
Peter Baumann <Peter.B.Baumann@stud.informatik.uni-erlangen.de>
writes:
> This allows one to see the root commit as a diff in commands like git-log,
> git-show and git-whatchanged. It also modifies git-diff-tree to act as --root
> was specified on the commandline. The default is set to true.
>
> Signed-off-by: Peter Baumann <Peter.B.Baumannn@stud.informatik.uni-erlangen.de>
> ---
> I'm not sure if making core.showroot acting on git-diff-tree is the
> right thing to do. Please check first bevore applying.
I agree that this "use --root by default" belongs to Porcelain
layer, not the plumbing. We would probably want to do this in
the same way as we do the color in diff.c::git_diff_ui_config().
^ permalink raw reply
* Re: sizeof(struct ...)
From: Junio C Hamano @ 2006-11-23 20:47 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Gerrit Pape
In-Reply-To: <45659781.5050005@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Gerrit Pape schrieb:
>...
>> It's because sizeof(struct zip_local_header) is 32, zip_dir_header 48,
>> and zip_dir_trailer 24, breaking the zip files. Compiling with
>> -fpack-struct seemed to break other things, so I for now I ended up with
>> this (not so nice) workaround.
>
> Hm, yes, this use sizeof() is not strictly correct. But I'd very much
> like to keep being lazy and let the compiler to do the summing. How
> about this patch instead? Does it work for you, Gerrit?
>...
> @@ -35,6 +35,7 @@ struct zip_local_header {
> unsigned char size[4];
> unsigned char filename_length[2];
> unsigned char extra_length[2];
> + unsigned char _end[0];
> };
While I think relying on the compiler to add no pad in the
middle of the structure that has only unsigned char array
members, making the compiler to sum the member length using
offsetof() is a reasonable approach to avoid hardcoding the size
of on-disk structure representation, zero-length member is not
portable.
We need to deal with tail padding in any case, and this _end[N]
is essentially a manual tail padding when N > 0, so I think that
the code should work even if you change these to _end[1], and
that would be a reasonably clean solution to this problem.
^ permalink raw reply
* Re: Pull and fetch
From: Sean @ 2006-11-23 20:36 UTC (permalink / raw)
To: Paolo Ciarrocchi; +Cc: git
In-Reply-To: <20061123203950.5d47421f@paolo-desktop>
On Thu, 23 Nov 2006 20:39:50 +0100
Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> wrote:
> OK, make sense. So let's try with an experiment:
> paolo@paolo-desktop:~$ mkdir test
> paolo@paolo-desktop:~$ cd test
> paolo@paolo-desktop:~/test$ git-init-db
> defaulting to local storage area
At this point your "master" branch will be empty. You can see
this by doing a "git log"
> paolo@paolo-desktop:~/test$ git fetch ../git master:testbranch
> warning: no common commits
> remote: Generating pack...
If you do "git log" at this point you'll see your master branch
is still empty. All you've done is fetched the remote branch
into your new repo as "testbranch"
> [skip]
> paolo@paolo-desktop:~/test$ git pull ../git master:testbranchpull
> * refs/heads/testbranchpull: storing branch 'master' of ../git
> commit: e945f95
This command is different than fetch. After fetching the remote
branch into your repo as "testbranchpull", it then merges it with
your currently checked out branch (master). At this point if
you do a "git log" you'll see your master is now populated with
the results of the merge (well in this case a fast forward).
So this nicely shows the difference of fetch and pull, and explains
how your "master" branch came to have something in it.
> Now I have 3 branches:
> paolo@paolo-desktop:~/test$ git branch
> * master
> testbranch
> testbranchpull
>
> All the branches have the same content, I expect to see differences between testbranch
> and testbranchpull. The first one is the end result of a fetch, the second one is
> the end result of a pull.
Yes, they will all have the same content. As your local repo diverges
from the remote (as you make local commits) the difference between
fetch and pull will become more obvious and pronounced.
> git status always says:
> nothing to commit
>
> Why?
This just means you have no uncommitted local changes in your repo. Edit
one of the file, and before committing the change, do "git status" again
and it will list the file as modified.
> What will happen if I repeat the same commands:
> git fetch ../git master:testbranch
> git pull ../git master:testbranchpull
> after a change in the git master branch?
Try it out :o)
Afterward, both testbranch and testbranchpull will be direct copies
of the other repo's master. The fetch command will do nothing to
"master". However, after updating your testbranchpull, the pull
command will merge (or fast forward) its contents into "master".
Cheers,
^ permalink raw reply
* Re: Some tips for doing a CVS importer
From: Robin Rosenberg @ 2006-11-23 19:45 UTC (permalink / raw)
To: Shawn Pearce
Cc: lamikr, Jon Smirl, Carl Worth, Martin Langhoff, Git Mailing List
In-Reply-To: <20061121200508.GB22461@spearce.org>
tisdag 21 november 2006 21:05 skrev Shawn Pearce:
> lamikr <lamikr@cc.jyu.fi> wrote:
> > Shawn Pearce wrote:
> > > - No GUI.
> >
> > QGIT allows using some commands. I plan to try out the GIT eclipse
> > plugin in near future myself.
> > This mail list have some discussion and download link to it's repo in
> > archives.
> > (title: Java GIT/Eclipse GIT version 0.1.1, )
>
> I'm the author of that plugin. :-)
>
> Its not even capable of making a commit yet. The underling plumbing
> (aka jgit) can make commits but the Eclipse GUI has no function to
> actually invoke that plumbing and make a commit to the repository.
>
> The Eclipse plugin has apparently been a low priority for me.
> I haven't worked on it very recently. Robin Rosenburg has supposedly
> gotten the revision compare interface to work, but its slow as a
> duck in November due to jgit's pack reading code not running as
> fast as it should.
Slow it is. It is somewhat usable though, especially the quickdiff. I worked
the whole day with help from quickdiff today. The diff is computed against
HEAD^ (i.e. I get to see the changes that my topmost StGit patch introduces).
The project contains 20000+ files and six years of history. Reading the whole
history is out of the question with the current performance so I restrict
reading to 500 entries which is just about bearable. That's enough for
practical use with quickdiff and compare though. Improving jgit's speed 50
times will probably be enough to make jgit shine.
Activating the Git connection seems to be a problem with the egit projects,
i.e. it works sometimes, but not with my much bigger repo. The only problem
is that the first time is dog slow. The structure is different though, as my
repo has .project at the top, not one level down.
^ permalink raw reply
* Pull and fetch
From: Paolo Ciarrocchi @ 2006-11-23 19:39 UTC (permalink / raw)
To: git
Hi all,
I'm still reading git tutorial.txt and I'm again confused.
A more cautious Alice might wish to examine Bob's changes before
pulling them. She can do this by creating a temporary branch just
for the purpose of studying Bob's changes:
-------------------------------------
$ git fetch /home/bob/myrepo master:bob-incoming
-------------------------------------
which fetches the changes from Bob's master branch into a new branch
named bob-incoming. (Unlike git pull, git fetch just fetches a copy
of Bob's line of development without doing any merging). Then
-------------------------------------
$ git log -p master..bob-incoming
-------------------------------------
shows a list of all the changes that Bob made since he branched from
Alice's master branch.
OK, make sense. So let's try with an experiment:
paolo@paolo-desktop:~$ mkdir test
paolo@paolo-desktop:~$ cd test
paolo@paolo-desktop:~/test$ git-init-db
defaulting to local storage area
paolo@paolo-desktop:~/test$ git fetch ../git master:testbranch
warning: no common commits
remote: Generating pack...
[skip]
paolo@paolo-desktop:~/test$ git pull ../git master:testbranchpull
* refs/heads/testbranchpull: storing branch 'master' of ../git
commit: e945f95
Now I have 3 branches:
paolo@paolo-desktop:~/test$ git branch
* master
testbranch
testbranchpull
All the branches have the same content, I expect to see differences between testbranch
and testbranchpull. The first one is the end result of a fetch, the second one is
the end result of a pull.
git status always says:
nothing to commit
Why?
What will happen if I repeat the same commands:
git fetch ../git master:testbranch
git pull ../git master:testbranchpull
after a change in the git master branch?
Thanks in advance?
Kind regards,
Paolo Ciarrocchi
^ permalink raw reply
* Re: "stgit clean" has problems with removed generated files
From: Yann Dirson @ 2006-11-23 19:28 UTC (permalink / raw)
To: Catalin Marinas; +Cc: GIT list
In-Reply-To: <b0943d9e0611230833y5ffc14d3q922a821744ad05d@mail.gmail.com>
On Thu, Nov 23, 2006 at 04:33:42PM +0000, Catalin Marinas wrote:
> >fatal: Untracked working tree file 'include/asm-arm/constants.h' would be
> >overwritten by merge.
>
> That's a git error and I think it is the correct behaviour. It is
> safer to notify that a local file is overridden by a merge/switch
> operation rather than just losing its content.
Right, I do not discuss the behaviour of git here. But when I first
encountered this issue, I was really wondering about what was
happenning. It would be really helpful in such a case, if stgit was
able to pinpoint the precise patch which could not be popped. It could
also be helpful to tell when popping patches - currently it's done
"behind my back", and I could only understand what was happenning by
reading the code.
> >The root issue seems to be that stgit has problem with such generated
> >files, ie., files that were removed from version-control, but can still
> >legitimately exist in the tree. Dealing with them could possibly be
> >done (eg. allowing to back them up, and restore them when pushing the
> >annoying patch), but is not a trivial issue (eg. we still need to guard
> >the user against real conflicts).
>
> That's a GIT issue more than an StGIT one, unless GIT already has a
> way to deal with this and StGIT doesn't pass the right options.
I'm not sure if git itself should add support for this. IMHO, such an issue
is more related to the nature of patch management - git itself does not
have a need to repetedly pop/push paches, with related merge
hassles like this one.
> >First, when cleaning patches, we could first look up which patches are
> >to be removed, and only pop the necessary ones.
>
> OK, I mentioned it above as well. This should really be done for
> efficiency but it might not solve the problem if the empty patch is
> deeper into the stack.
Right, it could be advertised as good practice to burry those near the
bottom of the stack, and anyway to keep actively-changing ones near the
top.
> >BTW, maybe it would those make sense to allowthose special ranges in
> >most places a range is valid.
>
> Is there any other place where ranges could be used but aren't?
Hm, let's see... I'd say "export" (I have missed it already), "files"
and maybe "commit" and "pick", although the latter would require a syntax
for ranges in other branch.
While reviewing the various commands for this, I realized that "stg pop
<patch>" semantics is significantly different from "stg push <patch>" -
ie. it is an equivalent of "goto". What about turning it into a
float+pop, to better match the "push" behaviour ?
I also realized that "stg help <command>" output does not go through to
the pager, while eg. the help for "mail" is quite long.
Best regards,
--
^ permalink raw reply
* Re: sizeof(struct ...)
From: René Scharfe @ 2006-11-23 17:57 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Erik Mouw, Gerrit Pape, git
In-Reply-To: <4565CA02.20602@shadowen.org>
Andy Whitcroft schrieb:
> Perhaps we can look and see what a portable application like gzip or
> bzip2 do in this situation. They must have the same problem.
Info-ZIP's zip uses structs only for in-memory storage and has a write
function for each of them that writes the members one by one. I find
the structs in archive-zip.c easier to read, but I might be biased. ;-)
Anyway, archive-zip.c assumes that there is no padding between unsigned
char arrays and that an unsigned char is exactly one byte wide. The
additional current assumption -- that sizeof(struct ...) sums up the
sizes of all struct members -- is wrong on ARM, and the patches in this
thread correct this error.
So we're not as portable as Info-ZIP, but I think the assumptions above
hold true for all interesting architectures. And we have a readable
description of the on-disk ZIP file headers.
^ permalink raw reply
* Calling all bash completion experts..
From: Linus Torvalds @ 2006-11-23 17:45 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
So, I'm a happy but clueless user of the bash completion, and one thing
drives me wild: when it has found an exclusive completion, it doesn't add
the final space at the end, but just beeps at you when you <tab> again.
So I do "git repa<tab>" and get "git repack", which is fine, but I really
_wanted_ to get "git repack " (with the space at the end), since I've now
got a unique command, and that's the normal completion behaviour (ie I
want it to act the same way that "git-repa<tab>" would have acted).
The same is true of filename arguments, btw:
git commit cont<tab>com<tab><tab>
gives me
git commit contrib/completion/git-completion.bash
but again, it doesn't seem to do the right thing about the fact that it
was the unique choice, so it didn't add the final space, and when you
press <tab> more to get what the other choices are, it doesn't show you
any other choices (because there are none).
Now, without knowing the completion syntax, I assume it's the "-o nospace"
things in the completion file. So I'm wondering _why_ that "nospace" is
there, and whether this could be fixed?
^ permalink raw reply
* Re: sizeof(struct ...)
From: Erik Mouw @ 2006-11-23 16:42 UTC (permalink / raw)
To: Ren? Scharfe; +Cc: Andy Whitcroft, Gerrit Pape, git
In-Reply-To: <4565C8F4.6000606@lsrfire.ath.cx>
On Thu, Nov 23, 2006 at 05:14:44PM +0100, Ren? Scharfe wrote:
> Erik Mouw schrieb:
> > On Thu, Nov 23, 2006 at 04:45:09PM +0100, Ren? Scharfe wrote:
> >> Is there really a compiler that inserts padding between arrays of
> >> unsigned chars?
> >
> > Yes, that compiler is called "gcc".
> >
> > #include <stdio.h>
> >
> > struct foo {
> > unsigned char a[3];
> > unsigned char b[3];
> > };
> >
> > int main(void)
> > {
> > printf("%d\n", sizeof(struct foo));
> > return 0;
> > }
> >
> > On i386 that prints 6, on ARM it prints 8.
>
> Does it add 1 byte after a and and 1 after b or two after b?
> I suspect it's the latter case -- otherwise Gerrit's patch,
> which started this thread, wouldn't help solve his problem.
> Or the pad sizing follows complicated rules that I do not
> understand at the moment.
You're right, it adds the padding after b:
#define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
printf("%d %d\n", offsetof(struct foo, a), offsetof(struct foo, b));
prints "0 3" on ARM.
> Time to look for an ARM emulator, it seems.
objdump -D -S is your friend. I didn't have an ARM target ready, but at
least I know enough ARM assembly that I can see what it will print :)
Erik
--
+-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 --
^ permalink raw reply
* Re: "stgit clean" has problems with removed generated files
From: Catalin Marinas @ 2006-11-23 16:33 UTC (permalink / raw)
To: Yann Dirson; +Cc: GIT list
In-Reply-To: <20061123161135.GK5443@nan92-1-81-57-214-146.fbx.proxad.net>
On 23/11/06, Yann Dirson <ydirson@altern.org> wrote:
> In a kernel tree, the precise problem I had is due to generated files
> committed by error in an upstream branch (a BSP from a well-known
> vendor, indeed ;). The first patch in my stgir stack does some cleanup
> by removing them from git control (so that "make dep" does not cause
> them to change every so often).
>
> Now when I want to run "stg clean" for applied patches, stgit first
> wants to pop the whole stack, including that patch, which triggers the
> following error:
>
> fatal: Untracked working tree file 'include/asm-arm/constants.h' would be overwritten by merge.
That's a git error and I think it is the correct behaviour. It is
safer to notify that a local file is overridden by a merge/switch
operation rather than just losing its content.
> Obviously, removing all such files by hand allows to run "stg clean", as
> does (floating and) popping that patch and deleting it, or running "stg clean
> --unappplied".
Maybe 'stg clean' should only pop to the first empty applied patch
rather than popping all of them as it is also more efficient.
> The root issue seems to be that stgit has problem with such generated
> files, ie., files that were removed from version-control, but can still
> legitimately exist in the tree. Dealing with them could possibly be
> done (eg. allowing to back them up, and restore them when pushing the
> annoying patch), but is not a trivial issue (eg. we still need to guard
> the user against real conflicts).
That's a GIT issue more than an StGIT one, unless GIT already has a
way to deal with this and StGIT doesn't pass the right options.
> First, when cleaning patches, we could first look up which patches are
> to be removed, and only pop the necessary ones.
OK, I mentioned it above as well. This should really be done for
efficiency but it might not solve the problem if the empty patch is
deeper into the stack.
> Second, we could generalize the "clean" subcommand to accept arbitrary
> ranges, not only the "applied" and "unapplied" ones. A special case
> would be "stg clean that-patch", which would be a secure version of "stg
> delete".
Easy to fix as well.
> BTW, maybe it would those make sense to allowthose special ranges in
> most places a range is valid.
Is there any other place where ranges could be used but aren't?
--
^ permalink raw reply
* Re: sizeof(struct ...)
From: Andy Whitcroft @ 2006-11-23 16:19 UTC (permalink / raw)
To: René Scharfe; +Cc: Erik Mouw, Gerrit Pape, git
In-Reply-To: <4565C8F4.6000606@lsrfire.ath.cx>
René Scharfe wrote:
> Erik Mouw schrieb:
>> On Thu, Nov 23, 2006 at 04:45:09PM +0100, René Scharfe wrote:
>>> Is there really a compiler that inserts padding between arrays of
>>> unsigned chars?
>> Yes, that compiler is called "gcc".
>>
>> #include <stdio.h>
>>
>> struct foo {
>> unsigned char a[3];
>> unsigned char b[3];
>> };
>>
>> int main(void)
>> {
>> printf("%d\n", sizeof(struct foo));
>> return 0;
>> }
>>
>> On i386 that prints 6, on ARM it prints 8.
>
> Does it add 1 byte after a and and 1 after b or two after b?
> I suspect it's the latter case -- otherwise Gerrit's patch,
> which started this thread, wouldn't help solve his problem.
> Or the pad sizing follows complicated rules that I do not
> understand at the moment.
>
> Time to look for an ARM emulator, it seems.
Perhaps we can look and see what a portable application like gzip or
bzip2 do in this situation. They must have the same problem.
^ 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