* Re: git-p4: Fix sync errors due to new server version
From: Junio C Hamano @ 2010-02-01 20:47 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: David Aguilar, simon, git, pal_engstad
In-Reply-To: <alpine.LFD.2.00.1002011455440.1681@xanadu.home>
Thanks; will queue it to 'master'.
^ permalink raw reply
* Re: Better cooperation between checkouts and stashing
From: Junio C Hamano @ 2010-02-01 20:40 UTC (permalink / raw)
To: Markus Elfring; +Cc: git
In-Reply-To: <4B67227A.7030908@web.de>
Markus Elfring <Markus.Elfring@web.de> writes:
> The content control tool "Git" maintains a single file system view that can be
> actively worked on. It can be switched to different topic branches by the
> command "git checkout". If the current active working copy contains "dirty"
> changes, they need to be stashed away before each switch to a different issue.
> http://ariejan.net/2008/04/23/git-using-the-stash/
>
> I imagine that there are opportunities for further improvements.
> - How do you think about the feature that a checkout performs also a stash
> operation before by default and a stash would offer the option to checkout a
> branch afterwards in one step?
If you are starting from "if your work tree is dirty, you MUST stash
before checking out another branch", the suggestion is understandable.
But the thing is, that starting point is not quite correct. And the end
result is that such a change you are suggesting would inconvenience people
greatly, I am afraid, if not designed carefully (I'll outline at the end).
Checking out another branch (branch switching) is designed to carry your
local modification across with you. This is to allow you to start working
on something, realize that your changes are better suited for another
branch, and at that point after the fact "git checkout" that other branch,
while keeping what you have done so far.
If the original branch you started your work from and the branch you are
checking out have different contents in files you have changed (aka "your
changes conflict at the paths level"), without -m option, "git checkout"
refuses to check out the other branch, because it will need a three-way
merge to carry your changes forward, and you might not be prepared to
resolve conflicts resulting from such a merge.
In practice, however, your changes often don't conflict with the changes
between the branches at the paths level, and "git checkout" lets you carry
your local changes across just fine. I'd say this is the majority of the
case especially for experienced git users, as they tend to commit in
smaller and more logical units than those from other SCM background.
Forcing auto-stash on them would mean they now have to pop the stash,
after checking out the other branch, which is not an improvement for them
(and remember, soon you will be part of "them" after getting used to git).
Doing an auto-pop in addition to your auto-stash "to help them" is even
worse, as you essentially made "git checkout other-branch" to always use
"-m" option.
I actually have explained this at least a few times here in the past:
http://thread.gmane.org/gmane.comp.version-control.git/77700/focus=77708
http://thread.gmane.org/gmane.comp.version-control.git/135661/focus=135663
but I don't see anything that states clearly that "checkout" is designed
to carry your local changes across in any documentation (I gave a cursory
look to the user manual, tutorial and checkout manual page). Probably
"git checkout --help" needs a "Switching branches" section, just like the
planned enhancement for "Detached HEAD" section.
We _can_ start experimenting with an option (similar to "checkout -m"),
which does:
- Internally try 'git checkout other-branch' without disturbing the user
with any error message; if it does not fail due to paths level
conflicts, we have successfully checked out the other branch and we are
done;
- If the above trial would fail for some other reason (perhaps your index
was unmerged), then don't do anything---just fail as before;
- Internally run 'git stash', then run 'git checkout other-branch'. If
either of these steps fail, you are in deep trouble. Design what you
have to do carefully in this case (I won't do that in this message);
- Then finally run 'git stash pop'.
^ permalink raw reply
* Re: [PATCH] fast-import: Stream very large blobs directly to pack
From: Junio C Hamano @ 2010-02-01 20:14 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, Sverre Rabbelier, git, Nicolas Pitre
In-Reply-To: <20100201152826.GE8916@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Junio C Hamano <gitster@pobox.com> wrote:
>> I'll queued this with a result of my conflict resolution to 'pu' for now
>> but please double check after I push it out.
>
> The strtoumax call got messed up. Squash this into your merge:
>
> diff --git a/fast-import.c b/fast-import.c
> index e6ebcf6..9c65a24 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2800,7 +2800,7 @@ static int parse_one_option(const char *option)
> if (!prefixcmp(option, "max-pack-size=")) {
> option_max_pack_size(option + 14);
> } else if (!prefixcmp(option, "big-file-threshold=")) {
> - big_file_threshold = strtoumax(option + 21, NULL, 0) * 1024 * 1024;
> + big_file_threshold = strtoumax(option + 19, NULL, 0) * 1024 * 1024;
> } else if (!prefixcmp(option, "depth=")) {
> option_depth(option + 6);
> } else if (!prefixcmp(option, "active-branches=")) {
Ah, of course. Sorry for not being careful and thanks for pointing it
out.
^ permalink raw reply
* Re: git-p4: Fix sync errors due to new server version
From: Nicolas Pitre @ 2010-02-01 19:57 UTC (permalink / raw)
To: David Aguilar; +Cc: gitster, simon, git, pal_engstad
In-Reply-To: <20100201193851.GA6134@gmail.com>
On Mon, 1 Feb 2010, David Aguilar wrote:
>
> Hi Junio
>
> It looks like we forgot about this patch:
>
> http://article.gmane.org/gmane.comp.version-control.git/137723
>
> The patch looks good and Pal added his sign-off
> (I believe that and not inlining the patch were the original
> setbacks).
>
> Simon, are there any reasons to not apply it?
FWIW the patch looks fine to me. Not that I'm affected by the new
server version, but the patch can't break anything.
Nicolas
^ permalink raw reply
* Re: git-p4: Fix sync errors due to new server version
From: David Aguilar @ 2010-02-01 19:38 UTC (permalink / raw)
To: gitster; +Cc: simon, git, pal_engstad
Hi Junio
It looks like we forgot about this patch:
http://article.gmane.org/gmane.comp.version-control.git/137723
The patch looks good and Pal added his sign-off
(I believe that and not inlining the patch were the original
setbacks).
Simon, are there any reasons to not apply it?
Thanks,
--
David
^ permalink raw reply
* Better cooperation between checkouts and stashing
From: Markus Elfring @ 2010-02-01 18:50 UTC (permalink / raw)
To: git
Hello,
The content control tool "Git" maintains a single file system view that can be
actively worked on. It can be switched to different topic branches by the
command "git checkout". If the current active working copy contains "dirty"
changes, they need to be stashed away before each switch to a different issue.
http://ariejan.net/2008/04/23/git-using-the-stash/
I imagine that there are opportunities for further improvements.
- How do you think about the feature that a checkout performs also a stash
operation before by default and a stash would offer the option to checkout a
branch afterwards in one step?
- Would you like to offer a more powerful filter expression for the command "git
stash list" to make the navigation between the available intermediate work
results easier?
Regards,
Markus
^ permalink raw reply
* Re: pack.packSizeLimit, safety checks
From: Junio C Hamano @ 2010-02-01 18:45 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Sergio, git
In-Reply-To: <alpine.LFD.2.00.1002011240510.1681@xanadu.home>
Nicolas Pitre <nico@fluxnic.net> writes:
> While I share your concern for advance warning and such, I think those
> concerns are worth an effort proportional to the depth of user exposure.
> Like for the THREADED_DELTA_SEARCH case, I'm wondering how much pain if
> at all might be saved with a transition plan vs the cost of maintaining
> that plan and carrying the discrepancy further.
Absolutely. I just wanted to hear that --max-pack-size command line
option is not widely used and it is Ok to change it.
^ permalink raw reply
* Re: master^ is not a local branch -- huh?!?
From: Jay Soffian @ 2010-02-01 18:27 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Junio C Hamano, Steve Diver, Mark Lodato, Sverre Rabbelier,
Git List, Ron1, Jacob Helwig
In-Reply-To: <alpine.LFD.2.00.1002011309020.1681@xanadu.home>
On Mon, Feb 1, 2010 at 1:12 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
>> I thought that a documentation update in this area was already planned?
>
> Jay Soffian (added to CC) agreed to augment the documentation with the
> comprehensive explanation he posted to the list lately.
I talked to him, he didn't forget, and he'll try his best to submit a
patch this week. :-)
j.
^ permalink raw reply
* Re: Speed of conversion from Subversion repository to Git
From: Jacob Helwig @ 2010-02-01 18:17 UTC (permalink / raw)
To: Markus Elfring; +Cc: git
In-Reply-To: <4B6717DF.8020004@web.de>
On Mon, Feb 1, 2010 at 10:05, Markus Elfring <Markus.Elfring@web.de> wrote:
> Hello,
>
> I am trying to convert the YaST Subversion repository into a Git variant on my
> system. Unfortunately, a command like "svn2git
> http://svn.opensuse.org/svn/yast/" does not finish this attempt after 1,5 days
> (> 715:13.24 CPU time) here. I would appreciate suggestions for improvements of
> the execution speed.
> (Renicing of the process "git-svn" does not help to increase its processor
> utilisation in noticeable ways.)
>
> Regards,
> Markus
>
We're in the process of converting to Git at work, and I've found that
svn-all-fast-export was the best option for us. git-svn's behavior of
a branch's entire history every time it found a new branch (even
though it has already seen 3/4 of the revisions involved) caused it to
be far too slow for our needs. Admittedly, this was ~1 year ago that
I tried git-svn to do a test conversion, and things might have
improved since then.
svn-all-fast-export does require access to the raw repository, instead
of just (http|svn|svn+ssh):// access, but it does allow you to split
out one SVN "repository" into multiple Git repositories in one go.
-Jacob
^ permalink raw reply
* Re: master^ is not a local branch -- huh?!?
From: Nicolas Pitre @ 2010-02-01 18:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jay Soffian, Steve Diver, Mark Lodato, Sverre Rabbelier, Git List,
Ron1, Jacob Helwig
In-Reply-To: <7vpr4o3lg9.fsf@alter.siamese.dyndns.org>
On Mon, 1 Feb 2010, Junio C Hamano wrote:
> I do not think we need yet another term "scratchpad" for this, but what is
> important is that both introductory and full documentation explain the
> detached HEAD well.
>
> Currently we say:
>
> Detached HEAD
> -------------
>
> It is sometimes useful to be able to 'checkout' a commit that is
> not at the tip of one of your branches. The most obvious
> example is to check out the commit at a tagged official release
> point, like this:
>
> ------------
> $ git checkout v2.6.18
> ------------
>
> Earlier versions of git did not allow this and asked you to
> create a temporary branch using the `-b` option, but starting from
> version 1.5.0, the above command 'detaches' your HEAD from the
> current branch and directly points at the commit named by the tag
> (`v2.6.18` in the example above).
>
> If read carefully (some may argue that it does not need a very careful
> reading to get it, though), this hints that "detached HEAD" state is a
> substitute for using a temporary branch, but it may not be strong enough.
>
> I thought that a documentation update in this area was already planned?
Jay Soffian (added to CC) agreed to augment the documentation with the
comprehensive explanation he posted to the list lately.
Nicolas
^ permalink raw reply
* [PATCH] Updates for dirty submodules in release notes and user manual
From: Jens Lehmann @ 2010-02-01 18:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaavvi4r5.fsf@alter.siamese.dyndns.org>
In the release notes "git status" was not mentioned, also shortly explain
the "-dirty" output generated by diff.
Added a paragraph to the "Pitfalls with submodules" section in
user-manual.txt describing new and old behavior of "git status" and "git
diff" for dirty submodules.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
Documentation/RelNotes-1.7.0.txt | 12 +++++++-----
Documentation/user-manual.txt | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/Documentation/RelNotes-1.7.0.txt b/Documentation/RelNotes-1.7.0.txt
index 6510d42..323ae54 100644
--- a/Documentation/RelNotes-1.7.0.txt
+++ b/Documentation/RelNotes-1.7.0.txt
@@ -200,11 +200,13 @@ release, unless otherwise noted.
from non-root dir, 2010-01-26) may be merged to older maintenance
branches.
- * When "git diff" is asked to compare the work tree with something,
- it used to consider that a checked-out submodule with uncommitted
- changes is not modified; this could cause people to forget committing
- these changes in the submodule before committing in the superproject.
- It now considers such a change as a modification.
+ * When using "git status" or asking "git diff" to compare the work tree
+ with something, they used to consider that a checked-out submodule with
+ uncommitted changes is not modified; this could cause people to forget
+ committing these changes in the submodule before committing in the
+ superproject. They now consider such a change as a modification and
+ "git diff" will append a "-dirty" to the work tree side when generating
+ patch output or when used with the --submodule option.
--
exec >/var/tmp/1
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 517daca..fe6fb72 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -3640,6 +3640,26 @@ Did you forget to 'git add'?
Unable to checkout '261dfac35cb99d380eb966e102c1197139f7fa24' in submodule path 'a'
-------------------------------------------------
+In older git versions it could be easily forgotten to commit new or modified
+files in a submodule, which silently leads to similar problems as not pushing
+the submodule changes. Starting with git 1.7.0 both "git status" and "git diff"
+in the superproject show submodules as modified when they contain new or
+modified files to protect against accidentally committing such a state. "git
+diff" will also add a "-dirty" to the work tree side when generating patch
+output or used with the --submodule option:
+
+-------------------------------------------------
+$ git diff
+diff --git a/sub b/sub
+--- a/sub
++++ b/sub
+@@ -1 +1 @@
+-Subproject commit 3f356705649b5d566d97ff843cf193359229a453
++Subproject commit 3f356705649b5d566d97ff843cf193359229a453-dirty
+$ git diff --submodule
+Submodule sub 3f35670..3f35670-dirty:
+-------------------------------------------------
+
You also should not rewind branches in a submodule beyond commits that were
ever recorded in any superproject.
--
1.7.0.rc1.156.g05baa
^ permalink raw reply related
* Re: master^ is not a local branch -- huh?!?
From: Sergei Organov @ 2010-02-01 17:58 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
In-Reply-To: <7vpr4o3lg9.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Steve Diver <squelch2@googlemail.com> writes:
[...]
> If read carefully (some may argue that it does not need a very careful
> reading to get it, though), this hints that "detached HEAD" state is a
> substitute for using a temporary branch, but it may not be strong
> enough.
For my rather fresh eye it looks more like unnamed (anonymous?) branch
than a temporary one. Doesn't detached HEAD behave exactly like a
regular HEAD but pointing to the tip of an unnamed branch?
-- Sergei.
^ permalink raw reply
* Speed of conversion from Subversion repository to Git
From: Markus Elfring @ 2010-02-01 18:05 UTC (permalink / raw)
To: git
Hello,
I am trying to convert the YaST Subversion repository into a Git variant on my
system. Unfortunately, a command like "svn2git
http://svn.opensuse.org/svn/yast/" does not finish this attempt after 1,5 days
(> 715:13.24 CPU time) here. I would appreciate suggestions for improvements of
the execution speed.
(Renicing of the process "git-svn" does not help to increase its processor
utilisation in noticeable ways.)
Regards,
Markus
^ permalink raw reply
* Re: pack.packSizeLimit, safety checks
From: Nicolas Pitre @ 2010-02-01 18:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sergio, git
In-Reply-To: <7vvdeg50x4.fsf@alter.siamese.dyndns.org>
On Mon, 1 Feb 2010, Junio C Hamano wrote:
> Nicolas Pitre <nico@fluxnic.net> writes:
>
> > Grrrrr. This is a terrible discrepency given that all the other
> > arguments in Git are always byte based, with the optional k/m/g suffix,
> > by using git_parse_ulong(). So IMHO I'd just change --max-pack-size to
> > be in line with all the rest and have it accept bytes instead of MB.
> > And of course I'd push such a change to be included in v1.7.0 along with
> > the other incompatible fixes.
>
> All of the "other incompatible" changes had ample leading time for
> transition with warnings and all.
>
> I am afraid that doing this "unit change" is way too late for 1.7.0, and
> it makes me somewhat unhappy to hear such a suggestion. It belittles all
> the careful planning that has been done for these other changes to help
> protect the users from transition pain.
>
> Introduce --max-pack-megabytes that is a synonym for --max-pack-size for
> now, and warn when --max-pack-size is used; warn that --max-pack-size will
> count in bytes in 1.8.0. Ship 1.7.0 with that change. --max-pack-bytes
> can also be added if you feel like, while at it.
>
> But changing the unit --max-pack-size counts in to bytes in 1.7.0 feels
> a bit too irresponsible for the existing users.
Thing is... I don't know if the --max-pack-size argument is really that
used. I'd expect people relying on that feature to use the config
variable instead, given that 'git gc' has no idea about --max-pack-size
anyway. People using the --max-pack-size argument directly are probably
doing so only to experiment with it, and then setting the config
variable, probably using the wrong unit. The fact that such a
discrepancy just came to our attention after all the time this feature
has existed is certainly a good indicator of its popularity.
I understand the really unfortunate timing for such a change. OTOH
there is a big advantage to bundle as much incompatibilities together at
the same time, as people will be prepared for such things already.
While I share your concern for advance warning and such, I think those
concerns are worth an effort proportional to the depth of user exposure.
Like for the THREADED_DELTA_SEARCH case, I'm wondering how much pain if
at all might be saved with a transition plan vs the cost of maintaining
that plan and carrying the discrepancy further.
Nicolas
^ permalink raw reply
* Re: Wishlist for branch management
From: H. Peter Anvin @ 2010-02-01 17:49 UTC (permalink / raw)
To: Petr Baudis; +Cc: Git Mailing List
In-Reply-To: <20100201091907.GJ9553@machine.or.cz>
On 02/01/2010 01:19 AM, Petr Baudis wrote:
> I'm used to git symbolic-ref HEAD. I like the fact that human-friendly
> and scripting interfaces are mostly separated to different commands.
It's makes the learning curve for scripting git unnecessarily large.
Furthermore, it's rather unrealistic for people who are not git people
to follow this dictum -- I have seen the git scripts that the kernel
people write, including myself, and they generally tend to use the same
commands they're used to using themselves.
So I think it's actually a really bad UI design. It helps making clear
what is scriptable (stable) and what isn't, but expecting people to use
a different mental rubric when scripting than when working on the
command line is completely unrealistic.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply
* Re: Problem listing GIT repository with accents
From: Jeff King @ 2010-02-01 17:40 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Sixt, Ellié Computing Open Source Program,
Pierre Habouzit, git
In-Reply-To: <7v3a1k50sw.fsf@alter.siamese.dyndns.org>
On Mon, Feb 01, 2010 at 09:21:51AM -0800, Junio C Hamano wrote:
> Unless we document "this function is to C-quote a (portion of a) string,
> either to the end or up to the given length", however, future callers may
> incorrectly assume that with length the function can be fed anything and
> would C-quote that piece of memory. The argument name "const char *name"
> already suggests that is not an arbitrary binary rubbish, changing that to
> "str" would probably make that a bit stronger documentation, or we could
> explicitly say "this is a (early part of a) NUL-terminated string" in a
> comment.
>
> But your one-liner patch would actually be a smaller change than any of
> them and makes the whole problem disappear; wouldn't it be a far better
> solution?
Sure, if you are going to bother to document it to future-proof against
new callers, you might as well just make it more flexible with my
one-liner. I don't think it will impact the behavior of any existing
callers either way.
I was just going to not bother, but perhaps while we have spent some
brain cycles on it, it is better to just fix it. Either way is fine with
me.
-Peff
^ permalink raw reply
* Re: master^ is not a local branch -- huh?!?
From: Junio C Hamano @ 2010-02-01 17:38 UTC (permalink / raw)
To: Steve Diver
Cc: Nicolas Pitre, Mark Lodato, Sverre Rabbelier, Git List, Ron1,
Jacob Helwig
In-Reply-To: <31a97741002010352x1ad27f26ia4d51857bb2d2d4f@mail.gmail.com>
Steve Diver <squelch2@googlemail.com> writes:
> Would it be a safe assumption to describe a 'detached HEAD' state as
> being synonymous with a (local) personal scratchpad or temporary
> workspace based on and from the original committed object?
A commonly used term since we started discussing the detached HEAD late
2006 (v1.5.0 timeframe) is a "temporary branch" or a "throw-away" branch.
See c847f53 (Detached HEAD (experimental), 2007-01-01), for example.
I do not think we need yet another term "scratchpad" for this, but what is
important is that both introductory and full documentation explain the
detached HEAD well.
Currently we say:
Detached HEAD
-------------
It is sometimes useful to be able to 'checkout' a commit that is
not at the tip of one of your branches. The most obvious
example is to check out the commit at a tagged official release
point, like this:
------------
$ git checkout v2.6.18
------------
Earlier versions of git did not allow this and asked you to
create a temporary branch using the `-b` option, but starting from
version 1.5.0, the above command 'detaches' your HEAD from the
current branch and directly points at the commit named by the tag
(`v2.6.18` in the example above).
If read carefully (some may argue that it does not need a very careful
reading to get it, though), this hints that "detached HEAD" state is a
substitute for using a temporary branch, but it may not be strong enough.
I thought that a documentation update in this area was already planned?
^ permalink raw reply
* [PATCH v3] fast-import: Stream very large blobs directly to pack
From: Shawn O. Pearce @ 2010-02-01 17:27 UTC (permalink / raw)
To: git, Junio C Hamano
If a blob is larger than the configured big-file-threshold, instead
of reading it into a single buffer obtained from malloc, stream it
onto the end of the current pack file. Streaming the larger objects
into the pack avoids the 4+ GiB memory footprint that occurs when
fast-import is processing 2+ GiB blobs.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Changed --big-file-threshold option to be in bytes.
Squashed the documentation patch from earlier today.
Documentation/config.txt | 14 +++
Documentation/git-fast-import.txt | 6 ++
fast-import.c | 179 +++++++++++++++++++++++++++++++++----
t/t5705-clone-2gb.sh | 2 +-
t/t9300-fast-import.sh | 46 ++++++++++
5 files changed, 227 insertions(+), 20 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index f7728ec..b16a20b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -409,6 +409,20 @@ You probably do not need to adjust this value.
+
Common unit suffixes of 'k', 'm', or 'g' are supported.
+core.bigFileThreshold::
+ Files larger than this size are stored deflated, without
+ attempting delta compression. Storing large files without
+ delta compression avoids excessive memory usage, at the
+ slight expense of increased disk usage.
++
+Default is 512 MiB on all platforms. This should be reasonable
+for most projects as source code and other text files can still
+be delta compressed, but larger binary media files won't be.
++
+Common unit suffixes of 'k', 'm', or 'g' are supported.
++
+Currently only linkgit:git-fast-import[1] honors this setting.
+
core.excludesfile::
In addition to '.gitignore' (per-directory) and
'.git/info/exclude', git looks into this file for patterns
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index e6d364f..79c5f69 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -50,6 +50,12 @@ OPTIONS
importers may wish to lower this, such as to ensure the
resulting packfiles fit on CDs.
+--big-file-threshold=<n>::
+ Maximum size of a blob that fast-import will attempt to
+ create a delta for, expressed in bytes. The default is 512m
+ (512 MiB). Some importers may wish to lower this on systems
+ with constrained memory.
+
--depth=<n>::
Maximum delta depth, for blob and tree deltification.
Default is 10.
diff --git a/fast-import.c b/fast-import.c
index 60d0aa2..2105310 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -280,6 +280,7 @@ struct recent_command
/* Configured limits on output */
static unsigned long max_depth = 10;
static off_t max_packsize = (1LL << 32) - 1;
+static uintmax_t big_file_threshold = 512 * 1024 * 1024;
static int force_update;
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
static int pack_compression_seen;
@@ -1003,7 +1004,7 @@ static void cycle_packfile(void)
static size_t encode_header(
enum object_type type,
- size_t size,
+ uintmax_t size,
unsigned char *hdr)
{
int n = 1;
@@ -1159,6 +1160,118 @@ static int store_object(
return 0;
}
+static void truncate_pack(off_t to)
+{
+ if (ftruncate(pack_data->pack_fd, to)
+ || lseek(pack_data->pack_fd, to, SEEK_SET) != to)
+ die_errno("cannot truncate pack to skip duplicate");
+ pack_size = to;
+}
+
+static void stream_blob(uintmax_t len, unsigned char *sha1out, uintmax_t mark)
+{
+ size_t in_sz = 64 * 1024, out_sz = 64 * 1024;
+ unsigned char *in_buf = xmalloc(in_sz);
+ unsigned char *out_buf = xmalloc(out_sz);
+ struct object_entry *e;
+ unsigned char sha1[20];
+ unsigned long hdrlen;
+ off_t offset;
+ git_SHA_CTX c;
+ z_stream s;
+ int status = Z_OK;
+
+ /* Determine if we should auto-checkpoint. */
+ if ((pack_size + 60 + len) > max_packsize
+ || (pack_size + 60 + len) < pack_size)
+ cycle_packfile();
+
+ offset = pack_size;
+
+ hdrlen = snprintf((char *)out_buf, out_sz, "blob %" PRIuMAX, len) + 1;
+ if (out_sz <= hdrlen)
+ die("impossibly large object header");
+
+ git_SHA1_Init(&c);
+ git_SHA1_Update(&c, out_buf, hdrlen);
+
+ memset(&s, 0, sizeof(s));
+ deflateInit(&s, pack_compression_level);
+
+ hdrlen = encode_header(OBJ_BLOB, len, out_buf);
+ if (out_sz <= hdrlen)
+ die("impossibly large object header");
+
+ s.next_out = out_buf + hdrlen;
+ s.avail_out = out_sz - hdrlen;
+
+ while (status != Z_STREAM_END) {
+ if (0 < len && !s.avail_in) {
+ size_t cnt = in_sz < len ? in_sz : (size_t)len;
+ size_t n = fread(in_buf, 1, cnt, stdin);
+ if (!n && feof(stdin))
+ die("EOF in data (%" PRIuMAX " bytes remaining)", len);
+
+ git_SHA1_Update(&c, in_buf, n);
+ s.next_in = in_buf;
+ s.avail_in = n;
+ len -= n;
+ }
+
+ status = deflate(&s, len ? 0 : Z_FINISH);
+
+ if (!s.avail_out || status == Z_STREAM_END) {
+ size_t n = s.next_out - out_buf;
+ write_or_die(pack_data->pack_fd, out_buf, n);
+ pack_size += n;
+ s.next_out = out_buf;
+ s.avail_out = out_sz;
+ }
+
+ switch (status) {
+ case Z_OK:
+ case Z_BUF_ERROR:
+ case Z_STREAM_END:
+ continue;
+ default:
+ die("unexpected deflate failure: %d", status);
+ }
+ }
+ deflateEnd(&s);
+ git_SHA1_Final(sha1, &c);
+
+ if (sha1out)
+ hashcpy(sha1out, sha1);
+
+ e = insert_object(sha1);
+
+ if (mark)
+ insert_mark(mark, e);
+
+ if (e->offset) {
+ duplicate_count_by_type[OBJ_BLOB]++;
+ truncate_pack(offset);
+
+ } else if (find_sha1_pack(sha1, packed_git)) {
+ e->type = OBJ_BLOB;
+ e->pack_id = MAX_PACK_ID;
+ e->offset = 1; /* just not zero! */
+ duplicate_count_by_type[OBJ_BLOB]++;
+ truncate_pack(offset);
+
+ } else {
+ e->depth = 0;
+ e->type = OBJ_BLOB;
+ e->pack_id = pack_id;
+ e->offset = offset;
+ object_count++;
+ object_count_by_type[OBJ_BLOB]++;
+ }
+
+ free(in_buf);
+ free(out_buf);
+}
+
/* All calls must be guarded by find_object() or find_mark() to
* ensure the 'struct object_entry' passed was written by this
* process instance. We unpack the entry by the offset, avoiding
@@ -1704,7 +1817,7 @@ static void parse_mark(void)
next_mark = 0;
}
-static void parse_data(struct strbuf *sb)
+static int parse_data(struct strbuf *sb, uintmax_t limit, uintmax_t *len_res)
{
strbuf_reset(sb);
@@ -1728,9 +1841,15 @@ static void parse_data(struct strbuf *sb)
free(term);
}
else {
- size_t n = 0, length;
+ uintmax_t len = strtoumax(command_buf.buf + 5, NULL, 10);
+ size_t n = 0, length = (size_t)len;
- length = strtoul(command_buf.buf + 5, NULL, 10);
+ if (limit && limit < len) {
+ *len_res = len;
+ return 0;
+ }
+ if (length < len)
+ die("data is too large to use in this context");
while (n < length) {
size_t s = strbuf_fread(sb, length - n, stdin);
@@ -1742,6 +1861,7 @@ static void parse_data(struct strbuf *sb)
}
skip_optional_lf();
+ return 1;
}
static int validate_raw_date(const char *src, char *result, int maxlen)
@@ -1806,14 +1926,32 @@ static char *parse_ident(const char *buf)
return ident;
}
-static void parse_new_blob(void)
+static void parse_and_store_blob(
+ struct last_object *last,
+ unsigned char *sha1out,
+ uintmax_t mark)
{
static struct strbuf buf = STRBUF_INIT;
+ uintmax_t len;
+ if (parse_data(&buf, big_file_threshold, &len))
+ store_object(OBJ_BLOB, &buf, last, sha1out, mark);
+ else {
+ if (last) {
+ strbuf_release(&last->data);
+ last->offset = 0;
+ last->depth = 0;
+ }
+ stream_blob(len, sha1out, mark);
+ skip_optional_lf();
+ }
+}
+
+static void parse_new_blob(void)
+{
read_next_command();
parse_mark();
- parse_data(&buf);
- store_object(OBJ_BLOB, &buf, &last_blob, NULL, next_mark);
+ parse_and_store_blob(&last_blob, NULL, next_mark);
}
static void unload_one_branch(void)
@@ -1924,15 +2062,12 @@ static void file_change_m(struct branch *b)
* another repository.
*/
} else if (inline_data) {
- static struct strbuf buf = STRBUF_INIT;
-
if (p != uq.buf) {
strbuf_addstr(&uq, p);
p = uq.buf;
}
read_next_command();
- parse_data(&buf);
- store_object(OBJ_BLOB, &buf, &last_blob, sha1, 0);
+ parse_and_store_blob(&last_blob, sha1, 0);
} else if (oe) {
if (oe->type != OBJ_BLOB)
die("Not a blob (actually a %s): %s",
@@ -2058,15 +2193,12 @@ static void note_change_n(struct branch *b)
die("Invalid ref name or SHA1 expression: %s", p);
if (inline_data) {
- static struct strbuf buf = STRBUF_INIT;
-
if (p != uq.buf) {
strbuf_addstr(&uq, p);
p = uq.buf;
}
read_next_command();
- parse_data(&buf);
- store_object(OBJ_BLOB, &buf, &last_blob, sha1, 0);
+ parse_and_store_blob(&last_blob, sha1, 0);
} else if (oe) {
if (oe->type != OBJ_BLOB)
die("Not a blob (actually a %s): %s",
@@ -2232,7 +2364,7 @@ static void parse_new_commit(void)
}
if (!committer)
die("Expected committer but didn't get one");
- parse_data(&msg);
+ parse_data(&msg, 0, NULL);
read_next_command();
parse_from(b);
merge_list = parse_merge(&merge_count);
@@ -2353,7 +2485,7 @@ static void parse_new_tag(void)
tagger = NULL;
/* tag payload/message */
- parse_data(&msg);
+ parse_data(&msg, 0, NULL);
/* build the tag object */
strbuf_reset(&new_data);
@@ -2473,11 +2605,15 @@ static int git_pack_config(const char *k, const char *v, void *cb)
pack_compression_seen = 1;
return 0;
}
+ if (!strcmp(k, "core.bigfilethreshold")) {
+ long n = git_config_int(k, v);
+ big_file_threshold = 0 < n ? n : 0;
+ }
return git_default_config(k, v, cb);
}
static const char fast_import_usage[] =
-"git fast-import [--date-format=f] [--max-pack-size=n] [--depth=n] [--active-branches=n] [--export-marks=marks.file]";
+"git fast-import [--date-format=f] [--max-pack-size=n] [--big-file-threshold=n] [--depth=n] [--active-branches=n] [--export-marks=marks.file]";
int main(int argc, const char **argv)
{
@@ -2518,7 +2654,12 @@ int main(int argc, const char **argv)
}
else if (!prefixcmp(a, "--max-pack-size="))
max_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;
- else if (!prefixcmp(a, "--depth=")) {
+ else if (!prefixcmp(a, "--big-file-threshold=")) {
+ unsigned long v;
+ if (!git_parse_ulong(a + 21, &v))
+ usage(fast_import_usage);
+ big_file_threshold = v;
+ } else if (!prefixcmp(a, "--depth=")) {
max_depth = strtoul(a + 8, NULL, 0);
if (max_depth > MAX_DEPTH)
die("--depth cannot exceed %u", MAX_DEPTH);
diff --git a/t/t5705-clone-2gb.sh b/t/t5705-clone-2gb.sh
index 9f52154..adfaae8 100755
--- a/t/t5705-clone-2gb.sh
+++ b/t/t5705-clone-2gb.sh
@@ -31,7 +31,7 @@ test_expect_success 'setup' '
echo "data 5" &&
echo ">2gb" &&
cat commit) |
- git fast-import &&
+ git fast-import --big-file-threshold=2 &&
test ! -f exit-status
'
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index b49815d..513db86 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1254,4 +1254,50 @@ test_expect_success \
'Q: verify note for third commit' \
'git cat-file blob refs/notes/foobar:$commit3 >actual && test_cmp expect actual'
+##
+## R: very large blobs
+##
+blobsize=$((2*1024*1024 + 53))
+test-genrandom bar $blobsize >expect
+cat >input <<INPUT_END
+commit refs/heads/big-file
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+R - big file
+COMMIT
+
+M 644 inline big1
+data $blobsize
+INPUT_END
+cat expect >>input
+cat >>input <<INPUT_END
+M 644 inline big2
+data $blobsize
+INPUT_END
+cat expect >>input
+echo >>input
+
+test_expect_success \
+ 'R: blob bigger than threshold' \
+ 'test_create_repo R &&
+ git --git-dir=R/.git fast-import --big-file-threshold=1 <input'
+test_expect_success \
+ 'R: verify created pack' \
+ ': >verify &&
+ for p in R/.git/objects/pack/*.pack;
+ do
+ git verify-pack -v $p >>verify || exit;
+ done'
+test_expect_success \
+ 'R: verify written objects' \
+ 'git --git-dir=R/.git cat-file blob big-file:big1 >actual &&
+ test_cmp expect actual &&
+ a=$(git --git-dir=R/.git rev-parse big-file:big1) &&
+ b=$(git --git-dir=R/.git rev-parse big-file:big2) &&
+ test $a = $b'
+test_expect_success \
+ 'R: blob appears only once' \
+ 'n=$(grep $a verify | wc -l) &&
+ test 1 = $n'
+
test_done
--
1.7.0.rc0.170.g7207c
--
Shawn.
^ permalink raw reply related
* Re: Problem listing GIT repository with accents
From: Junio C Hamano @ 2010-02-01 17:21 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Sixt, Ellié Computing Open Source Program,
Pierre Habouzit, git
In-Reply-To: <20100201134454.GA796@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> And here is what the "quote embedded NULs" patch would look like on top.
> It's actually pretty straightforward, but the more I think of it, the
> more I think it is probably not worthwhile. Not only are we quoting
> paths, which should not have embedded NULs, but it requires that the
> caller always pass the length explicitly, and clearly we are not doing
> that all or even most of the time. So while this would fix the low-level
> "this function quotes an arbitrary string" case, for it to be of any use
> all of the code paths leading to it would need to be audited to handle
> NUL-embedded strings.
Thanks; I think your analysis is very sound.
The current callers do not care (for a good reason). They are dealing
with a pathname, and either they are feeding a string on which there is a
pathname followed by something else and they know where that something
else begins (they give maxlen because they don't want to or cannot NUL
terminate the string in place while calling this function), or they know
they want to quote to the end of the string but they haven't counted how
long it is (they say "I don't care---just quote to the end"). In either
way, they don't expect giving too long a maxlen will go beyond the end of
the string past the terminating NUL.
Unless we document "this function is to C-quote a (portion of a) string,
either to the end or up to the given length", however, future callers may
incorrectly assume that with length the function can be fed anything and
would C-quote that piece of memory. The argument name "const char *name"
already suggests that is not an arbitrary binary rubbish, changing that to
"str" would probably make that a bit stronger documentation, or we could
explicitly say "this is a (early part of a) NUL-terminated string" in a
comment.
But your one-liner patch would actually be a smaller change than any of
them and makes the whole problem disappear; wouldn't it be a far better
solution?
^ permalink raw reply
* Re: pack.packSizeLimit, safety checks
From: Junio C Hamano @ 2010-02-01 17:20 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Johannes Sixt, Nicolas Pitre, Sergio, git
In-Reply-To: <20100201162816.GA9394@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Yup. WTF was I thinking when I did megabytes as the default unit
> on the command line...
That thing is new, so it is worth fixing before 1.7.0 final.
Please make it so.
Thanks.
^ permalink raw reply
* Re: pack.packSizeLimit, safety checks
From: Junio C Hamano @ 2010-02-01 17:19 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Sergio, git
In-Reply-To: <alpine.LFD.2.00.1002011100550.1681@xanadu.home>
Nicolas Pitre <nico@fluxnic.net> writes:
> Grrrrr. This is a terrible discrepency given that all the other
> arguments in Git are always byte based, with the optional k/m/g suffix,
> by using git_parse_ulong(). So IMHO I'd just change --max-pack-size to
> be in line with all the rest and have it accept bytes instead of MB.
> And of course I'd push such a change to be included in v1.7.0 along with
> the other incompatible fixes.
All of the "other incompatible" changes had ample leading time for
transition with warnings and all.
I am afraid that doing this "unit change" is way too late for 1.7.0, and
it makes me somewhat unhappy to hear such a suggestion. It belittles all
the careful planning that has been done for these other changes to help
protect the users from transition pain.
Introduce --max-pack-megabytes that is a synonym for --max-pack-size for
now, and warn when --max-pack-size is used; warn that --max-pack-size will
count in bytes in 1.8.0. Ship 1.7.0 with that change. --max-pack-bytes
can also be added if you feel like, while at it.
But changing the unit --max-pack-size counts in to bytes in 1.7.0 feels
a bit too irresponsible for the existing users.
^ permalink raw reply
* Re: [PATCH] git-svn: persistent memoization
From: Andrew Myrick @ 2010-02-01 16:44 UTC (permalink / raw)
To: Eric Wong; +Cc: git, sam
In-Reply-To: <20100201040312.GA26199@dcvr.yhbt.net>
On Jan 31, 2010, at 8:03 PM, Eric Wong wrote:
> Andrew Myrick <amyrick@apple.com> wrote:
>> Make memoization of the svn:mergeinfo processing functions persistent with
>> Memoize::Storable so that the memoization tables don't need to be regenerated
>> every time the user runs git-svn fetch.
>>
>> The Memoize::Storable hashes are stored in ENV{GIT_DIR}/svn/caches.
>
> Hi Andrew,
>
> Perhaps "$ENV{GIT_DIR}/svn/.caches" is better here since older versions
> of git svn used "$ENV{GIT_DIR}/svn/$refname" in the top-level and
> "caches" may conflict with existing repos.
>
>> -use File::Path qw/mkpath/;
>> +use File::Path qw/mkpath make_path/;
>
> File::Path::make_path is very recent not in Perls distributed by most
> vendors. My 5.10.0 installation (Debian stable) doesn't have it, and I
> also don't see a good reason to use it over the traditional mkpath.
>
> I think I'll squash the following patch and Ack. Let me know if
> you have any objections, thanks.!
> (also wraps long lines to 80 chars)
Makes sense to me. Thanks, Eric.
-Andrew
^ permalink raw reply
* Re: [PATCH 0/2] upload-pack: pre- and post- hooks
From: Shawn O. Pearce @ 2010-02-01 16:36 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Arun Raghavan, git
In-Reply-To: <alpine.LFD.2.00.1002011116320.1681@xanadu.home>
Nicolas Pitre <nico@fluxnic.net> wrote:
> On Mon, 1 Feb 2010, Shawn O. Pearce wrote:
> I think such hooks could be allowed only if triggered explicitly by the
> upload-pack caller, such as git-daemon. That's probably the only
> scenario where a useful use case can be justified for them anyway.
>
> And of course, to avoid any security problems, the actual hooks must not
> be provided by the repository owner but provided externally, like from
> git-daemon, via some upload-pack command line arguments. This way the
> hooks are really controlled by the system administrator managing
> git-daemon and not by any random git repository owner.
>
> That should be good enough for all the use cases those hooks were
> originally designed for.
Oooh, I like that.
If the paths to the hooks are passed in on the command line of
git-upload-pack, and git-daemon takes those options and passes
them through, you're right, we probably get everything we need.
Gitosis can still use the hooks if it wants, since it controls
the call of git-upload-pack.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 0/2] upload-pack: pre- and post- hooks
From: Nicolas Pitre @ 2010-02-01 16:30 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Arun Raghavan, git
In-Reply-To: <20100201152010.GC8916@spearce.org>
On Mon, 1 Feb 2010, Shawn O. Pearce wrote:
> Arun Raghavan <ford_prefect@gentoo.org> wrote:
> > This patch set reintroduces the post-upload-pack hook and adds a
> > pre-upload-pack hook. These are now only built if 'ALLOW_INSECURE_HOOKS' is set
> > at build time. The idea is that only system administrators who need this
> > functionality and are sure the potential insecurity is not relevant to their
> > system will enable it.
>
> *sigh*
>
> I guess this is better, having it off by default, but allowing an
> administrator who needs this feature to build a custom package.
>
> Unfortunately... I'm sure some distro out there is going to think
> they know how to compile Git better than we do, and enable this by
> default, exposing their users to a security hole. Ask the OpenSSL
> project about how well distros package code... :-\
>
> I'd like a bit more than just a compile time flag.
I think such hooks could be allowed only if triggered explicitly by the
upload-pack caller, such as git-daemon. That's probably the only
scenario where a useful use case can be justified for them anyway.
And of course, to avoid any security problems, the actual hooks must not
be provided by the repository owner but provided externally, like from
git-daemon, via some upload-pack command line arguments. This way the
hooks are really controlled by the system administrator managing
git-daemon and not by any random git repository owner.
That should be good enough for all the use cases those hooks were
originally designed for.
Nicolas
^ permalink raw reply
* Re: pack.packSizeLimit, safety checks
From: Shawn O. Pearce @ 2010-02-01 16:28 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Nicolas Pitre, Sergio, git
In-Reply-To: <4B6700CF.1090106@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> wrote:
> Nicolas Pitre schrieb:
> > Grrrrr. This is a terrible discrepency given that all the other
> > arguments in Git are always byte based, with the optional k/m/g suffix,
> > by using git_parse_ulong(). So IMHO I'd just change --max-pack-size to
> > be in line with all the rest and have it accept bytes instead of MB.
> > And of course I'd push such a change to be included in v1.7.0 along with
> > the other incompatible fixes.
>
> While at it, also change --big-file-threshold that fast-import learnt the
> other day...
Yup. WTF was I thinking when I did megabytes as the default unit
on the command line...
--
Shawn.
^ 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