* [PATCH 2/2] fast-import: fix 'from 0{40}' test
From: Dmitry Ivankov @ 2011-09-18 21:20 UTC (permalink / raw)
To: git
Cc: Jonathan Nieder, Shawn O. Pearce, David Barr, Sverre Rabbelier,
Dmitry Ivankov
In-Reply-To: <1316380846-15845-1-git-send-email-divanorama@gmail.com>
parse_from_existing() has a special case for null_sha1 treating it
as a start of an orphaned branch. It is how null_sha1 parent is
treated in fast-import. For example parse_reset_branch() clears
sha1 of a branch but leaves it in a lookup table.
Looking at parse_from_existing() call sites, we can seen that it is
only called for sha1's that come from get_sha1() or an existing
object. So fast-import internals don't give it null_sha1 explicitly
and the only way for it to appear is direct '0{40}' in the input.
Don't treat null_sha1 as a magic sha1 in parse_from_existing thus
making 'from 0{40}' an invalid input. (Unless there is a commit
object having null_sha1, of course. And object with null_sha1 would
be a lot of trouble in fast-import regardless of this patch.)
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
fast-import.c | 17 ++++++-----------
t/t9300-fast-import.sh | 2 +-
2 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 742e7da..827434a 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2488,18 +2488,13 @@ static void parse_from_commit(struct branch *b, char *buf, unsigned long size)
static void parse_from_existing(struct branch *b)
{
- if (is_null_sha1(b->sha1)) {
- hashclr(b->branch_tree.versions[0].sha1);
- hashclr(b->branch_tree.versions[1].sha1);
- } else {
- unsigned long size;
- char *buf;
+ unsigned long size;
+ char *buf;
- buf = read_object_with_reference(b->sha1,
- commit_type, &size, b->sha1);
- parse_from_commit(b, buf, size);
- free(buf);
- }
+ buf = read_object_with_reference(b->sha1,
+ commit_type, &size, b->sha1);
+ parse_from_commit(b, buf, size);
+ free(buf);
}
static int parse_from(struct branch *b)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 8cc3f16..0784d50 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -381,7 +381,7 @@ data 0
from 0000000000000000000000000000000000000000
INPUT_END
-test_expect_failure 'B: fail on "from 0{40}"' '
+test_expect_success 'B: fail on "from 0{40}"' '
test_must_fail git fast-import <input
'
rm -f .git/objects/pack_* .git/objects/index_*
--
1.7.3.4
^ permalink raw reply related
* [PATCH 1/2] fast-import: add 'commit from 0{40}' failing test
From: Dmitry Ivankov @ 2011-09-18 21:20 UTC (permalink / raw)
To: git
Cc: Jonathan Nieder, Shawn O. Pearce, David Barr, Sverre Rabbelier,
Dmitry Ivankov
In-Reply-To: <1316380846-15845-1-git-send-email-divanorama@gmail.com>
Following shouldn't be allowed, while it is:
commit refs/heads/some
committer ...
data ...
from `null_sha1`
It is treated as if 'from' was omitted. But it is allowed to just
omit 'from' actually. And `null_sha1` being special in fast-import
is an internal implementation detail.
Add a test as described.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
t/t9300-fast-import.sh | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 1a6c066..8cc3f16 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -375,6 +375,18 @@ test_expect_success 'B: fail on invalid branch name "bad[branch]name"' '
rm -f .git/objects/pack_* .git/objects/index_*
cat >input <<INPUT_END
+commit refs/heads/zeromaster
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data 0
+
+from 0000000000000000000000000000000000000000
+INPUT_END
+test_expect_failure 'B: fail on "from 0{40}"' '
+ test_must_fail git fast-import <input
+'
+rm -f .git/objects/pack_* .git/objects/index_*
+
+cat >input <<INPUT_END
commit TEMP_TAG
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
data <<COMMIT
--
1.7.3.4
^ permalink raw reply related
* [PATCH/RFC 0/2] fast-import: commit from null_sha1
From: Dmitry Ivankov @ 2011-09-18 21:20 UTC (permalink / raw)
To: git
Cc: Jonathan Nieder, Shawn O. Pearce, David Barr, Sverre Rabbelier,
Dmitry Ivankov
Not so sure how null_sha1 parent should be treated in fast-import.
Absent parent is represented as null_sha1 to the user in reflog,
but isn't allowed as an argument for porcelain nor shows in most
plumbing commands afaik.
These patches make fast-import treat
commit refs/heads/master
...
from `null_sha1`
like any other missing parent sha1 - reject such input.
Note: if we'll want this input to be valid, some other adjustments
to fast-import logic may be needed for consistency.
Dmitry Ivankov (2):
fast-import: add 'commit from 0{40}' failing test
fast-import: fix 'from 0{40}' test
fast-import.c | 17 ++++++-----------
t/t9300-fast-import.sh | 12 ++++++++++++
2 files changed, 18 insertions(+), 11 deletions(-)
--
1.7.3.4
^ permalink raw reply
* fast-import wishlist (Re: [RFC] fast-import: note deletion command)
From: Jonathan Nieder @ 2011-09-18 21:03 UTC (permalink / raw)
To: Dmitry Ivankov
Cc: Sverre Rabbelier, Git List, David Barr, Shawn O. Pearce,
Thomas Rast, Johan Herland
In-Reply-To: <CA+gfSn9CsL4tz30B62mDzALdyy1RTFiRT4a1zdJ8pR8aTdcpXA@mail.gmail.com>
Dmitry Ivankov wrote:
> On Mon, Sep 19, 2011 at 2:39 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> Is this perhaps a good moment to also think about branch deletion?
It might be a good temporal moment, but it's not quite the right
thread for it. :) If someone wants to maintain a fast-import wishlist
in a wiki somewhere, it sounds like it could be useful to some people
(I prefer to read patches and stories about particular use cases,
myself :)).
>> That came up earlier as well, and thinking about that might give us
>> some insights in how to deal with deletions uniformly.
>
> Also maybe marks deletion, getting mark sha1, resetting a mark with
> explicit sha1. But most probably not tags deletion.
>
> And going much further on commands, following look nice to have:
> - 'ls' storing result to a mark (to allow us not to compute sha1/store
> object if we don't want to)
> - marks namespaces (to keep ls mark separately, mark deletion will
> do too, if it's only a single temporary mark). Maybe like ::nr:mark.
Regards,
Jonathan
^ permalink raw reply
* Re: [RFC] fast-import: note deletion command
From: Dmitry Ivankov @ 2011-09-18 20:59 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Jonathan Nieder, Git List, David Barr, Shawn O. Pearce,
Thomas Rast, Johan Herland
In-Reply-To: <CAGdFq_hpA95Kj4eMr4e1duuXTpr+OkkwF4K5bTapXEi9UjWcSA@mail.gmail.com>
On Mon, Sep 19, 2011 at 2:39 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Heya,
>
>>
>> If I were doing it:
>>
>> - advertise "N 0{40} <commit>" as the historical way to delete a
>> note, and make sure it keeps working
>>
>> - introduce 'ND' or 'notedelete <commit>' as a human-friendly
>> synonym, keeping in mind that NC and NR could be introduced in the
>> future if there is demand for them.
>
> Is this perhaps a good moment to also think about branch deletion?
> That came up earlier as well, and thinking about that might give us
> some insights in how to deal with deletions uniformly.
Also maybe marks deletion, getting mark sha1, resetting a mark with
explicit sha1. But most probably not tags deletion.
And going much further on commands, following look nice to have:
- 'ls' storing result to a mark (to allow us not to compute sha1/store
object if we don't want to)
- marks namespaces (to keep ls mark separately, mark deletion will
do too, if it's only a single temporary mark). Maybe like ::nr:mark.
>
> --
> Cheers,
>
> Sverre Rabbelier
>
^ permalink raw reply
* Re: [RFC] fast-import: note deletion command
From: Sverre Rabbelier @ 2011-09-18 20:39 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Dmitry Ivankov, Git List, David Barr, Shawn O. Pearce,
Thomas Rast, Johan Herland
In-Reply-To: <20110918203506.GG2308@elie>
Heya,
On Sun, Sep 18, 2011 at 22:35, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> ND commit_sha1 # One may think there are also NC, NR, NM
>> N delete commit_sha1
>> # Only :mark, full-40-byte-sha1 and 'inline' are allowed currently.
>> # So no clashes arise, but then one may also want M 'delete' path
>> # command to work too.
>
> If I were doing it:
>
> - advertise "N 0{40} <commit>" as the historical way to delete a
> note, and make sure it keeps working
>
> - introduce 'ND' or 'notedelete <commit>' as a human-friendly
> synonym, keeping in mind that NC and NR could be introduced in the
> future if there is demand for them.
Is this perhaps a good moment to also think about branch deletion?
That came up earlier as well, and thinking about that might give us
some insights in how to deal with deletions uniformly.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [RFC] fast-import: note deletion command
From: Jonathan Nieder @ 2011-09-18 20:35 UTC (permalink / raw)
To: Dmitry Ivankov
Cc: Git List, David Barr, Shawn O. Pearce, Thomas Rast, Johan Herland,
Sverre Rabbelier
In-Reply-To: <CA+gfSn9sdTzQghqQp6hcO-9kN9mPx2JLRig79Rgx2FqGWXXp=A@mail.gmail.com>
(+cc: Sverre for fast-import, Johan for notes, Thomas as a user of
the note import feature, snipping less for their benefit)
Dmitry Ivankov wrote:
> Hi,
>
> fast-import is able to write notes provided a command
> N <note_content_data_specification> <commit_specification>
>
> There is no documented command to delete a note. It can't be
> easily replaced with Delete command on notes tree because
> of notes fanout.
>
> But fast-import interprets 0{40} data_specification as a note
> deletion command. It is consistent with fast-import internals
> where null_sha1 is used for absent/unknown object. But is
> not consistent with file Modify command nor looks like a common
> convention in git ui.
Right.
> So the questions are:
> - should 0{40} not be treated as deletion toggle? Downside is that
> it is used as one in t/t9301-fast-import-notes.sh.
> - how should a documented way to delete notes look like?
> ND commit_sha1 # One may think there are also NC, NR, NM
> N delete commit_sha1
> # Only :mark, full-40-byte-sha1 and 'inline' are allowed currently.
> # So no clashes arise, but then one may also want M 'delete' path
> # command to work too.
If I were doing it:
- advertise "N 0{40} <commit>" as the historical way to delete a
note, and make sure it keeps working
- introduce 'ND' or 'notedelete <commit>' as a human-friendly
synonym, keeping in mind that NC and NR could be introduced in the
future if there is demand for them.
- don't add any new magic "N <magic keyword> <commit>" commands,
since it's hard to know when they will clash with some VCS's
convention for object names.
I imagine there are plenty of other acceptable ways to accomplish
these things, though. :) Thanks for looking into it.
^ permalink raw reply
* [RFC] fast-import: note deletion command
From: Dmitry Ivankov @ 2011-09-18 20:07 UTC (permalink / raw)
To: Git List, Jonathan Nieder, David Barr, Shawn O. Pearce
Hi,
fast-import is able to write notes provided a command
N <note_content_data_specification> <commit_specification>
There is no documented command to delete a note. It can't be
easily replaced with Delete command on notes tree because
of notes fanout.
But fast-import interprets 0{40} data_specification as a note
deletion command. It is consistent with fast-import internals
where null_sha1 is used for absent/unknown object. But is
not consistent with file Modify command nor looks like a common
convention in git ui.
So the questions are:
- should 0{40} not be treated as deletion toggle? Downside is that
it is used as one in t/t9301-fast-import-notes.sh.
- how should a documented way to delete notes look like?
ND commit_sha1 # One may think there are also NC, NR, NM
N delete commit_sha1
# Only :mark, full-40-byte-sha1 and 'inline' are allowed currently.
# So no clashes arise, but then one may also want M 'delete' path
# command to work too.
^ permalink raw reply
* Re: Pull From Mirrored Repository
From: Jean Sacren @ 2011-09-18 20:06 UTC (permalink / raw)
To: git
In-Reply-To: <CAH5451m1WJ0H9G8uHbCqZ5acO-xEynGKjMk=6g75buJy1S7iSw@mail.gmail.com>
From: Andrew Ardill <andrew.ardill@gmail.com>
Date: Mon, 19 Sep 2011 01:15:47 +1000
>
> Am I correct in thinking that you have done the following:
>
> $ git clone original-server/repo.git
> $ cd repo
> $ git pull mirror-server/repo.git
Essentially that's what I did.
> and now you want the remote ref 'origin/master' to point to the same
> thing as 'mirror/master' (assuming that you create the ref for the
> mirror)?
The idea is to update the local tree from multiple sites so long as one
of them is still running. And the update shall make no difference
whether it is from the master site or not.
> This can be done (by editing the file .git/refs/remotes/origin/master
> and specifying the correct sha1) however this is probably not the best
> idea. It is best for git to know the actual state of the remote the
> last time you synced with it so that it can act intelligently the next
> time you sync with it.
It turns out to be more of a bug with git. After a successful pull from
a master or mirror site, the SHA1 shall be updated among all the master
files in .git/refs/remotes/origin or .git/refs/remotes/[WHATEVER_MIRROR]
as the tip of the local tree is updated.
> If you intend to sync with the mirror, you might as well set it as the
> upstream ref of you master branch. That way you will not get the 'Your
> branch is ahead' message when you pull new updates (unless you have
> local changes, of course).
> This can be done by:
> $ git checkout master
> $ git branch --set-upstream mirror
This is a nice hack, but we want all the changes stay with the master
branch. It's easier to use one master branch for tracking all updates
from the upstream.
Thank you for all your help!
--
Jean Sacren
^ permalink raw reply
* Re: [PATCH 3/3] fast-import: rename object_count to pack_object_count
From: Dmitry Ivankov @ 2011-09-18 19:51 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce, David Barr
In-Reply-To: <20110918193205.GF2308@elie>
On Mon, Sep 19, 2011 at 1:32 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Dmitry Ivankov wrote:
>
>> object_count is used to count objects that'll go to the current pack.
>> While object_count_by_* are used to count total amount of objects and
>> are not used to determine if current packfile is empty.
>>
>> Rename (and move declaration) object_count to pack_object_count to
>> avoid possible confusion.
>
> No strong opinion on this one. I guess the important thing is that
> you are moving the declaration to the group of declarations labelled as
>
> /* The .pack file being generated */
>
> . Is it important to rename the variable while at it (which will
> disrupt other patches in flight using that variable if they exist)?
Not that important. Maybe a huge comment will do more and better.
object_count++ still appears near object_count_by_type[type]++, but
hopefully one will look for their declarations and thus avoid the confusion.
--- a/fast-import.c
+++ b/fast-import.c
@@ -290,7 +290,6 @@ static uintmax_t object_count_by_type[1 << TYPE_BITS];
static uintmax_t duplicate_count_by_type[1 << TYPE_BITS];
static uintmax_t delta_count_by_type[1 << TYPE_BITS];
static uintmax_t delta_count_attempts_by_type[1 << TYPE_BITS];
-static unsigned long object_count;
static unsigned long branch_count;
static unsigned long branch_load_count;
static int failure;
@@ -310,8 +309,16 @@ static unsigned int atom_cnt;
static struct atom_str **atom_table;
/* The .pack file being generated */
+/*
+ * objects that are being written to the current pack
+ * all *must* have current pack_id in struct object_entry.
+ * And object_count *must* be a count of object_entry's
+ * having current pack_id. This data is used to create
+ * index file once current pack_file is finished.
+ */
static struct pack_idx_option pack_idx_opts;
static unsigned int pack_id;
+static unsigned long object_count;
static struct sha1file *pack_file;
static struct packed_git *pack_data;
static struct packed_git **all_packs;
^ permalink raw reply
* Re: Recovering from a bad object
From: jonsmirl @ 2011-09-18 19:48 UTC (permalink / raw)
To: weigelt, Git Mailing List
In-Reply-To: <20110918193116.GB6334@nibiru.local>
On Sun, Sep 18, 2011 at 3:31 PM, Enrico Weigelt <weigelt@metux.de> wrote:
> * Junio C Hamano <gitster@pobox.com> wrote:
>
>> > The object is likely at kernel.org.
>> > Can git go fetch it somehow?
>>
>> The protocol is designed specifically to disallow "I guessed that the
>> trade secret is contained within an object whose object name is this,
>> please feed me that object" kind of requests, so in general no.
>
> Maybe the local side (in some special repair mode) could check if
> some of the remote refs (and their referenced graphs) are broken
> and refetch them completely. If the remote side(s) got the objects
> you're missing (visibile to you), you'll be done after that.
>
> For example:
>
> A local branch "foo" is based on origin/master, somewhere in the
> line behind origin/master some objects are broken:
>
> It would find out that origin/master points to broken/missing objects
> and refetch it completely afresh (same as it would if they had never
> been fetched yet).
>
That's describes the lines I was thinking along.
>
> Perhaps it's not that hard to implement: just requires a special
> git-fetch operation mode that ignores locally existing objects,
> and a few lines of shell code that simply fetches the whole remote
> to some temporary namespaces (and drop that when done). Ends up in
> the same traffic as a fresh clone, but at least runs automatically.
>
>
> cu
> --
> ----------------------------------------------------------------------
> Enrico Weigelt, metux IT service -- http://www.metux.de/
>
> phone: +49 36207 519931 email: weigelt@metux.de
> mobile: +49 151 27565287 icq: 210169427 skype: nekrad666
> ----------------------------------------------------------------------
> Embedded-Linux / Portierung / Opensource-QM / Verteilte Systeme
> ----------------------------------------------------------------------
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Recovering from a bad object
From: Enrico Weigelt @ 2011-09-18 19:31 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <7vy61ywjpa.fsf@alter.siamese.dyndns.org>
* Junio C Hamano <gitster@pobox.com> wrote:
> > The object is likely at kernel.org.
> > Can git go fetch it somehow?
>
> The protocol is designed specifically to disallow "I guessed that the
> trade secret is contained within an object whose object name is this,
> please feed me that object" kind of requests, so in general no.
Maybe the local side (in some special repair mode) could check if
some of the remote refs (and their referenced graphs) are broken
and refetch them completely. If the remote side(s) got the objects
you're missing (visibile to you), you'll be done after that.
For example:
A local branch "foo" is based on origin/master, somewhere in the
line behind origin/master some objects are broken:
It would find out that origin/master points to broken/missing objects
and refetch it completely afresh (same as it would if they had never
been fetched yet).
Perhaps it's not that hard to implement: just requires a special
git-fetch operation mode that ignores locally existing objects,
and a few lines of shell code that simply fetches the whole remote
to some temporary namespaces (and drop that when done). Ends up in
the same traffic as a fresh clone, but at least runs automatically.
cu
--
----------------------------------------------------------------------
Enrico Weigelt, metux IT service -- http://www.metux.de/
phone: +49 36207 519931 email: weigelt@metux.de
mobile: +49 151 27565287 icq: 210169427 skype: nekrad666
----------------------------------------------------------------------
Embedded-Linux / Portierung / Opensource-QM / Verteilte Systeme
----------------------------------------------------------------------
^ permalink raw reply
* Re: [PATCH 3/3] fast-import: rename object_count to pack_object_count
From: Jonathan Nieder @ 2011-09-18 19:32 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, Shawn O. Pearce, David Barr
In-Reply-To: <1316372508-7173-4-git-send-email-divanorama@gmail.com>
Dmitry Ivankov wrote:
> object_count is used to count objects that'll go to the current pack.
> While object_count_by_* are used to count total amount of objects and
> are not used to determine if current packfile is empty.
>
> Rename (and move declaration) object_count to pack_object_count to
> avoid possible confusion.
No strong opinion on this one. I guess the important thing is that
you are moving the declaration to the group of declarations labelled as
/* The .pack file being generated */
. Is it important to rename the variable while at it (which will
disrupt other patches in flight using that variable if they exist)?
^ permalink raw reply
* Re: [PATCH 2/3] fast-import: fix corner case for checkpoint
From: Jonathan Nieder @ 2011-09-18 19:28 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, Shawn O. Pearce, David Barr
In-Reply-To: <1316372508-7173-3-git-send-email-divanorama@gmail.com>
Dmitry Ivankov wrote:
> checkpoint command makes fast-import finish current pack and write out
> branches/tags and marks. In case no new objects are added in current
> pack fast-import falls back to no-op. While it is possible that refs
> or marks need to be updated (to point to old objects).
>
> Make fast-import always dump them on checkpoint. But as before do not
> cycle_packfile if there are no objects to write.
Yeah, that would be annoying to run into. Rearranging the description
a little for clarity and brevity:
fast-import: update refs on checkpoint even if there are no new objects
During an import using the fast-import command, it is possible for
no new objects to have been added between two checkpoints requested
with the SIGUSR1 signal or the "checkpoint" command. Even in this
case, fast-import should write out any updated refs and marks to
fulfill the second checkpoint request.
As before, fast-import will not write an empty pack and start a new
one when there are no new objects to write out.
With that change,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: Maint-only commits
From: Enrico Weigelt @ 2011-09-18 19:11 UTC (permalink / raw)
To: git
In-Reply-To: <6416457.30612.1305580526325.JavaMail.root@mail.hq.genarts.com>
* Stephen Bash <bash@genarts.com> wrote:
> Hi all-
>
> In my office we've recently run into three separate fixes
> required on our maintenance branch that should not be
> included in master (our normal workflow is to make changes
> on maint, tag, release, and then merge to master). Normally
> these "maint only" fixes are interspersed with commits that
> should go back into master. In the past the "maint only"
> commits were rare, so I'd carefully use "merge -s ours"
> to avoid including the "maint only" changes in master.
> But now I'm wondering if there's a better process/workflow?
Of course, there is: use topic branches and rebase.
Assuming you've found a bug in maint, which is also still
in master.
#1: for off a topic branch (for that bug) from maint
#2: fix the bug there
#3: rebase to latest maint (if changed meanwhile) and test carefully
#4: (ff-)merge your bugfix branch to maint
#5: rebase bugfix branch to master (maybe incremental, if they
went too far away from another) and test carefully
#6: (ff-)merge bugfix branch to master
#7: drop that topic branch, as you're done now.
cu
--
----------------------------------------------------------------------
Enrico Weigelt, metux IT service -- http://www.metux.de/
phone: +49 36207 519931 email: weigelt@metux.de
mobile: +49 151 27565287 icq: 210169427 skype: nekrad666
----------------------------------------------------------------------
Embedded-Linux / Portierung / Opensource-QM / Verteilte Systeme
----------------------------------------------------------------------
^ permalink raw reply
* Re: [PATCH 1/3] fast-import: die if we produce too many (MAX_PACK_ID) packs
From: Jonathan Nieder @ 2011-09-18 19:17 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, Shawn O. Pearce, David Barr
In-Reply-To: <1316372508-7173-2-git-send-email-divanorama@gmail.com>
Dmitry Ivankov wrote:
> In fast-import pack_id is 16-bit with MAX_PACK_ID reserved to identify
> pre-existing objects. It is unlikely to wrap under reasonable settings
> but still things in fast-import will break once it happens.
>
> Add a check and immediate die() as the simplest reaction to being unable
> to continue the import.
Makes a lot of sense. A few possible minor clarity improvements:
- missing commas after "In fast-import" and before "with MAX_PACK_ID
reserved"
- "pre-existing objects": it would be clearer to say something like
"objects this fast-import process instance did not write out to a
packfile", like the comment before gfi_unpack_entry() does
- I suppose "under reasonable settings" means "with a reasonable
max-pack-size setting"?
- "things will break" is a bit vague.
- "immediate" -> "immediately"
Maybe:
In fast-import, pack_id is a 16-bit unsigned integer, with MAX_PACK_ID
(2^16 - 1) reserved for use by objects that are not in a packfile that
this fast-import process instance wrote. It is unusual for pack_id to
hit MAX_PACK_ID with a reasonable --max-pack-size setting, but when it
does, the pack_id stored in each "struct object_entry" wraps and
fast-import gets utterly confused.
Add a check and immediately die() so the operator can at least see what
went wrong instead of experiencing an unexplained broken import.
With or without that clarification,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks! A test would still be nice, if someone has time to write one.
^ permalink raw reply
* [PATCH 2/3] fast-import: fix corner case for checkpoint
From: Dmitry Ivankov @ 2011-09-18 19:01 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Shawn O. Pearce, David Barr, Dmitry Ivankov
In-Reply-To: <1316372508-7173-1-git-send-email-divanorama@gmail.com>
checkpoint command makes fast-import finish current pack and write out
branches/tags and marks. In case no new objects are added in current
pack fast-import falls back to no-op. While it is possible that refs
or marks need to be updated (to point to old objects).
Make fast-import always dump them on checkpoint. But as before do not
cycle_packfile if there are no objects to write.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
fast-import.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 907cb05..014a807 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3025,12 +3025,11 @@ static void parse_ls(struct branch *b)
static void checkpoint(void)
{
checkpoint_requested = 0;
- if (object_count) {
+ if (object_count)
cycle_packfile();
- dump_branches();
- dump_tags();
- dump_marks();
- }
+ dump_branches();
+ dump_tags();
+ dump_marks();
}
static void parse_checkpoint(void)
--
1.7.3.4
^ permalink raw reply related
* [PATCH 3/3] fast-import: rename object_count to pack_object_count
From: Dmitry Ivankov @ 2011-09-18 19:01 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Shawn O. Pearce, David Barr, Dmitry Ivankov
In-Reply-To: <1316372508-7173-1-git-send-email-divanorama@gmail.com>
object_count is used to count objects that'll go to the current pack.
While object_count_by_* are used to count total amount of objects and
are not used to determine if current packfile is empty.
Rename (and move declaration) object_count to pack_object_count to
avoid possible confusion.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
fast-import.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 014a807..8f2411b 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -290,7 +290,6 @@ static uintmax_t object_count_by_type[1 << TYPE_BITS];
static uintmax_t duplicate_count_by_type[1 << TYPE_BITS];
static uintmax_t delta_count_by_type[1 << TYPE_BITS];
static uintmax_t delta_count_attempts_by_type[1 << TYPE_BITS];
-static unsigned long object_count;
static unsigned long branch_count;
static unsigned long branch_load_count;
static int failure;
@@ -316,6 +315,7 @@ static struct sha1file *pack_file;
static struct packed_git *pack_data;
static struct packed_git **all_packs;
static off_t pack_size;
+static unsigned long pack_object_count;
/* Table of objects we've written. */
static unsigned int object_entry_alloc = 5000;
@@ -880,7 +880,7 @@ static void start_packfile(void)
pack_data = p;
pack_size = sizeof(hdr);
- object_count = 0;
+ pack_object_count = 0;
all_packs = xrealloc(all_packs, sizeof(*all_packs) * (pack_id + 1));
all_packs[pack_id] = p;
@@ -894,17 +894,17 @@ static const char *create_index(void)
struct object_entry_pool *o;
/* Build the table of object IDs. */
- idx = xmalloc(object_count * sizeof(*idx));
+ idx = xmalloc(pack_object_count * sizeof(*idx));
c = idx;
for (o = blocks; o; o = o->next_pool)
for (e = o->next_free; e-- != o->entries;)
if (pack_id == e->pack_id)
*c++ = &e->idx;
- last = idx + object_count;
+ last = idx + pack_object_count;
if (c != last)
die("internal consistency error creating the index");
- tmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts, pack_data->sha1);
+ tmpfile = write_idx_file(NULL, idx, pack_object_count, &pack_idx_opts, pack_data->sha1);
free(idx);
return tmpfile;
}
@@ -953,7 +953,7 @@ static void end_packfile(void)
struct packed_git *old_p = pack_data, *new_p;
clear_delta_base_cache();
- if (object_count) {
+ if (pack_object_count) {
unsigned char cur_pack_sha1[20];
char *idx_name;
int i;
@@ -963,7 +963,7 @@ static void end_packfile(void)
close_pack_windows(pack_data);
sha1close(pack_file, cur_pack_sha1, 0);
fixup_pack_header_footer(pack_data->pack_fd, pack_data->sha1,
- pack_data->pack_name, object_count,
+ pack_data->pack_name, pack_object_count,
cur_pack_sha1, pack_size);
close(pack_data->pack_fd);
idx_name = keep_pack(create_index());
@@ -1103,7 +1103,7 @@ static int store_object(
e->type = type;
e->pack_id = pack_id;
e->idx.offset = pack_size;
- object_count++;
+ pack_object_count++;
object_count_by_type[type]++;
crc32_begin(pack_file);
@@ -1267,7 +1267,7 @@ static void stream_blob(uintmax_t len, unsigned char *sha1out, uintmax_t mark)
e->pack_id = pack_id;
e->idx.offset = offset;
e->idx.crc32 = crc32_end(pack_file);
- object_count++;
+ pack_object_count++;
object_count_by_type[OBJ_BLOB]++;
}
@@ -3025,7 +3025,7 @@ static void parse_ls(struct branch *b)
static void checkpoint(void)
{
checkpoint_requested = 0;
- if (object_count)
+ if (pack_object_count)
cycle_packfile();
dump_branches();
dump_tags();
--
1.7.3.4
^ permalink raw reply related
* [PATCH 1/3] fast-import: die if we produce too many (MAX_PACK_ID) packs
From: Dmitry Ivankov @ 2011-09-18 19:01 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Shawn O. Pearce, David Barr, Dmitry Ivankov
In-Reply-To: <1316372508-7173-1-git-send-email-divanorama@gmail.com>
In fast-import pack_id is 16-bit with MAX_PACK_ID reserved to identify
pre-existing objects. It is unlikely to wrap under reasonable settings
but still things in fast-import will break once it happens.
Add a check and immediate die() as the simplest reaction to being unable
to continue the import.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
fast-import.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 742e7da..907cb05 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1009,6 +1009,8 @@ static void end_packfile(void)
static void cycle_packfile(void)
{
end_packfile();
+ if (pack_id >= MAX_PACK_ID)
+ die("too many (%u) packs produced", pack_id);
start_packfile();
}
--
1.7.3.4
^ permalink raw reply related
* [PATCH 0/3] fast-import: fix pack_id corner cases
From: Dmitry Ivankov @ 2011-09-18 19:01 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Shawn O. Pearce, David Barr, Dmitry Ivankov
[1/3] is just a precaution unlikely to happen as having 65536+ packs
produced in fast-import looks extremely rare.
[2/2] is more real bug. Shouldn't be too hard to reproduce, but I'm
currently too lazy to as it is quite rare and not likely to get broken
again.
[3/3] is pure cosmetic change while I'm on pack_id topic.
Dmitry Ivankov (3):
fast-import: die if we produce too many (MAX_PACK_ID) packs
fast-import: fix corner case for checkpoint
fast-import: rename object_count to pack_object_count
fast-import.c | 29 +++++++++++++++--------------
1 files changed, 15 insertions(+), 14 deletions(-)
--
1.7.3.4
^ permalink raw reply
* Re: [RFC/PATCH] Configurable hyperlinking in gitk
From: Jakub Narebski @ 2011-09-18 18:50 UTC (permalink / raw)
To: Chris Packham; +Cc: Jeff Epler, git
In-Reply-To: <4E7467B7.1090201@gmail.com>
Chris Packham <judge.packham@gmail.com> writes:
> On 17/09/11 14:29, Jeff Epler wrote:
> > Some time ago I hardcoded this into gitk for $DAY_JOB and find it very
> > useful. I made it configurable in the hopes that it might be adopted
> > upstream. (unfortunately, the configurable version is radically
> > different from the original hard-coded version, so I can't say this
> > has had much testing yet)
>
> This is definitely something folks at my $dayjob would be interested in.
> We've already done some customisation of gitweb to do something similar.
> I'm not actually sure what the changes where or how configurable they
> are. I'll see if I can dig them out on Monday someone else might want to
> polish them into something suitable (I might do it myself if I get some
> tuits).
That would be nice. So called "committags" support was long planned
for gitweb, and even some preliminary work exists...
> > There are probably better names for the configuration options, too.
>
> It'd be nice if the config variables weren't gitk specific. .re and .sub
> could be applied to gitweb and maybe other git viewers outside of
> gig.git might decide to use them. My bikeshedding suggestion would be to
> just drop the gitk prefix and have linkify.re and linkify.sub.
Perhaps more descriptive name, i.e.
linkify.<name>.regexp
linkify.<name>.subst
would be better?
I guess that regexp is an extended regular expression, isn't it?
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v2] Add a credential-helper for KDE
From: Jeff King @ 2011-09-18 18:49 UTC (permalink / raw)
To: Lukas Sandström; +Cc: Git Mailing List
In-Reply-To: <4E7605CA.7020204@gmail.com>
On Sun, Sep 18, 2011 at 04:52:58PM +0200, Lukas Sandström wrote:
> Right. Multiple usernames per "unique" context is supported in this version.
> I looked at the git-credential-storage helper when I wrote the first patch,
> which didn't have obvious support for multiple usernames per unique context.
Yeah, sorry about that. The -cache helper is much more fully fleshed out
(though I have improved the -store helper in the past few days to handle
multiple usernames better).
> Keeping the username outside the token is probably a good thing, but perhaps it
> should be clarified in the api-docs that multiple usernames has to be supported.
OK, I'll try to write up a clarification.
> Also; what about rejecting credentials. This code currently deletes just a
> username/password pair if a username is specified, and all credentials associated
> with the token if only --unique and --reject is specified. Is this correct/expected
> behavior?
Yes, that's what I think should happen, and what both of my helpers do.
In practice, I don't think it will be called that way by git, which
will always be rejecting a username we just tried. But I wanted to leave
things flexible in case a user wants to manually remove a credential
from a store.
> When I first wrote the helper I tried to immediately ask for a new password if a
> credential was rejected, but this didn't work with the HTTP auth code, since it
> doesn't retry the auth with the new credentials after a reject. I think it would
> be better if we asked for a new password instead of just saying "auth failed" and
> having the user retry the fetch/pull when the stored credentials are incorrect.
Yeah, I had a patch early on to retry authentication a few times before
exiting, but I wondered how helpful it was. It's usually pretty easy to
retry your command again via shell history, and sometimes looping on
asking for authentication can be annoying (because things like askpass
will actually grab the keyboard focus).
So I dunno what is best. I don't consider it a big deal, but maybe
others do.
Even if we did do the retry from git, the helper shouldn't ask
immediately for the new credential inside a --reject. It should wait to
be invoked again asking for the password. I know this may be an extra
fork/exec/startup cycle, but it keeps the interface to the helper simple
and flexible.
-Peff
^ permalink raw reply
* Re: Most elegant way to reference to SVN from GIT?
From: Jakub Narebski @ 2011-09-18 18:47 UTC (permalink / raw)
To: Manuel Reimer; +Cc: git, Jeff King
In-Reply-To: <j54keg$ug9$1@dough.gmane.org>
Manuel Reimer <Manuel.Spam@nurfuerspam.de> writes:
> Jeff King wrote:
> > You can pull their commits into git using git-svn, and then merge them.
Nb. there is also so called "remote helper" approach, where you can
use repositories using other SCM just like native remote Git
repositories. But I think it is in early beta, or even alpha stages -
- adding support for Subversion was this year Google Summer of Code
2011 project.
> Tried that, but maybe I did something wrong.
>
> If I have a git-svn repo inside my git repo, then for some reason my
> own repo doesn't known the git-svn repo. Means that I cannot push
> changes, that occured in the SVN repo, to my GIT server.
Errr... isn't it more or less expected for _any_ remote repository
added, be it native (git) or Subversion one?
Remote tracking branches are not usually pushed / fetched, unless you
clone in a mirror mode, or set refspecs for fecthing / pushing by
hand...
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH] git-web--browse: invoke kfmclient directly
From: Jeff King @ 2011-09-18 18:38 UTC (permalink / raw)
To: Chris Packham; +Cc: git, chriscool, jepler
In-Reply-To: <1316341224-4359-1-git-send-email-judge.packham@gmail.com>
On Sun, Sep 18, 2011 at 10:20:24PM +1200, Chris Packham wrote:
> Instead of using eval which causes problems when a URL contains an
> appropriately escaped ampersand (\&).
I think this probably should just remove all of the evals. I don't see
how any of them is doing any good, and they're actively breaking URLs
that need quoting.
Hmm. Actually, the one for custom browser commands might need it,
because that one is expected to be a shell snippet. I suspect the
simplest thing is to do something like:
eval "$browser_cmd \"\$@\""
The other option would be to actually shell-quote each argument, which
is a pain to do in the shell (but is what C git does).
> For what it's worth I've included a testcase that detects my problem. I'm not
> sure if the testcase is really worth it because the test library suppresses X
> applications and even if it didn't the testcase is fairly trivial and might
> just annoy people by opening web-browsers (and it snaps up the last t99xx
> prefix).
Ick, yeah. Actually starting real browsers interacts too much with the
world outside of the test scripts. The results will be annoying (new
browser windows) and cause non-deterministic test results.
If you want to make a test, I think you would do better with something
like:
echo someurl_with_&_in_it >expect &&
git config browser.custom.cmd echo &&
git web--browse --browser=custom someurl_with_&_in_it >actual &&
test_cmp expect actual
That won't test that we are invoking kfmclient correctly, obviously, but
you can confirm at least that URLs are making it through to the browser
script intact.
-Peff
^ permalink raw reply
* Re: git subtree merging with a SVN remote?
From: Ronan Keryell @ 2011-09-18 18:00 UTC (permalink / raw)
To: Manuel Reimer; +Cc: git
In-Reply-To: <j550cl$218$1@dough.gmane.org>
>>>>> On Sun, 18 Sep 2011 16:44:33 +0200, Manuel Reimer <Manuel.Spam@nurfuerspam.de> said:
Manuel> Hello, I want to use "subtree merging" to get stuff from an
Manuel> external SVN repo into my project.
Manuel> Is it possible to do this directly using the external SVN
Manuel> server or should I set up a GIT mirror of this SVN repo?
I guess you can move *manually* the files from the SVN repository into
the git repository directly. :-)
More seriously, if you want to automate things, you have to go through a
git svn clone process.
Manuel> Is this "subtree merging" a pure local thing or do I push
Manuel> some information about this to the central GIT server?
Since the concept of central git server is a pure convention, you can
rely only on your local git repository only.
For a compiler project, I use these kinds of tricks to subtree 5
upstream svn projects with the ability to apply on top of them some
local commits, for example to avoid some upstream errors (it has just
happened last Friday :-( ).
Having all this operational is a little bit tedious, so I've developed
scripts to have all the infrastructure running and easy to be updated.
You may read
http://download.par4all.org/doc/organization/par4all_organization.htdoc/par4all_organization.html#x1-240007
or the PDF version and look at the p4a_git command I use to run this process:
https://git.hpc-project.com/cgit/par4all/tree/src/dev/p4a_git?h=p4a-own
https://git.hpc-project.com/cgit/par4all/tree/src/dev/p4a_git_lib.bash?h=p4a-own
and there are also some scripts around in the same directory to
bootstrap the infrastructure.
If you want to look at the resulting project history, study the p4a
branch from the Par4All project.
I've found this very powerful and efficient... once it is correctly
set up. :-)
I hope that helps,
--
Ronan KERYELL |\/ Cell: +33 613 143 766
HPC Project, Inc. |/) Ronan.Keryell@hpc-project.com
5201 Great America Parkway #3241 K skype:keryell
Santa Clara, CA 95054 |\ http://hpc-project.com
USA | \
^ 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