* Re: Memory issue with fast-import, why track branches?
From: Shawn O. Pearce @ 2008-12-21 22:17 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git list
In-Reply-To: <94a0d4530812202154l26dfe0dfm49397c63dbfdfdf9@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> wrote:
> I tracked down an issue I have when importing a big repository. For
> some reason memory usage keeps increasing until there is no more
> memory.
>
> After looking at the code my guess is that I have a humongous amount
> of branches.
>
> Actually they are not really branches, but refs. For each git commit
> there's an original mtn ref that I store in 'refs/mtn/sha1', but since
> I'm using 'commit refs/mtn/sha1' to store it, a branch is created for
> every commit.
>
> I guess there are many ways to fix the issue, but for starters I
> wonder why is fast-import keeping track of all the branches? In my
> case I would like fast-import to work exactly the same if I specify
> branches or not (I'll update them later).
Because fast-import has to buffer them until the pack file is done.
The objects aren't available to the repository until after a
checkpoint is sent or until the stream ends. Either way until
then fast-import has to buffer the refs so they don't get exposed
to other git processes reading that same repository, because they
would point to objects that the process cannot find.
I guess it could release the brnach memory after it dumps the
branches in a checkpoint, but its memory allocators work under an
assumption that strings (like branch and file names) will be reused
heavily by the frontend and thus they are poooled inside of a string
pool. The branch objects are also pooled inside of a common alloc
pool, to ammortize the cost of malloc's block headers out over the
data used.
IOW, fast-import was designed for ~5k branches, not ~1 million
unique branches.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-21 22:17 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.; +Cc: Robin Rosenberg, git
In-Reply-To: <200812211513.26808.bss@iguanasuicide.net>
From: Robin Rosenberg <robin.rosenberg.lists@dewire.com>
Subject: git-revert: record the parent against which a revert was made
As described in Documentation/howto/revert-a-faulty-merge.txt, re-merging
from a previously reverted a merge of a side branch may need a revert of
the revert beforehand. Record against which parent the revert was made in
the commit, so that later the user can figure out what went on.
[jc: original had the logic in the message reversed, so I tweaked it.]
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
"Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> I still think the parent we are reverting to should be mentioned in the
> automatically generated commit message, even if it is the first parent. Even
> if it is decided to elide that information for the first parent, I agree
> that, at least for now, the "-m" should still be required when reverting a
> merge commit.
Ok, so here is Robin's patch with a bit of rewording. I want to have
something usable now, so that I can tag -rc4 and still have time left
for sipping my Caipirinha in the evening ;-)
I think we later _could_ use this information inside ancestry traversal
made while computing the merge base in such a way to eliminate the
necessity of the "revert of the revert". When we see a message that
records a revert of a merge, we keep a mental note of it, and when we
encounter such a merge during the ancestry traversal, we pretend as if
the merge never happened (i.e. instead we traverse only the named
parent).
But that needs more thought, and we do not have to do that now.
builtin-revert.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git c/builtin-revert.c w/builtin-revert.c
index 4038b41..fae0fe8 100644
--- c/builtin-revert.c
+++ w/builtin-revert.c
@@ -352,6 +352,11 @@ static int revert_or_cherry_pick(int argc, const char **argv)
add_to_msg(oneline_body + 1);
add_to_msg("\"\n\nThis reverts commit ");
add_to_msg(sha1_to_hex(commit->object.sha1));
+
+ if (commit->parents->next) {
+ add_to_msg(",\nreverting damages made to %s");
+ add_to_msg(sha1_to_hex(parent->object.sha1));
+ }
add_to_msg(".\n");
} else {
base = parent;
^ permalink raw reply related
* Re: [PATCH] Style changes per suggestions from Junio in #git
From: Shawn O. Pearce @ 2008-12-21 22:22 UTC (permalink / raw)
To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1229895454-19498-3-git-send-email-tyler@slide.com>
"R. Tyler Ballance" <tyler@slide.com> wrote:
> Moving includes into git-compat-util.h, move away from malloc(2)
Obviously this should just be squashed into the prior patch.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 2/2] fast-import: add special '-' blob reference to use the previous one.
From: Junio C Hamano @ 2008-12-21 22:24 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Felipe Contreras, git
In-Reply-To: <20081221221149.GB17355@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Hmph, so if create a new path with a blob of "-" the repository
> will be corrupt because the zero id was used and error was produced.
>
> Actually I think you have the same bug in the prior patch with the
> mode being inherited. I wonder if we shouldn't put error checking
> in too to validate that versions[0] describes a file entry.
Why are these patches necessary?
The proposed commit message describes what it does, but does not give hint
to even guess being able to use this new feature helps in what situation.
As far as I can see, these changes allow the exporter to say "this aspect
of the new data is the same as the previous one", but I thought that the
way in which fast-import works already revolves around "you have this
tree, and the next tree is different from it in this and that way." Why
does one need be able to mention "this is the same as the previous one"
explicitly in the first place?
^ permalink raw reply
* Re: [PATCH] Add support for changing packed_git_window_size at process start time
From: Shawn O. Pearce @ 2008-12-21 22:28 UTC (permalink / raw)
To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1229895454-19498-2-git-send-email-tyler@slide.com>
"R. Tyler Ballance" <tyler@slide.com> wrote:
> "Works" insofar that it will alter the packed_git_window_size variable in environment.c
> when the environment is set up. It /doesn't/ work when commands like git-diff(1) and git-log(1)
> call get_revision() which seems to disregard the setting if the packed_window_size is set to something
> low (i.e. ulimit -v 32768)
I think you are tweaking the wrong variable here. Its more than
just the window size that matters to how much of the ulimit we use.
Its also packed_git_limit, which on a 64 bit system is 8 GB.
That's probably why get_revision doesn't seem to honor this as
a setting. Yea, its down to 0.85% of your ulimit per window,
but we'll still try to open a new window because we have space
left before the 8 GB limit.
I think this is a good idea, trying to fit within the ulimit
rather than assuming we can take whatever we please. But you
also need to drop the packed_git_limit down.
My suggestion is this:
packed_git_limit = as->rlim_cur * 0.85;
packed_git_window_size = packed_git_limit / 4;
or maybe / 2. You really want at least 2 windows available within
your limit.
> @@ -75,6 +78,13 @@ static void setup_git_env(void)
> git_graft_file = getenv(GRAFT_ENVIRONMENT);
> if (!git_graft_file)
> git_graft_file = git_pathdup("info/grafts");
> +
> + if (DYNAMIC_WINDOW_SIZE) {
> + struct rlimit *as = malloc(sizeof(struct rlimit));
> + if ( (getrlimit(RLIMIT_AS, as) == 0) && ((int)(as->rlim_cur) > 0) )
> + packed_git_window_size = (unsigned int)(as->rlim_cur * DYNAMIC_WINDOW_SIZE_PERCENTAGE);
> + free(as);
> + }
> }
--
Shawn.
^ permalink raw reply
* Re: [PATCH 2/2] fast-import: add special '-' blob reference to use the previous one.
From: Shawn O. Pearce @ 2008-12-21 22:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Felipe Contreras, git
In-Reply-To: <7vlju9kvyg.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > Hmph, so if create a new path with a blob of "-" the repository
> > will be corrupt because the zero id was used and error was produced.
> >
> > Actually I think you have the same bug in the prior patch with the
> > mode being inherited. I wonder if we shouldn't put error checking
> > in too to validate that versions[0] describes a file entry.
>
> Why are these patches necessary?
>
> The proposed commit message describes what it does, but does not give hint
> to even guess being able to use this new feature helps in what situation.
> As far as I can see, these changes allow the exporter to say "this aspect
> of the new data is the same as the previous one", but I thought that the
> way in which fast-import works already revolves around "you have this
> tree, and the next tree is different from it in this and that way." Why
> does one need be able to mention "this is the same as the previous one"
> explicitly in the first place?
Hmm. Actually, imagine you were dumping from git-diff output style
stream into a fast-import stream.
If a file changes only content, the mode is shown in the index line.
Yay us. But what if the index line wasn't present in the diff? You
don't know the prior mode of the file, but you do have its content.
If a file changes only mode, we get no content hints in the diff.
How do you send that into fast-import without making the frontend
keep track of every path's current mode?
Though I agree, these details should be described in the commit
messages, not left as an exercise for the maintainer to make up.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-21 22:38 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.; +Cc: Robin Rosenberg, git
In-Reply-To: <7vprjlkwbb.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Ok, so here is Robin's patch with a bit of rewording. I want to have
> something usable now, so that I can tag -rc4 and still have time left
> for sipping my Caipirinha in the evening ;-)
> ...
> + add_to_msg(",\nreverting damages made to %s");
> + add_to_msg(sha1_to_hex(parent->object.sha1));
Crap. Scratch that. Obviously I should have done this:
diff --git a/builtin-revert.c b/builtin-revert.c
index 4038b41..c188150 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -352,6 +352,11 @@ static int revert_or_cherry_pick(int argc, const char **argv)
add_to_msg(oneline_body + 1);
add_to_msg("\"\n\nThis reverts commit ");
add_to_msg(sha1_to_hex(commit->object.sha1));
+
+ if (commit->parents->next) {
+ add_to_msg(",\nreverting damages made to ");
+ add_to_msg(sha1_to_hex(parent->object.sha1));
+ }
add_to_msg(".\n");
} else {
base = parent;
--
1.6.1.rc3.72.gf4bf6
^ permalink raw reply related
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Robin Rosenberg @ 2008-12-21 22:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Boyd Stephen Smith Jr., git
In-Reply-To: <7vprjlkwbb.fsf@gitster.siamese.dyndns.org>
söndag 21 december 2008 23:17:12 skrev Junio C Hamano:
> From: Robin Rosenberg <robin.rosenberg.lists@dewire.com>
> Subject: git-revert: record the parent against which a revert was made
>
> As described in Documentation/howto/revert-a-faulty-merge.txt, re-merging
> from a previously reverted a merge of a side branch may need a revert of
> the revert beforehand. Record against which parent the revert was made in
> the commit, so that later the user can figure out what went on.
>
> [jc: original had the logic in the message reversed, so I tweaked it.]
No need for this comment.
> + add_to_msg(",\nreverting damages made to %s");
maybe "changes" is more neutrral language. I also think you break
the line too early.
Are we done now?
-- robin
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-21 22:46 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Boyd Stephen Smith Jr., git
In-Reply-To: <200812212340.46375.robin.rosenberg.lists@dewire.com>
Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:
> söndag 21 december 2008 23:17:12 skrev Junio C Hamano:
>> From: Robin Rosenberg <robin.rosenberg.lists@dewire.com>
>> Subject: git-revert: record the parent against which a revert was made
>>
>> As described in Documentation/howto/revert-a-faulty-merge.txt, re-merging
>> from a previously reverted a merge of a side branch may need a revert of
>> the revert beforehand. Record against which parent the revert was made in
>> the commit, so that later the user can figure out what went on.
>>
>> [jc: original had the logic in the message reversed, so I tweaked it.]
> No need for this comment.
Ok.
>> + add_to_msg(",\nreverting damages made to %s");
> maybe "changes" is more neutrral language. I also think you break
> the line too early.
The above (without %s which shouldn't have been there) would give:
This reverts commit efe05b019ca19328d27c07ef32b4698a7f36166f,
reverting damages made to ec9f0ea3e6ecf1237223dec8428e7bb73d339320.
Do you want:
This reverts commit efe05b019ca19328d27c07ef32b4698a7f36166f, reversing
changes made to ec9f0ea3e6ecf1237223dec8428e7bb73d339320.
this instead?
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Robin Rosenberg @ 2008-12-21 22:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Boyd Stephen Smith Jr., git
In-Reply-To: <7vd4flkuy2.fsf@gitster.siamese.dyndns.org>
söndag 21 december 2008 23:46:45 skrev Junio C Hamano:
> Do you want:
>
> This reverts commit efe05b019ca19328d27c07ef32b4698a7f36166f, reversing
> changes made to ec9f0ea3e6ecf1237223dec8428e7bb73d339320.
Yes, it fills the paragraph nicely. It is a normal text flow after all.
-- robin
^ permalink raw reply
* [Patch] Minor corrections to help & error messages
From: Richard Hartmann @ 2008-12-21 23:53 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 232 bytes --]
Hi all,
the two patches should be so obvious, there is not much
need to explain them.
I noticed that quite a few options in the manpages are
not in alphabetical order. Would a patch which fixes
all of those be accepted?
Richard
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-help-entries-alphabetical.patch --]
[-- Type: text/x-diff; name=0001-Make-help-entries-alphabetical.patch, Size: 1435 bytes --]
From 59b291d79cdcdb5fe235202cb45705dd4f8f352d Mon Sep 17 00:00:00 2001
From: Richard Hartmann <richih.mailinglist@gmail.com>
Date: Sun, 21 Dec 2008 23:28:25 +0100
Subject: [PATCH] Make help entries alphabetical
Switched the order. If there is a need for more changes, I can do them later.
Signed-off-by: Richard Hartmann <richih@net.in.tum.de>
---
Documentation/config.txt | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 21ea165..52786c7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -601,10 +601,6 @@ diff.autorefreshindex::
affects only 'git-diff' Porcelain, and not lower level
'diff' commands, such as 'git-diff-files'.
-diff.suppress-blank-empty::
- A boolean to inhibit the standard behavior of printing a space
- before each empty output line. Defaults to false.
-
diff.external::
If this config variable is set, diff generation is not
performed using the internal diff machinery, but using the
@@ -639,6 +635,10 @@ diff.renames::
will enable basic rename detection. If set to "copies" or
"copy", it will detect copies, as well.
+diff.suppress-blank-empty::
+ A boolean to inhibit the standard behavior of printing a space
+ before each empty output line. Defaults to false.
+
fetch.unpackLimit::
If the number of objects fetched over the git native
transfer is below this
--
1.5.6.5
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Always-show-which-directory-is-not-a-git-repository.patch --]
[-- Type: text/x-diff; name=0002-Always-show-which-directory-is-not-a-git-repository.patch, Size: 2264 bytes --]
From bc332cec43d1a389786c36ee893a0a1d09b112bc Mon Sep 17 00:00:00 2001
From: Richard Hartmann <richih.mailinglist@gmail.com>
Date: Mon, 22 Dec 2008 00:17:32 +0100
Subject: [PATCH] Always show which directory is not a git repository
Unify all
fatal: Not a git repository
error messages so they include path information.
Signed-off-by: Richard Hartmann <richih@net.in.tum.de>
---
contrib/workdir/git-new-workdir | 2 +-
perl/Git.pm | 4 ++--
setup.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
index 7959eab..993cacf 100755
--- a/contrib/workdir/git-new-workdir
+++ b/contrib/workdir/git-new-workdir
@@ -22,7 +22,7 @@ branch=$3
# want to make sure that what is pointed to has a .git directory ...
git_dir=$(cd "$orig_git" 2>/dev/null &&
git rev-parse --git-dir 2>/dev/null) ||
- die "\"$orig_git\" is not a git repository!"
+ die "Not a git repository: \"$orig_git\""
case "$git_dir" in
.git)
diff --git a/perl/Git.pm b/perl/Git.pm
index dde9105..8392a68 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -204,14 +204,14 @@ sub repository {
unless (-d "$dir/refs" and -d "$dir/objects" and -e "$dir/HEAD") {
# Mimick git-rev-parse --git-dir error message:
- throw Error::Simple('fatal: Not a git repository');
+ throw Error::Simple("fatal: Not a git repository: $dir");
}
my $search = Git->repository(Repository => $dir);
try {
$search->command('symbolic-ref', 'HEAD');
} catch Git::Error::Command with {
# Mimick git-rev-parse --git-dir error message:
- throw Error::Simple('fatal: Not a git repository');
+ throw Error::Simple("fatal: Not a git repository: $dir");
}
$opts{Repository} = abs_path($dir);
diff --git a/setup.c b/setup.c
index 833ced2..6b277b6 100644
--- a/setup.c
+++ b/setup.c
@@ -468,7 +468,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
*nongit_ok = 1;
return NULL;
}
- die("Not a git repository");
+ die("Not a git repository (or any of the parent directories): %s", DEFAULT_GIT_DIR_ENVIRONMENT);
}
if (chdir(".."))
die("Cannot change to %s/..: %s", cwd, strerror(errno));
--
1.5.6.5
^ permalink raw reply related
* Re: What's in git.git (Dec 2008, #03; Sun, 21)
From: Paul Mackerras @ 2008-12-21 23:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Johannes Schindelin, Johannes Sixt, Robin Rosenberg,
Boyd Stephen Smith Jr.
In-Reply-To: <7vskohpvj7.fsf@gitster.siamese.dyndns.org>
Junio C Hamano writes:
> * Paul has one patch on his gitk branch I have fetched but not merged yet;
> waiting for his go-ahead.
Sorry. I have now flushed out my queue of pending patches; please
pull.
Thanks,
Paul.
^ permalink raw reply
* Re: [PATCH] gitk: Swap positions of 'next' and 'prev' buttons in the 'Find' section.
From: Paul Mackerras @ 2008-12-21 22:59 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <4948E779.9080909@viscovery.net>
Johannes Sixt writes:
> The button order 'prev' left of 'next' feels more natural than the other
> way round, in particular, compared to the order of the forward and backward
> arrows that are in the line above.
The next/prev labels are a bit awkward in any case, since "next" finds
an earlier commit - one further down the list, and "prev" finds a
later commit - one further back in the list. A better solution might
be to make images for arrows pointing up and down and use compound
buttons so the buttons say Find ^ and Find v (if you image the ^/v as
up and down arrows).
I also find myself often wanting a "Find first" button, i.e. search
starting at the top of the list.
Paul.
^ permalink raw reply
* Re: [PATCHv2] Have manpage reference new documentation on reverting merges.
From: Boyd Stephen Smith Jr. @ 2008-12-22 0:04 UTC (permalink / raw)
To: git; +Cc: Nanako Shiraishi
[-- Attachment #1: Type: text/plain, Size: 966 bytes --]
On Sunday 2008 December 21 15:54:18 you wrote:
> "Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> > I took the alternative approach.
>
> Thanks. I was thinking about doing this instead; how the reference to the
> HOW-TO is done is different, and I am hoping that it would give better
> result for HTML version at least.
I like the different link style (still learning asciidoc here), although I am
mainly concerned with the manpage output.
However, I intentionally avoided using the word "branch" in my prose, because
that implies (to me) that the ref is what is remembered and important.
That's not technically correct, as refs are not part of the history that is
considered during the later merge.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCHv3] Have git revert documentation reference new HOWTO on reverting faulty merges.
From: Boyd Stephen Smith Jr. @ 2008-12-22 0:26 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nanako Shiraishi
In-Reply-To: <7vy6y9kxdh.fsf@gitster.siamese.dyndns.org>
Signed-off-by: Boyd Stephen Smith Jr <bss@iguanasuicide.net>
---
On Sunday 2008 December 21 15:54:18 Junio C Hamano wrote:
> Thanks. I was thinking about doing this instead; how the reference to the
> HOW-TO is done is different, and I am hoping that it would give better
> result for HTML version at least.
Here's a pruned version of my prose, with your link style. I prefer my prose
because I don't want to use the word "branch", which implies refs having some
important in the later merges, which is not the case from what I understand.
Documentation/git-revert.txt | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index caa0729..ee9cb82 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -44,6 +44,14 @@ OPTIONS
option specifies the parent number (starting from 1) of
the mainline and allows revert to reverse the change
relative to the specified parent.
++
+Reverting a merge commit declares that you will never want the tree changes
+brought in by the merge. As a result, later merges will only bring in tree
+changes introduced by commits that are not ancestors of the revert commit.
+This may or may not be what you want.
++
+See the link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To] for
+more details.
--no-edit::
With this option, 'git-revert' will not start the commit
--
1.6.0.2
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
^ permalink raw reply related
* Re: [PATCH 2/2] fast-import: add special '-' blob reference to use the previous one.
From: Felipe Contreras @ 2008-12-22 2:08 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081221221149.GB17355@spearce.org>
On Mon, Dec 22, 2008 at 12:11 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> @@ -1862,7 +1864,7 @@ static void file_change_m(struct branch *b)
>> const char *endp;
>> struct object_entry *oe = oe;
>> unsigned char sha1[20];
>> - uint16_t mode, inline_data = 0;
>> + uint16_t mode, inline_data = 0, empty_blob = 0;
>
> Its not the empty blob, its the inherited/assumed blob...
Right. I thought: in order to use the inherited blob, you should not
specify any blob (leave it empty, or blank).
But yeah, 'inherited' does the job too.
>> @@ -1893,6 +1895,10 @@ static void file_change_m(struct branch *b)
>> } else if (!prefixcmp(p, "inline")) {
>> inline_data = 1;
>> p += 6;
>> + } else if (!prefixcmp(p, "- ")) {
>> + hashclr(sha1);
>> + empty_blob = 1;
>> + p += 1;
>
> Hmph, so if create a new path with a blob of "-" the repository
> will be corrupt because the zero id was used and error was produced.
>
> Actually I think you have the same bug in the prior patch with the
> mode being inherited. I wonder if we shouldn't put error checking
> in too to validate that versions[0] describes a file entry.
Yes, in my tests I found that issue in the previous patch and I have a
fix for that (set a default mode), but I haven't fixed this one. Do
you know what should be the behavior? I think it should 'die'.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/2] fast-import: add special '-' blob reference to use the previous one.
From: Felipe Contreras @ 2008-12-22 2:10 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20081221223335.GF17355@spearce.org>
On Mon, Dec 22, 2008 at 12:33 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>>
>> > Hmph, so if create a new path with a blob of "-" the repository
>> > will be corrupt because the zero id was used and error was produced.
>> >
>> > Actually I think you have the same bug in the prior patch with the
>> > mode being inherited. I wonder if we shouldn't put error checking
>> > in too to validate that versions[0] describes a file entry.
>>
>> Why are these patches necessary?
Yeah, I realized I didn't explain that after sending the patches.
>> The proposed commit message describes what it does, but does not give hint
>> to even guess being able to use this new feature helps in what situation.
>> As far as I can see, these changes allow the exporter to say "this aspect
>> of the new data is the same as the previous one", but I thought that the
>> way in which fast-import works already revolves around "you have this
>> tree, and the next tree is different from it in this and that way." Why
>> does one need be able to mention "this is the same as the previous one"
>> explicitly in the first place?
>
> Hmm. Actually, imagine you were dumping from git-diff output style
> stream into a fast-import stream.
>
> If a file changes only content, the mode is shown in the index line.
> Yay us. But what if the index line wasn't present in the diff? You
> don't know the prior mode of the file, but you do have its content.
>
> If a file changes only mode, we get no content hints in the diff.
> How do you send that into fast-import without making the frontend
> keep track of every path's current mode?
>
> Though I agree, these details should be described in the commit
> messages, not left as an exercise for the maintainer to make up.
Exactly. That's what happens with monotone; you usually have the
contents or the new mode, but not both at the same time.
--
Felipe Contreras
^ permalink raw reply
* [PATCH] fast-import: Cleanup mode setting.
From: Felipe Contreras @ 2008-12-22 2:19 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras
"S_IFREG | mode" probably is only required for 0644 and 0755.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
fast-import.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index a6bce66..f0e08ac 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1872,12 +1872,13 @@ static void file_change_m(struct branch *b)
if (!p)
die("Corrupt mode: %s", command_buf.buf);
switch (mode) {
+ case 0644:
+ case 0755:
+ mode |= S_IFREG;
case S_IFREG | 0644:
case S_IFREG | 0755:
case S_IFLNK:
case S_IFGITLINK:
- case 0644:
- case 0755:
/* ok */
break;
default:
@@ -1944,7 +1945,7 @@ static void file_change_m(struct branch *b)
typename(type), command_buf.buf);
}
- tree_content_set(&b->branch_tree, p, sha1, S_IFREG | mode, NULL);
+ tree_content_set(&b->branch_tree, p, sha1, mode, NULL);
}
static void file_change_d(struct branch *b)
--
1.6.0.6.5.ga66c
^ permalink raw reply related
* Re: [PATCH 1/2] fast-import: add special mode; copy from parent.
From: Felipe Contreras @ 2008-12-22 2:23 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081221220757.GA17355@spearce.org>
On Mon, Dec 22, 2008 at 12:07 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> + if (!prefixcmp(p, "- ")) {
>> + mode = 0;
>> + p += 2;
>
> This part made me wonder, why are we always doing "S_IFREG | mode"
> further down?
My guess is because 0644 and 0755; doing S_IFREG | mode doesn't
achieve anything for the other modes.
I just sent a patch that I hope makes that more visible.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 1/2] fast-import: add special mode; copy from parent.
From: Felipe Contreras @ 2008-12-22 2:25 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras
In-Reply-To: <1229825502-963-1-git-send-email-felipe.contreras@gmail.com>
On Sun, Dec 21, 2008 at 4:11 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> Documentation/git-fast-import.txt | 1 +
> fast-import.c | 41 +++++++++++++++++++++---------------
> 2 files changed, 25 insertions(+), 17 deletions(-)
Please disregard this patch, it doesn't work as I expected. I'll send
an updated one soon.
--
Felipe Contreras
^ permalink raw reply
* Re: Memory issue with fast-import, why track branches?
From: Felipe Contreras @ 2008-12-22 2:36 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git list
In-Reply-To: <20081221221702.GC17355@spearce.org>
On Mon, Dec 22, 2008 at 12:17 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> I tracked down an issue I have when importing a big repository. For
>> some reason memory usage keeps increasing until there is no more
>> memory.
>>
>> After looking at the code my guess is that I have a humongous amount
>> of branches.
>>
>> Actually they are not really branches, but refs. For each git commit
>> there's an original mtn ref that I store in 'refs/mtn/sha1', but since
>> I'm using 'commit refs/mtn/sha1' to store it, a branch is created for
>> every commit.
>>
>> I guess there are many ways to fix the issue, but for starters I
>> wonder why is fast-import keeping track of all the branches? In my
>> case I would like fast-import to work exactly the same if I specify
>> branches or not (I'll update them later).
>
> Because fast-import has to buffer them until the pack file is done.
> The objects aren't available to the repository until after a
> checkpoint is sent or until the stream ends. Either way until
> then fast-import has to buffer the refs so they don't get exposed
> to other git processes reading that same repository, because they
> would point to objects that the process cannot find.
>
> I guess it could release the brnach memory after it dumps the
> branches in a checkpoint, but its memory allocators work under an
> assumption that strings (like branch and file names) will be reused
> heavily by the frontend and thus they are poooled inside of a string
> pool. The branch objects are also pooled inside of a common alloc
> pool, to ammortize the cost of malloc's block headers out over the
> data used.
>
> IOW, fast-import was designed for ~5k branches, not ~1 million
> unique branches.
My point is: why is it not designed for 0 branches? In many places in
the code there's the assumption that the tree = branch, but that's not
always the case. You can specify a 'from sha1' and then the branch
becomes irrelevant.
In fact in monotone some commits are not part of any branch, and many
are part of multiple branches. Those cases can't be handled by
fast-import right now. Not to mention random refs like 'ref/mtn/foo'
which would come in handy for my script.
Now my question is: would it be possible to get rid of the notion of
branches on fast-import and go for refs instead?
On the other hand if branch memory is freed after a checkpoint then
there's no limit to how many 'branches' can be handled.
--
Felipe Contreras
^ permalink raw reply
* [PATCH] bash completion: add 'rename' subcommand to git-remote
From: Markus Heidelberg @ 2008-12-22 2:56 UTC (permalink / raw)
To: gitster; +Cc: git
Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
contrib/completion/git-completion.bash | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 108859e..e0d96f3 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1367,7 +1367,7 @@ _git_config ()
_git_remote ()
{
- local subcommands="add rm show prune update"
+ local subcommands="add rename rm show prune update"
local subcommand="$(__git_find_subcommand "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
@@ -1375,7 +1375,7 @@ _git_remote ()
fi
case "$subcommand" in
- rm|show|prune)
+ rename|rm|show|prune)
__gitcomp "$(__git_remotes)"
;;
update)
--
1.6.1.rc3.64.ga0b5a
^ permalink raw reply related
* Re: Git weekly links: 2008-51
From: Felipe Contreras @ 2008-12-22 3:04 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git list
In-Reply-To: <m3ej02d3tq.fsf@localhost.localdomain>
On Sat, Dec 20, 2008 at 9:50 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> "Felipe Contreras" <felipe.contreras@gmail.com> writes:
>
>> This week tortoisegit stole the spotlight. Maybe there weren't many
>> other links, or maybe I failed to notice them. Also, many people liked
>> the comment of Linus Torvalds regarding C++ in git.
>>
>> blog version:
>> http://gitlog.wordpress.com/2008/12/20/git-weekly-links-2008-51/
>>
>> == Articles ==
>>
>> Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
>> Linus Torvalds creates some buzz
>> http://lwn.net/Articles/249460/
>
> A not countered counter:
> C++ is a horrible language
> http://skepticalmethodologist.wordpress.com/2008/12/17/c-is-a-horrible-language/
>
>> == General links ==
>>
>> tortoisegit
>> http://code.google.com/p/tortoisegit/
>
> What about "Git Extensions":
> https://sourceforge.net/projects/gitextensions/
> http://github.com/spdr870/gitextensions/
>
> And "TortoiseGit Challenge":
> http://github.com/blog/256-tortoisegit-challenge
I didn't see many people bookmarking those links, but I added them to
the blog post as your picks.
--
Felipe Contreras
^ permalink raw reply
* Re: git-diff should not fire up $PAGER, period!
From: Miles Bader @ 2008-12-22 3:27 UTC (permalink / raw)
To: jidanni; +Cc: git
In-Reply-To: <8763lixyps.fsf_-_@jidanni.org>
Just (setenv "PAGER" "cat") in .emacs.
[I actually have it set to /bin/cat, not sure if that's meaningful or not.]
-Miles
--
.Numeric stability is probably not all that important when you're guessing.
^ permalink raw reply
* Re: git-diff should not fire up $PAGER, period!
From: Miles Bader @ 2008-12-22 3:28 UTC (permalink / raw)
To: Linus Torvalds; +Cc: jidanni, git
In-Reply-To: <alpine.LFD.2.00.0812171401300.14014@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> And then you have the _gall_ to talk about "unix design"...
Another beer?
-Miles
--
Suburbia: where they tear out the trees and then name streets after them.
^ 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