Mike Hommey [off-list ref] writes:
The --nosort option disabled the internal sorting used by pack-objects,
and runs the sliding window along the object list litterally as given on
stdin.
I think this is a good way to give people an easier way to experiment.
But it makes me wonder if this is disabling too much, and if the list
should be sorted at least by type, as we won't delta different types of
objects against each other.
At the beginning of try_delta(), when we see that the next candidate is
of a different type, we return -1 telling the caller that "No object in
the window will ever be a good delta base for the current object, please
abort". This relies on the fact that we sort by type first, so I think
one of the following is necessary:
(1) you weaken this check (return 0, saying "This did not delta well but
do not give up yet"),
(2) you document this well so that --nosort user will know, or
(3) you sort --nosort input by type.
I would obviously add the appropriate documentation for this flag if this
is accepted. I'll also try to send another documentation patch for
pack-objects with some information compiled from Linus's explanation to my
last message about pack-objects.
I need to rant here a bit.
Sometimes people say "Here is my patch. If this is accepted, I'll add
documentation and tests". My reaction is, "Don't you, as the person who
proposes that change, believe in your patch deeply enough yourself to be
willing to perfect it, to make it suitable for consumption by the
general public, whether it is included in my tree or not? A change that
even you do not believe in deeply enough probably to perfect would not
benefit the general public, so thanks but no thanks, I'll pass."
Fortunately we haven't had this problem too many times on this list.
I would not have minded at all if you said:
Obviously, appropriate documentation and tests are needed before
inclusion, but I am sending this out primarily to seek opinions
from the list to make sure this is going in the right direction,
iow, this is an RFC.
What bugged me was the phrase "if this is accepted".
On Fri, Dec 07, 2007 at 01:25:24PM -0800, Junio C Hamano wrote:
I need to rant here a bit.
Sometimes people say "Here is my patch. If this is accepted, I'll add
documentation and tests". My reaction is, "Don't you, as the person who
proposes that change, believe in your patch deeply enough yourself to be
willing to perfect it, to make it suitable for consumption by the
general public, whether it is included in my tree or not? A change that
even you do not believe in deeply enough probably to perfect would not
benefit the general public, so thanks but no thanks, I'll pass."
As you can seen from my other message, I'm *actually* not sure this is
really material for git as a VCS. I will add documentation unrelated to
--nosort to pack-objects anyways.
Mike
On Fri, 7 Dec 2007, Mike Hommey wrote:
As you can seen from my other message, I'm *actually* not sure this is
really material for git as a VCS. I will add documentation unrelated to
--nosort to pack-objects anyways.
Well, I have serious doubts about this patch in the first place.
I think it is simply unneeded.
If you want pack-objects not to change the sort order because you have
some sorting of your own, externally implemented, then you simply have
to run git-pack-objects feeding it the list of object SHA1s along with a
tag of your own which will effectively impose the sorting you want,
based on that tag.
Objects with the same tag will still be sorted amongst themselves which
is still a good thing.
for example, you may have something like:
git rev-list --all --objects |
sed -e 's|foo/logs/.*|LOGS|' |
git pack-objects ...
This will effectively cluster all foo/logs/* files together for delta
compression regardless of their actual name. Maybe that's what you
really want?
Nicolas
On Fri, Dec 07, 2007 at 05:20:59PM -0500, Nicolas Pitre wrote:
On Fri, 7 Dec 2007, Mike Hommey wrote:
quoted
As you can seen from my other message, I'm *actually* not sure this is
really material for git as a VCS. I will add documentation unrelated to
--nosort to pack-objects anyways.
Well, I have serious doubts about this patch in the first place.
I think it is simply unneeded.
If you want pack-objects not to change the sort order because you have
some sorting of your own, externally implemented, then you simply have
to run git-pack-objects feeding it the list of object SHA1s along with a
tag of your own which will effectively impose the sorting you want,
based on that tag.
Objects with the same tag will still be sorted amongst themselves which
is still a good thing.
for example, you may have something like:
git rev-list --all --objects |
sed -e 's|foo/logs/.*|LOGS|' |
git pack-objects ...
This will effectively cluster all foo/logs/* files together for delta
compression regardless of their actual name. Maybe that's what you
really want?
I've been thinking about this, but I'm not sure the list won't be
mixed up with the rest of the sort... I'll just try to see what the
sorted list look like...
Mike