Git development
 help / color / mirror / Atom feed
* [PATCH 3/3] gitweb: Output valid utf8 in git_blame_common('data')
From: Jakub Narebski @ 2011-12-17  9:15 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin
In-Reply-To: <1324113324-21328-1-git-send-email-jnareb@gmail.com>

From: Jürgen Kreileder <jk@blackdown.de>

Otherwise when javascript-actions are enabled gitweb shown broken
author names in the tooltips on blame pages ('blame_incremental'
view).

Signed-off-by: Jürgen Kreileder <jk@blackdown.de>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index dcf4658..d24763b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6244,7 +6244,9 @@ sub git_blame_common {
 			-type=>"text/plain", -charset => "utf-8",
 			-status=> "200 OK");
 		local $| = 1; # output autoflush
-		print while <$fd>;
+		while (my $line = <$fd>) {
+			print to_utf8($line);
+		}
 		close $fd
 			or print "ERROR $!\n";
 
-- 
1.7.6

^ permalink raw reply related

* [PATCH 2/3] gitweb: esc_html() site name for title in OPML
From: Jakub Narebski @ 2011-12-17  9:15 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin
In-Reply-To: <1324113324-21328-1-git-send-email-jnareb@gmail.com>

From: Jürgen Kreileder <jk@blackdown.de>

This escapes the site name in OPML (XML uses the same escaping rules
as HTML).  Also fixes encoding issues because esc_html() uses
to_utf8().

Signed-off-by: Jürgen Kreileder <jk@blackdown.de>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 35126cd..dcf4658 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7863,11 +7863,12 @@ sub git_opml {
 		-charset => 'utf-8',
 		-content_disposition => 'inline; filename="opml.xml"');
 
+	my $title = esc_html($site_name);
 	print <<XML;
 <?xml version="1.0" encoding="utf-8"?>
 <opml version="1.0">
 <head>
-  <title>$site_name OPML Export</title>
+  <title>$title OPML Export</title>
 </head>
 <body>
 <outline text="git RSS feeds">
-- 
1.7.6

^ permalink raw reply related

* [PATCH 1/3] gitweb: Call to_utf8() on input string in chop_and_escape_str()
From: Jakub Narebski @ 2011-12-17  9:15 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin
In-Reply-To: <1324113324-21328-1-git-send-email-jnareb@gmail.com>

From: Jürgen Kreileder <jk@blackdown.de>

a) To fix the comparison with the chopped string,
   otherwise we compare bytes with characters, as
   chop_str() must run to_utf8() for correct operation
b) To give the title attribute correct encoding;
   we need to mark strings as UTF-8 before outpur

