* [PATCH 0/2] gitweb: patch view
From: Giuseppe Bilotta @ 2008-11-29 13:41 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
I recently discovered that the commitdiff_plain view is not exactly
something that can be used by git am directly (for example, the subject
line gets duplicated in the commit message body after using git am).
Since I'm not sure if it was the case to fix the plain view because I
don't know what its intended usage was, I prepared a new view,
uncreatively called 'patch', that exposes git format-patch output
directly.
The second patch exposes it from commitdiff view (obviosly), but also
from shortlog view, when less than 16 patches are begin shown.
Giuseppe Bilotta (2):
gitweb: add patch view
gitweb: links to patch action in commitdiff and shortlog view
gitweb/gitweb.perl | 35 +++++++++++++++++++++++++++++++++--
1 files changed, 33 insertions(+), 2 deletions(-)
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)
From: Nguyen Thai Ngoc Duy @ 2008-11-29 13:02 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <alpine.LNX.1.00.0811281938250.19665@iabervon.org>
On 11/29/08, Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Fri, 28 Nov 2008, Junio C Hamano wrote:
>
> > "Shawn O. Pearce" <spearce@spearce.org> writes:
> >
> > > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > ...
> > >> In other words, unless there is more interest in that feature, enough to
> > >> generate a well-understood design before a good implementation, I'd rather
> > >> see this patch series dropped.
> > >
> > > Ack. I agree with every remark made by Dscho, and also want to cry "wolf".
> > >
> > > I haven't had time to read the patch series. Its big and intrusive
> > > and I just don't need the feature.
> >
> > Well, "me neither". Although I personally think resisting changes until
> > it becomes absolutely necessary is a good discipline, we also need to
> > recognise that there is a chicken-and-egg problem. When you have a
> > potentially useful feature, unless people actually try using it in the
> > field, you won't discover the drawbacks in either the design nor the
> > implementation, let alone any improvements.
>
>
> I just looked over most of it (skipping the generic index extension
> portion). It looks to me like it's introducing an extra concept to avoid
> actually fixing maybe-bugs in the "assume unchanged" implementation
> when used with files that have been changed intentionally (with the user
> intending git to overlook this change). Sparse checkout is essentially a
> special case of this, where the user has changed the working directory
> radically (not populating it at all) and wants git to carry on as if this
> was not the case (with a certain amount of porcelain code to cause this to
> happen automatically).
I chose to use another bit because I did not want to change the
behaviour of CE_VALID bit: it assumes all files are present.
> If there's any need for this to be distinguished from "assume unchanged",
> I think it should be used with, not instead of, the CE_VALID bit; and it
> could probably use some bit in the stat info section, since we don't need
> stat info if we know by assumption that the entry is valid.
Interesting. I'll think more about this.
> -Daniel
> *This .sig left intentionally blank*
>
> --
> 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
>
--
Duy
^ permalink raw reply
* Re: [JGIT PATCH 0/4] RepositoryTestCase cleanups
From: Robin Rosenberg @ 2008-11-29 12:01 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, fonseca
In-Reply-To: <20081127214916.GD23984@spearce.org>
torsdag 27 november 2008 22:49:16 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > Ok, so here is an attempt to improve the ability of the JGit's unit
> > tests to delete temporary repositories. This has probably been seen
> > by many, but Jonas Fonseca raised the issue.
>
> Hmpph. This takes 19 seconds to run the suite, where it used to be
> only 2 seconds on the same system. The slower run isn't something
> I'm too happy about, actually I'd like to make the run even faster
> than 2 seconds.
So maybe we should clean up less. Every new test repo we create has
a new name so we could do without cleaning up so much. The cleanup
however is a verification that we close (and can close) our resources,
though it only works on Windows :/ On unix we could spawn lsof but that
is really really slow.
> If the issue is mmap'd files, why don't we instead disable mmap
> on Windows during JUnit tests, and use the non-mmap variant of
> pack access? At least do that for the bulk of the tests, and
> then have a single test case which tests the mmap code path but
> has careful System.gc calls in place to try and ensure we can
> actually clean up the temporary files.
We would then need some other really slow test to play rough with
memory mapping and gc. As I mentioned above it is actually about
closing resources in general, mmapped files being an especially
nasty case.
> I'd actually like to get that Repository refactoring done soon,
> someone else was asking about it for the RefDatabase (to store the
> refs in a SQL database so JGit ties into JTA) but I may also want
> it for Gerrit 2 - I'm looking at doing something that would put
> 200,000 refs per year into a repository. That's so large that
> most operations can't afford to scan the entire ref database,
> and it really cannot be loose. ;-)
Would be cool, but having that diff engine is more important to me.
>
>
> Refactoring repository is a fair chunk of work, disabling the mmap
> feature under Windows in JUnit may be easier. Hmm, according to
> WindowCache's <clinit> its default by false. Why is it enabling
> on Windows? The only code that calls WindowCache.reconfigure()
> is in the Eclipse plugin, so pure JGit unit tests shouldn't be
> turning on mmap code *at all*.
>
> Which also points out a gap in our tests. Nothing new, we have
> lots of gaps. *sigh*
Yep. :/
-- robin
^ permalink raw reply
* [PATCHv2ter 2/2] gitweb: clean up gitweb_check_feature() calls
From: Giuseppe Bilotta @ 2008-11-29 11:53 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1227904793-1821-3-git-send-email-giuseppe.bilotta@gmail.com>
Since gitweb_check_feature now returns a scalar, the parentheses around
the variables initialized from gitweb_check_feature() are not needed.
We remove them for stylistic consistency and to prevent unnecessary
ambiguity in the code.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
Third attempt, hopefully I got the commit message right, finally 8-P
gitweb/gitweb.perl | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 128d7ad..b0d00ea 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -828,7 +828,7 @@ sub href (%) {
}
}
- my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
# try to put as many parameters as possible in PATH_INFO:
# - project name
@@ -2119,7 +2119,7 @@ sub git_get_projects_list {
$filter ||= '';
$filter =~ s/\.git$//;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
if (-d $projects_list) {
# search in directory
@@ -2965,7 +2965,7 @@ EOF
}
print "</div>\n";
- my ($have_search) = gitweb_check_feature('search');
+ my $have_search = gitweb_check_feature('search');
if (defined $project && $have_search) {
if (!defined $searchtext) {
$searchtext = "";
@@ -2979,7 +2979,7 @@ EOF
$search_hash = "HEAD";
}
my $action = $my_uri;
- my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
$action .= "/".esc_url($project);
}
@@ -3472,7 +3472,7 @@ sub is_patch_split {
sub git_difftree_body {
my ($difftree, $hash, @parents) = @_;
my ($parent) = $parents[0];
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
print "<div class=\"list_head\">\n";
if ($#{$difftree} > 10) {
print(($#{$difftree} + 1) . " files changed:\n");
@@ -3986,7 +3986,7 @@ sub git_project_list_body {
# actually uses global variable $project
my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
my @projects = fill_project_list_info($projlist, $check_forks);
$order ||= $default_projects_order;
@@ -4446,7 +4446,7 @@ sub git_summary {
my @taglist = git_get_tags_list(16);
my @headlist = git_get_heads_list(16);
my @forklist;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
if ($check_forks) {
@forklist = git_get_projects_list($project);
@@ -4475,7 +4475,7 @@ sub git_summary {
}
# Tag cloud
- my $show_ctags = (gitweb_check_feature('ctags'))[0];
+ my $show_ctags = gitweb_check_feature('ctags');
if ($show_ctags) {
my $ctags = git_get_project_ctags($project);
my $cloud = git_populate_project_tagcloud($ctags);
@@ -4765,7 +4765,7 @@ sub git_blob {
$expires = "+1d";
}
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
or die_error(500, "Couldn't cat $file_name, $hash");
my $mimetype = blob_mimetype($fd, $file_name);
@@ -4858,7 +4858,7 @@ sub git_tree {
my $ref = format_ref_marker($refs, $hash_base);
git_header_html();
my $basedir = '';
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
if (defined $hash_base && (my %co = parse_commit($hash_base))) {
my @views_nav = ();
if (defined $file_name) {
@@ -5856,7 +5856,7 @@ insensitive).</p>
<dt><b>commit</b></dt>
<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>
EOT
- my ($have_grep) = gitweb_check_feature('grep');
+ my $have_grep = gitweb_check_feature('grep');
if ($have_grep) {
print <<EOT;
<dt><b>grep</b></dt>
@@ -5873,7 +5873,7 @@ EOT
<dt><b>committer</b></dt>
<dd>Name and e-mail of the committer and date of commit will be scanned for the given pattern.</dd>
EOT
- my ($have_pickaxe) = gitweb_check_feature('pickaxe');
+ my $have_pickaxe = gitweb_check_feature('pickaxe');
if ($have_pickaxe) {
print <<EOT;
<dt><b>pickaxe</b></dt>
@@ -5925,7 +5925,7 @@ sub git_shortlog {
sub git_feed {
my $format = shift || 'atom';
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
# Atom: http://www.atomenabled.org/developers/syndication/
# RSS: http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCHv2 2/2] gitweb: clean up gitweb_check_feature() calls
From: Jakub Narebski @ 2008-11-29 11:18 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <1227957301-7417-1-git-send-email-giuseppe.bilotta@gmail.com>
Giuseppe Bilotta wrote:
> Since git_check_feature now returns a scalar, the parentheses around the
^^^
\---- missed one.
> variables initialized from gitweb_check_feature() are not needed.
>
> We remove them for stylistic consistency and to prevent unnecessary
> ambiguity in the code.
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>
> Fixed the commit message, use this instead of the previous one.
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCHv2 2/2] gitweb: clean up gitweb_check_feature() calls
From: Giuseppe Bilotta @ 2008-11-29 11:15 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1227904793-1821-3-git-send-email-giuseppe.bilotta@gmail.com>
Since git_check_feature now returns a scalar, the parentheses around the
variables initialized from gitweb_check_feature() are not needed.
We remove them for stylistic consistency and to prevent unnecessary
ambiguity in the code.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
Fixed the commit message, use this instead of the previous one.
gitweb/gitweb.perl | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 128d7ad..b0d00ea 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -828,7 +828,7 @@ sub href (%) {
}
}
- my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
# try to put as many parameters as possible in PATH_INFO:
# - project name
@@ -2119,7 +2119,7 @@ sub git_get_projects_list {
$filter ||= '';
$filter =~ s/\.git$//;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
if (-d $projects_list) {
# search in directory
@@ -2965,7 +2965,7 @@ EOF
}
print "</div>\n";
- my ($have_search) = gitweb_check_feature('search');
+ my $have_search = gitweb_check_feature('search');
if (defined $project && $have_search) {
if (!defined $searchtext) {
$searchtext = "";
@@ -2979,7 +2979,7 @@ EOF
$search_hash = "HEAD";
}
my $action = $my_uri;
- my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
$action .= "/".esc_url($project);
}
@@ -3472,7 +3472,7 @@ sub is_patch_split {
sub git_difftree_body {
my ($difftree, $hash, @parents) = @_;
my ($parent) = $parents[0];
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
print "<div class=\"list_head\">\n";
if ($#{$difftree} > 10) {
print(($#{$difftree} + 1) . " files changed:\n");
@@ -3986,7 +3986,7 @@ sub git_project_list_body {
# actually uses global variable $project
my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
my @projects = fill_project_list_info($projlist, $check_forks);
$order ||= $default_projects_order;
@@ -4446,7 +4446,7 @@ sub git_summary {
my @taglist = git_get_tags_list(16);
my @headlist = git_get_heads_list(16);
my @forklist;
- my ($check_forks) = gitweb_check_feature('forks');
+ my $check_forks = gitweb_check_feature('forks');
if ($check_forks) {
@forklist = git_get_projects_list($project);
@@ -4475,7 +4475,7 @@ sub git_summary {
}
# Tag cloud
- my $show_ctags = (gitweb_check_feature('ctags'))[0];
+ my $show_ctags = gitweb_check_feature('ctags');
if ($show_ctags) {
my $ctags = git_get_project_ctags($project);
my $cloud = git_populate_project_tagcloud($ctags);
@@ -4765,7 +4765,7 @@ sub git_blob {
$expires = "+1d";
}
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
or die_error(500, "Couldn't cat $file_name, $hash");
my $mimetype = blob_mimetype($fd, $file_name);
@@ -4858,7 +4858,7 @@ sub git_tree {
my $ref = format_ref_marker($refs, $hash_base);
git_header_html();
my $basedir = '';
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
if (defined $hash_base && (my %co = parse_commit($hash_base))) {
my @views_nav = ();
if (defined $file_name) {
@@ -5856,7 +5856,7 @@ insensitive).</p>
<dt><b>commit</b></dt>
<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>
EOT
- my ($have_grep) = gitweb_check_feature('grep');
+ my $have_grep = gitweb_check_feature('grep');
if ($have_grep) {
print <<EOT;
<dt><b>grep</b></dt>
@@ -5873,7 +5873,7 @@ EOT
<dt><b>committer</b></dt>
<dd>Name and e-mail of the committer and date of commit will be scanned for the given pattern.</dd>
EOT
- my ($have_pickaxe) = gitweb_check_feature('pickaxe');
+ my $have_pickaxe = gitweb_check_feature('pickaxe');
if ($have_pickaxe) {
print <<EOT;
<dt><b>pickaxe</b></dt>
@@ -5925,7 +5925,7 @@ sub git_shortlog {
sub git_feed {
my $format = shift || 'atom';
- my ($have_blame) = gitweb_check_feature('blame');
+ my $have_blame = gitweb_check_feature('blame');
# Atom: http://www.atomenabled.org/developers/syndication/
# RSS: http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCHv2 0/2] fixes to gitweb feature check code
From: Giuseppe Bilotta @ 2008-11-29 11:13 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <200811291148.15182.jnareb@gmail.com>
2008/11/29 Jakub Narebski <jnareb@gmail.com>:
> Dnia piątek 28. listopada 2008 21:39, Giuseppe Bilotta napisał:
>> The first patch introduces git_get_feature() to clearly distinguish the
>> feature retrieval from the boolean feature check (which is kept at
>> git_check_feature()). The new function is used where appropriate.
> It is gitweb_get_feature() (for retrieval) and gitweb_check_feature()
> (for checking), not git_get_feature() and git_check_feature().
Ahem. Right. I'll resend the second patch with a fixed subject and
commit message 8-P
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCHv2 0/2] fixes to gitweb feature check code
From: Jakub Narebski @ 2008-11-29 10:48 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <1227904793-1821-1-git-send-email-giuseppe.bilotta@gmail.com>
Dnia piątek 28. listopada 2008 21:39, Giuseppe Bilotta napisał:
> This is v2 of the gitweb feature check fix patch, which has now been
> split into into a code patch and cleanup patch.
>
> The first patch introduces git_get_feature() to clearly distinguish the
> feature retrieval from the boolean feature check (which is kept at
> git_check_feature()). The new function is used where appropriate.
>
> The second patch cleans up use of git_check_feature(): since the
> function now returns a boolean instead of an array, the often-used
> construct
> my ($somevar) = git_check_feature('somefeat');
> although still valid, becomes a rather clumsy stylistic choice, as it
> introduces an unnecessary ambiguity. Make it clear that we're now
> dealing with scalars by using scalar assignment.
It is gitweb_get_feature() (for retrieval) and gitweb_check_feature()
(for checking), not git_get_feature() and git_check_feature().
I really like this series; making coding easier by removing need to
use special construct, and making it easier to avoid mistakes is always
a good thing.
> Giuseppe Bilotta (2):
> gitweb: fixes to gitweb feature check code
> gitweb: clean up git_check_feature() calls
>
> gitweb/gitweb.perl | 52 +++++++++++++++++++++++++++++++++++-----------------
> 1 files changed, 35 insertions(+), 17 deletions(-)
I also like the split, as it separates improvement (much easier to use
and less error prone gitweb_check_feature()) from style cleanup.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: timestamps not git-cloned
From: Thomas Rast @ 2008-11-29 10:16 UTC (permalink / raw)
To: Stephen R. van den Berg; +Cc: Chris Frey, jidanni, dhruvakm, git
In-Reply-To: <20081129092231.GA32630@cuci.nl>
[-- Attachment #1: Type: text/plain, Size: 1726 bytes --]
Stephen R. van den Berg wrote:
> Chris Frey wrote:
> >If this is the important bit, perhaps git-archive could be changed
> >to create tarballs with file timestamps based on their commit dates.
>
> Based on the principle of least surprise, I'd consider this a rather good
> idea.
Unless I'm missing something, this would make git-archive rather more
expensive than it is now: Tree objects do not record any timestamps,
so figuring out the last commit that changed a file requires a full
history walk in the worst case[*]. (This is another side-effect of
not versioning files.) On the other hand, current git-archive's
running time depends only on the size of the tree-ish given, including
all subtrees and blobs.
My unscientific guesstimates on how much work this would be, in a
random (old) linux-2.6 clone:
$ git rev-parse HEAD
e013e13bf605b9e6b702adffbe2853cfc60e7806
$ time git ls-tree -r -t $(git rev-list HEAD~5000..HEAD) >/dev/null
real 0m1.385s
user 0m1.164s
sys 0m0.220s
$ git rev-list HEAD | wc -l
117812
So reading (and dumping) all those trees and subtrees incurs a penalty
on the order of 30 seconds. Compare to the current running time of
git-archive:
$ time git archive --format=tar HEAD >/dev/null
real 0m2.790s
user 0m2.684s
sys 0m0.072s
Of course, the ratio will keep getting worse as history gets longer.
- Thomas
[*] I think to really have a "worst case" here, you need at least one
file in every leaf directory that has not changed since the root
commit, and another that changes in every commit to force the search
to really read every subtree.
--
Thomas Rast
trast@{inf,student}.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: timestamps not git-cloned
From: Stephen R. van den Berg @ 2008-11-29 9:22 UTC (permalink / raw)
To: Chris Frey; +Cc: jidanni, dhruvakm, git
In-Reply-To: <20081129085406.GA20428@foursquare.net>
Chris Frey wrote:
>On Fri, Nov 28, 2008 at 01:06:00PM +0800, jidanni@jidanni.org wrote:
>> I hate it when I get some latest WhizBang.tgz, only to untar it to
>> find all the files' dates the same, when in fact the README hasn't
>> been touched in seven years, but you can't tell that from ls -l. I
>> recall some content tracker was involved.
>If this is the important bit, perhaps git-archive could be changed
>to create tarballs with file timestamps based on their commit dates.
Based on the principle of least surprise, I'd consider this a rather good
idea.
--
Sincerely,
Stephen R. van den Berg.
To people that say "I could care less" - well, why don't you?
^ permalink raw reply
* Re: timestamps not git-cloned
From: Chris Frey @ 2008-11-29 8:54 UTC (permalink / raw)
To: jidanni; +Cc: dhruvakm, git
In-Reply-To: <87tz9sv3rb.fsf@jidanni.org>
On Fri, Nov 28, 2008 at 01:06:00PM +0800, jidanni@jidanni.org wrote:
> I hate it when I get some latest WhizBang.tgz, only to untar it to
> find all the files' dates the same, when in fact the README hasn't
> been touched in seven years, but you can't tell that from ls -l. I
> recall some content tracker was involved.
If this is the important bit, perhaps git-archive could be changed
to create tarballs with file timestamps based on their commit dates.
- Chris
^ permalink raw reply
* [PATCH 3/3] git add --intent-to-add: do not let an empty blob committed by accident
From: Junio C Hamano @ 2008-11-29 3:56 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Johannes Schindelin, git
In-Reply-To: <7vvdu72nq9.fsf@gitster.siamese.dyndns.org>
Writing a tree out of an index with an "intent to add" entry is blocked.
This implies that you cannot "git commit" from such a state; however "git
commit -a" still works.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-write-tree.c | 2 +-
cache-tree.c | 10 +++++++---
t/t3701-add-interactive.sh | 18 ++++++++++++++++++
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/builtin-write-tree.c b/builtin-write-tree.c
index 52a3c01..9d64050 100644
--- a/builtin-write-tree.c
+++ b/builtin-write-tree.c
@@ -42,7 +42,7 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
die("%s: error reading the index", me);
break;
case WRITE_TREE_UNMERGED_INDEX:
- die("%s: error building trees; the index is unmerged?", me);
+ die("%s: error building trees", me);
break;
case WRITE_TREE_PREFIX_ERROR:
die("%s: prefix %s not found", me, prefix);
diff --git a/cache-tree.c b/cache-tree.c
index 5f8ee87..3d8f218 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -155,13 +155,17 @@ static int verify_cache(struct cache_entry **cache,
funny = 0;
for (i = 0; i < entries; i++) {
struct cache_entry *ce = cache[i];
- if (ce_stage(ce)) {
+ if (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {
if (10 < ++funny) {
fprintf(stderr, "...\n");
break;
}
- fprintf(stderr, "%s: unmerged (%s)\n",
- ce->name, sha1_to_hex(ce->sha1));
+ if (ce_stage(ce))
+ fprintf(stderr, "%s: unmerged (%s)\n",
+ ce->name, sha1_to_hex(ce->sha1));
+ else
+ fprintf(stderr, "%s: not added yet\n",
+ ce->name);
}
}
if (funny)
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index e95663d..473ef85 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -133,6 +133,24 @@ test_expect_success 'real edit works' '
test_cmp expected output
'
+test_expect_success 'cannot commit with i-t-a entry' '
+ git reset --hard &&
+ echo xyzzy >rezrov &&
+ echo frotz >nitfol &&
+ git add rezrov &&
+ git add -N nitfol &&
+ test_must_fail git commit
+'
+
+test_expect_success 'can commit with an unrelated i-t-a entry in index' '
+ git reset --hard &&
+ echo xyzzy >rezrov &&
+ echo frotz >nitfol &&
+ git add rezrov &&
+ git add -N nitfol &&
+ git commit -m partial rezrov
+'
+
if test "$(git config --bool core.filemode)" = false
then
say 'skipping filemode tests (filesystem does not properly support modes)'
--
1.6.0.4.850.g6bd829
^ permalink raw reply related
* [PATCH 2/3] git add --intent-to-add: fix removal of cached emptiness
From: Junio C Hamano @ 2008-11-29 3:55 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Johannes Schindelin, git
In-Reply-To: <7vvdu72nq9.fsf@gitster.siamese.dyndns.org>
This uses the extended index flag mechanism introduced earlier to mark
the entries added to the index via "git add -N" with CE_INTENT_TO_ADD.
The logic to detect an "intent to add" entry for the purpose of allowing
"git rm --cached $path" is tightened to check not just for a staged empty
blob, but with the CE_INTENT_TO_ADD bit. This protects an empty blob that
was explicitly added and then modified in the work tree from being dropped
with this sequence:
$ >empty
$ git add empty
$ echo "non empty" >empty
$ git rm --cached empty
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-rm.c | 2 +-
cache.h | 3 ++-
read-cache.c | 2 ++
t/t3600-rm.sh | 4 ++--
4 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/builtin-rm.c b/builtin-rm.c
index 3d03da0..c11f455 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -115,7 +115,7 @@ static int check_local_mod(unsigned char *head, int index_only)
* "intent to add" entry.
*/
if (local_changes && staged_changes) {
- if (!index_only || !is_empty_blob_sha1(ce->sha1))
+ if (!index_only || !(ce->ce_flags & CE_INTENT_TO_ADD))
errs = error("'%s' has staged content different "
"from both the file and the HEAD\n"
"(use -f to force removal)", name);
diff --git a/cache.h b/cache.h
index ef2e7f9..f15b3fc 100644
--- a/cache.h
+++ b/cache.h
@@ -176,10 +176,11 @@ struct cache_entry {
/*
* Extended on-disk flags
*/
+#define CE_INTENT_TO_ADD 0x20000000
/* CE_EXTENDED2 is for future extension */
#define CE_EXTENDED2 0x80000000
-#define CE_EXTENDED_FLAGS (0)
+#define CE_EXTENDED_FLAGS (CE_INTENT_TO_ADD)
/*
* Safeguard to avoid saving wrong flags:
diff --git a/read-cache.c b/read-cache.c
index abc627b..fa30a0f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -546,6 +546,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
ce->ce_flags = namelen;
if (!intent_only)
fill_stat_cache_info(ce, st);
+ else
+ ce->ce_flags |= CE_INTENT_TO_ADD;
if (trust_executable_bit && has_symlinks)
ce->ce_mode = create_ce_mode(st_mode);
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 5b4d6f7..b7d46e5 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -187,8 +187,8 @@ test_expect_success 'but with -f it should work.' '
test_must_fail git ls-files --error-unmatch baz
'
-test_expect_failure 'refuse to remove cached empty file with modifications' '
- touch empty &&
+test_expect_success 'refuse to remove cached empty file with modifications' '
+ >empty &&
git add empty &&
echo content >empty &&
test_must_fail git rm --cached empty
--
1.6.0.4.850.g6bd829
^ permalink raw reply related
* [PATCH 1/3] builtin-rm.c: explain and clarify the "local change" logic
From: Junio C Hamano @ 2008-11-29 3:51 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Johannes Schindelin, git
In-Reply-To: <7vk5an4cba.fsf_-_@gitster.siamese.dyndns.org>
Explain the logic to check local modification a bit more in the comment,
especially because the existing comment that talks about "git rm --cached"
was placed in a part that was not about "--cached" at all.
Also clarify "if .. else if .." structure.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This is the first patch of a re-rolled sesries of the previous one, and
again applies to the master plus the first commit from nd/narrow.
builtin-rm.c | 53 ++++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 42 insertions(+), 11 deletions(-)
diff --git a/builtin-rm.c b/builtin-rm.c
index b7126e3..3d03da0 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -31,7 +31,8 @@ static void add_list(const char *name)
static int check_local_mod(unsigned char *head, int index_only)
{
- /* items in list are already sorted in the cache order,
+ /*
+ * Items in list are already sorted in the cache order,
* so we could do this a lot more efficiently by using
* tree_desc based traversal if we wanted to, but I am
* lazy, and who cares if removal of files is a tad
@@ -71,25 +72,55 @@ static int check_local_mod(unsigned char *head, int index_only)
*/
continue;
}
+
+ /*
+ * "rm" of a path that has changes need to be treated
+ * carefully not to allow losing local changes
+ * accidentally. A local change could be (1) file in
+ * work tree is different since the index; and/or (2)
+ * the user staged a content that is different from
+ * the current commit in the index.
+ *
+ * In such a case, you would need to --force the
+ * removal. However, "rm --cached" (remove only from
+ * the index) is safe if the index matches the file in
+ * the work tree or the HEAD commit, as it means that
+ * the content being removed is available elsewhere.
+ */
+
+ /*
+ * Is the index different from the file in the work tree?
+ */
if (ce_match_stat(ce, &st, 0))
local_changes = 1;
+
+ /*
+ * Is the index different from the HEAD commit? By
+ * definition, before the very initial commit,
+ * anything staged in the index is treated by the same
+ * way as changed from the HEAD.
+ */
if (no_head
|| get_tree_entry(head, name, sha1, &mode)
|| ce->ce_mode != create_ce_mode(mode)
|| hashcmp(ce->sha1, sha1))
staged_changes = 1;
- if (local_changes && staged_changes &&
- !(index_only && is_empty_blob_sha1(ce->sha1)))
- errs = error("'%s' has staged content different "
- "from both the file and the HEAD\n"
- "(use -f to force removal)", name);
+ /*
+ * If the index does not match the file in the work
+ * tree and if it does not match the HEAD commit
+ * either, (1) "git rm" without --cached definitely
+ * will lose information; (2) "git rm --cached" will
+ * lose information unless it is about removing an
+ * "intent to add" entry.
+ */
+ if (local_changes && staged_changes) {
+ if (!index_only || !is_empty_blob_sha1(ce->sha1))
+ errs = error("'%s' has staged content different "
+ "from both the file and the HEAD\n"
+ "(use -f to force removal)", name);
+ }
else if (!index_only) {
- /* It's not dangerous to "git rm --cached" a
- * file if the index matches the file or the
- * HEAD, since it means the deleted content is
- * still available somewhere.
- */
if (staged_changes)
errs = error("'%s' has changes staged in the index\n"
"(use --cached to keep the file, "
--
1.6.0.4.850.g6bd829
^ permalink raw reply related
* Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)
From: Daniel Barkalow @ 2008-11-29 1:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <7voczz4cfb.fsf@gitster.siamese.dyndns.org>
On Fri, 28 Nov 2008, Junio C Hamano wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > ...
> >> In other words, unless there is more interest in that feature, enough to
> >> generate a well-understood design before a good implementation, I'd rather
> >> see this patch series dropped.
> >
> > Ack. I agree with every remark made by Dscho, and also want to cry "wolf".
> >
> > I haven't had time to read the patch series. Its big and intrusive
> > and I just don't need the feature.
>
> Well, "me neither". Although I personally think resisting changes until
> it becomes absolutely necessary is a good discipline, we also need to
> recognise that there is a chicken-and-egg problem. When you have a
> potentially useful feature, unless people actually try using it in the
> field, you won't discover the drawbacks in either the design nor the
> implementation, let alone any improvements.
I just looked over most of it (skipping the generic index extension
portion). It looks to me like it's introducing an extra concept to avoid
actually fixing maybe-bugs in the "assume unchanged" implementation
when used with files that have been changed intentionally (with the user
intending git to overlook this change). Sparse checkout is essentially a
special case of this, where the user has changed the working directory
radically (not populating it at all) and wants git to carry on as if this
was not the case (with a certain amount of porcelain code to cause this to
happen automatically).
If there's any need for this to be distinguished from "assume unchanged",
I think it should be used with, not instead of, the CE_VALID bit; and it
could probably use some bit in the stat info section, since we don't need
stat info if we know by assumption that the entry is valid.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: summaries in git add --patch
From: William Pursell @ 2008-11-29 0:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8wr48g98.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> William Pursell <bill.pursell@gmail.com> writes:
>
>> Here's a new patch. Instead of displaying the summary and then
>> the current hunk, it implements a 'goto' command.
>
> I take it that this is for discussion not for immediate inclusion.
>
Yes. I tend to think of all of my patches as being merely
for discussion since I'm not terribly familiar with the code
base and expect to miss many things. I'm flattered that
you would even consider them for inclusion. For that matter,
I'm flattered that you have even responded to my submissions!
>> @@ -799,6 +801,7 @@ sub help_patch_cmd {
>> y - stage this hunk
>> n - do not stage this hunk
>> a - stage this and all the remaining hunks in the file
>> +g - select a hunk to jump to
>> d - do not stage this hunk nor any of the remaining hunks in the file
>> j - leave this hunk undecided, see next undecided hunk
>> J - leave this hunk undecided, see next hunk
>
> Since you took 'g' after "go to", help text should also say "go to",
> instead of "jump to" for the mnemonics value, iow, to help people
> remember.
Agreed.
>> @@ -836,6 +839,27 @@ sub patch_update_cmd {
>> }
>> }
>>
>> +sub select_new_hunk {
>> + my $ri = shift;
>> + my @hunk = @_;
>> + my ($i, $response);
>> + print " '+' stage, '-' don't stage\n";
>> + for ( $i = 0; $i < @hunk; $i++ ) {
>> + my $status = " ";
>> + if( defined $hunk[$i]{USE} ) {
>> + $status = $hunk[$i]{USE} ? "+" : "-";
>> + }
>
> Style.
>
> (1) SP between language construct and open parenthesis, as opposed to
> no extra SP between function name and open parenthesis;
>
> (2) No extra SP around what is enclosed in parentheses.
My apologies for that. I do try to conform, but this
sort of habit is hard to change. Especially in perl,
where code so often looks like a cartoon character's
speech bubble while swearing (eg #@$!%#@@), I
like to put space inside my parens. I'm fully aware
that this is not the preferred style here, and I
am trying to conform. Is there a style validating
pre-commit hook script available?
>> + printf "%s%3d: %s",
>> + $status,
>> + $i + 1,
>> + $hunk[$i]{SUMMARY};
>> + }
>
> I think this "for ()" loop part, including the comment about +/- notation,
> should be separated into a function so that you can implement a separate
> "l"ist command like you did in the other patch, using the same function.
My thought is that 'g' would replace 'l', but as per your previous
email 'l' would be a reasonable status command, so it
makes sense to factor it out.
>
>> + printf "goto which hunk? ";
>> + $response = <STDIN>;
>> + chomp $response;
>> + $$ri = $response - 1;
>
> What happens when $response is (1) a non number, (2) outside range (both
> negative and positive), or (3) EOF?
>
> Sending ref to scalar and returning the value by assigning is a bad taste.
> Why shouldn't this function just return an integer to be assigned to $ix
> by the caller? If you want to use pass-by-ref to show off your Perl-fu, I
> think \@hunk would be what you would want to for performance reasons.
Ack. I have no Perl-fu, I'm just not familiar with the
idioms and thought this was accepted in perl. I agree
that it's poor judgement, and can only attribute my
usage of it here to laziness. ( At one point I was passing
$ix, and when I realized I wanted to modify it at the caller
it was just easier to pass by reference.)
>
>> @@ -919,7 +943,7 @@ sub patch_update_file {
>> for (@{$hunk[$ix]{DISPLAY}}) {
>> print;
>> }
>> - print colored $prompt_color, "Stage this hunk [y/n/a/d$other/?]? ";
>> + print colored $prompt_color, "Stage this hunk [y/n/a/d/g$other/?]? ";
>
> When there is only one hunk, we do not give j nor k. Should we give g in
> such a case? Why?
I would agree that g should be invalid when only one hunk is
available. I hadn't considered that case.
>
>> @@ -937,6 +961,16 @@ sub patch_update_file {
>> }
>> next;
>> }
>> + elsif ($line =~ /^g/) {
>> + chomp ($line);
>> + if ($line =~ /^g$/) {
>> + select_new_hunk (\$ix, @hunk);
>> + }
>> + else {
>> + $ix = (substr $line, 1) - 1;
>> + }
>
> The same "input validation" issue exists here. it would make sense to:
>
> - Make choose_hunk(@hunk) that calls list_hunks(@hunk) that gives the
> summary, reads one line, and returns that line;
>
> - Make the caller here to look like this:
>
> elsif ($line =~ s/^g//) {
> chomp($line);
> if ($line eq '') {
> $line = choose_hunk(@hunk);
> }
> if ($line !~ /^\d+$/) {
> print STDERR "Eh '$line', what number is that?\n";
> next;
> } elsif (0 < $line && $line <= $num) {
> $ix = $line - 1;
> } else {
> print STDERR "Sorry, you have only $num hunks\n";
> }
> }
>
>> + next;
>> + }
>> elsif ($line =~ /^d/i) {
>> while ($ix < $num) {
>> if (!defined $hunk[$ix]{USE}) {
I will try to incorporate your ideas into a workable,
includable patch. I think the '/' regex search can
become part of the choose_hunk() routine so that
an integer response means select by number while
a '/re' response means jump forward to next matching
hunk. Also, instead of storing the summary line in
the hunk, it will probably be better to generate on
the fly during the display routine.
Thanks for the feedback.
--
William Pursell
^ permalink raw reply
* [PATCH] git add --intent-to-add: fix removal of cached emptiness
From: Junio C Hamano @ 2008-11-29 0:15 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Johannes Schindelin, git
In-Reply-To: <7voczz4cfb.fsf@gitster.siamese.dyndns.org>
This uses the extended index flag mechanism introduced earlier to mark
the entries added to the index via "git add -N" with CE_INTENT_TO_ADD.
The logic to detect an "intent to add" entry for the purpose of allowing
"git rm --cached $path" is tightened to check not just for a staged empty
blob, but with the CE_INTENT_TO_ADD bit. This protects an empty blob that
was explicitly added and then modified in the work tree from being dropped
with this sequence:
$ >empty
$ git add empty
$ echo "non empty" >empty
$ git rm --cached empty
An index an "intent to add" entry is blocked. This implies that you
cannot "git commit" from such a state; however "git commit -a" still
works.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This applies on top of the result of merging commit 06aaaa0 (Extend
index to save more flags, 2008-10-01) from nd/narrow topic into 'master'.
builtin-rm.c | 11 ++++++-----
builtin-write-tree.c | 2 +-
cache-tree.c | 10 +++++++---
cache.h | 3 ++-
read-cache.c | 2 ++
t/t3600-rm.sh | 4 ++--
t/t3701-add-interactive.sh | 18 ++++++++++++++++++
7 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/builtin-rm.c b/builtin-rm.c
index b7126e3..8debcec 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -73,14 +73,15 @@ static int check_local_mod(unsigned char *head, int index_only)
}
if (ce_match_stat(ce, &st, 0))
local_changes = 1;
- if (no_head
- || get_tree_entry(head, name, sha1, &mode)
- || ce->ce_mode != create_ce_mode(mode)
- || hashcmp(ce->sha1, sha1))
+ if (no_head ||
+ ((get_tree_entry(head, name, sha1, &mode)
+ || ce->ce_mode != create_ce_mode(mode)
+ || hashcmp(ce->sha1, sha1)) &&
+ !(ce->ce_flags & CE_INTENT_TO_ADD)))
staged_changes = 1;
if (local_changes && staged_changes &&
- !(index_only && is_empty_blob_sha1(ce->sha1)))
+ !(index_only && (ce->ce_flags & CE_INTENT_TO_ADD)))
errs = error("'%s' has staged content different "
"from both the file and the HEAD\n"
"(use -f to force removal)", name);
diff --git a/builtin-write-tree.c b/builtin-write-tree.c
index 52a3c01..9d64050 100644
--- a/builtin-write-tree.c
+++ b/builtin-write-tree.c
@@ -42,7 +42,7 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
die("%s: error reading the index", me);
break;
case WRITE_TREE_UNMERGED_INDEX:
- die("%s: error building trees; the index is unmerged?", me);
+ die("%s: error building trees", me);
break;
case WRITE_TREE_PREFIX_ERROR:
die("%s: prefix %s not found", me, prefix);
diff --git a/cache-tree.c b/cache-tree.c
index 5f8ee87..3d8f218 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -155,13 +155,17 @@ static int verify_cache(struct cache_entry **cache,
funny = 0;
for (i = 0; i < entries; i++) {
struct cache_entry *ce = cache[i];
- if (ce_stage(ce)) {
+ if (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {
if (10 < ++funny) {
fprintf(stderr, "...\n");
break;
}
- fprintf(stderr, "%s: unmerged (%s)\n",
- ce->name, sha1_to_hex(ce->sha1));
+ if (ce_stage(ce))
+ fprintf(stderr, "%s: unmerged (%s)\n",
+ ce->name, sha1_to_hex(ce->sha1));
+ else
+ fprintf(stderr, "%s: not added yet\n",
+ ce->name);
}
}
if (funny)
diff --git a/cache.h b/cache.h
index ef2e7f9..f15b3fc 100644
--- a/cache.h
+++ b/cache.h
@@ -176,10 +176,11 @@ struct cache_entry {
/*
* Extended on-disk flags
*/
+#define CE_INTENT_TO_ADD 0x20000000
/* CE_EXTENDED2 is for future extension */
#define CE_EXTENDED2 0x80000000
-#define CE_EXTENDED_FLAGS (0)
+#define CE_EXTENDED_FLAGS (CE_INTENT_TO_ADD)
/*
* Safeguard to avoid saving wrong flags:
diff --git a/read-cache.c b/read-cache.c
index abc627b..fa30a0f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -546,6 +546,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
ce->ce_flags = namelen;
if (!intent_only)
fill_stat_cache_info(ce, st);
+ else
+ ce->ce_flags |= CE_INTENT_TO_ADD;
if (trust_executable_bit && has_symlinks)
ce->ce_mode = create_ce_mode(st_mode);
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 5b4d6f7..b7d46e5 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -187,8 +187,8 @@ test_expect_success 'but with -f it should work.' '
test_must_fail git ls-files --error-unmatch baz
'
-test_expect_failure 'refuse to remove cached empty file with modifications' '
- touch empty &&
+test_expect_success 'refuse to remove cached empty file with modifications' '
+ >empty &&
git add empty &&
echo content >empty &&
test_must_fail git rm --cached empty
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index e95663d..473ef85 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -133,6 +133,24 @@ test_expect_success 'real edit works' '
test_cmp expected output
'
+test_expect_success 'cannot commit with i-t-a entry' '
+ git reset --hard &&
+ echo xyzzy >rezrov &&
+ echo frotz >nitfol &&
+ git add rezrov &&
+ git add -N nitfol &&
+ test_must_fail git commit
+'
+
+test_expect_success 'can commit with an unrelated i-t-a entry in index' '
+ git reset --hard &&
+ echo xyzzy >rezrov &&
+ echo frotz >nitfol &&
+ git add rezrov &&
+ git add -N nitfol &&
+ git commit -m partial rezrov
+'
+
if test "$(git config --bool core.filemode)" = false
then
say 'skipping filemode tests (filesystem does not properly support modes)'
--
1.6.0.4.850.g6bd829
^ permalink raw reply related
* Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)
From: Junio C Hamano @ 2008-11-29 0:13 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Johannes Schindelin, git
In-Reply-To: <20081128192033.GF23984@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> ...
>> In other words, unless there is more interest in that feature, enough to
>> generate a well-understood design before a good implementation, I'd rather
>> see this patch series dropped.
>
> Ack. I agree with every remark made by Dscho, and also want to cry "wolf".
>
> I haven't had time to read the patch series. Its big and intrusive
> and I just don't need the feature.
Well, "me neither". Although I personally think resisting changes until
it becomes absolutely necessary is a good discipline, we also need to
recognise that there is a chicken-and-egg problem. When you have a
potentially useful feature, unless people actually try using it in the
field, you won't discover the drawbacks in either the design nor the
implementation, let alone any improvements.
> But I feel like if it were in fact merged I'll fall over some bug
> in it sometime soon and be forced to stop and debug it.
Exactly. That is how you make progress.
Having said that, I am willing to carry it over in 'next' outside 'master'
for the 1.6.1 cycle, as three people who are most likely to be able to fix
any potential issues are not using that feature.
> Heck at
> the least I'll have to go back to JGit's index code and implement
> the new file format.
I am sorry to dissapoint you but I am planning to use the first one in the
series, which is the one that adds extended index flag bits, for the fix
to an unrelated feature.
^ permalink raw reply
* Re: remote branch checkout issue, why its *(no branch)
From: Björn Steinbrink @ 2008-11-28 23:24 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Sudipta GHOSH, git
In-Reply-To: <m3k5anjxcd.fsf@localhost.localdomain>
On 2008.11.28 14:32:56 -0800, Jakub Narebski wrote:
> What you can do is to create _local_ branch, closely tied to
> remote-tracking branch 'origin/B1', by using:
>
> $ git branch -b B1 --track origin/B1
You mixed up checkout and branch there ;-)
It's either
git branch B1 origin/B1 # Just create a branch B1
or
git checkout -b B1 origin/B1 # Create and checkout B1
I omitted the "--track" as, by default, it is implied when you create a
new branch from a remote tracking branch.
Björn
^ permalink raw reply
* Re: Grafting mis-aligned trees.
From: Boyd Stephen Smith Jr. @ 2008-11-28 23:01 UTC (permalink / raw)
To: git; +Cc: Michael J Gruber
In-Reply-To: <492289B0.4010903@drmicha.warpmail.net>
[-- Attachment #1: Type: text/plain, Size: 2827 bytes --]
On Tuesday 2008 November 18 03:24, Michael J Gruber wrote:
> Boyd Stephen Smith Jr. venit, vidit, dixit 17.11.2008 23:45:
> > I haven't gotten a response from my subscription email, so please CC me
> > on any replies.
> >
> > So, I've been managaing the source I had from a client project in git and
> > have a non-linear history. Currently, two tips (production and testing)
> > but there are many feature branches that were git-merge'd in, not
> > rebased.
> >
> > Now, I've gotten the full tree. Turns out all the source code I was
> > working on was in a subdirectory "project/web". I'd like to "graft" the
> > *changes* I made onto the full tree.
> >
> > I figured this might be a job for git-filter-branch. Certainly, that did
> > the job of moving all my changes into the subdirectory. But, now I want
> > to do something that's a combination or git-rebase and git-filter-branch.
> > I want to replay the *patches/deltas* (like rebase) on top of the full
> > tree I have, but *maintain the non-liear history* (like filter-branch).
> >
> > Can anyone think of a recipe for me?
> >
> > Trees look something like this right now.
> >
> > <some history> -> FT
> >
> > TI -> <non-linear history> -> A -> <non-linear history> -> C
> > \ \ \
> > -> PI ------------------------> B ------------------------> D
> >
> > I'd like to have it look something like:
> >
> > <some history> -> FT -> <non-linear history> -> A' -> <non-linear
> > history> -> C' \ \ \
> > -> PI' ----------------------> B' -----------------------> D'
> >
> > A', B', C', and D' are different commits, but the diff (and history)
> > between FT and A' is the same as the diff (and history) between TI and A.
> >
> > Again, please CC me on any replies.
>
> [CCing is customary here anyways.]
>
> So, your base directory for TI and FT is different, right? I.e.: In the
> TI repo, your project sits at the root, whereas in the FT repo it sits
> in project/web?
Yes.
> Has FT advanced since you took the initial subdir
> snapshot for TI?
No. Well, maybe. I think the subdir diff is fairly trivial if not empty. TI
is an import from the code actually present on the testing server. FT was
the a subversion repository obtained later after some hullabaloo with the
ex-development house.
Right now this tree is effectively all mine, so I can always graft in commits
to synchronize the common subtree of FT and TI, if that makes things easier.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss03@volumehost.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.org/ \_/
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Removing part of the history
From: Jakub Narebski @ 2008-11-28 22:38 UTC (permalink / raw)
To: Samuel Lucas Vaz de Mello; +Cc: git
In-Reply-To: <49306150.6010701@datacom.ind.br>
Samuel Lucas Vaz de Mello <samuellucas@datacom.ind.br> writes:
> I have a repository with code imported from CVS.
>
> I first imported from CVS using git 1.5.6 and we started to develop
> in git branches. I run cvsimport periodically and merged the
> changes from CVS (legacy developers) to our git branches. After
> some time, I realized that the import was broken, upgraded to git
> 1.6 and reimported the data. Now the imported data seems to be
> correct and I finished with a history tree like this:
>
> (our branch1) X--Y--Z
> merges> / \
> (our branch2) J--K--L--M--N--O--P
> merges> / / /
> (cvs old) ...A--B--C--D--E /
> merges> /
> (cvs new) ...A'-B'-C'-D'-E'-F'
>
>
> There is any way to get rid of the (broken) cvs-old tree?
>
> I've tried to rebase, but the commits from A' and A are different,
> it tries to reapply all history in CVS.
>
> I've tried to checkout a point before our changes start (B') and
> then, using a script, cherry-pick all commits up to HEAD that are
> not in cvs-old branch. This approach didn't handled well merges
> between our branches (K-L-M and X-Y).
>
> The history from the beginning up to the point where our changes
> started (...A--B) is quite large, so it would be nice for us to get
> rid of it.
>
> Any suggestion on how to handle this?
You can use grafts (see "Repository Layout" documentation) to prune
'cvs old' branches, by adding for each merge commit <c_i> with one
side leading to 'cvs old' branch line to .git/info/grafts
<c_i> <c_i^1>
where <c_i> is full hexadecimal object name.
Then check in gitk or some other history viewer if the history now
looks like you wanted (looks sane).
Then use "git filter-branch --all" to turn grafts into reality, check
out correctness, then remove refs/original/* and do git-gc.
HTH
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: remote branch checkout issue, why its *(no branch)
From: Jakub Narebski @ 2008-11-28 22:32 UTC (permalink / raw)
To: Sudipta GHOSH; +Cc: git
In-Reply-To: <9e6474ae0811281415n7b4596afq7b3ce25816d9a639@mail.gmail.com>
"Sudipta GHOSH" <sudipta.in@gmail.com> writes:
> In our git repository there are few branches like
>
> master
> origin/B1
> origin/B2
>
> After doing #git checkout origin/B1 I found that my current branch is
> *(no branch)
>
> #git branch
> *(no branch)
> master
>
> Why its showing *(no branch) why not origin/B1
>
> How I can checkout origin/B1 and after that my current branch will be
> origin/B1 not the *(no branch)
First, 'origin/B1' is so called remote-tracking branch, and its full
name is refs/remotes/origin/B1. Those branches are updated
_automatically_ by git on fetch, to follow branch 'B1' in 'origin'
repository. Therefore, to not lose _your_ changes git doesn't allow
checking out remote-tracking branches. You can checkout _state_ of
such branch into anonymous noname branch, so called in git jargon
"detached HEAD".
What you can do is to create _local_ branch, closely tied to
remote-tracking branch 'origin/B1', by using:
$ git branch -b B1 --track origin/B1
Git would set up configuration for this branch in such way that "git
pull" command given on 'B1' branch would "do the right thing"(TM)
I really recommend reading "Git User's Manual" or "The Git Community
Book" first...
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] git-p4: fix keyword-expansion regex
From: Junio C Hamano @ 2008-11-28 22:29 UTC (permalink / raw)
To: Simon Hausmann; +Cc: Pete Wyckoff, git
In-Reply-To: <200811271745.30963.simon@lst.de>
Simon Hausmann <simon@lst.de> writes:
> On Wednesday 26 November 2008 Junio C Hamano, wrote:
>> I will queue this to 'pu' and wait for an Ack from git-p4 folks. Thanks.
>
> Acked-by: Simon Hausmann <simon@lst.de>
Thanks.
^ permalink raw reply
* remote branch checkout issue, why its *(no branch)
From: Sudipta GHOSH @ 2008-11-28 22:15 UTC (permalink / raw)
To: git
Hi,
In Our git repository there are few branches like
master
orgin/B1
origin/B2
After doing #git checkout orgin/B1 I found that my current branch is
*(no branch)
#git branch
*(no branch)
master
Why its showing *(no branch) why not origin/B1
How I can checkout orgin/B1 and after that my current branch will be
orgin/B1 not the *(no branch)
Regards,
-SG
^ permalink raw reply
* Re: Removing part of the history
From: Miklos Vajna @ 2008-11-28 21:34 UTC (permalink / raw)
To: Samuel Lucas Vaz de Mello; +Cc: git
In-Reply-To: <49306150.6010701@datacom.ind.br>
[-- Attachment #1: Type: text/plain, Size: 639 bytes --]
On Fri, Nov 28, 2008 at 07:23:28PM -0200, Samuel Lucas Vaz de Mello <samuellucas@datacom.ind.br> wrote:
> There is any way to get rid of the (broken) cvs-old tree?
>
> I've tried to rebase, but the commits from A' and A are different, it
> tries to reapply all history in CVS.
Have you tried git rebase --onto?
> I've tried to checkout a point before our changes start (B') and then,
> using a script, cherry-pick all commits up to HEAD that are not in
> cvs-old branch. This approach didn't handled well merges between our
> branches (K-L-M and X-Y).
git rebase --preserve-merges is probably what you are searching for.
[-- Attachment #2: 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