* Re: [PATCH 1/2] diff --stat: use asymptotic scaling in graph
From: Junio C Hamano @ 2006-10-14 19:06 UTC (permalink / raw)
To: apodtele; +Cc: git
In-Reply-To: <d620685f0610130656u55079a1fkc2c98a82f3aa4a33@mail.gmail.com>
apodtele <apodtele@gmail.com> writes:
> Before my patch is completely forgotten, let me critique the current
> approach. Currently everything is great and beautiful unless one
> particular change adds a couple of hundred lines, say, to a man page.
> With large changes in play, small changes are squashed to a single
> character. Would you argue that this scenario correctly represent
> importance of man pages? Would you say, that it's not misleading that
> 1-, 2-, and 5-liners all look the same as long as a man page is
> prominently shown? Moreover, 1-, 2-, and 5- liners may look different
> depending on the size of that man page. The current approach is not
> invariant; it is, however, normalized as needed. "Normalized" is good,
> "as needed" is bad.
One thing that mildly irritates me has been:
git log --stat v2.6.17..
which, as you correctly point out, shows the bad effect of
scaling per commit. "Normalized as needed" is good. What's bad
is "not normalizing across things we show".
Even with your non-linear scaling, you would need to make sure
every commit gets the same graph width; I do not think they
currently do, due to name part scaling.
People are used to seeing the traditional diffstat output, so
any improvement you make that is different from it (including
e.g. "being able to show differences between 1- and 2- liner
patch when a monster 800- liner happens to be in the same patch
set", which is a worthwhile goal) will look bizarre and/or
misleading to them and they would not like it.
With the change to align things in the middle, it might become
easier to accept, because then it is _so_ obviously different
from traditional diffstat, it is very clear to people that the
output is different but still they can easily figure out that
longer bars are for larger changes.
And this new output needs to be an option.
^ permalink raw reply
* Re: [PATCH 1/2] Delete ref $frotz by moving ref file to "deleted-$frotz~ref".
From: Junio C Hamano @ 2006-10-14 18:47 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <20061014153949.2994a114.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> The idea is that moving:
>
> $GIT_DIR/refs/<refpath>/frotz
>
> to:
>
> $GIT_DIR/deleted-refs/<refpath>/frotz~ref
>
> maybe cheaper and safer than repacking the refs without the
> deleted one.
Before actually implementing delete_ref(), we discussed this
"deleted-refs/" idea. but I do not think it is a direction we
would want to go.
Ref deletion is an operation that happens far far rarer than
updates and lookups, and I deliberately chose to optimize for
the latter.
There are valid reasons to delete refs, and one most frequent
one would be to discard throw-away temporary branches you may
have needed to switch to when your work was interrupted. But
even counting that kind of deletion, I imagine that you would
not be creating and removing more than one branch per every 10
commits you would create, and I also imagine you would be
invoking not less than 5 operations that inspect project
history, such as git-log and git-diff, between commits you make.
An operation to build a new commit itself needs at least two
lookups (one to see what's current upfront, and another to see
nobody touched it upon lockless update). Most history-related
operations at least need to look at one (typically, HEAD), and
any refname you use to spell the name of an object or revision
range (e.g. "v2.6.17..v2.6.18~10" needs to look at tags/v2.6.17
and tags/v2.6.18). Optimizing for deletion path at the expense
of giving even a tiny penalty to lookup path is optimizing for a
wrong case, and that is why I rejected deleted-refs/ idea when I
originally did the delete_ref() implementation.
Having said that, I would definitely think there still are rooms
for optimization in the current implementation. For example, I
do not recall offhand if I made the code to unconditionally
repack without the deleted one, or only repack when we know the
ref being deleted exists in the packed refs file. The latter
obviously would be more efficient and if we currently do not do
that, making it do so is a very welcomed change. Especially,
given that the latest code does not pack branch heads by
default, when a temporary throw-away branch is discarded, it is
far more likely that it is not packed and we do not need to
repack.
Independent from this topic of "removing deleted from packed" vs
"using deleted-refs/", I think we still do not handle .git/logs/
hierarchy correctly in the current code when ref deletion is
involved. We already made it to correctly unlink/rmdir/mkdir
on-demand for .git/refs/ hierarchy and I think we need to have
code that parallels that for the .git/logs/ side.
^ permalink raw reply
* Re: [PATCH] Fix tracing when GIT_TRACE is set to an empty string.
From: Junio C Hamano @ 2006-10-14 18:11 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <20061014160525.c013e6d2.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> trace.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/trace.c b/trace.c
> index f9efc91..495e5ed 100644
> --- a/trace.c
> +++ b/trace.c
> @@ -55,7 +55,8 @@ static int get_trace_fd(int *need_close)
> {
> char *trace = getenv("GIT_TRACE");
>
> - if (!trace || !strcmp(trace, "0") || !strcasecmp(trace, "false"))
> + if (!trace || !strcmp(trace, "") ||
> + !strcmp(trace, "0") || !strcasecmp(trace, "false"))
> return 0;
> if (!strcmp(trace, "1") || !strcasecmp(trace, "true"))
> return STDERR_FILENO;
> --
> 1.4.3.rc2.gadba
This means that
GIT_TRACE= git-foo
and
unset GIT_TRACE ; git-foo
should behave the same way, right?
If so that sounds sensible.
^ permalink raw reply
* Re: VCS comparison table
From: Jakub Narebski @ 2006-10-14 17:42 UTC (permalink / raw)
To: Jon Smirl; +Cc: git
In-Reply-To: <9e4733910610141018gdfa62a7o9a2bb2819d5c2ecc@mail.gmail.com>
Jon Smirl wrote:
>> About "checkouts", i.e. working directories with repository
>> elsewhere: you can use GIT_DIR environmental variable or "git
>> --git-dir" option, or symlinks, and if Nguyen Thai Ngoc D proposal
>> to have .gitdir/.git "symref"-like file to point to repository
>> passes, we can use that.
>
> I believe they mean checking out only the latest few revisions
> instead of copying the whole repo. This issue is a problem for
> Mozilla. If you want to change a line in the git version you have to
> download the entire 500MB tree with full history.
>From http://bazaar-vcs.org/RcsComparisons
A "Checkout" is a working tree that points elsewhere for its RCS data.
You can always do like Linux kernel did, splitting repository into
current and historical part (which would contain also dead branches),
and creating and publishing current-historical graft file, to join
history if needed.
>> Partial checkouts are only partially supported as of now; it means
>> you have to do some lowe level stuff to do partial checkout, and be
>> carefull when comitting. BTW it depends what you mean by partial
>> checkout, but they are somewhat incompatibile with atomic commits
>> to snapshot based repository.
>
> I believe partial checkout means being able to check one directory
> tree out of the repo and work on it while ignoring what is happening
> in the rest of the repo. This is another issue for Mozilla which has
> multiple dependent projects checked into a single repo.
So split different projects into different repositories. There was some
helper program (git-splitrepo or something like that) for that posted
on git mailing list. And use "superrepository" to gather all projects
together (see last discussion about subprojects on git mailing list).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: VCS comparison table
From: Jon Smirl @ 2006-10-14 17:18 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <egr3ud$nqm$1@sea.gmane.org>
On 10/14/06, Jakub Narebski <jnareb@gmail.com> wrote:
> Jon Smirl wrote:
>
> > It refers to this comparison chart between source control systems.
> > http://bazaar-vcs.org/RcsComparisons
>
> It is quite obvious that comparison of programs of given type (SMC)
> on some program site (Bazaar-NG) is usually biased towards said program,
> perhaps unconsciously: by emphasizing the features which were important
> for developers of said program.
>
> > Does it accurately reflect the current status of git? Is their
> > assessment of git's rename capability correct?
>
> For example simple namespace for git: you can use shortened sha1
> (even to only 6 characters, although usually 8 are used), you can
> use tags, you can use ref^m~n syntax.
>
> I'm not sure about "No" in "Supports Repository". Git supports multiple
> branches in one repository, and what's better supports development using
> multiple branches, but cannot for example do a diff or a cherry-pick
> between repositories (well, you can use git-format-patch/git-am to
> cherry-pick changes between repositories...).
>
> About "checkouts", i.e. working directories with repository elsewhere:
> you can use GIT_DIR environmental variable or "git --git-dir" option,
> or symlinks, and if Nguyen Thai Ngoc D proposal to have .gitdir/.git
> "symref"-like file to point to repository passes, we can use that.
I believe they mean checking out only the latest few revisions instead
of copying the whole repo. This issue is a problem for Mozilla. If you
want to change a line in the git version you have to download the
entire 500MB tree with full history.
>
> Partial checkouts are only partially supported as of now; it means
> you have to do some lowe level stuff to do partial checkout, and be
> carefull when comitting. BTW it depends what you mean by partial
> checkout, but they are somewhat incompatibile with atomic commits
> to snapshot based repository.
I believe partial checkout means being able to check one directory
tree out of the repo and work on it while ignoring what is happening
in the rest of the repo. This is another issue for Mozilla which has
multiple dependent projects checked into a single repo.
>
> Git supports renames in its own way; it doesn't use file ids, nor
> remember renames (the new "note" header for use e.g. by porcelains
> didn't pass if I remember correctly). But it does *detect* moving
> _contents_, and even *copying* _contents_ when requested. And of
> course it detect renames in merges.
>
> Git doesn't have some "plugin framework", but because it has many
> "plumbing" commands, it is easy to add new commands, and also new
> merge strategies, using shell scripts, Perl, Python and of course C.
> So the answer would be "Somewhat", as git has plugable merge strategies,
> or even "Yes" at it is easy to add new git command.
>
> > They want changes via IRC. "Please discuss changes to this table on
> > the freenode IRC network channel #bzr, or on the mailing list."
>
> Gaah, subscribe-to-post mailing list!
It is annoying, but subscribe with the no delivery option.
> --
> Jakub Narebski
> Warsaw, Poland
> ShadeHawk on #git
>
>
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: VCS comparison table
From: Jakub Narebski @ 2006-10-14 16:40 UTC (permalink / raw)
To: git; +Cc: bazaar-ng
In-Reply-To: <9e4733910610140807p633f5660q49dd2d2111c9f5fe@mail.gmail.com>
Jon Smirl wrote:
> It refers to this comparison chart between source control systems.
> http://bazaar-vcs.org/RcsComparisons
It is quite obvious that comparison of programs of given type (SMC)
on some program site (Bazaar-NG) is usually biased towards said program,
perhaps unconsciously: by emphasizing the features which were important
for developers of said program.
> Does it accurately reflect the current status of git? Is their
> assessment of git's rename capability correct?
For example simple namespace for git: you can use shortened sha1
(even to only 6 characters, although usually 8 are used), you can
use tags, you can use ref^m~n syntax.
I'm not sure about "No" in "Supports Repository". Git supports multiple
branches in one repository, and what's better supports development using
multiple branches, but cannot for example do a diff or a cherry-pick
between repositories (well, you can use git-format-patch/git-am to
cherry-pick changes between repositories...).
About "checkouts", i.e. working directories with repository elsewhere:
you can use GIT_DIR environmental variable or "git --git-dir" option,
or symlinks, and if Nguyen Thai Ngoc D proposal to have .gitdir/.git
"symref"-like file to point to repository passes, we can use that.
Partial checkouts are only partially supported as of now; it means
you have to do some lowe level stuff to do partial checkout, and be
carefull when comitting. BTW it depends what you mean by partial
checkout, but they are somewhat incompatibile with atomic commits
to snapshot based repository.
Git supports renames in its own way; it doesn't use file ids, nor
remember renames (the new "note" header for use e.g. by porcelains
didn't pass if I remember correctly). But it does *detect* moving
_contents_, and even *copying* _contents_ when requested. And of
course it detect renames in merges.
Git doesn't have some "plugin framework", but because it has many
"plumbing" commands, it is easy to add new commands, and also new
merge strategies, using shell scripts, Perl, Python and of course C.
So the answer would be "Somewhat", as git has plugable merge strategies,
or even "Yes" at it is easy to add new git command.
> They want changes via IRC. "Please discuss changes to this table on
> the freenode IRC network channel #bzr, or on the mailing list."
Gaah, subscribe-to-post mailing list!
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* VCS comparison table
From: Jon Smirl @ 2006-10-14 15:07 UTC (permalink / raw)
To: Git Mailing List
I was reading Brendan's blog post about Mozilla 2
http://weblogs.mozillazine.org/roadmap/archives/2006/10/mozilla_2.html
It refers to this comparison chart between source control systems.
http://bazaar-vcs.org/RcsComparisons
Does it accurately reflect the current status of git? Is their
assessment of git's rename capability correct?
They want changes via IRC. "Please discuss changes to this table on
the freenode IRC network channel #bzr, or on the mailing list. The
terms used in the table have precise meanings, and not all VCS's use
the same term in the same way - which means that some translation is
needed to fill it in properly."
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* [PATCH] Re: git-svn: fix commits over svn+ssh://
From: Pierre HABOUZIT @ 2006-10-14 14:57 UTC (permalink / raw)
To: Eric Wong, 392702; +Cc: git
In-Reply-To: <11608165571212-git-send-email-normalperson@yhbt.net>
[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]
I confirm that fixes it indeed.
On Sat, Oct 14, 2006 at 02:02:37AM -0700, Eric Wong wrote:
> Once a get_commit_editor has been called from an SVN session, RA
> layer operations are not allowed (well, unless you're using
> file:// or http(s)://). So we'll pass an alternate SVN::Ra
> object to our editor object for running 'check-path'.
>
> This should fix commits over svnserve (svn:// without ssh, too).
>
> Closes Debian bug #392702, thanks to Pierre Habouzit for
> reporting the bug.
>
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ---
> git-svn.perl | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index a128d90..0f968c8 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -536,7 +536,7 @@ sub commit_lib {
> $SVN = libsvn_connect($repo);
> my $ed = SVN::Git::Editor->new(
> { r => $r_last,
> - ra => $SVN,
> + ra => $SVN_LOG,
> c => $c,
> svn_path => $SVN_PATH
> },
> @@ -832,7 +832,7 @@ sub commit_diff {
> $SVN ||= libsvn_connect($repo);
> my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef, 0) : ();
> my $ed = SVN::Git::Editor->new({ r => $SVN->get_latest_revnum,
> - ra => $SVN, c => $tb,
> + ra => $SVN_LOG, c => $tb,
> svn_path => $SVN_PATH
> },
> $SVN->get_commit_editor($_message,
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] Fix tracing when GIT_TRACE is set to an empty string.
From: Christian Couder @ 2006-10-14 14:05 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
trace.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/trace.c b/trace.c
index f9efc91..495e5ed 100644
--- a/trace.c
+++ b/trace.c
@@ -55,7 +55,8 @@ static int get_trace_fd(int *need_close)
{
char *trace = getenv("GIT_TRACE");
- if (!trace || !strcmp(trace, "0") || !strcasecmp(trace, "false"))
+ if (!trace || !strcmp(trace, "") ||
+ !strcmp(trace, "0") || !strcasecmp(trace, "false"))
return 0;
if (!strcmp(trace, "1") || !strcasecmp(trace, "true"))
return STDERR_FILENO;
--
1.4.3.rc2.gadba
^ permalink raw reply related
* [PATCH 2/2] When packing and pruning refs, remove "deleted-refs".
From: Christian Couder @ 2006-10-14 13:41 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
When running "git pack-refs --prune" we have to also prune
deleted ref files. To do this, remove recusively everything
under "$GIT_DIR/deleted-refs".
The new "remove_all_recursive" function has been copied from
"remove_empty_dir_recursive" in "refs.c".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-pack-refs.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 1087657..84ca170 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -54,12 +54,54 @@ static void prune_ref(struct ref_to_prun
}
}
+static int remove_all_recursive(char *path)
+{
+ DIR *dir = opendir(path);
+ struct dirent *e;
+ int ret = 0;
+ int len = strlen(path);
+
+ if (!dir)
+ return unlink(path);
+ if (path[len-1] != '/')
+ path[len++] = '/';
+ while ((e = readdir(dir)) != NULL) {
+ struct stat st;
+ int namlen;
+ if ((e->d_name[0] == '.') &&
+ ((e->d_name[1] == 0) ||
+ ((e->d_name[1] == '.') && e->d_name[2] == 0)))
+ continue; /* "." and ".." */
+
+ namlen = strlen(e->d_name);
+ if ((len + namlen < PATH_MAX) &&
+ strcpy(path + len, e->d_name) &&
+ !lstat(path, &st) &&
+ (S_ISDIR(st.st_mode) ?
+ !remove_all_recursive(path) :
+ !unlink(path)))
+ continue; /* happy */
+
+ /* path too long, stat, rmdir or unlink fails */
+ ret = -1;
+ break;
+ }
+ closedir(dir);
+ if (!ret) {
+ path[len] = 0;
+ ret = rmdir(path);
+ }
+ return ret;
+}
+
static void prune_refs(struct ref_to_prune *r)
{
while (r) {
prune_ref(r);
r = r->next;
}
+
+ remove_all_recursive(git_path("deleted-refs"));
}
static struct lock_file packed;
--
1.4.3.rc2.gbcf275-dirty
^ permalink raw reply related
* [PATCH 1/2] Delete ref $frotz by moving ref file to "deleted-$frotz~ref".
From: Christian Couder @ 2006-10-14 13:39 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
The idea is that moving:
$GIT_DIR/refs/<refpath>/frotz
to:
$GIT_DIR/deleted-refs/<refpath>/frotz~ref
maybe cheaper and safer than repacking the refs without the
deleted one.
On the other hand now when resolving a ref we have to check
if a related deleted ref file exists.
The new "delete_ref" function is similar to "write_ref_sha1".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
refs.c | 160 ++++++++++++++++++++++++++++++++++++++++------------------------
refs.h | 2 +
2 files changed, 102 insertions(+), 60 deletions(-)
diff --git a/refs.c b/refs.c
index 89cffaf..6a8da58 100644
--- a/refs.c
+++ b/refs.c
@@ -176,6 +176,38 @@ static struct ref_list *get_loose_refs(v
return cached_refs.loose;
}
+static char *get_del_ref_name(const char *ref_name)
+{
+ size_t ref_name_len = strlen(ref_name);
+ char *delref_name = xmalloc(ref_name_len + 13);
+
+ strncpy(delref_name, "deleted-", 8);
+ strncpy(delref_name + 8, ref_name, ref_name_len);
+ strcpy(delref_name + 8 + ref_name_len, "~ref");
+
+ return delref_name;
+}
+
+static int lstat_del_ref_file(const char *ref_name, struct stat *st)
+{
+ char *delref_name = get_del_ref_name(ref_name);
+ char *delref_file = git_path("%s", delref_name);
+
+ int ret = lstat(delref_file, st);
+
+ free(delref_name);
+ return ret;
+}
+
+static char *get_del_ref_file(const char *ref_name)
+{
+ char *delref_name = get_del_ref_name(ref_name);
+ char *delref_file = xstrdup(git_path("%s", delref_name));
+
+ free(delref_name);
+ return delref_file;
+}
+
/* We allow "recursive" symbolic refs. Only within reason, though */
#define MAXDEPTH 5
@@ -204,6 +236,17 @@ const char *resolve_ref(const char *ref,
* born. It is NOT OK if we are resolving for
* reading.
*/
+
+ if (!lstat_del_ref_file(ref, &st)) {
+ /* The ref has been deleted. */
+ if (flag)
+ *flag |= REF_ISDELETED;
+ if (reading)
+ return NULL;
+ hashclr(sha1);
+ return ref;
+ }
+
if (lstat(path, &st) < 0) {
struct ref_list *list = get_packed_refs();
while (list) {
@@ -541,12 +584,14 @@ static struct ref_lock *lock_ref_sha1_ba
struct ref_lock *lock;
struct stat st;
int last_errno = 0;
+ int default_flag;
+ int *ref_flag = flag ? flag : &default_flag;
int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
lock = xcalloc(1, sizeof(struct ref_lock));
lock->lock_fd = -1;
- ref = resolve_ref(ref, lock->old_sha1, mustexist, flag);
+ ref = resolve_ref(ref, lock->old_sha1, mustexist, ref_flag);
if (!ref && errno == EISDIR) {
/* we are trying to lock foo but we used to
* have foo/bar which now does not exist;
@@ -559,7 +604,7 @@ static struct ref_lock *lock_ref_sha1_ba
error("there are still refs under '%s'", orig_ref);
goto error_return;
}
- ref = resolve_ref(orig_ref, lock->old_sha1, mustexist, flag);
+ ref = resolve_ref(orig_ref, lock->old_sha1, mustexist, ref_flag);
}
if (!ref) {
last_errno = errno;
@@ -568,7 +613,8 @@ static struct ref_lock *lock_ref_sha1_ba
goto error_return;
}
if (is_null_sha1(lock->old_sha1)) {
- /* The ref did not exist and we are creating it.
+ /* The ref did not exist (or has been deleted)
+ * and we are (re)creating it.
* Make sure there is no existing ref that is packed
* whose name begins with our refname, nor a ref whose
* name is a proper prefix of our refname.
@@ -580,15 +626,19 @@ static struct ref_lock *lock_ref_sha1_ba
int len = strlen(list->name);
int cmplen = (namlen < len) ? namlen : len;
const char *lead = (namlen < len) ? list->name : ref;
+ struct stat st;
if (!strncmp(ref, list->name, cmplen) &&
- lead[cmplen] == '/') {
+ lead[cmplen] == '/' &&
+ lstat_del_ref_file(list->name, &st)) {
error("'%s' exists; cannot create '%s'",
list->name, ref);
goto error_return;
}
list = list->next;
}
+ if (*ref_flag & REF_ISDELETED)
+ lock->del_file = get_del_ref_file(ref);
}
lock->lk = xcalloc(1, sizeof(struct lock_file));
@@ -627,77 +677,60 @@ struct ref_lock *lock_any_ref_for_update
return lock_ref_sha1_basic(ref, old_sha1, NULL);
}
-static struct lock_file packlock;
-
-static int repack_without_ref(const char *refname)
+/* Remove the ref "refs/$frotz" by creating "deleted-refs/$frotz~ref". */
+int delete_ref(const char *ref_name, unsigned char *sha1)
{
- struct ref_list *list, *packed_ref_list;
- int fd;
- int found = 0;
+ int flag = 0;
+ static char term = '\n';
+ struct ref_lock *lock = lock_ref_sha1_basic(ref_name, sha1, &flag);
- packed_ref_list = get_packed_refs();
- for (list = packed_ref_list; list; list = list->next) {
- if (!strcmp(refname, list->name)) {
- found = 1;
- break;
- }
+ if (!lock)
+ return -1;
+ if (flag & REF_ISDELETED) {
+ unlock_ref(lock);
+ return 0; /* Already deleted is ok. */
}
- if (!found)
- return 0;
- memset(&packlock, 0, sizeof(packlock));
- fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
- if (fd < 0)
- return error("cannot delete '%s' from packed refs", refname);
-
- for (list = packed_ref_list; list; list = list->next) {
- char line[PATH_MAX + 100];
- int len;
-
- if (!strcmp(refname, list->name))
- continue;
- len = snprintf(line, sizeof(line), "%s %s\n",
- sha1_to_hex(list->sha1), list->name);
- /* this should not happen but just being defensive */
- if (len > sizeof(line))
- die("too long a refname '%s'", list->name);
- write_or_die(fd, line, len);
+ if (write(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
+ write(lock->lock_fd, &term, 1) != 1
+ || close(lock->lock_fd) < 0) {
+ error("Couldn't write %s", lock->lk->filename);
+ unlock_ref(lock);
+ return -1;
}
- return commit_lock_file(&packlock);
-}
-
-int delete_ref(const char *refname, unsigned char *sha1)
-{
- struct ref_lock *lock;
- int err, i, ret = 0, flag = 0;
-
- lock = lock_ref_sha1_basic(refname, sha1, &flag);
- if (!lock)
- return 1;
+ invalidate_cached_refs();
if (!(flag & REF_ISPACKED)) {
/* loose */
- i = strlen(lock->lk->filename) - 5; /* .lock */
+ int i = strlen(lock->lk->filename) - 5; /* .lock */
lock->lk->filename[i] = 0;
- err = unlink(lock->lk->filename);
- if (err) {
- ret = 1;
+ if (unlink(lock->lk->filename)) {
error("unlink(%s) failed: %s",
lock->lk->filename, strerror(errno));
+ unlock_ref(lock);
+ return -1;
}
lock->lk->filename[i] = '.';
}
- /* removing the loose one could have resurrected an earlier
- * packed one. Also, if it was not loose we need to repack
- * without it.
- */
- ret |= repack_without_ref(refname);
-
- err = unlink(lock->log_file);
- if (err && errno != ENOENT)
+ if (unlink(lock->log_file) && errno != ENOENT)
fprintf(stderr, "warning: unlink(%s) failed: %s",
lock->log_file, strerror(errno));
- invalidate_cached_refs();
+ if (!lock->del_file)
+ lock->del_file = get_del_ref_file(ref_name);
+ if (safe_create_leading_directories(lock->del_file)) {
+ error("unable to create directory for %s: %s",
+ lock->del_file, strerror(errno));
+ unlock_ref(lock);
+ return -1;
+ }
+ if (rename(lock->lk->filename, lock->del_file)) {
+ error("rename(%s -> %s) failed: %s",
+ lock->lk->filename, lock->del_file,
+ strerror(errno));
+ unlock_ref(lock);
+ return -1;
+ }
+ lock->lock_fd = -1;
unlock_ref(lock);
- return ret;
+ return 0;
}
void unlock_ref(struct ref_lock *lock)
@@ -710,6 +743,7 @@ void unlock_ref(struct ref_lock *lock)
}
free(lock->ref_name);
free(lock->log_file);
+ free(lock->del_file);
free(lock);
}
@@ -786,6 +820,12 @@ int write_ref_sha1(struct ref_lock *lock
unlock_ref(lock);
return -1;
}
+ if (lock->del_file && unlink(lock->del_file) && errno != ENOENT) {
+ error("unlink(%s) failed: %s",
+ lock->del_file, strerror(errno));
+ unlock_ref(lock);
+ return -1;
+ }
if (commit_lock_file(lock->lk)) {
error("Couldn't set %s", lock->ref_name);
unlock_ref(lock);
diff --git a/refs.h b/refs.h
index a57d437..5aa7755 100644
--- a/refs.h
+++ b/refs.h
@@ -4,6 +4,7 @@ #define REFS_H
struct ref_lock {
char *ref_name;
char *log_file;
+ char *del_file;
struct lock_file *lk;
unsigned char old_sha1[20];
int lock_fd;
@@ -16,6 +17,7 @@ struct ref_lock {
*/
#define REF_ISSYMREF 01
#define REF_ISPACKED 02
+#define REF_ISDELETED 04
typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int flags, void *cb_data);
extern int head_ref(each_ref_fn, void *);
extern int for_each_ref(each_ref_fn, void *);
--
1.4.3.rc2.gbcf275-dirty
^ permalink raw reply related
* RE: git-svn and u-boot broken.
From: Joakim Tjernlund @ 2006-10-14 13:13 UTC (permalink / raw)
To: 'Eric Wong'; +Cc: 'Randal L. Schwartz', git
> > Also, which version of git-svn are you running? Also, which
> > version of
> > Subversion (client library, and server (if possible) are
> you running?
>
> git 1.4.2.3
> subversion 1.3.2-r1
>
> I will try upgrading to newer subverison.
subversion 1.4.0 didn't help
PS.
I can't find the dcommit command in git-cvs, has it been removed?
if so, then the docs need an update.
^ permalink raw reply
* [PATCH] clone: the given repository dir should be relative to $PWD
From: Yasushi SHOJI @ 2006-10-14 12:02 UTC (permalink / raw)
To: git
the repository argument for git-clone should be relative to $PWD
instead of the given target directory. The old behavior gave us
surprising success and you need a few minute to know why it worked.
GIT_DIR is already exported so no need to cd into $D. And this makes
$PWD for git-fetch-pack, which is the actual command to take the given
repository dir, the same as git-clone.
Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>
---
While I'm not sure this is a feature we rely on or not, and I don't
want to change the way people work, IMHO the old behaviour isn't
appropriate for such higher level porcelain.
The patch should be for post 1.4.3.
git-clone.sh | 2 +-
t/t5600-clone-fail-cleanup.sh | 6 ++++++
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/git-clone.sh b/git-clone.sh
index 3998c55..bf54a11 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -312,7 +312,7 @@ yes,yes)
fi
;;
*)
- cd "$D" && case "$upload_pack" in
+ case "$upload_pack" in
'') git-fetch-pack --all -k $quiet "$repo" ;;
*) git-fetch-pack --all -k $quiet "$upload_pack" "$repo" ;;
esac >"$GIT_DIR/CLONE_HEAD" || {
diff --git a/t/t5600-clone-fail-cleanup.sh b/t/t5600-clone-fail-cleanup.sh
index 0c6a363..041be04 100755
--- a/t/t5600-clone-fail-cleanup.sh
+++ b/t/t5600-clone-fail-cleanup.sh
@@ -25,6 +25,12 @@ test_create_repo foo
# clone doesn't like it if there is no HEAD. Is that a bug?
(cd foo && touch file && git add file && git commit -m 'add file' >/dev/null 2>&1)
+# source repository given to git-clone should be relative to the
+# current path not to the target dir
+test_expect_failure \
+ 'clone of non-existent (relative to $PWD) source should fail' \
+ 'git-clone ../foo baz'
+
test_expect_success \
'clone should work now that source exists' \
'git-clone foo bar'
--
1.4.2.3
^ permalink raw reply related
* [PATCH 1/2] Add hash_sha1_file()
From: Rene Scharfe @ 2006-10-14 10:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
Most callers of write_sha1_file_prepare() are only interested in the
resulting hash but don't care about the returned file name or the header.
This patch adds a simple wrapper named hash_sha1_file() which does just
that, and converts potential callers.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
builtin-apply.c | 8 ++------
cache-tree.c | 8 ++------
cache.h | 1 +
merge-recursive.c | 5 +----
sha1_file.c | 34 ++++++++++++++++------------------
5 files changed, 22 insertions(+), 34 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index e3ef044..cbe5977 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1783,8 +1783,6 @@ static int apply_binary(struct buffer_de
{
const char *name = patch->old_name ? patch->old_name : patch->new_name;
unsigned char sha1[20];
- unsigned char hdr[50];
- int hdrlen;
/* For safety, we require patch index line to contain
* full 40-byte textual SHA1 for old and new, at least for now.
@@ -1800,8 +1798,7 @@ static int apply_binary(struct buffer_de
/* See if the old one matches what the patch
* applies to.
*/
- write_sha1_file_prepare(desc->buffer, desc->size,
- blob_type, sha1, hdr, &hdrlen);
+ hash_sha1_file(desc->buffer, desc->size, blob_type, sha1);
if (strcmp(sha1_to_hex(sha1), patch->old_sha1_prefix))
return error("the patch applies to '%s' (%s), "
"which does not match the "
@@ -1846,8 +1843,7 @@ static int apply_binary(struct buffer_de
name);
/* verify that the result matches */
- write_sha1_file_prepare(desc->buffer, desc->size, blob_type,
- sha1, hdr, &hdrlen);
+ hash_sha1_file(desc->buffer, desc->size, blob_type, sha1);
if (strcmp(sha1_to_hex(sha1), patch->new_sha1_prefix))
return error("binary patch to '%s' creates incorrect result (expecting %s, got %s)", name, patch->new_sha1_prefix, sha1_to_hex(sha1));
}
diff --git a/cache-tree.c b/cache-tree.c
index 323c68a..d388848 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -344,12 +344,8 @@ #if DEBUG
#endif
}
- if (dryrun) {
- unsigned char hdr[200];
- int hdrlen;
- write_sha1_file_prepare(buffer, offset, tree_type, it->sha1,
- hdr, &hdrlen);
- }
+ if (dryrun)
+ hash_sha1_file(buffer, offset, tree_type, it->sha1);
else
write_sha1_file(buffer, offset, tree_type, it->sha1);
free(buffer);
diff --git a/cache.h b/cache.h
index 97debd0..aa3a562 100644
--- a/cache.h
+++ b/cache.h
@@ -245,6 +245,7 @@ char *enter_repo(char *path, int strict)
extern int sha1_object_info(const unsigned char *, char *, unsigned long *);
extern void * unpack_sha1_file(void *map, unsigned long mapsize, char *type, unsigned long *size);
extern void * read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size);
+extern int hash_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *sha1);
extern int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *return_sha1);
extern char *write_sha1_file_prepare(void *buf,
unsigned long len,
diff --git a/merge-recursive.c b/merge-recursive.c
index 611cd95..2ba43ae 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1235,13 +1235,10 @@ int merge(struct commit *h1,
if (merged_common_ancestors == NULL) {
/* if there is no common ancestor, make an empty tree */
struct tree *tree = xcalloc(1, sizeof(struct tree));
- unsigned char hdr[40];
- int hdrlen;
tree->object.parsed = 1;
tree->object.type = OBJ_TREE;
- write_sha1_file_prepare(NULL, 0, tree_type, tree->object.sha1,
- hdr, &hdrlen);
+ hash_sha1_file(NULL, 0, tree_type, tree->object.sha1);
merged_common_ancestors = make_virtual_commit(tree, "ancestor");
}
diff --git a/sha1_file.c b/sha1_file.c
index 27b1ebb..6c64ec4 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1501,6 +1501,15 @@ static void setup_object_header(z_stream
stream->avail_out -= hdr;
}
+int hash_sha1_file(void *buf, unsigned long len, const char *type,
+ unsigned char *sha1)
+{
+ unsigned char hdr[50];
+ int hdrlen;
+ write_sha1_file_prepare(buf, len, type, sha1, hdr, &hdrlen);
+ return 0;
+}
+
int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1)
{
int size;
@@ -1784,8 +1793,6 @@ int index_pipe(unsigned char *sha1, int
unsigned long size = 4096;
char *buf = xmalloc(size);
int ret;
- unsigned char hdr[50];
- int hdrlen;
if (read_pipe(fd, &buf, &size)) {
free(buf);
@@ -1796,10 +1803,8 @@ int index_pipe(unsigned char *sha1, int
type = blob_type;
if (write_object)
ret = write_sha1_file(buf, size, type, sha1);
- else {
- write_sha1_file_prepare(buf, size, type, sha1, hdr, &hdrlen);
- ret = 0;
- }
+ else
+ ret = hash_sha1_file(buf, size, type, sha1);
free(buf);
return ret;
}
@@ -1809,8 +1814,6 @@ int index_fd(unsigned char *sha1, int fd
unsigned long size = st->st_size;
void *buf;
int ret;
- unsigned char hdr[50];
- int hdrlen;
buf = "";
if (size)
@@ -1823,10 +1826,8 @@ int index_fd(unsigned char *sha1, int fd
type = blob_type;
if (write_object)
ret = write_sha1_file(buf, size, type, sha1);
- else {
- write_sha1_file_prepare(buf, size, type, sha1, hdr, &hdrlen);
- ret = 0;
- }
+ else
+ ret = hash_sha1_file(buf, size, type, sha1);
if (size)
munmap(buf, size);
return ret;
@@ -1855,12 +1856,9 @@ int index_path(unsigned char *sha1, cons
return error("readlink(\"%s\"): %s", path,
errstr);
}
- if (!write_object) {
- unsigned char hdr[50];
- int hdrlen;
- write_sha1_file_prepare(target, st->st_size, blob_type,
- sha1, hdr, &hdrlen);
- } else if (write_sha1_file(target, st->st_size, blob_type, sha1))
+ if (!write_object)
+ hash_sha1_file(target, st->st_size, blob_type, sha1);
+ else if (write_sha1_file(target, st->st_size, blob_type, sha1))
return error("%s: failed to insert into database",
path);
free(target);
--
1.4.3.rc2.gdce3
^ permalink raw reply related
* [PATCH 2/2] Make write_sha1_file_prepare() static
From: Rene Scharfe @ 2006-10-14 10:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
There are no callers of write_sha1_file_prepare() left outside of
sha1_file.c, so make it static.
---
cache.h | 6 ------
sha1_file.c | 9 +++------
2 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/cache.h b/cache.h
index aa3a562..c354701 100644
--- a/cache.h
+++ b/cache.h
@@ -247,12 +247,6 @@ extern void * unpack_sha1_file(void *map
extern void * read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size);
extern int hash_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *sha1);
extern int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *return_sha1);
-extern char *write_sha1_file_prepare(void *buf,
- unsigned long len,
- const char *type,
- unsigned char *sha1,
- unsigned char *hdr,
- int *hdrlen);
extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned long size, const char *type);
diff --git a/sha1_file.c b/sha1_file.c
index 6c64ec4..d111be7 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1347,12 +1347,9 @@ void *read_object_with_reference(const u
}
}
-char *write_sha1_file_prepare(void *buf,
- unsigned long len,
- const char *type,
- unsigned char *sha1,
- unsigned char *hdr,
- int *hdrlen)
+static char *write_sha1_file_prepare(void *buf, unsigned long len,
+ const char *type, unsigned char *sha1,
+ unsigned char *hdr, int *hdrlen)
{
SHA_CTX c;
--
1.4.3.rc2.gdce3
^ permalink raw reply related
* Re: On blame/pickaxe
From: Junio C Hamano @ 2006-10-14 10:26 UTC (permalink / raw)
To: git
In-Reply-To: <7vr6xddm7h.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> 4. Passing more blame.
>
> Instead of taking responsibility for the remainder, there are
> other ways to find other people to pass blame on. That's what
> the "NEEDSWORK" comment in pass_blame() is about.
I've spent a few hours tonight to further work (eh, "have fun")
on this. The version at the tip of "pu" implements detection of
a case like this:
> A typical example is a change that moves one static C function
> from lower part of the file to upper part of the same file,
> because you added a new caller in the middle. The path in your
> parent and the path in you would look like this:
>
> parent you
>
> A static foo() {
> B }
> C A
> D B
> E C
> F D
> G ... call foo();
> static foo() { E
> } F
> I G
> J I
>
> With the common part finding code with diff, you will be able to
> pass blames for lines A B C D E F G I J to your parent. You are
> truly guilty for introducing "... call foo();". The problem
> here is that in addition, you will be blamed for the lines that
> implement "static foo() { ... }" at the beginning of your file.
You can use the pickaxe on its source itself, like this:
git pickaxe -n master..pu builtin-pickaxe.c
If you compare this with output from:
git log --pretty=short -p master..pu builtin-pickaxe.c
you will notice the line-movement detection in action.
During the course of development, I had to move quite a few
static functions around so that they are defined before their
first call site. This is partly because I am very bad at
initial planning (who is?) and this still being in experimental
stage I did not bother declaring static functions upfront as
forward declarations.
For example, commit db3f0f2 introduces find_last_in_target()
function, but it was moved up by commit b5c0e4f (near the tip of
"pu"). pickaxe blames the implementation of it to db3f0f20, and
also notices that the bulk of its logic was actually copied from
the implementation of pass_blame_to_parent() function in commit
b14dc9ef (the initial commit that introduced builtin-pickaxe.c).
What _ought_ to come next is to detect line movement across
files, but I'll go to bed for now.
^ permalink raw reply
* RE: git-svn and u-boot broken.
From: Joakim Tjernlund @ 2006-10-14 9:51 UTC (permalink / raw)
To: 'Eric Wong'; +Cc: 'Randal L. Schwartz', git
In-Reply-To: <20061014062812.GC29149@localdomain>
> -----Original Message-----
> From: Eric Wong [mailto:normalperson@yhbt.net]
> Sent: den 14 oktober 2006 08:28
> To: Joakim Tjernlund
> Cc: 'Randal L. Schwartz'; git@vger.kernel.org
> Subject: Re: git-svn and u-boot broken.
>
> Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
> >
> >
> > > -----Original Message-----
> > > From: Randal L. Schwartz [mailto:merlyn@stonehenge.com]
> > > Sent: den 13 oktober 2006 21:40
> > > To: Joakim Tjernlund
> > > Cc: git@vger.kernel.org
> > > Subject: Re: git-svn and u-boot broken.
> > >
> > > >>>>> "Joakim" == Joakim Tjernlund
> > > <joakim.tjernlund@transmode.se> writes:
> > >
> > > Joakim> First, I had to change this (from memory) in git-svn:
> > > Joakim> my $got = SVN::TxDelta::send_stream($fh, @$atd,
> > > $self->{pool});
> > > Joakim> to
> > > Joakim> my $got;
> > > Joakim> if ( $got ) {
> > > Joakim> $got = SVN::TxDelta::send_stream($fh, @$atd,
> > > $self->{pool});
> > > Joakim> } else {
> > > Joakim> $got = $exp
> > > Joakim> }
> > > Joakim> I am no perl programmer so please change as you se fit.
>
> With your correction following merlyn's comments, it could
> work[1]. I'm
> curious as to what change git-svn printed right before
> hitting an error
from memory:
Usage: svn_txdelta_send_stream(x,y,z,m)
> here, and would like to see the files (before and after) that were
> printed by git-svn.
Sorry, don't have that hande ATM
>
> I'm not sure what change would show up in git but cause
> apply_textdelta
> to return undef...
>
> [1] - I am tempted to apply a change based on yours, but this
> could just
> be working around another bug somewhere else (either in git-svn or
> subversion).
>
> > > That doesn't make any sense. You'll never run the if-true
> > > branch there.
> > > The value of $got immediately following "my $got;" is
> always undef.
> >
> > git-pull --squash seems to do what I want but know I get this when
> > committing to svn:
> > git-svn commit remotes/git-svn..svn-branch
> > ...
> > A cpu/mpc824x/drivers/i2c_export.h
> > A cpu/mpc824x/drivers/i2o.h
> > A cpu/mpc824x/drivers/i2o/Makefile
> > A cpu/mpc824x/drivers/i2o/Makefile_pc
> > Cannot allocate memory at /usr/bin/git-svn line 1462
> > 3072 at /usr/bin/git-svn line 547
> >
> main::commit_lib('2ff9bcb7908d2752f643695ab3a28f9ababfafab')
> called at /usr/bin/git-svn line 457
> > main::commit('remotes/git-svn..svn-branch') called
> at /usr/bin/git-svn line 149
>
> How large is the changeset you're committing?
> (git show --stat 2ff9bcb7908d2752f643695ab3a28f9ababfafab)
That gives me a huge listing that ends in:
4306 files changed, 1194819 insertions(+), 0 deletions(-)
got some entries that look like this:
board/MAI/bios_emulator/scitech/makedefs/qnxnto.mk | 157
.../bios_emulator/scitech/makedefs/rules/bc16.mk | 69
.../bios_emulator/scitech/makedefs/rules/bc3.mk
whats the ... for?
>
> I actually have a fair amount of fork() hacks in git-svn to workaround
> leaks in the SVN:: libraries (hard to avoid otherwise in Perl).
> I haven't noticed (or looked for) file-descriptor leakage very heard,
> though.
>
> Also, which version of git-svn are you running? Also, which
> version of
> Subversion (client library, and server (if possible) are you running?
git 1.4.2.3
subversion 1.3.2-r1
I will try upgrading to newer subverison.
distribution: Gentoo
^ permalink raw reply
* [PATCH] git-svn: fix commits over svn+ssh://
From: Eric Wong @ 2006-10-14 9:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, 392702, Eric Wong
Once a get_commit_editor has been called from an SVN session, RA
layer operations are not allowed (well, unless you're using
file:// or http(s)://). So we'll pass an alternate SVN::Ra
object to our editor object for running 'check-path'.
This should fix commits over svnserve (svn:// without ssh, too).
Closes Debian bug #392702, thanks to Pierre Habouzit for
reporting the bug.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
git-svn.perl | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index a128d90..0f968c8 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -536,7 +536,7 @@ sub commit_lib {
$SVN = libsvn_connect($repo);
my $ed = SVN::Git::Editor->new(
{ r => $r_last,
- ra => $SVN,
+ ra => $SVN_LOG,
c => $c,
svn_path => $SVN_PATH
},
@@ -832,7 +832,7 @@ sub commit_diff {
$SVN ||= libsvn_connect($repo);
my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef, 0) : ();
my $ed = SVN::Git::Editor->new({ r => $SVN->get_latest_revnum,
- ra => $SVN, c => $tb,
+ ra => $SVN_LOG, c => $tb,
svn_path => $SVN_PATH
},
$SVN->get_commit_editor($_message,
--
1.4.3.rc2.g1cbb0
^ permalink raw reply related
* Re: git-svn and u-boot broken.
From: Eric Wong @ 2006-10-14 6:28 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: 'Randal L. Schwartz', git
In-Reply-To: <00e301c6ef2f$16993ac0$1267a8c0@Jocke>
Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
>
>
> > -----Original Message-----
> > From: Randal L. Schwartz [mailto:merlyn@stonehenge.com]
> > Sent: den 13 oktober 2006 21:40
> > To: Joakim Tjernlund
> > Cc: git@vger.kernel.org
> > Subject: Re: git-svn and u-boot broken.
> >
> > >>>>> "Joakim" == Joakim Tjernlund
> > <joakim.tjernlund@transmode.se> writes:
> >
> > Joakim> First, I had to change this (from memory) in git-svn:
> > Joakim> my $got = SVN::TxDelta::send_stream($fh, @$atd,
> > $self->{pool});
> > Joakim> to
> > Joakim> my $got;
> > Joakim> if ( $got ) {
> > Joakim> $got = SVN::TxDelta::send_stream($fh, @$atd,
> > $self->{pool});
> > Joakim> } else {
> > Joakim> $got = $exp
> > Joakim> }
> > Joakim> I am no perl programmer so please change as you se fit.
With your correction following merlyn's comments, it could work[1]. I'm
curious as to what change git-svn printed right before hitting an error
here, and would like to see the files (before and after) that were
printed by git-svn.
I'm not sure what change would show up in git but cause apply_textdelta
to return undef...
[1] - I am tempted to apply a change based on yours, but this could just
be working around another bug somewhere else (either in git-svn or
subversion).
> > That doesn't make any sense. You'll never run the if-true
> > branch there.
> > The value of $got immediately following "my $got;" is always undef.
>
> git-pull --squash seems to do what I want but know I get this when
> committing to svn:
> git-svn commit remotes/git-svn..svn-branch
> ...
> A cpu/mpc824x/drivers/i2c_export.h
> A cpu/mpc824x/drivers/i2o.h
> A cpu/mpc824x/drivers/i2o/Makefile
> A cpu/mpc824x/drivers/i2o/Makefile_pc
> Cannot allocate memory at /usr/bin/git-svn line 1462
> 3072 at /usr/bin/git-svn line 547
> main::commit_lib('2ff9bcb7908d2752f643695ab3a28f9ababfafab') called at /usr/bin/git-svn line 457
> main::commit('remotes/git-svn..svn-branch') called at /usr/bin/git-svn line 149
How large is the changeset you're committing?
(git show --stat 2ff9bcb7908d2752f643695ab3a28f9ababfafab)
I actually have a fair amount of fork() hacks in git-svn to workaround
leaks in the SVN:: libraries (hard to avoid otherwise in Perl).
I haven't noticed (or looked for) file-descriptor leakage very heard,
though.
Also, which version of git-svn are you running? Also, which version of
Subversion (client library, and server (if possible) are you running?
There seems to be a lot of new bugreports to git-svn lately and very
little has changed in git-svn the past few months...
--
Eric Wong
^ permalink raw reply
* RE: git-svn and u-boot broken.
From: Joakim Tjernlund @ 2006-10-14 1:21 UTC (permalink / raw)
To: 'Randal L. Schwartz'; +Cc: git
In-Reply-To: <86slhsgg3f.fsf@blue.stonehenge.com>
> -----Original Message-----
> From: Randal L. Schwartz [mailto:merlyn@stonehenge.com]
> Sent: den 13 oktober 2006 21:40
> To: Joakim Tjernlund
> Cc: git@vger.kernel.org
> Subject: Re: git-svn and u-boot broken.
>
> >>>>> "Joakim" == Joakim Tjernlund
> <joakim.tjernlund@transmode.se> writes:
>
> Joakim> First, I had to change this (from memory) in git-svn:
> Joakim> my $got = SVN::TxDelta::send_stream($fh, @$atd,
> $self->{pool});
> Joakim> to
> Joakim> my $got;
> Joakim> if ( $got ) {
> Joakim> $got = SVN::TxDelta::send_stream($fh, @$atd,
> $self->{pool});
> Joakim> } else {
> Joakim> $got = $exp
> Joakim> }
> Joakim> I am no perl programmer so please change as you se fit.
>
> That doesn't make any sense. You'll never run the if-true
> branch there.
> The value of $got immediately following "my $got;" is always undef.
git-pull --squash seems to do what I want but know I get this when
committing to svn:
git-svn commit remotes/git-svn..svn-branch
...
A cpu/mpc824x/drivers/i2c_export.h
A cpu/mpc824x/drivers/i2o.h
A cpu/mpc824x/drivers/i2o/Makefile
A cpu/mpc824x/drivers/i2o/Makefile_pc
Cannot allocate memory at /usr/bin/git-svn line 1462
3072 at /usr/bin/git-svn line 547
main::commit_lib('2ff9bcb7908d2752f643695ab3a28f9ababfafab') called at /usr/bin/git-svn line 457
main::commit('remotes/git-svn..svn-branch') called at /usr/bin/git-svn line 149
Jocke
^ permalink raw reply
* Re: [PATCH] git-pickaxe: blame rewritten.
From: Junio C Hamano @ 2006-10-13 22:50 UTC (permalink / raw)
To: ltuikov; +Cc: git
In-Reply-To: <20061013215942.36919.qmail@web31806.mail.mud.yahoo.com>
Luben Tuikov <ltuikov@yahoo.com> writes:
>> If there were a corresponding line
>> in "the parent commit" for that line, we would not have assigned
>> the blame to the commit, but the blame would have been passed
>> down to "the parent commit" already.
>
> Indeed.
So is the topic retracted?
Actually, I kind of know what you want. However, I just do not
think blame is the right place to do that.
Suppose you have three lines with this history (for simplicity
the ancestry is linear, time flows from left to right):
Revisions 0 1 2 3 4 5
line 1 a a a A A A
line 2 b b x y B B
line 3 c C C C C C
and we dig from commit 5. The three lines will get blamed to
commit 3, 4 and 1 respectively.
last change lineno data
3 1 A
4 2 B
1 3 C
You click lineno #2 and what you will see is this:
Revisions 0 1 2 3 4
line 1 a a a A A
line 2 b b x y B
line 3 c C C C C
last change lineno data
3 1 A
4 2 B
1 3 C
In such a case, we already know the current commit we are
looking at (commit 4) is what introduced 'B', so there is no
more digging down from that line, but we would like to peel that
'B' to reveal what was behind it (in this case, 'y'). Clicking
line 1 would take you to commit 3 and that happens to show the
line after the line that had 'A' had 'y', but clicking line 3
would take you to commit 1 and you would miss changes that had
'x' or 'y' on the line before the line that had 'C'. So "click
surrounding ones to see how things changed" is not quite the
right answer to the question "Ok, we know 'B' appeared in the
commit that got blamed. What, if anything, was there instead
before that?"
You would need to diff commit 4 and commit 3 (after all, we know
4 introduced 'B' on line 2 so we know rev 3 is already different
and does not have 'B' there) and guess what's on that line, and
if you have more than one parent for commit 4 you would need to
do that for each parent and show that in some human readable
way. This will be expensive but is a useful thing to do "on
demand".
I think "on demand" is really the key word here. We can spend
extra cycles to find that out when we know we are interested in
what, if any, was there where we have 'B' today. Spending that
extra cycle to all lines in the blamed file will end up wasted
if we are using this for "iteratively dig deeper" in gitweb.
That is why I do not think blame is the right place to do this.
What you can do to improve gitweb is to change the URL that each
lineno on the blame page has, so that when it is pointing at the
current commit (i.e. the commit we are looking at is the one
that was blamed for the line), have it run an equivalent of
"git-diff-tree -m -p commit", and guess what line that line that
did not exist in any of the parent (in our example, line 2 that
had 'B') is in the parent, and show 'git-blame' for that file in
that parent around that line.
^ permalink raw reply
* Re: [PATCH 2/2] git-repack: -b to pass --delta-base-offset
From: Junio C Hamano @ 2006-10-13 22:11 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0610131757050.2435@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
>> So how about
>>
>> [core]
>> repackUseDeltaBase = true
>
> I agree, although "repackUseDeltaBase" is a bit vague as both delta
> types do use a delta base.
True; core.repackUseDeltaBaseOffset that is.
^ permalink raw reply
* Re: [PATCH] git-pickaxe: blame rewritten.
From: Luben Tuikov @ 2006-10-13 21:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7rjc2vg.fsf@assigned-by-dhcp.cox.net>
--- Junio C Hamano <junkio@cox.net> wrote:
> Luben Tuikov <ltuikov@yahoo.com> writes:
>
> > --- Junio C Hamano <junkio@cox.net> wrote:
> >> Luben Tuikov <ltuikov@yahoo.com> writes:
> >>
> >> > Junio, is it possible to also print the "previous" commit?
> >> > I mean, is it tenable to print the commit such that
> >> > a "git-diff C B -- A:file" will give a diff of the block of lines
> >> > we're looking at?
> >>
> >> There is no single "previous" in general. Which side of the
> >> merge would you take?
> >
> > The parent commit.
>
> There is no single "the parent commit" in general. Which side
> of the merge would you take?
Yes, I realise that...
I guess I'm trying to get to a successful implementation of
the intention of commit 65910395c08e3dc4be685a9a9f60adfa61c89aa5
(later reverted for a good reason).
It is ok if this is not possible. After all, the absolutely
unambiguous way is blame->commit->blame->commit->..., etc,
due to multiple parenting.
> Also remeber, when we blame a line to a revision (unless we do
> not limit the blame with v2.6.18.. and --since=2.weeks which
> only git-pickaxe can do), the line is known to have been
> introduced by _that_ commit.
That is what we want.
(fully agree with your previous comment that we limit _after_
placing blame on a commit...)
> If there were a corresponding line
> in "the parent commit" for that line, we would not have assigned
> the blame to the commit, but the blame would have been passed
> down to "the parent commit" already.
Indeed.
Luben
^ permalink raw reply
* Re: [PATCH 2/2] git-repack: -b to pass --delta-base-offset
From: Nicolas Pitre @ 2006-10-13 21:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v64endi6x.fsf@assigned-by-dhcp.cox.net>
On Fri, 13 Oct 2006, Junio C Hamano wrote:
> Nicolas Pitre <nico@cam.org> writes:
>
> > On Thu, 12 Oct 2006, Junio C Hamano wrote:
> >
> >> This new option makes the resulting pack express the delta base
> >> with more compact "offset" format.
> >
> > Actually I thought about making it the default whenever git-pack-objects
> > supported it, and use a negative option with git-repack to disable it
> > instead.
> >
> > The fact is that there is little reason for not using delta base offsets
> > in most cases and specifying -b all the time would become more of an
> > annoyance.
> >
> > What do you think?
>
> The only time it matters is if the packs in repository being
> repacked needs to be readable by older git, which I think is
> only when somebody with older git uses commit walkers to
> download the pack into a remote repository to use. Using or not
> using delta-base-offset is tied to each repository, and in a
> sense that is similar to "repository format version".
>
> So how about
>
> [core]
> repackUseDeltaBase = true
I agree, although "repackUseDeltaBase" is a bit vague as both delta
types do use a delta base.
Nicolas
^ permalink raw reply
* Re: [PATCH] git-pickaxe: blame rewritten.
From: Junio C Hamano @ 2006-10-13 21:38 UTC (permalink / raw)
To: Luben Tuikov; +Cc: git
In-Reply-To: <20061013205418.5087.qmail@web31803.mail.mud.yahoo.com>
Luben Tuikov <ltuikov@yahoo.com> writes:
> --- Junio C Hamano <junkio@cox.net> wrote:
>> Luben Tuikov <ltuikov@yahoo.com> writes:
>>
>> > Junio, is it possible to also print the "previous" commit?
>> > I mean, is it tenable to print the commit such that
>> > a "git-diff C B -- A:file" will give a diff of the block of lines
>> > we're looking at?
>>
>> There is no single "previous" in general. Which side of the
>> merge would you take?
>
> The parent commit.
There is no single "the parent commit" in general. Which side
of the merge would you take?
Also remeber, when we blame a line to a revision (unless we do
not limit the blame with v2.6.18.. and --since=2.weeks which
only git-pickaxe can do), the line is known to have been
introduced by _that_ commit. If there were a corresponding line
in "the parent commit" for that line, we would not have assigned
the blame to the commit, but the blame would have been passed
down to "the parent commit" already.
^ 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