Signed-off-by: Jürgen Kreileder <jk@blackdown.de>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f80f259..35126cd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1696,6 +1696,7 @@ sub chop_and_escape_str {
 	my ($str) = @_;
 
 	my $chopped = chop_str(@_);
+	$str = to_utf8($str);
 	if ($chopped eq $str) {
 		return esc_html($chopped);
 	} else {
-- 
1.7.6

^ permalink raw reply related

* [PATCH 0/3] gitweb: Various to_utf8 / esc_html fixes
From: Jakub Narebski @ 2011-12-17  9:15 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin, Jakub Narebski

This is post-release resend of Jürgen patches (which were sent
during feature-freeze).

I have slightly extended commit messages, and added my ACK.

Jürgen Kreileder (3):
  gitweb: Call to_utf8() on input string in chop_and_escape_str()
  gitweb: esc_html() site name for title in OPML
  gitweb: Output valid utf8 in git_blame_common('data')

 gitweb/gitweb.perl |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

-- 
1.7.6

^ permalink raw reply

* Re: [PATCH] Escape file:// URL's to meet subversion SVN::Ra requirements
From: Jonathan Nieder @ 2011-12-17  8:45 UTC (permalink / raw)
  To: Eric Wong; +Cc: Ben Walton, git
In-Reply-To: <20111102220941.GA3925@dcvr.yhbt.net>

Hi Eric,

Eric Wong wrote:

> I don't have much time to help you fix it, but I got numerous errors on
> SVN 1.6.x (svn 1.6.12).  Can you make sure things continue to work on
> 1.6 and earlier, also?
[...]
> Here are the tests that failed for me:
>
> make[1]: *** [t9100-git-svn-basic.sh] Error 1
[...]

I tried the patch with subversion 1.6.17dfsg-3 from Debian and all
tests passed.  Odd.  Could you send output from running the test with
-v -i with Ben's patch applied?

Ah, the subversion CHANGES file mentions a bugfix that might explain
the difference:

| Version 1.6.13
[...]
|    * properly canonicalize a URL (r984928, -31)

Thanks,
Jonathan

^ permalink raw reply

* Re: [msysGit] [PATCH] gitk: fix the display of files when filtered by path
From: Junio C Hamano @ 2011-12-17  6:31 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: Martin von Zweigbergk, Pat Thoyts, Johannes Schindelin,
	David Aguilar, Git, msysGit
In-Reply-To: <20111215225945.GG20629@bloggs.ozlabs.ibm.com>

Paul Mackerras <paulus@samba.org> writes:

> On Thu, Dec 15, 2011 at 11:42:38AM -0800, Martin von Zweigbergk wrote:
>
>> git://git.kernel.org/pub/scm/gitk/gitk.git is still down.
>
> I have just created a repository on ozlabs.org for gitk, since I don't
> have kernel.org access at this point.  The repository is:
>
> git://ozlabs.org/~paulus/gitk.git
> ...
> Your patches are in the master branch.  I applied them back in July
> but then kernel.org went down.

Thanks.

All pulled and resulted in one liner update to the draft Release Notes for
the next release.

diff --git a/Documentation/RelNotes/1.7.9.txt b/Documentation/RelNotes/1.7.9.txt
index cd3c256..f476667 100644
--- a/Documentation/RelNotes/1.7.9.txt
+++ b/Documentation/RelNotes/1.7.9.txt
@@ -4,6 +4,8 @@ Git v1.7.9 Release Notes (draft)
 Updates since v1.7.8
 --------------------
 
+ * Accumulated gitk updates since early this year.
+
  * git-gui updated to 0.16.0.
 
  * git-p4 (in contrib/) updates.

^ permalink raw reply related

* Re: [PATCH] docs: brush up obsolete bits of git-fsck manpage
From: Junio C Hamano @ 2011-12-17  6:10 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111217012811.GC20225@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Using "--patience", on the other hand, does find it as a common line,

Ahh, that must be it.  Thanks, and sorry for the noise.

^ permalink raw reply

* Re: git-p4 issue
From: Michael Horowitz @ 2011-12-17  5:52 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: git
In-Reply-To: <loom.20110513T162233-874@post.gmane.org>

Vitor,

I know it has been a long time, but I finally tried the below, and it
works for me.  I was doing things wrong at first, but I think I
finally understand what is going on...

First, the branchList is great, that is what I needed.  Now I can
completely ignore the p4 branches.  It would be great if there was an
option to not have it query p4 branches at all, because we have so
many branches here it takes forever, and I am just using branchList.
What I did for now was to use your branchUser option, and pass it a
bogus username, like "XXXXXX".  At least this way it returns fast, but
would be nice to not have it try to query at all.

I think I understand now why all this is needed if you want it to
detect branches on its own.  Since in Perforce branches are just
directories, there is no way to tell if a directory is a branch or
not, and people could organize branches in an arbitrary directory
structure.  I was thinking you could look at sibling directories under
the same parent, which may be common, but there is no way to know for
sure, so better to be explicit.

The other odd issue is that if you do not add the new branch to
branchList (or p4 branches if you are using them) before you do the
git-p4 sync, then since it goes through change numbers in order, it
will never go back again and catch the branch in a change it already
synced.  This is why I could never get it to work, because I never had
it in place before I synced the relevant change.  What I realized
though is that I can always do a "git-p4 sync --detect-branches
//depot/foo/bar/@all" again, even though I already have the most
recent change, and it will re-detect any branches I missed (given I
have since added them to branchList).

What I had been thinking though is that if it had already detected
branches once, it could at least update the remote p4 branches it
already knew about to point at the latest revision, just not try to
detect new ones, and not rely on branchList.  Of course you would
still need branchList to detect any new branches.

Anyway, works a lot better than it did before.

Thanks,

Mike


On Fri, May 13, 2011 at 10:31 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> Hi Michael,
>
> Michael Horowitz <michael.horowitz <at> ieee.org> writes:
>
>>
>> Vitor,
>>
>> > Could you please search for the following set of patches in this
>> > mailing list?
>> >
>> > [PATCH v2 0/3] git-p4: Improve branch support
>> >
>> > I think I sent v2 twice somehow, so please make sure you pick the
>> > latest ;)
>> > In these patches I add the possibility to use a "git-p4.branchList"
>> > configuration to define the branches. The patch is still to be
>> > approved because most people in the mailing list do not use branch
>> > detection, but I use it daily and it is working in my side. Could
>> > you please test it?
>>
>> Can you send this patch again?  It looks like you had previously
>> responded to the list only, so I never got this message, as I wasn't
>> on the list at the time (I am now).  I only saw this because I was
>> searching the archive for something else.  I searched for the patch,
>> but the actual patch body doesn't seem to be in the archive.
>
> I am also not in the mailing list, I just follow its RSS and try to
> follow up on the git-p4 related topics ;) That is the reason why you
> were not included in the reply.
>
> But I have been a bit busy and did not see this email passing by. Sorry.
> Luckily I had a tab opened in this thread, which I looked at today
> wondering what it was about! :P
>
> Please follow the link to the thread [1] and you can open each of the
> entries. Take care not to apply patch 1/3 as it may require you to clone
> everything again.
>
>> Thanks,
>>
>> Mike
>>
>
> Regards,
> Vitor
>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/167998/focus=168000
>
>
> --
> 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

^ permalink raw reply

