* Re: What's in git.git (Dec 2008, #03; Sun, 21)
From: Paul Mackerras @ 2008-12-21 23:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Johannes Schindelin, Johannes Sixt, Robin Rosenberg,
Boyd Stephen Smith Jr.
In-Reply-To: <7vskohpvj7.fsf@gitster.siamese.dyndns.org>
Junio C Hamano writes:
> * Paul has one patch on his gitk branch I have fetched but not merged yet;
> waiting for his go-ahead.
Sorry. I have now flushed out my queue of pending patches; please
pull.
Thanks,
Paul.
^ permalink raw reply
* [Patch] Minor corrections to help & error messages
From: Richard Hartmann @ 2008-12-21 23:53 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 232 bytes --]
Hi all,
the two patches should be so obvious, there is not much
need to explain them.
I noticed that quite a few options in the manpages are
not in alphabetical order. Would a patch which fixes
all of those be accepted?
Richard
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-help-entries-alphabetical.patch --]
[-- Type: text/x-diff; name=0001-Make-help-entries-alphabetical.patch, Size: 1435 bytes --]
From 59b291d79cdcdb5fe235202cb45705dd4f8f352d Mon Sep 17 00:00:00 2001
From: Richard Hartmann <richih.mailinglist@gmail.com>
Date: Sun, 21 Dec 2008 23:28:25 +0100
Subject: [PATCH] Make help entries alphabetical
Switched the order. If there is a need for more changes, I can do them later.
Signed-off-by: Richard Hartmann <richih@net.in.tum.de>
---
Documentation/config.txt | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 21ea165..52786c7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -601,10 +601,6 @@ diff.autorefreshindex::
affects only 'git-diff' Porcelain, and not lower level
'diff' commands, such as 'git-diff-files'.
-diff.suppress-blank-empty::
- A boolean to inhibit the standard behavior of printing a space
- before each empty output line. Defaults to false.
-
diff.external::
If this config variable is set, diff generation is not
performed using the internal diff machinery, but using the
@@ -639,6 +635,10 @@ diff.renames::
will enable basic rename detection. If set to "copies" or
"copy", it will detect copies, as well.
+diff.suppress-blank-empty::
+ A boolean to inhibit the standard behavior of printing a space
+ before each empty output line. Defaults to false.
+
fetch.unpackLimit::
If the number of objects fetched over the git native
transfer is below this
--
1.5.6.5
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Always-show-which-directory-is-not-a-git-repository.patch --]
[-- Type: text/x-diff; name=0002-Always-show-which-directory-is-not-a-git-repository.patch, Size: 2264 bytes --]
From bc332cec43d1a389786c36ee893a0a1d09b112bc Mon Sep 17 00:00:00 2001
From: Richard Hartmann <richih.mailinglist@gmail.com>
Date: Mon, 22 Dec 2008 00:17:32 +0100
Subject: [PATCH] Always show which directory is not a git repository
Unify all
fatal: Not a git repository
error messages so they include path information.
Signed-off-by: Richard Hartmann <richih@net.in.tum.de>
---
contrib/workdir/git-new-workdir | 2 +-
perl/Git.pm | 4 ++--
setup.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
index 7959eab..993cacf 100755
--- a/contrib/workdir/git-new-workdir
+++ b/contrib/workdir/git-new-workdir
@@ -22,7 +22,7 @@ branch=$3
# want to make sure that what is pointed to has a .git directory ...
git_dir=$(cd "$orig_git" 2>/dev/null &&
git rev-parse --git-dir 2>/dev/null) ||
- die "\"$orig_git\" is not a git repository!"
+ die "Not a git repository: \"$orig_git\""
case "$git_dir" in
.git)
diff --git a/perl/Git.pm b/perl/Git.pm
index dde9105..8392a68 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -204,14 +204,14 @@ sub repository {
unless (-d "$dir/refs" and -d "$dir/objects" and -e "$dir/HEAD") {
# Mimick git-rev-parse --git-dir error message:
- throw Error::Simple('fatal: Not a git repository');
+ throw Error::Simple("fatal: Not a git repository: $dir");
}
my $search = Git->repository(Repository => $dir);
try {
$search->command('symbolic-ref', 'HEAD');
} catch Git::Error::Command with {
# Mimick git-rev-parse --git-dir error message:
- throw Error::Simple('fatal: Not a git repository');
+ throw Error::Simple("fatal: Not a git repository: $dir");
}
$opts{Repository} = abs_path($dir);
diff --git a/setup.c b/setup.c
index 833ced2..6b277b6 100644
--- a/setup.c
+++ b/setup.c
@@ -468,7 +468,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
*nongit_ok = 1;
return NULL;
}
- die("Not a git repository");
+ die("Not a git repository (or any of the parent directories): %s", DEFAULT_GIT_DIR_ENVIRONMENT);
}
if (chdir(".."))
die("Cannot change to %s/..: %s", cwd, strerror(errno));
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Robin Rosenberg @ 2008-12-21 22:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Boyd Stephen Smith Jr., git
In-Reply-To: <7vd4flkuy2.fsf@gitster.siamese.dyndns.org>
söndag 21 december 2008 23:46:45 skrev Junio C Hamano:
> Do you want:
>
> This reverts commit efe05b019ca19328d27c07ef32b4698a7f36166f, reversing
> changes made to ec9f0ea3e6ecf1237223dec8428e7bb73d339320.
Yes, it fills the paragraph nicely. It is a normal text flow after all.
-- robin
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-21 22:46 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Boyd Stephen Smith Jr., git
In-Reply-To: <200812212340.46375.robin.rosenberg.lists@dewire.com>
Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:
> söndag 21 december 2008 23:17:12 skrev Junio C Hamano:
>> From: Robin Rosenberg <robin.rosenberg.lists@dewire.com>
>> Subject: git-revert: record the parent against which a revert was made
>>
>> As described in Documentation/howto/revert-a-faulty-merge.txt, re-merging
>> from a previously reverted a merge of a side branch may need a revert of
>> the revert beforehand. Record against which parent the revert was made in
>> the commit, so that later the user can figure out what went on.
>>
>> [jc: original had the logic in the message reversed, so I tweaked it.]
> No need for this comment.
Ok.
>> + add_to_msg(",\nreverting damages made to %s");
> maybe "changes" is more neutrral language. I also think you break
> the line too early.
The above (without %s which shouldn't have been there) would give:
This reverts commit efe05b019ca19328d27c07ef32b4698a7f36166f,
reverting damages made to ec9f0ea3e6ecf1237223dec8428e7bb73d339320.
Do you want:
This reverts commit efe05b019ca19328d27c07ef32b4698a7f36166f, reversing
changes made to ec9f0ea3e6ecf1237223dec8428e7bb73d339320.
this instead?
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Robin Rosenberg @ 2008-12-21 22:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Boyd Stephen Smith Jr., git
In-Reply-To: <7vprjlkwbb.fsf@gitster.siamese.dyndns.org>
söndag 21 december 2008 23:17:12 skrev Junio C Hamano:
> From: Robin Rosenberg <robin.rosenberg.lists@dewire.com>
> Subject: git-revert: record the parent against which a revert was made
>
> As described in Documentation/howto/revert-a-faulty-merge.txt, re-merging
> from a previously reverted a merge of a side branch may need a revert of
> the revert beforehand. Record against which parent the revert was made in
> the commit, so that later the user can figure out what went on.
>
> [jc: original had the logic in the message reversed, so I tweaked it.]
No need for this comment.
> + add_to_msg(",\nreverting damages made to %s");
maybe "changes" is more neutrral language. I also think you break
the line too early.
Are we done now?
-- robin
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-21 22:38 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.; +Cc: Robin Rosenberg, git
In-Reply-To: <7vprjlkwbb.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Ok, so here is Robin's patch with a bit of rewording. I want to have
> something usable now, so that I can tag -rc4 and still have time left
> for sipping my Caipirinha in the evening ;-)
> ...
> + add_to_msg(",\nreverting damages made to %s");
> + add_to_msg(sha1_to_hex(parent->object.sha1));
Crap. Scratch that. Obviously I should have done this:
diff --git a/builtin-revert.c b/builtin-revert.c
index 4038b41..c188150 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -352,6 +352,11 @@ static int revert_or_cherry_pick(int argc, const char **argv)
add_to_msg(oneline_body + 1);
add_to_msg("\"\n\nThis reverts commit ");
add_to_msg(sha1_to_hex(commit->object.sha1));
+
+ if (commit->parents->next) {
+ add_to_msg(",\nreverting damages made to ");
+ add_to_msg(sha1_to_hex(parent->object.sha1));
+ }
add_to_msg(".\n");
} else {
base = parent;
--
1.6.1.rc3.72.gf4bf6
^ permalink raw reply related
* Re: [PATCH 2/2] fast-import: add special '-' blob reference to use the previous one.
From: Shawn O. Pearce @ 2008-12-21 22:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Felipe Contreras, git
In-Reply-To: <7vlju9kvyg.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > Hmph, so if create a new path with a blob of "-" the repository
> > will be corrupt because the zero id was used and error was produced.
> >
> > Actually I think you have the same bug in the prior patch with the
> > mode being inherited. I wonder if we shouldn't put error checking
> > in too to validate that versions[0] describes a file entry.
>
> Why are these patches necessary?
>
> The proposed commit message describes what it does, but does not give hint
> to even guess being able to use this new feature helps in what situation.
> As far as I can see, these changes allow the exporter to say "this aspect
> of the new data is the same as the previous one", but I thought that the
> way in which fast-import works already revolves around "you have this
> tree, and the next tree is different from it in this and that way." Why
> does one need be able to mention "this is the same as the previous one"
> explicitly in the first place?
Hmm. Actually, imagine you were dumping from git-diff output style
stream into a fast-import stream.
If a file changes only content, the mode is shown in the index line.
Yay us. But what if the index line wasn't present in the diff? You
don't know the prior mode of the file, but you do have its content.
If a file changes only mode, we get no content hints in the diff.
How do you send that into fast-import without making the frontend
keep track of every path's current mode?
Though I agree, these details should be described in the commit
messages, not left as an exercise for the maintainer to make up.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Add support for changing packed_git_window_size at process start time
From: Shawn O. Pearce @ 2008-12-21 22:28 UTC (permalink / raw)
To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1229895454-19498-2-git-send-email-tyler@slide.com>
"R. Tyler Ballance" <tyler@slide.com> wrote:
> "Works" insofar that it will alter the packed_git_window_size variable in environment.c
> when the environment is set up. It /doesn't/ work when commands like git-diff(1) and git-log(1)
> call get_revision() which seems to disregard the setting if the packed_window_size is set to something
> low (i.e. ulimit -v 32768)
I think you are tweaking the wrong variable here. Its more than
just the window size that matters to how much of the ulimit we use.
Its also packed_git_limit, which on a 64 bit system is 8 GB.
That's probably why get_revision doesn't seem to honor this as
a setting. Yea, its down to 0.85% of your ulimit per window,
but we'll still try to open a new window because we have space
left before the 8 GB limit.
I think this is a good idea, trying to fit within the ulimit
rather than assuming we can take whatever we please. But you
also need to drop the packed_git_limit down.
My suggestion is this:
packed_git_limit = as->rlim_cur * 0.85;
packed_git_window_size = packed_git_limit / 4;
or maybe / 2. You really want at least 2 windows available within
your limit.
> @@ -75,6 +78,13 @@ static void setup_git_env(void)
> git_graft_file = getenv(GRAFT_ENVIRONMENT);
> if (!git_graft_file)
> git_graft_file = git_pathdup("info/grafts");
> +
> + if (DYNAMIC_WINDOW_SIZE) {
> + struct rlimit *as = malloc(sizeof(struct rlimit));
> + if ( (getrlimit(RLIMIT_AS, as) == 0) && ((int)(as->rlim_cur) > 0) )
> + packed_git_window_size = (unsigned int)(as->rlim_cur * DYNAMIC_WINDOW_SIZE_PERCENTAGE);
> + free(as);
> + }
> }
--
Shawn.
^ permalink raw reply
* Re: [PATCH 2/2] fast-import: add special '-' blob reference to use the previous one.
From: Junio C Hamano @ 2008-12-21 22:24 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Felipe Contreras, git
In-Reply-To: <20081221221149.GB17355@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Hmph, so if create a new path with a blob of "-" the repository
> will be corrupt because the zero id was used and error was produced.
>
> Actually I think you have the same bug in the prior patch with the
> mode being inherited. I wonder if we shouldn't put error checking
> in too to validate that versions[0] describes a file entry.
Why are these patches necessary?
The proposed commit message describes what it does, but does not give hint
to even guess being able to use this new feature helps in what situation.
As far as I can see, these changes allow the exporter to say "this aspect
of the new data is the same as the previous one", but I thought that the
way in which fast-import works already revolves around "you have this
tree, and the next tree is different from it in this and that way." Why
does one need be able to mention "this is the same as the previous one"
explicitly in the first place?
^ permalink raw reply
* Re: [PATCH] Style changes per suggestions from Junio in #git
From: Shawn O. Pearce @ 2008-12-21 22:22 UTC (permalink / raw)
To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1229895454-19498-3-git-send-email-tyler@slide.com>
"R. Tyler Ballance" <tyler@slide.com> wrote:
> Moving includes into git-compat-util.h, move away from malloc(2)
Obviously this should just be squashed into the prior patch.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-21 22:17 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.; +Cc: Robin Rosenberg, git
In-Reply-To: <200812211513.26808.bss@iguanasuicide.net>
From: Robin Rosenberg <robin.rosenberg.lists@dewire.com>
Subject: git-revert: record the parent against which a revert was made
As described in Documentation/howto/revert-a-faulty-merge.txt, re-merging
from a previously reverted a merge of a side branch may need a revert of
the revert beforehand. Record against which parent the revert was made in
the commit, so that later the user can figure out what went on.
[jc: original had the logic in the message reversed, so I tweaked it.]
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
"Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> I still think the parent we are reverting to should be mentioned in the
> automatically generated commit message, even if it is the first parent. Even
> if it is decided to elide that information for the first parent, I agree
> that, at least for now, the "-m" should still be required when reverting a
> merge commit.
Ok, so here is Robin's patch with a bit of rewording. I want to have
something usable now, so that I can tag -rc4 and still have time left
for sipping my Caipirinha in the evening ;-)
I think we later _could_ use this information inside ancestry traversal
made while computing the merge base in such a way to eliminate the
necessity of the "revert of the revert". When we see a message that
records a revert of a merge, we keep a mental note of it, and when we
encounter such a merge during the ancestry traversal, we pretend as if
the merge never happened (i.e. instead we traverse only the named
parent).
But that needs more thought, and we do not have to do that now.
builtin-revert.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git c/builtin-revert.c w/builtin-revert.c
index 4038b41..fae0fe8 100644
--- c/builtin-revert.c
+++ w/builtin-revert.c
@@ -352,6 +352,11 @@ static int revert_or_cherry_pick(int argc, const char **argv)
add_to_msg(oneline_body + 1);
add_to_msg("\"\n\nThis reverts commit ");
add_to_msg(sha1_to_hex(commit->object.sha1));
+
+ if (commit->parents->next) {
+ add_to_msg(",\nreverting damages made to %s");
+ add_to_msg(sha1_to_hex(parent->object.sha1));
+ }
add_to_msg(".\n");
} else {
base = parent;
^ permalink raw reply related
* Re: Memory issue with fast-import, why track branches?
From: Shawn O. Pearce @ 2008-12-21 22:17 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git list
In-Reply-To: <94a0d4530812202154l26dfe0dfm49397c63dbfdfdf9@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> wrote:
> I tracked down an issue I have when importing a big repository. For
> some reason memory usage keeps increasing until there is no more
> memory.
>
> After looking at the code my guess is that I have a humongous amount
> of branches.
>
> Actually they are not really branches, but refs. For each git commit
> there's an original mtn ref that I store in 'refs/mtn/sha1', but since
> I'm using 'commit refs/mtn/sha1' to store it, a branch is created for
> every commit.
>
> I guess there are many ways to fix the issue, but for starters I
> wonder why is fast-import keeping track of all the branches? In my
> case I would like fast-import to work exactly the same if I specify
> branches or not (I'll update them later).
Because fast-import has to buffer them until the pack file is done.
The objects aren't available to the repository until after a
checkpoint is sent or until the stream ends. Either way until
then fast-import has to buffer the refs so they don't get exposed
to other git processes reading that same repository, because they
would point to objects that the process cannot find.
I guess it could release the brnach memory after it dumps the
branches in a checkpoint, but its memory allocators work under an
assumption that strings (like branch and file names) will be reused
heavily by the frontend and thus they are poooled inside of a string
pool. The branch objects are also pooled inside of a common alloc
pool, to ammortize the cost of malloc's block headers out over the
data used.
IOW, fast-import was designed for ~5k branches, not ~1 million
unique branches.
--
Shawn.
^ permalink raw reply
* [PATCH] git-commit: colored status when color.ui is set
From: Markus Heidelberg @ 2008-12-21 22:15 UTC (permalink / raw)
To: gitster; +Cc: git
When using "git commit" and there was nothing to commit (the editor
wasn't launched), the status output wasn't colored, even though color.ui
was set. Only when setting color.status it worked.
Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
builtin-commit.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index ac88dc2..7cf227a 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -948,6 +948,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
git_config(git_commit_config, NULL);
+ if (wt_status_use_color == -1)
+ wt_status_use_color = git_use_color_default;
+
argc = parse_and_validate_options(argc, argv, builtin_commit_usage, prefix);
index_file = prepare_index(argc, argv, prefix);
--
1.6.1.rc3.62.g7c0a2
^ permalink raw reply related
* [PATCH] git-status -v: colored diff when color.ui is set
From: Markus Heidelberg @ 2008-12-21 22:14 UTC (permalink / raw)
To: gitster; +Cc: git
When using "git status -v", the diff output wasn't colored, even though
color.ui was set. Only when setting color.diff it worked.
Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
builtin-commit.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index e88b78f..ac88dc2 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -866,6 +866,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
if (wt_status_use_color == -1)
wt_status_use_color = git_use_color_default;
+ if (diff_use_color_default == -1)
+ diff_use_color_default = git_use_color_default;
+
argc = parse_and_validate_options(argc, argv, builtin_status_usage, prefix);
index_file = prepare_index(argc, argv, prefix);
--
1.6.1.rc3.62.g7c0a2
^ permalink raw reply related
* Re: [PATCH 2/2] fast-import: add special '-' blob reference to use the previous one.
From: Shawn O. Pearce @ 2008-12-21 22:11 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <1229825502-963-2-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> wrote:
> @@ -1862,7 +1864,7 @@ static void file_change_m(struct branch *b)
> const char *endp;
> struct object_entry *oe = oe;
> unsigned char sha1[20];
> - uint16_t mode, inline_data = 0;
> + uint16_t mode, inline_data = 0, empty_blob = 0;
Its not the empty blob, its the inherited/assumed blob...
> @@ -1893,6 +1895,10 @@ static void file_change_m(struct branch *b)
> } else if (!prefixcmp(p, "inline")) {
> inline_data = 1;
> p += 6;
> + } else if (!prefixcmp(p, "- ")) {
> + hashclr(sha1);
> + empty_blob = 1;
> + p += 1;
Hmph, so if create a new path with a blob of "-" the repository
will be corrupt because the zero id was used and error was produced.
Actually I think you have the same bug in the prior patch with the
mode being inherited. I wonder if we shouldn't put error checking
in too to validate that versions[0] describes a file entry.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 1/2] fast-import: add special mode; copy from parent.
From: Shawn O. Pearce @ 2008-12-21 22:07 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <1229825502-963-1-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> wrote:
> + if (!prefixcmp(p, "- ")) {
> + mode = 0;
> + p += 2;
This part made me wonder, why are we always doing "S_IFREG | mode"
further down?
> + } else {
> + p = get_mode(p, &mode);
> + if (!p)
> + die("Corrupt mode: %s", command_buf.buf);
> + switch (mode) {
> + case S_IFREG | 0644:
> + case S_IFREG | 0755:
> + case S_IFLNK:
> + case S_IFGITLINK:
> + case 0644:
> + case 0755:
> + /* ok */
> + break;
> + default:
> + die("Corrupt mode: %s", command_buf.buf);
> + }
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Style changes per suggestions from Junio in #git
From: Matthieu Moy @ 2008-12-21 22:03 UTC (permalink / raw)
To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1229895454-19498-3-git-send-email-tyler@slide.com>
"R. Tyler Ballance" <tyler@slide.com> writes:
> Moving includes into git-compat-util.h, move away from malloc(2)
Usually, those cleanup patches are merged with the actual patch before
inclusion. This helps review (i.e. let reviewers not have to say or
think "you shouldn't do that - oh, ok, you're actually not doing it"),
and avoids having bad commits at all in Junio's repository.
--
Matthieu
^ permalink raw reply
* Re: [PATCHv2] Have manpage reference new documentation on reverting merges.
From: Junio C Hamano @ 2008-12-21 21:54 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.; +Cc: git, Nanako Shiraishi
In-Reply-To: <E1LEVxk-0001jS-Cc@rei.iguanasuicide.net>
"Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> I took the alternative approach.
Thanks. I was thinking about doing this instead; how the reference to the
HOW-TO is done is different, and I am hoping that it would give better
result for HTML version at least.
Documentation/git-revert.txt | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git c/Documentation/git-revert.txt w/Documentation/git-revert.txt
index caa0729..b87f2b3 100644
--- c/Documentation/git-revert.txt
+++ w/Documentation/git-revert.txt
@@ -44,6 +44,15 @@ OPTIONS
option specifies the parent number (starting from 1) of
the mainline and allows revert to reverse the change
relative to the specified parent.
++
+By reverting a merge, you are telling git that you never want the changes
+the merge made to your tree. If the branch the reverted merge merged is
+updated later, and you merge from it again, git remembers this, and only
+brings in the changes on the branch that are made after the previously
+reverted merge. This may or may not be what you want.
++
+See link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To] for
+more details.
--no-edit::
With this option, 'git-revert' will not start the commit
^ permalink raw reply related
* [PATCH] Style changes per suggestions from Junio in #git
From: R. Tyler Ballance @ 2008-12-21 21:37 UTC (permalink / raw)
To: git; +Cc: R. Tyler Ballance
In-Reply-To: <1229895454-19498-2-git-send-email-tyler@slide.com>
Moving includes into git-compat-util.h, move away from malloc(2)
Signed-off-by: R. Tyler Ballance <tyler@slide.com>
---
environment.c | 9 +++------
git-compat-util.h | 2 ++
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/environment.c b/environment.c
index a3b6bab..aa36360 100644
--- a/environment.c
+++ b/environment.c
@@ -7,8 +7,6 @@
* even if you might want to know where the git directory etc
* are.
*/
-#include <sys/time.h>
-#include <sys/resource.h>
#include "cache.h"
@@ -80,10 +78,9 @@ static void setup_git_env(void)
git_graft_file = git_pathdup("info/grafts");
if (DYNAMIC_WINDOW_SIZE) {
- struct rlimit *as = malloc(sizeof(struct rlimit));
- if ( (getrlimit(RLIMIT_AS, as) == 0) && ((int)(as->rlim_cur) > 0) )
- packed_git_window_size = (unsigned int)(as->rlim_cur * DYNAMIC_WINDOW_SIZE_PERCENTAGE);
- free(as);
+ struct rlimit as;
+ if (getrlimit(RLIMIT_AS, &as) == 0 && (int)as.rlim_cur > 0)
+ packed_git_window_size = (unsigned int)(as.rlim_cur * DYNAMIC_WINDOW_SIZE_PERCENTAGE);
}
}
diff --git a/git-compat-util.h b/git-compat-util.h
index 9603ca6..dad2dc8 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -188,6 +188,8 @@ extern int git_munmap(void *start, size_t length);
#else /* NO_MMAP */
#include <sys/mman.h>
+#include <sys/time.h>
+#include <sys/resource.h>
/* This value must be multiple of (pagesize * 2) */
#define DEFAULT_PACKED_GIT_WINDOW_SIZE \
--
^ permalink raw reply related
* [PATCH] Add support for changing packed_git_window_size at process start time
From: R. Tyler Ballance @ 2008-12-21 21:37 UTC (permalink / raw)
To: git; +Cc: R. Tyler Ballance
In-Reply-To: <1229895454-19498-1-git-send-email-tyler@slide.com>
"Works" insofar that it will alter the packed_git_window_size variable in environment.c
when the environment is set up. It /doesn't/ work when commands like git-diff(1) and git-log(1)
call get_revision() which seems to disregard the setting if the packed_window_size is set to something
low (i.e. ulimit -v 32768)
Signed-off-by: R. Tyler Ballance <tyler@slide.com>
---
environment.c | 10 ++++++++++
git-compat-util.h | 4 ++++
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/environment.c b/environment.c
index e278bce..a3b6bab 100644
--- a/environment.c
+++ b/environment.c
@@ -7,6 +7,9 @@
* even if you might want to know where the git directory etc
* are.
*/
+#include <sys/time.h>
+#include <sys/resource.h>
+
#include "cache.h"
char git_default_email[MAX_GITNAME];
@@ -75,6 +78,13 @@ static void setup_git_env(void)
git_graft_file = getenv(GRAFT_ENVIRONMENT);
if (!git_graft_file)
git_graft_file = git_pathdup("info/grafts");
+
+ if (DYNAMIC_WINDOW_SIZE) {
+ struct rlimit *as = malloc(sizeof(struct rlimit));
+ if ( (getrlimit(RLIMIT_AS, as) == 0) && ((int)(as->rlim_cur) > 0) )
+ packed_git_window_size = (unsigned int)(as->rlim_cur * DYNAMIC_WINDOW_SIZE_PERCENTAGE);
+ free(as);
+ }
}
int is_bare_repository(void)
diff --git a/git-compat-util.h b/git-compat-util.h
index e20b1e8..9603ca6 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -182,6 +182,8 @@ extern int git_munmap(void *start, size_t length);
/* This value must be multiple of (pagesize * 2) */
#define DEFAULT_PACKED_GIT_WINDOW_SIZE (1 * 1024 * 1024)
+#define DYNAMIC_WINDOW_SIZE 0
+#define DYNAMIC_WINDOW_SIZE_PERCENTAGE 0
#else /* NO_MMAP */
@@ -192,6 +194,8 @@ extern int git_munmap(void *start, size_t length);
(sizeof(void*) >= 8 \
? 1 * 1024 * 1024 * 1024 \
: 32 * 1024 * 1024)
+#define DYNAMIC_WINDOW_SIZE 1
+#define DYNAMIC_WINDOW_SIZE_PERCENTAGE 0.85
#endif /* NO_MMAP */
--
^ permalink raw reply related
* Dynamically adjusting packed_git_window_size
From: R. Tyler Ballance @ 2008-12-21 21:37 UTC (permalink / raw)
To: git
Internally we are using a custom build of Git, and one of the patches
that I apply to newer builds of Git is one to adjust the
DEFAULT_PACKED_GIT_WINDOW_SIZE in git-compat-util.h so Git won't trample
all over our ulimit values on the 64-bit dev machines.
To do away with this, I've got these two (really one) set of patches to
adjust the packed_git_window_size when setup_git_env() is called to a
fraction of the "addressspace" limit (RLIMIT_AS). If the user's
environment defines "ulimit -v" as "unlimited", this code will not take
effect.
It's worth noting that this doesn't force Git to respect these limits,
I'm still tracking down an issue hiding in get_revision() where I'm
experiencing mmap(2) failures executing a `git log` command with
restrictive ulimit settings (Linus, since you were so "pleased" with my
last epic gdb fail, here's today's):
(gdb)
open_packed_git (p=0x71f2e0) at sha1_file.c:733
733 /* We leave these file descriptors open with sliding mmap;
(gdb)
735 */
(gdb)
741 return error("cannot set FD_CLOEXEC");
(gdb)
746 if (hdr.hdr_signature != htonl(PACK_SIGNATURE))
(gdb)
Recursive internal problem.
[1] 17381 abort GIT_PAGER= gdb git
tyler@starfruit:~/source/git/main>
Oi vei.
Cheers,
-R. Tyler Ballance
^ permalink raw reply
* [PATCHv2] Have manpage reference new documentation on reverting merges.
From: Boyd Stephen Smith Jr. @ 2008-12-21 21:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nanako Shiraishi
In-Reply-To: <7vtz8ytft0.fsf@gitster.siamese.dyndns.org>
Signed-off-by: Boyd Stephen Smith Jr <bss@iguanasuicide.net>
---
On Saturday 2008 December 20 20:36:43 Junio C Hamano wrote:
> "Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> > + Reverting a merge commit does not completely "undo" the effect of the
> > + merge and it may make future merges more difficult. For more details,
> > + please read Documentation/howto/revert-a-faulty-merge.txt.
>
> I think these new lines need to be dedented, and the previous blank line
> should be turned into a line with a single '+'.
Fixed, I wasn't familiar with asciidoc.
> I'd also suggest removing "does not ... merge and it" from the above
> sentence to avoid confusing readers, because people who read only the
> above and do not read the howto document may get a wrong impression that
> the resulting tree may have some changes that came from the merge even
> after the revert, which is not the case.
>
> An alternative is to give a complete but brief explanation. Perhaps like
> this:
I took the alternative approach.
Documentation/git-revert.txt | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index caa0729..daa77a9 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -44,6 +44,13 @@ OPTIONS
option specifies the parent number (starting from 1) of
the mainline and allows revert to reverse the change
relative to the specified parent.
++
+Reverting a merge commit declares to git that you will never want the tree
+changes brought in by the merge but never alters the history changes caused by
+the merge. The history allows git to remember you rejected the tree changes
+and, as a result, later merges will only bring in changes introduced by commits
+that are not ancestors of the revert commit. This may or may not be what you
+want. See the 'revert-a-faulty-merge' HOWTO for more details.
--no-edit::
With this option, 'git-revert' will not start the commit
--
1.5.6
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
^ permalink raw reply related
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Boyd Stephen Smith Jr. @ 2008-12-21 21:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Robin Rosenberg, git
In-Reply-To: <7vwsdtmg5m.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]
On Sunday 2008 December 21 14:23:17 Junio C Hamano wrote:
> "Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> > Then why not have "-m 1" be assumed instead of forcing the user to
> > specify it?
>
> The reason we don't is because until very recently we did not even allow
> you to revert a merge relative to any parent. We wanted to avoid
> surprising people who are _relying on_ that behaviour to make sure that
> they do not revert a merge by accident.
That makes sense.
> We could certainly do what you suggest to imply "-m 1" when the commit
> requested to be reverted happens to be a merge, but we shouldn't be doing
> that without thinking things through. It will break people's longstanding
> expectations.
I wasn't really suggesting that. I was pointing out what I thought was an
inconsistency: making the user specify the parent but not making the commit
message specify the parent.
I still think the parent we are reverting to should be mentioned in the
automatically generated commit message, even if it is the first parent. Even
if it is decided to elide that information for the first parent, I agree
that, at least for now, the "-m" should still be required when reverting a
merge commit.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-21 20:23 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.; +Cc: Robin Rosenberg, git
In-Reply-To: <200812211359.31991.bss@iguanasuicide.net>
"Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> Then why not have "-m 1" be assumed instead of forcing the user to specify it?
The reason we don't is because until very recently we did not even allow
you to revert a merge relative to any parent. We wanted to avoid
surprising people who are _relying on_ that behaviour to make sure that
they do not revert a merge by accident.
We could certainly do what you suggest to imply "-m 1" when the commit
requested to be reverted happens to be a merge, but we shouldn't be doing
that without thinking things through. It will break people's longstanding
expectations.
^ permalink raw reply
* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Boyd Stephen Smith Jr. @ 2008-12-21 19:59 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Junio C Hamano, git
In-Reply-To: <200812211109.36788.robin.rosenberg.lists@dewire.com>
[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]
On Sunday 2008 December 21 04:09:36 Robin Rosenberg wrote:
> söndag 21 december 2008 04:11:13 skrev Boyd Stephen Smith Jr.:
> > On Saturday 2008 December 20 20:37:16 Junio C Hamano wrote:
> > > Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:
> > > > An alternative, would be "removing changes relative to .."
> > > > (mainline).
> > >
> > > But that is exactly what "This reverts commit X" means, isn't it?
> >
> > When X is a merge commit, the phrase "the reverts commit X" is ambiguous.
> > Did you revert the tree to X^, X^2, or X^8? I'd be fine with "This
> > reverts commit X to X^y", but we definitely need some mention of X^y.
>
> One could consider keeping the contributions from ^1 a special case and not
> mention the parent, making it look like any revert commit. I guess most
> merge reverts are like this in practice.
Then why not have "-m 1" be assumed instead of forcing the user to specify it?
If we force the user to specify that information, shouldn't we hold the code
to the same standard and have it output a message with that information?
I think git should mention the parent to which we reverted whenever there are
multiple parents.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
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