* Re: [PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Jonathan Nieder @ 2011-12-17  3:38 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, Junio C Hamano, git, Brandon Casey
In-Reply-To: <20111217012118.GB20225@sigill.intra.peff.net>

Jeff King wrote:

>                          Or maybe just drop the C ones (and probably the
> objc one based on other parts of the thread) and do the rest?

Yes, that.

This way, someone wondering why the C ones are not used by default can
easily look at your patch, see that they are utterly broken, and help
us fix it.  That's how progress is made.

^ permalink raw reply

* Re:  Adding hooks.directory config option; wiring into run_hook
From: Aaron Schrab @ 2011-12-17  3:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christopher Dale, git
In-Reply-To: <7vmxasie6k.fsf@alter.siamese.dyndns.org>

At 10:06 -0800 16 Dec 2011, Junio C Hamano <gitster@pobox.com> wrote:
>Christopher Dale <chrelad@gmail.com> writes:
>
>> ...
>> trusted path execution policies. These systems require that any file
>> that can be executed exhibit at least the following characteristics:
>>
>>   * The executable, it's directory, and each directory above it are
>>     not writable.
>>
>> Since the hooks directory is within a directory that by it's very nature
>> requires write permissions,...
>
>Sorry, but I am not interested at all. If you can write into $GIT_DIR/config
>then you can point at any directory with hooks.directory and that would mean
>it would defeat your "trusted path execution policies".

How does that defeat the policy?  It would certainly allow somebody who 
can write to $GIT_DIR to disable hooks, use hooks that were meant for a 
different repository, or perhaps even use executables that weren't 
intended to be hooks.  If the proposed configuration option were 
modified by an attacker to point to some directory that doesn't meet the 
requirements, then the underlying system would still prevent execution; 
the same result that would happen if they'd try to install hooks in the 
traditional location.

I see other problems with that policy, at least with the short 
description that was provided.  Unless there are also restrictions on 
the allowed owners of the executable and its containing directories, an 
attacker would still be able to install new executables and then remove 
write permissions before trying to use them.  But, that flaw (if it 
exists) wouldn't be affected by the proposed change to git.

Requiring that all executables on a secured system be installed by an 
admin doesn't seem to be a completely unreasonable requirement.  The 
supplied patch looks to be fairly small and easy to understand, so I 
wouldn't think that including it would present much of a problem for 
maintaining git.

The option might also be useful to allow the same hooks directory to be 
used for multiple repositories, although symlinks would likely be a 
better way to do that.

^ permalink raw reply

* Re:  Adding hooks.directory config option; wiring into run_hook
From: Aaron Schrab @ 2011-12-17  1:58 UTC (permalink / raw)
  To: Christopher Dale; +Cc: git
In-Reply-To: <CADQnX_e76LzuRUDOKFOsRHU=e8Cw+qh5x1BdW5HMEdMmP5PaHg@mail.gmail.com>

At 12:00 -0600 16 Dec 2011, Christopher Dale <chrelad@gmail.com> wrote:
>Since the hooks directory is within a directory that by it's very 
>nature requires write permissions,

Are you sure about this?  I'd think that this would mainly be used for 
bare repositories.  For a test I created a bare repository, removed 
write permission, changed the owner to root, then pushed into it from 
another repository.  So it seems that, at least for basic usage, a bare 
repository can be modified even if with write permission removed since 
no entries are being created or removed at the top level.

For this to be a workaround, new repositories would need to be created 
by an admin.  The requirement that the containing directory not be 
writeable wouldn't necessarily be an issue; at least on Linux I'm able 
to create files/directories as root even in a non-writeable directory.

^ permalink raw reply

* Re: [PATCH] docs: brush up obsolete bits of git-fsck manpage
From: Jeff King @ 2011-12-17  1:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy5ucgsic.fsf@alter.siamese.dyndns.org>

On Fri, Dec 16, 2011 at 12:40:11PM -0800, Junio C Hamano wrote:

> By the way did you hand-tweak your patch in any way?
> 
> I am not complaining that it does not apply (it does), but I am curious
> how you got the line that begins with "corruption it finds ..." neatly in
> preimage and postimage; it could become a common line but doing so makes
> the patch unreadable and that is what I am getting from "git show" after
> applying the patch.

No, I don't think I tweaked it at all. You can fetch the original commit
(888b4eb) at:

  git://github.com/peff/git.git jk/fsck-docs

Running "git show" produces the same output as the patch I sent. I
double-checked with older versions of git, and they all produce the same
output for me. Ditto for applying what I sent and running "git show" on
the result.

Using "--patience", on the other hand, does find it as a common line,
Weird.

-Peff

^ permalink raw reply

* Re: [PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Jeff King @ 2011-12-17  1:21 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git, Brandon Casey
In-Reply-To: <4EEBC0A7.3030303@kdbg.org>

On Fri, Dec 16, 2011 at 11:05:27PM +0100, Johannes Sixt wrote:

> Am 16.12.2011 20:21, schrieb Jeff King:
> > I'm not clear from what you wrote on whether you were saying it is
> > simply sub-optimal, or whether on balance it is way worse than the
> > default funcname matching.
> 
> I'm saying the latter. Okay, we're talking "only" about hunk headers.
> But when you are reviewing patches, they are *extremely* useful and a
> time-saver; when they are wrong or not present, they are exactly the
> opposite.

Right. I don't think it is worth arguing "well, it's only funcname
headers". Because that same argument applies to both the advantages
(i.e., hopefully with the patch we are generating better funcname
headers) and disadvantage (i.e., it seems that we might be generating
worse funcname headers).

So it is really a question of "how good" or "how bad" for each style.

> Sure I have. What I didn't say (sorry for that!), but wanted to hint at
> is that this is to experiment with a pattern in order to ultimately
> improve the built-in pattern. The topic came up just the other day, and
> I took Thomas Rast's suggestion to experiment with a simplified pattern:
> 
> http://thread.gmane.org/gmane.comp.version-control.git/186355/focus=186439
> 
> But as is, the built-in pattern misses way too many anchor points in C++
> code.

Yeah, I can certainly agree that the patterns could be better.

Maybe we should just table the extension mapping for now, then, and see
if the patterns improve? Or maybe just drop the C ones (and probably the
objc one based on other parts of the thread) and do the rest?

-Peff

^ permalink raw reply

* Re: [PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Jeff King @ 2011-12-17  1:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, git, Brandon Casey
In-Reply-To: <7vehw4ia5x.fsf@alter.siamese.dyndns.org>

On Fri, Dec 16, 2011 at 11:33:30AM -0800, Junio C Hamano wrote:

> I think we recently saw that the optional built-in one for C did not even
> understand a function that returns a pointer, and nobody complained about
> it for a long time,

Yeah. That implies to me that either people don't really care about
this feature, or that they are not actually using it because it requires
special configuration (we are not even using it in git.git, for
example).

> > And if it is bad on balance, is the right solution to avoid exposing
> > people to it, or is it to make our patterns better?
> 
> Can't we do both, by avoid exposing normal users to broken one while
> people who want to improve the pattern based one work on unbreak it?

Sure, we can do both. But if nobody is eating the dogfood, it will never
grow to taste better. Maybe we should start by using diff=c in git
itself?

> > I.e., is it fixable,
> > or is it simply too hard a problem to get right in the general case, and
> > we shouldn't turn it on by default?
> 
> I do not think that is the "either-or" question. My impression has been
> that even if it is fixable, it is too broken and produces worse result
> than the simple default in its current form.

What I meant by the either-or was: if it is fixable, then we should fix
it and consider turning it on as a default. If it's too hard to get
right, then we probably never want it on by default, and people who do
like it can turn it on (presumably because it works on their code
style).

-Peff

^ permalink raw reply

* Re: git-p4 using notes
From: Michael Horowitz @ 2011-12-17  0:57 UTC (permalink / raw)
  To: Luke Diamand; +Cc: git
In-Reply-To: <4EEBA24F.8030103@diamand.org>

That would be great.  If you need me to help test, let me know.
Unfortunately I don't know Python and I know very little about the
internals of git, so I can't help much more than that...

Mike



On Fri, Dec 16, 2011 at 2:55 PM, Luke Diamand <luke@diamand.org> wrote:
> On 16/12/11 16:07, Michael Horowitz wrote:
>>
>> For those of you using git-p4 because of a company requirement to use
>> Perforce, but really wish you could use git only, the most frustrating
>> part is the fact that when changes are submitted, the commit message
>> is rewritten to include a reference to the P4 change number which is
>> used by the sync.  When syncing back changes, this causes the commit
>> hash to be different, and so blows away your old commit and any parent
>> commit references and such.
>>
>> I read someplace, I can't remember where at this point, that if git-p4
>> used notes to write the P4 change information, that would not impact
>> the commit hash, so when merging back, things would not be
>> overwritten, and you can maintain branches and commit history properly
>> in git.
>>
>> I just ran into this project, where it seems that someone has
>> re-written git-p4 to use notes: https://github.com/ermshiperete/git-p4
>>
>> I was wondering if any of the maintainers of git-p4 has considered
>> this, and might want to leverage this work to merge into the main git
>> repo, possibly with an option to choose between the two behaviors.
>
>
> I'm not sure I qualify for such a grand title as maintainer, but I was going
> to give this a go in the new year as it would be quite useful, unless
> someone beat me to it. I want to fix some problems with labels first though.
>
> Regards!
> Luke

^ permalink raw reply

* Re: git-p4.skipSubmitEdit
From: Michael Horowitz @ 2011-12-17  0:49 UTC (permalink / raw)
  To: Luke Diamand; +Cc: Pete Wyckoff, L. A. Linden Levy, git
In-Reply-To: <CAFLRbor3Gnqhudmg8G_U37Nbo7ENoCEy0iFVGRP4i_AmatJHxw@mail.gmail.com>

Oh, and in the case where you do edit the template, it doesn't give
you an error or anything, it looks like it succeeded, but you'll
notice the change never got submitted to Perforce.  If you look
carefully though, you can see it reverting each of your edited files
in the P4 tree.

Mike



On Fri, Dec 16, 2011 at 7:46 PM, Michael Horowitz
<michael.horowitz@ieee.org> wrote:
> Actually, it is the opposite.  Bailout works fine, it is when I ":wq"
> in Vi for example, that it fails to submit and reverts all my changes.
>
>        if os.stat(template_file).st_mtime <= mtime:
>            while True:
>                response = raw_input("Submit template unchanged.
> Submit anyway? [y]es, [n]o (skip this patch) ")
>                if response == 'y':
>                    return True
>                if response == 'n':
>                    return False
>        # I think this else needs to be added here, so when the file
> has been modified since it was opened in the editor, it will properly
> submit the change.
>        else:
>            return True
>
>
> Mike
>
>
>
> On Fri, Dec 16, 2011 at 2:50 PM, Luke Diamand <luke@diamand.org> wrote:
>>
>> On 16/12/11 15:38, Michael Horowitz wrote:
>>>
>>> All,
>>>
>>> It appears that this change has introduced a bug that causes submit to
>>> fail every time if you do not skip the submit edit.
>>>
>>>  From what I can tell, this is because the new edit_template method
>>> does not return True at the end.
>>
>>
>> Could you say exactly what you're doing?
>>
>> I've just tried it myself and it seems to work fine:
>>
>> git-p4 clone
>> git commit -m 'a change'
>> git-p4 submit
>> <quit from editor, with/without modifying it>
>>
>> And I couldn't see any paths in edit_template that returned without a value of True, except the one where the user decides to bail out.
>>
>> This is with Pete's skipSubmitEdit change.
>>
>> Thanks!
>> Luke

^ permalink raw reply

* Re: Revisiting metadata storage
From: Jonathan Nieder @ 2011-12-17  0:48 UTC (permalink / raw)
  To: Hilco Wijbenga
  Cc: Richard Hartmann, Ronan Keryell, Git List, Martin von Zweigbergk
In-Reply-To: <CAE1pOi2GW=3o7QgTEcUYbjif3WokpVdgL6UdKXu9x0yKH-vrGw@mail.gmail.com>

(+cc: Martin, who has been doing excellent work on "git rebase", just
 in case he's curious)
Hi again,

Hilco Wijbenga wrote:

> Yes, I guess the problem is that it uses the worktree as its workspace.

That's a comfort.  Thanks for explaining.

> (I know others disagree but to me it's a bug that Git touches files
> that it doesn't actually change.)

No, I somewhat agree.  If a command is touching more files than it needs
to, then that is likely to be a bug, or at least an opportunity for
improvement.

Where we might disagree is in how many files "git rebase" needs to
touch.  So let's consider your use case.

[...]
> I usually rebase in the morning to get an up-to-date tree. Is that
> considered too often? Perhaps it's my Subversion background but I'm
> not comfortable diverging too much. Is that too paranoid? :-)
>
> So IIUC, I can do "git rebase master" even after multiple "git merge master"s?

The second question is easy --- the answer is "yes".

Your first question is more a matter of opinion.  I will just say a
little about "git rebase", to help you decide for yourself.

The original and still-primary purpose of "git rebase" is to refresh and
clean up a short patch series that is going to be submitted by email
to some project, by making the series apply to a newer basis version.
You can imitate what it does fairly simply by hand:

	# on master
	git checkout -b master-rebased new-upstream
	git cherry-pick HEAD..master; # [*]
	git branch -M master

That is, we check out the new basis version and apply any "local"
changes on top of it one at a time, using human help to resolve
conflicts as necessary.

This procedure has the nice property that it is dead simple.  It also
is easily tweaked to produce an "interactive" variant that reorders
the patches or runs other commands in between applying patches (for
example, you can ask git to run the test suite after each commit when
rebasing by adding "exec make test" after each "pick" line in the
editor shown by "git rebase -i").  And in the end you have a nice
patch series that applies without fuzz and doesn't require people
reading your patches to think about the older code base they were
originally written against.

However, rebasing has a few downsides.

The most important one is that each time you rebase, you are making
new, untested commits.  When you rebase the 300-patch series that
you have been debugging in collaboration with other people, you
can no longer say "these changes have been in use and being tested
for a few months now; chances are we have already ironed out most
of the obvious bugs".  The usual heuristic that patches towards the
beginning of the series most likely work better and are less likely to
have introduced that new crash that makes your program not work at all
no longer applies, since when you rebase, you can easily introduce
a mistake in conflict resolution.  All "local" code is new code.

In the history

  A --- o --- o --- B [upstream]
   \
    P --- Q --- R [master]

after rebasing

  A --- o --- o --- B --- P' --- Q' --- R' [master]

it is even tempting for people to not test the intermediate commits P'
and Q' before publishing their work, resulting in a history where
intermediate commits involved in telling the story do not even build.
So building and testing old versions to track down a change in
behavior (e.g., with "git bisect") becomes hard.  The history is not
actual history.

That is easy to mitigate by only rebasing your small, _private_ patch
series that is not part of meaningful history.  When asking others to
incorporate the changes into permanent history, the contributor
hopefully carefully checks over them for sanity and checks each
intermediate version before they can be applied.  And history on a
large, public scale is still stable.

For similar reasons, rebasing can make life difficult for people
trying to write patches based on your patches.  The section RECOVERING
FROM UPSTREAM REBASE in the git-rebase(1) manual page has more on
that.

If you want to incorporate changes into your branch and preserve the
history of well-tested commits (for example, if you are the upstream
maintainer, pulling in changes from other people), a command to do
this is git-merge(1).  It does not have to rewind or rewrite anything;
it just uses a 3-way merge algorithm to apply the new changes and
writes a commit indicating it has done so and with pointers to the two
parent commits so history consumers can see the full story.

Another consideration.

When using Subversion and working against the trunk, I find myself
using "svn update" every day and right before commiting.  Otherwise, I
may be forced to deal with a painful conflict resolution, or worse,
commit a change to one file that uses an API that has been removed in
another file.

However, when using git, I do not find myself needing to do that.
Instead, most work pertaining to a particular goal happens on a branch
specific to that topic, I pick one version to develop against, and I
mostly stick to it.  This way, I am not distracted by irrelevant
breakage or other changes introduced in areas orthogonal to my topic.

"But how do you make sure your changes work with the current
codebase?" you might ask.  Here:

	# on branch "topic"

	# switch to working on an anonymous branch, or rather no
	# branch at all
	git checkout --detach
	# grab latest changes to test against
	git merge origin/master
	# test!
	make test

The "git merge" step does not present me with the same conflicts it
did yesterday thanks to the "git rerere" facility (since I have
[rerere] enabled set to true).  If my topic needs some nontrivial
reconciliation with the wider changes in the project (if there is an
API change, say), I might use "git merge" when on branch topic (i.e.,
_not_ detached) to record the resolution and use the commit message to
describe what happened.  Or I might just rebase.

Because of the nature of patches applied by mail, before sending the
patches out, I either rebase one last time or very loudly mention
which old version of the codebase the patch series applies to.
Usually the former.  But this step would not be necessary if asking
people to pull from me using a protocol that transfers the actual
objects.

Hope that helps,
Jonathan

[*] Actually, "git rebase" is a little smarter than that, in that it
notices and skips patches that have already been applied in
new-upstream.  A better imitation would be to use

	git cherry-pick --cherry-pick --right-only HEAD...master

Even better is to look at git-rebase.sh to see what it actually does.
:)

^ permalink raw reply

* Re: git-p4.skipSubmitEdit
From: Michael Horowitz @ 2011-12-17  0:46 UTC (permalink / raw)
  To: Luke Diamand; +Cc: Pete Wyckoff, L. A. Linden Levy, git
In-Reply-To: <4EEBA106.9010001@diamand.org>

Actually, it is the opposite.  Bailout works fine, it is when I ":wq"
in Vi for example, that it fails to submit and reverts all my changes.

        if os.stat(template_file).st_mtime <= mtime:
            while True:
                response = raw_input("Submit template unchanged.
Submit anyway? [y]es, [n]o (skip this patch) ")
                if response == 'y':
                    return True
                if response == 'n':
                    return False
        # I think this else needs to be added here, so when the file
has been modified since it was opened in the editor, it will properly
submit the change.
        else:
            return True


Mike



On Fri, Dec 16, 2011 at 2:50 PM, Luke Diamand <luke@diamand.org> wrote:
>
> On 16/12/11 15:38, Michael Horowitz wrote:
>>
>> All,
>>
>> It appears that this change has introduced a bug that causes submit to
>> fail every time if you do not skip the submit edit.
>>
>>  From what I can tell, this is because the new edit_template method
>> does not return True at the end.
>
>
> Could you say exactly what you're doing?
>
> I've just tried it myself and it seems to work fine:
>
> git-p4 clone
> git commit -m 'a change'
> git-p4 submit
> <quit from editor, with/without modifying it>
>
> And I couldn't see any paths in edit_template that returned without a value of True, except the one where the user decides to bail out.
>
> This is with Pete's skipSubmitEdit change.
>
> Thanks!
> Luke

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2011, #05; Thu, 15)
From: Junio C Hamano @ 2011-12-17  0:39 UTC (permalink / raw)
  To: git
In-Reply-To: <7vd3bpl6i8.fsf@alter.siamese.dyndns.org>

I don't want to do issue #06 yet, so only differences from #05 are listed
here for today's updates.  Extra sets of eyeballs on commits in 'pu' are
very much appreciated.

* tr/grep-threading (2011-12-16) 3 commits
 - grep: disable threading in non-worktree case
 - grep: enable threading with -p and -W using lazy attribute lookup
 - grep: load funcname patterns for -W

The second one was updated to include thread-utils.h in grep.h

* cn/maint-lf-to-crlf-filter (2011-12-16) 1 commit
 - lf_to_crlf_filter(): tell the caller we added "\n" when draining
 (this branch is used by jc/maint-lf-to-crlf-keep-crlf.)

Fixes up an earlier fix already in 'master'.

* jc/maint-lf-to-crlf-keep-crlf (2011-12-16) 1 commit
 - lf_to_crlf_filter(): resurrect CRLF->CRLF hack
 (this branch uses cn/maint-lf-to-crlf-filter.)

Builds on top of it.

* jc/request-pull-show-head-4 (2011-12-16) 1 commit
  (merged to 'next' on 2011-12-16 at bea51ac)
 + request-pull: update the "pull" command generation logic

Fixes up an earlier update already in 'master'.

* jk/doc-fsck (2011-12-16) 1 commit
 - docs: brush up obsolete bits of git-fsck manpage

* jk/follow-rename-score (2011-12-16) 1 commit
 - use custom rename score during --follow

* jk/pretty-reglog-ent (2011-12-16) 1 commit
 - pretty: give placeholders to reflog identity

Three fairly straightforward patches from Peff; will merge to 'next'
soonish.

^ permalink raw reply

* Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning
From: Jonathan Nieder @ 2011-12-16 23:59 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <4EEBC9D6.6010204@ramsay1.demon.co.uk>

Ramsay Jones wrote:

>         CC builtin/checkout.o
>     builtin/checkout.c: In function `cmd_checkout':
>     builtin/checkout.c:160: warning: 'mode' might be used uninitialized \
>         in this function
[...]
> [Note that only 2 out of the 3 versions of gcc I use issues this
> warning]

Which version of gcc is that?  Is gcc getting more sane, so we won't
have to worry about this after a while, or is the false positive a
new regression that should be reported to them?

^ permalink raw reply

* Re: Strange behaviour with git-add, .gitignore and a tracked file
From: Junio C Hamano @ 2011-12-16 23:42 UTC (permalink / raw)
  To: David ‘Bombe’ Roden; +Cc: git
In-Reply-To: <201112162258.04661.bombe@pterodactylus.net>

David ‘Bombe’ Roden  <bombe@pterodactylus.net> writes:

> This behaviour was reproduced with 1.7.7.4 (on OS X), 1.7.5.4, 1.7.1, 
> 1.7.8-247-g10f4eb6 (latest master as of a couple of hours ago) (all Linux).

Doesn't that suggest perhaps it could be a designed behaviour?

Perhaps to help users to realize that their .gitignore pattern may want to
be improved?

^ permalink raw reply

* Re: [BUG] attribute "eol" with "crlf"
From: Junio C Hamano @ 2011-12-16 23:34 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Carlos Martín Nieto, Ralf Thielow, git
In-Reply-To: <7vmxasgqlm.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> ...
> What you said is _technically_ correct in that sense.
>
> However, I think the CRLF filter used to have a hack to strip "\r" if the
> repository data records "\r" at the end of line. This was intended to help
> people who checked in such a broken text file (if it is a text file, then
> raw ascii CR does not have a place in it in the repository representation)
> and it was a useful hack to help people recover from such mistakes to
> start the project from DOS-only world (with CRLF in the repository data)
> and migrate to cross platform world (with LF in the repository data, CRLF
> in the DOS working tree).  I suspect that the streaming filter conversion
> may not have the same hack in it.

Perhaps something like this, but I do not use CRLF myself, so it probably
needs to be checked by extra sets of eyes.

Thanks.

-- >8 --
Subject: lf_to_crlf_filter(): resurrect CRLF->CRLF hack

The non-streaming version of the filter counts CRLF and LF in the whole
buffer, and returns without doing anything when they match (i.e. what is
recorded in the object store already uses CRLF). This was done to help
people who added files from the DOS world before realizing they want to go
cross platform and adding .gitattributes to tell Git that they only want
CRLF in their working tree.

The streaming version of the filter does not want to read the whole thing
before starting to work, as that defeats the whole point of streaming. So
we instead check what byte follows CR whenever we see one, and add CR
before LF only when the LF does not immediately follow CR already to keep
CRLF as is.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 convert.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/convert.c b/convert.c
index c028275..8daf4e4 100644
--- a/convert.c
+++ b/convert.c
@@ -879,7 +879,8 @@ int is_null_stream_filter(struct stream_filter *filter)
 
 struct lf_to_crlf_filter {
 	struct stream_filter filter;
-	unsigned want_lf:1;
+	unsigned has_held:1;
+	char held;
 };
 
 static int lf_to_crlf_filter_fn(struct stream_filter *filter,
@@ -889,10 +890,14 @@ static int lf_to_crlf_filter_fn(struct stream_filter *filter,
 	size_t count, o = 0;
 	struct lf_to_crlf_filter *lf_to_crlf = (struct lf_to_crlf_filter *)filter;
 
-	/* Output a pending LF if we need to */
-	if (lf_to_crlf->want_lf) {
-		output[o++] = '\n';
-		lf_to_crlf->want_lf = 0;
+	/*
+	 * We may be holding onto the CR to see if it is followed by a
+	 * LF, in which case we would need to go to the main loop.
+	 * Otherwise, just emit it to the output stream.
+	 */
+	if (lf_to_crlf->has_held && (lf_to_crlf->held != '\r' || !input)) {
+		output[o++] = lf_to_crlf->held;
+		lf_to_crlf->has_held = 0;
 	}
 
 	/* We are told to drain */
@@ -902,22 +907,57 @@ static int lf_to_crlf_filter_fn(struct stream_filter *filter,
 	}
 
 	count = *isize_p;
-	if (count) {
+	if (count || lf_to_crlf->has_held) {
 		size_t i;
+		int was_cr = 0;
+
+		if (lf_to_crlf->has_held) {
+			was_cr = 1;
+			lf_to_crlf->has_held = 0;
+		}
+
 		for (i = 0; o < *osize_p && i < count; i++) {
 			char ch = input[i];
+
 			if (ch == '\n') {
 				output[o++] = '\r';
-				if (o >= *osize_p) {
-					lf_to_crlf->want_lf = 1;
-					continue; /* We need to increase i */
-				}
+			} else if (was_cr) {
+				/*
+				 * Previous round saw CR and it is not followed
+				 * by a LF; emit the CR before processing the
+				 * current character.
+				 */
+				output[o++] = '\r';
 			}
+
+			/*
+			 * We may have consumed the last output slot,
+			 * in which case we need to break out of this
+			 * loop; hold the current character before
+			 * returning.
+			 */
+			if (*osize_p <= o) {
+				lf_to_crlf->has_held = 1;
+				lf_to_crlf->held = ch;
+				continue; /* break but increment i */
+			}
+
+			if (ch == '\r') {
+				was_cr = 1;
+				continue;
+			}
+
+			was_cr = 0;
 			output[o++] = ch;
 		}
 
 		*osize_p -= o;
 		*isize_p -= i;
+
+		if (!lf_to_crlf->has_held && was_cr) {
+			lf_to_crlf->has_held = 1;
+			lf_to_crlf->held = '\r';
+		}
 	}
 	return 0;
 }

^ permalink raw reply related

* Re: [BUG] attribute "eol" with "crlf"
From: Junio C Hamano @ 2011-12-16 23:24 UTC (permalink / raw)
  To: Ralf Thielow; +Cc: Junio C Hamano, Matthieu Moy, git
In-Reply-To: <CAN0XMOKts7UR6eSYWA9-xj-YCpprvhbqwfdbq4U6Hfrn0nUONQ@mail.gmail.com>

Ralf Thielow <ralf.thielow@googlemail.com> writes:

> Basicly I want to force the line endings of the files on my
> project. :/

You need to fix the data you have recorded with CRLF in the repository if
you are using eol=crlf, which means "This file wants to ..." (see below; I
do not want to type the same thing again).

> 2011/12/16 Junio C Hamano <gitster@pobox.com>:
>> Ralf Thielow <ralf.thielow@googlemail.com> writes:
>>
>>> So i have to commit ".gitattributes" and everything is fine for me after!?
>>
>> No.  Sorry if I was unclear, but I do not see which part was unclear in
>> what I wrote, so...
>>
>>>> The sequence adds "test\r\n" file without .gitattributes to have the
>>>> repository record that exact byte sequence for the file. But then later
>>>> goes around and says "This file wants to express the end of line with CRLF
>>>> on the filesystem, so please replace LF in the repository representation
>>>> to CRLF when checking out, and replace CRLF in the working tree to LF when
>>>> checking in".
>>>>
>>>> So it is not surprising that "\r\n" coming from the repository is replaced
>>>> to "\r\r\n" when checked out. As far as the repository data is concerned,
>>>> that line has a funny byte with value "\r" at the end, immediately before
>>>> the line terminator "\n".
>>>>
>>>> What you said is _technically_ correct in that sense.
>>>> ...

^ permalink raw reply

* Re: migrating from svn: How to clean up history?
From: Thomas Ferris Nicolaisen @ 2011-12-16 23:11 UTC (permalink / raw)
  To: Hartmut Goebel; +Cc: git
In-Reply-To: <4EE766D5.5040702@goebel-consult.de>

> I already did some experiments with `git filter-branch` without success. (I manage renaming the tags, though.)
>
> Any hints how to to this clean-up?

There are some examples of how to use filter-branch to remove empty
commits, and some other handy tips on this page:

http://thomasrast.ch/git/git-svn-conversion.html

^ permalink raw reply

* [PATCH] grep.h: Fix compilation error on mingw
From: Ramsay Jones @ 2011-12-16 22:56 UTC (permalink / raw)
  To: trast; +Cc: Junio C Hamano, GIT Mailing-list


In particular, gcc complains as follows:

        CC bisect.o
    In file included from revision.h:5,
                     from bisect.c:4:
    grep.h:138: error: expected '=', ',', ';', 'asm' or \
        '__attribute__' before 'grep_attr_mutex'
    make: *** [bisect.o] Error 1

In order to fix the error, we include the 'thread-utils.h' header
file in grep.h, since it provides a definition of pthread_mutex_t
(indirectly via compat/win32/pthread.h).

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Thomas,

If you need to re-roll your grep-threading series, could you please
squash this patch into your commit 53f4b6f7 (grep: enable threading
with -p and -W using lazy attribute lookup, 12-12-2011).

[Note: you could also remove the '#include "thread-utils.h"' in both
grep.c and builtin/grep.c, since it is now included from grep.h.]

Thanks!

ATB,
Ramsay Jones

 grep.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/grep.h b/grep.h
index 15d227c..b6f06cb 100644
--- a/grep.h
+++ b/grep.h
@@ -8,6 +8,7 @@ typedef int pcre;
 typedef int pcre_extra;
 #endif
 #include "kwset.h"
+#include "thread-utils.h"
 
 enum grep_pat_token {
 	GREP_PATTERN,
-- 
1.7.8

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox