* Re: [PATCH/RFC] launch_editor: ignore SIGINT while the editor has control
From: Jeff King @ 2012-11-08 15:33 UTC (permalink / raw)
To: Paul Fox; +Cc: git, Krzysztof Mazur, gitster
In-Reply-To: <20121107233515.107ED2E8089@grass.foxharp.boston.ma.us>
On Wed, Nov 07, 2012 at 06:35:15PM -0500, Paul Fox wrote:
> the user's editor likely catches SIGINT (ctrl-C). but if the user
> spawns a command from the editor and uses ctrl-C to kill that command,
> the SIGINT will likely also kill git itself. (depending on the
> editor, this can leave the terminal in an unusable state.)
>
> Signed-off-by: Paul Fox <pgf@foxharp.boston.ma.us>
Thanks, I think this makes sense.
> krzysztof wrote:
> ...
> > editor.c: In function 'launch_editor':
> > editor.c:42:3: warning: implicit declaration of function 'sigchain_push' [-Wimplicit-function-declaration]
> > editor.c:44:3: warning: implicit declaration of function 'sigchain_pop' [-Wimplicit-function-declaration]
>
> sigh. i had that initially, lost the patch, and then recreated
> without it. but i'm surprised my build (i did rebuild! :-) doesn't
> emit those errors. in any case, here's the fixed patch.
gcc will not warn about implicit declarations by default; try compiling
with "-Wall".
-Peff
^ permalink raw reply
* Re: [RFC] Add a new email notification script to "contrib"
From: Ævar Arnfjörð Bjarmason @ 2012-11-08 15:38 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Andy Parkins, Sitaram Chamarty, Stefan Näwe,
Junio C Hamano, Marc Branchaud, Matthieu Moy
In-Reply-To: <509BA2E7.4080102@alum.mit.edu>
On Thu, Nov 8, 2012 at 1:17 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 11/08/2012 12:39 PM, Ævar Arnfjörð Bjarmason wrote:
>> [...]
>
> I'm glad it's getting some use. Thanks for the feedback.
>
>> I'll test it out some more, the issues I've had with it so far in
>> migrating from the existing script + some custom hacks we have to it
>> have been:
>>
>> * Overly verbose default templates, easy to overwrite now. Might send
>> patches for some of them.
>
> The templating is currently not super flexible nor very well documented,
> but simple changes should be easy enough. I mostly carried over the
> text explanations from the old post-receive-email script; it is true
> that they are quite verbose.
>
>> * No ability to link to a custom gitweb, probably easy now.
>
> What do you mean by "a custom gitweb"? What are the commitmail issues
> involved?
Just for the E-Mail to include a link to
http://gitweb.example.com/git/....?h=$our_hash etc.
>> * If someone only pushes one commit I'd like to only have one e-mail
>> with the diff, but if they push multiple commits I'd like to have a
>> summary e-mail and replies to that which have the patches.
>>
>> It only seemed to support the latter mode, so you send out two
>> e-mails for pushing one commit.
>
> That's correct, and I've also thought about the feature that you
> described. I think it would be pretty easy to implement; it is only not
> quite obvious to which mailing list(s?) such emails should be sent.
>
>> * Ability to limit the number of lines, but not line length, that's
>> handy for some template repositories. Should be easy to add
>
> Should too-long lines be folded or truncated? Either way, it should be
> pretty straightforward (Python even has a textwrap module that could be
> used).
>
>> But in addition to that we have our own custom E-Mail notification
>> scripts for:
>>
>> * People can subscribe to changes to certain files. I.e. if you
>> modify very_important.c we'll send an E-Mail to a more widely seen
>> review list.
>>
>> * Invididuals can also edit a config file to watch individual files /
>> glob patterns of files, e.g. src/main.c or src/crypto*
>
> I implemented something like this back when we were using Subversion,
> but it didn't get much use and seemed like more configuration hassle
> than it was worth.
>
> If this were implemented and I asked for notifications about a
> particular file, and a particular reference change affects the file,
> what should I see?
>
> * The summary email for the reference change (yes/no)?
>
> * Detail emails for all commits within the reference change, or only for
> the individual commits that modify the file?
>
> * Should the detail emails include the full patch for the corresponding
> commit, or only the diffs affecting the file(s) of interest? (The
> latter would start to get expensive, because the script would have to
> generate individual emails per subscriber instead of letting sendmail
> fan the emails out across all subscribers.)
I think just sending the individual patch e-mails to all people who
subscribe to paths that got changed in that patch that match their
watchlist makes sense.
That's how an internal E-mailing script that I'm hopign to replace
with this works.
That script *also* supports sending the whole batch of patches pushed
in that push to someone watching any file that got modified in one of
the patches, in case you also want to get other stuff pushed in pushes
for files you're interested in.
But it doesn't generate individual E-Mails per recipient. I think that
way lies madness because as you rightly point out you have to start
worrying about the combinatorial nightmare of generating the E-mails
per subscriber.
>> I think a good way to support that would be to have either a path to a
>> config file with those watch specs, or a command so you could run "git
>> show ..." on some repo users can push to.
>
> *How* this feature would be configured depends strongly on how the repo
> is hosted. For example, gitolite has a well-developed scheme for how
> the server should be configured, and it would make sense to work
> together with that. Other people might configure user access via LDAP
> or Apache.
>
>> But overall it's very nice. I'll make some time to test it in my
>> organization (with lots of commits and people reading commit e-mails).
^ permalink raw reply
* Re: Long clone time after "done."
From: Jeff King @ 2012-11-08 15:56 UTC (permalink / raw)
To: Uri Moszkowicz; +Cc: git
In-Reply-To: <CAMJd5ASL1UPfzUmfXqACuYuGhXQjJLqbsjq7dfpRpF4K-hZFtA@mail.gmail.com>
On Wed, Nov 07, 2012 at 11:32:37AM -0600, Uri Moszkowicz wrote:
> #4 parse_object (sha1=0xb0ee98
> "\017C\205Wj\001`\254\356\307Z\332\367\353\233.\375P}D") at
> object.c:212
> #5 0x00000000004ae9ec in handle_one_ref (path=0xb0eec0
> "refs/tags/<removed>", sha1=0xb0ee98
> "\017C\205Wj\001`\254\356\307Z\332\367\353\233.\375P}D", flags=2,
> cb_data=<optimized out>) at pack-refs.
>
> [...]
>
> It looks like handle_one_ref() is called for each ref and most result
> in a call to read_sha1_file().
Right. When generating the packed-refs file, we include the "peeled"
reference for a tag (i.e., the commit that a tag object points to). So
we have to actually read any tag objects to get the value.
The upload-pack program generates a similar list, and I recently added
some optimizations. This code path could benefit from some of them by
using "peel_ref" instead of hand-rolling the tag dereferencing. The main
optimization, though, is reusing peeled values that are already in
packed-refs; we would probably need some additional magic to reuse the
values from the source repository.
However:
> It only takes a second or so for each call but when you have thousands
> of them (one for each ref) it adds up.
I am more concerned that it takes a second to read each tag. Even in my
pathological tests for optimizing upload-pack, peeling 50,000 refs took
only half a second.
> Adding --single-branch --branch <branch> doesn't appear to help as
> it is implemented afterwards. I would like to debug this problem
> further but am not familiar enough with the implementation to know
> what the next step is. Can anyone offer some suggestions? I don't see
> why a clone should be dependent on an O(#refs) operations.
Does this patch help? In a sample repo with 5000 annotated tags, it
drops my local clone time from 0.20s to 0.11s. Which is a big percentage
speedup, but this code isn't taking a long time in the first place for
me.
---
diff --git a/pack-refs.c b/pack-refs.c
index f09a054..3344749 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -40,13 +40,9 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
if (is_tag_ref) {
- struct object *o = parse_object(sha1);
- if (o->type == OBJ_TAG) {
- o = deref_tag(o, path, 0);
- if (o)
- fprintf(cb->refs_file, "^%s\n",
- sha1_to_hex(o->sha1));
- }
+ unsigned char peeled[20];
+ if (!peel_ref(path, peeled))
+ fprintf(cb->refs_file, "^%s\n", sha1_to_hex(peeled));
}
if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {
^ permalink raw reply related
* Re: [RFC] Add a new email notification script to "contrib"
From: Marc Branchaud @ 2012-11-08 16:03 UTC (permalink / raw)
To: Michael Haggerty
Cc: Ævar Arnfjörð Bjarmason, git, Andy Parkins,
Sitaram Chamarty, Stefan Näwe, Junio C Hamano, Matthieu Moy
In-Reply-To: <509B7E6B.1030407@alum.mit.edu>
On 12-11-08 04:42 AM, Michael Haggerty wrote:
> On 11/07/2012 10:47 PM, Ævar Arnfjörð Bjarmason wrote:
>> On Fri, Jul 20, 2012 at 12:01 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>>> On 07/14/2012 08:59 AM, mhagger@alum.mit.edu wrote:
>>>>
>>>> Add a new Python script, contrib/hooks/post-receive-multimail.py, that
>>>> can be used to send notification emails describing pushes into a git
>>>> repository. [...]
>>>
>>>
>>> Thanks to everybody for your feedback. I will try to incorporate it in a
>>> new version of the script, which I will put forward as a replacement for
>>> contrib/hooks/post-receive-email rather than as an alternative. But I have
>>> very little open-sourcing time these days, and will be on vacation next
>>> week, so please be patient (or feel free to lend a hand if you are so
>>> inclined).
>>
>> I'm curious as to whether you got around to this? I'd be interested in
>> updates on this script.
I installed the script in our main repository here. Some folks complained
about the "flood" of emails for large pushes, but having separate mailing
lists solved that.
Others really appreciate the individual messages with full patches. It's a
challenge to create one script that can satisfy all users, but IMHO this
comes pretty close.
> Thanks for asking. I have made many of the changes that were requested
> and/or I had planned:
>
> * Tentatively renamed the system to git-multimail
>
> * Store the configuration in namespace "multimailhook.*" instead of
> "hooks.*".
>
> * Reinstated (optional) support for including shortlogs since the last
> tag in announce emails.
>
> * Script to migrate post-receive-email configuration to the equivalent
> git-multimail config.
>
> * Better documentation (including of the migration process).
>
> * Made it easier to use Python code to customize the script's behavior
> (by allowing it to be imported as a Python module), plus a demo script
> showing this usage.
I look forward to trying out your updates. One thing I had to do to the
original script was override get_envelopesender() in GenericEnvironment to
use $USER if hooks.envelopesender is not set. (This is what the old
post-receive-email script does.)
This script is one of a couple of things on my git back-burner. Hopefully
things will settle down here soon and I'll be able to get back to some git
hacking in the next few weeks.
M.
^ permalink raw reply
* Re: [RFC] Add a new email notification script to "contrib"
From: Marc Branchaud @ 2012-11-08 16:24 UTC (permalink / raw)
To: Michael Haggerty
Cc: Ævar Arnfjörð Bjarmason, git, Andy Parkins,
Sitaram Chamarty, Stefan Näwe, Junio C Hamano, Matthieu Moy
In-Reply-To: <509BA2E7.4080102@alum.mit.edu>
On 12-11-08 07:17 AM, Michael Haggerty wrote:
> On 11/08/2012 12:39 PM, Ævar Arnfjörð Bjarmason wrote:
>> [...]
>
> I'm glad it's getting some use. Thanks for the feedback.
>
>> I'll test it out some more, the issues I've had with it so far in
>> migrating from the existing script + some custom hacks we have to it
>> have been:
>>
>> * Overly verbose default templates, easy to overwrite now. Might send
>> patches for some of them.
>
> The templating is currently not super flexible nor very well documented,
> but simple changes should be easy enough. I mostly carried over the
> text explanations from the old post-receive-email script; it is true
> that they are quite verbose.
>
>> * No ability to link to a custom gitweb, probably easy now.
>
> What do you mean by "a custom gitweb"? What are the commitmail issues
> involved?
We would also like to have a gitweb link in the summary email, like Ævar
describes.
>> * If someone only pushes one commit I'd like to only have one e-mail
>> with the diff, but if they push multiple commits I'd like to have a
>> summary e-mail and replies to that which have the patches.
>>
>> It only seemed to support the latter mode, so you send out two
>> e-mails for pushing one commit.
>
> That's correct, and I've also thought about the feature that you
> described. I think it would be pretty easy to implement; it is only not
> quite obvious to which mailing list(s?) such emails should be sent.
Overall, what should be the approach to the separate mailing lists?
Maybe I don't understand how the script is meant to work. We configured
things here with 'mailinglist' and 'commitlist' set to different lists. Now
if someone wants to get both the summary and per-commit emails, they need to
be on both lists. If I understand correctly, if all 4 mailing lists are
distinct, someone who wants all the emails needs to be on all of them. This
seems a little awkward.
I'd like there to be one list that always gets everything, and the other
lists should get subsets of the everything list.
M.
^ permalink raw reply
* Re: [RFC] Add a new email notification script to "contrib"
From: Ævar Arnfjörð Bjarmason @ 2012-11-08 16:37 UTC (permalink / raw)
To: marcnarc
Cc: Michael Haggerty, git, Andy Parkins, Sitaram Chamarty,
Stefan Näwe, Junio C Hamano, Matthieu Moy
In-Reply-To: <509BDCC3.1050107@xiplink.com>
On Thu, Nov 8, 2012 at 5:24 PM, Marc Branchaud <mbranchaud@xiplink.com> wrote:
> I'd like there to be one list that always gets everything, and the other
> lists should get subsets of the everything list.
Since it supports multiple mailing lists per category you can always
do (I can't remember the specific config keys, but it's not
important):
commits = all-git-activity@example.com,git-commits@example.com
tags = all-git-activity@example.com,git-tags@example.com
etc.
^ permalink raw reply
* Re: [PATCH 1/4] strbuf_split_buf(): use ALLOC_GROW()
From: Jeff King @ 2012-11-08 16:38 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git
In-Reply-To: <5098C21F.6030803@alum.mit.edu>
On Tue, Nov 06, 2012 at 08:54:07AM +0100, Michael Haggerty wrote:
> > I suspect this was not used originally because ALLOC_GROW relies on
> > alloc_nr, which does fast growth early on. At (x+16)*3/2, we end up with
> > 24 slots for the first allocation. We are typically splitting 1 or 2
> > values.
> >
> > It probably doesn't make a big difference in practice, though, as we're
> > talking about wasting less than 200 bytes on a 64-bit platform, and we
> > do not tend to keep large numbers of split lists around.
>
> I did a little bit of archeology, and found out that
>
> * ALLOC_GROW() did indeed exist when this code was developed, so it
> *could have* been used.
>
> * OTOH, I didn't find any indication on the mailing list that the
> choice not to use ALLOC_GROW() was a conscious decision.
>
> So history doesn't give us much guidance.
Thanks for digging.
> If the size of the initial allocation is a concern, then I would suggest
> adding a macro like ALLOC_SET_SIZE(ary,nr,alloc) that could be called to
> initialize the size to some number less than 24. Such a macro might be
> useful elsewhere, too. It wouldn't, of course, slow the growth rate
> *after* the first allocation.
I think we are getting into premature optimization territory. Let's
take your series as a cleanup, and we can worry about micro-optimizing
the allocation if and when it ever becomes an issue.
-Peff
^ permalink raw reply
* Re: Fwd: [PATCH] Remove terminal symbols from non-terminal console
From: Jeff King @ 2012-11-08 16:51 UTC (permalink / raw)
To: Michael Naumov; +Cc: git, gitster
In-Reply-To: <CABHRWd16k6B4Ybvc4k7z29_A9Q2wZVXA__Bov8Pst4cc2H0cmg@mail.gmail.com>
On Tue, Nov 06, 2012 at 10:17:21AM +1100, Michael Naumov wrote:
> As per discussion on msysgit user group:
> https://groups.google.com/forum/?fromgroups=#!topic/msysgit/U_a982_a3rc/discussion
> we found the following patch is required to get rid of weird terminal
> characters for other tools such as GitExtensions for Windows
>
> ---8<---
Please follow SubmittingPatches (missing signoff, weird use of scissors
marker that cuts out your commit message, and your commit message should
summarize the discussion).
> @@ -29,7 +29,7 @@ int recv_sideband(const char *me, int in_stream, int out)
>
> memcpy(buf, PREFIX, pf);
> term = getenv("TERM");
> - if (term && strcmp(term, "dumb"))
> + if (isatty(out) && term && strcmp(term, "dumb"))
> suffix = ANSI_SUFFIX;
> else
> suffix = DUMB_SUFFIX;
Is that right? The "out" fd is where we send sideband 1, and it is
almost always not going to be a tty. The suffix should be sent with
sideband 2, which goes to stderr. So I'd think you would want to check
isatty(2).
Also, most isatty checks also need to cover the case that a pager has
already been started. That is probably not worth worrying about here,
though, as we shouldn't be using a pager for commands that do network
communications (and if we do, omitting the magic line-clearing signal is
probably a sane thing to do).
I think the overall goal is a step in the right direction, though.
-Peff
^ permalink raw reply
* Re: [RFC] Add a new email notification script to "contrib"
From: Marc Branchaud @ 2012-11-08 16:46 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Michael Haggerty, git, Andy Parkins, Sitaram Chamarty,
Stefan Näwe, Junio C Hamano, Matthieu Moy
In-Reply-To: <CACBZZX75tytc=SMY6Y6TuaR3AQ5VL73NmjCT3dd3BRCrGkjpxA@mail.gmail.com>
On 12-11-08 11:37 AM, Ævar Arnfjörð Bjarmason wrote:
> On Thu, Nov 8, 2012 at 5:24 PM, Marc Branchaud <mbranchaud@xiplink.com> wrote:
>> I'd like there to be one list that always gets everything, and the other
>> lists should get subsets of the everything list.
>
> Since it supports multiple mailing lists per category you can always
> do (I can't remember the specific config keys, but it's not
> important):
>
> commits = all-git-activity@example.com,git-commits@example.com
> tags = all-git-activity@example.com,git-tags@example.com
>
> etc.
True enough. Still a bit awkward though, especially for folks who can't set
up emails lists easily. But I'm happy with this approach.
M.
^ permalink raw reply
* Re: [PATCH] gitweb: make remote_heads config setting work.
From: Jeff King @ 2012-11-08 16:56 UTC (permalink / raw)
To: Phil Pennock; +Cc: git, gitster
In-Reply-To: <20121105235047.GA78156@redoubt.spodhuis.org>
On Mon, Nov 05, 2012 at 06:50:47PM -0500, Phil Pennock wrote:
> Git configuration items can not contain underscores in their name; the
> 'remote_heads' feature can not be enabled on a per-repository basis with
> that name.
>
> This changes the git-config option to be `gitweb.remoteheads` but does
> not change the gitweb.conf option, to avoid backwards compatibility
> issues. We strip underscores from keys before looking through
> git-config output for them.
Makes sense. Thanks for considering the backwards compatibility angle.
Hopefully we can avoid adding names with underscores in the future, but I
think the mapping of "remote_head -> remotehead" is obvious enough that
we do not need to worry about deprecating and replacing the old name.
-Peff
^ permalink raw reply
* Re: Long clone time after "done."
From: Uri Moszkowicz @ 2012-11-08 17:20 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121108155607.GD15560@sigill.intra.peff.net>
I tried the patch but it doesn't appear to have helped :( Clone time
with it was ~32m.
Do you all by any chance have a tool to obfuscate a repository?
Probably I still wouldn't be permitted to distribute it but might make
the option slightly more palatable. Anything else that I can do to
help debug this problem?
On Thu, Nov 8, 2012 at 9:56 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 07, 2012 at 11:32:37AM -0600, Uri Moszkowicz wrote:
>
>> #4 parse_object (sha1=0xb0ee98
>> "\017C\205Wj\001`\254\356\307Z\332\367\353\233.\375P}D") at
>> object.c:212
>> #5 0x00000000004ae9ec in handle_one_ref (path=0xb0eec0
>> "refs/tags/<removed>", sha1=0xb0ee98
>> "\017C\205Wj\001`\254\356\307Z\332\367\353\233.\375P}D", flags=2,
>> cb_data=<optimized out>) at pack-refs.
>>
>> [...]
>>
>> It looks like handle_one_ref() is called for each ref and most result
>> in a call to read_sha1_file().
>
> Right. When generating the packed-refs file, we include the "peeled"
> reference for a tag (i.e., the commit that a tag object points to). So
> we have to actually read any tag objects to get the value.
>
> The upload-pack program generates a similar list, and I recently added
> some optimizations. This code path could benefit from some of them by
> using "peel_ref" instead of hand-rolling the tag dereferencing. The main
> optimization, though, is reusing peeled values that are already in
> packed-refs; we would probably need some additional magic to reuse the
> values from the source repository.
>
> However:
>
>> It only takes a second or so for each call but when you have thousands
>> of them (one for each ref) it adds up.
>
> I am more concerned that it takes a second to read each tag. Even in my
> pathological tests for optimizing upload-pack, peeling 50,000 refs took
> only half a second.
>
>> Adding --single-branch --branch <branch> doesn't appear to help as
>> it is implemented afterwards. I would like to debug this problem
>> further but am not familiar enough with the implementation to know
>> what the next step is. Can anyone offer some suggestions? I don't see
>> why a clone should be dependent on an O(#refs) operations.
>
> Does this patch help? In a sample repo with 5000 annotated tags, it
> drops my local clone time from 0.20s to 0.11s. Which is a big percentage
> speedup, but this code isn't taking a long time in the first place for
> me.
>
> ---
> diff --git a/pack-refs.c b/pack-refs.c
> index f09a054..3344749 100644
> --- a/pack-refs.c
> +++ b/pack-refs.c
> @@ -40,13 +40,9 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
>
> fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
> if (is_tag_ref) {
> - struct object *o = parse_object(sha1);
> - if (o->type == OBJ_TAG) {
> - o = deref_tag(o, path, 0);
> - if (o)
> - fprintf(cb->refs_file, "^%s\n",
> - sha1_to_hex(o->sha1));
> - }
> + unsigned char peeled[20];
> + if (!peel_ref(path, peeled))
> + fprintf(cb->refs_file, "^%s\n", sha1_to_hex(peeled));
> }
>
> if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {
^ permalink raw reply
* Re: [PATCH 0/2] Another minor cleanup involving string_lists
From: Jeff King @ 2012-11-08 17:38 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git
In-Reply-To: <1352104883-21053-1-git-send-email-mhagger@alum.mit.edu>
On Mon, Nov 05, 2012 at 09:41:21AM +0100, Michael Haggerty wrote:
> Nothing really earthshattering here. But it's funny how every time I
> look closely at a site where I think string_lists could be used, I
> find problems with the old code. In this case is_absolute_path() is
> called with an argument that is not a null-terminated string, which is
> incorrect (though harmless because the function only looks at the
> first two bytes of the string).
Thanks, the new version is much easier on the eyes.
> Another peculiarity of the (old and new) code is that it rejects
> "comments" even in paths taken from the colon-separated environment
> variable GIT_ALTERNATE_OBJECT_DIRECTORIES. The fix would be to change
> link_alt_odb_entries() to take a string_list and let the callers strip
> out comments when appropriate. But it didn't seem worth the extra
> code.
I don't think it's worth worrying about. Given that the entries must be
absolute paths anyway, we do not even have to worry about an insane path
starting with "#".
-Peff
^ permalink raw reply
* Re: [PATCH] Clarify how content states are to be built as the fast-import stream is interpreted.
From: Jeff King @ 2012-11-08 17:43 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20121105043101.GA24687@thyrsus.com>
On Sun, Nov 04, 2012 at 11:31:01PM -0500, Eric S. Raymond wrote:
>
> ---
> Documentation/git-fast-import.txt | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
Looks reasonable. Sign-off?
-Peff
^ permalink raw reply
* Re: [PATCH] git p4: RCS expansion should not span newlines
From: Jeff King @ 2012-11-08 17:47 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: git, Chris Goard
In-Reply-To: <20121104220402.GA9160@padd.com>
On Sun, Nov 04, 2012 at 05:04:02PM -0500, Pete Wyckoff wrote:
> This bug was introduced in cb585a9 (git-p4: keyword
> flattening fixes, 2011-10-16). The newline character
> is indeed special, and $File$ expansions should not try
> to match across multiple lines.
>
> Based-on-patch-by: Chris Goard <cgoard@gmail.com>
> Signed-off-by: Pete Wyckoff <pw@padd.com>
Thanks, I'll queue this for 'maint'. Seems obviously correct to me.
-Peff
^ permalink raw reply
* Re: [PATCH] gitweb.perl: fix %highlight_ext mappings
From: Jeff King @ 2012-11-08 18:01 UTC (permalink / raw)
To: rh; +Cc: git
In-Reply-To: <20121104094555.a46992b6d836c1e09524d2cc@lavabit.com>
On Sun, Nov 04, 2012 at 09:45:55AM -0800, rh wrote:
> The previous change created a dictionary of one-to-one elements when
> the intent was to map mutliple related types to one main type.
> e.g. bash, ksh, zsh, sh all map to sh since they share similar syntax
> This makes the mapping as the original change intended.
>
> Signed-off-by: Richard Hubbell <richard_hubbe11@lavabit.com>
Thanks.
> diff --git a/gitweb.cgi.orig b/gitweb.cgi
> index 060db27..155b238 100755
> --- a/gitweb.cgi.orig
> +++ b/gitweb.cgi
This is not the name of the source file in git.git, so "git am" choked.
I was able to fix it up locally, though. No need to resend.
-Peff
^ permalink raw reply
* Re: [PATCH as/check-ignore] t0007: fix tests on Windows
From: Jeff King @ 2012-11-08 18:04 UTC (permalink / raw)
To: Johannes Sixt
Cc: Nguyễn Thái Ngọc Duy, git, Junio C Hamano,
Adam Spiers
In-Reply-To: <5096D92A.7090600@kdbg.org>
On Sun, Nov 04, 2012 at 10:07:54PM +0100, Johannes Sixt wrote:
> The value of $global_excludes is sometimes part of the output
> that is tested for. Since git on Windows only sees DOS style paths,
> we have to ensure that the "expected" values are constructed in
> the same manner. To account for this, use $(pwd) to set the value
> of global_excludes.
>
> Additionally, add a SYMLINKS prerequisite to the tests involving
> symbolic links.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Thanks. I put this on top of as/check-ignore in 'pu' for reference, but
I am still expecting a re-roll from Adam or Duy at some point.
-Peff
^ permalink raw reply
* Re: git svn problem, probably a regression
From: Jeff King @ 2012-11-08 18:06 UTC (permalink / raw)
To: Joern Huxhorn; +Cc: git
In-Reply-To: <36370AA5-4BB9-4D36-95A7-BB3DA315C9E6@googlemail.com>
On Sun, Nov 04, 2012 at 09:31:17PM +0100, Joern Huxhorn wrote:
> git svn failed on me with the following error while cloning an SVN repository:
> r1216 = fcf69d5102378ee41217d60384b96549bf2173cb (refs/remotes/svn/trunk)
> Found possible branch point: svn+ssh://<repositoryName>@<IP address>/trunk => svn+ssh://<repositoryName>@<IP address>/tags/xxxx_2008-10-22, 1216
> Use of uninitialized value $u in substitution (s///) at /usr/local/Cellar/git/1.8.0/lib/Git/SVN.pm line 106.
> Use of uninitialized value $u in concatenation (.) or string at /usr/local/Cellar/git/1.8.0/lib/Git/SVN.pm line 106.
> refs/remotes/svn/asset-manager-redesign: 'svn+ssh://<IP address>' not found in ''
>
> I'm running git 1.8.0 via Homebrew on OS X. It was called via svn2git but I doubt that this is the culprit.
> A colleague of mine was able to perform the same operation with git 1.7.x (not sure which) on Debian so I assume that this is a regression.
>
> I just wanted to let you know.
If you have time and can reproduce the bug at will, it would be very
helpful to use "git-bisect" to pinpoint the exact commit that causes the
breakage.
-Peff
^ permalink raw reply
* Re: [PATCH] Clarify how content states are to be built as the fast-import stream is interpreted.
From: Jeff King @ 2012-11-08 18:32 UTC (permalink / raw)
To: Eric; +Cc: git
In-Reply-To: <20121108183038.GA4003@thyrsus.com>
On Thu, Nov 08, 2012 at 01:30:38PM -0500, Eric wrote:
> > Looks reasonable. Sign-off?
> >
>
> Ah, right. Shall resend with signoff. Probably Tuesday - I'm on the
> road, and the git instance on my laptop is prone to library-load issues.
>
> Or you can just add
>
> Signed-off-by: Eric S. Raymond <esr@thyrsus.com>
That's fine. I can fix it up locally. Thanks.
-Peff
^ permalink raw reply
* Re: [PATCH] gitweb.perl: fix %highlight_ext mappings
From: rh @ 2012-11-08 18:36 UTC (permalink / raw)
To: git
In-Reply-To: <20121108180157.GK15560@sigill.intra.peff.net>
On Thu, 8 Nov 2012 13:01:57 -0500
Jeff King <peff@peff.net> wrote:
> On Sun, Nov 04, 2012 at 09:45:55AM -0800, rh wrote:
>
> > The previous change created a dictionary of one-to-one elements when
> > the intent was to map mutliple related types to one main type.
> > e.g. bash, ksh, zsh, sh all map to sh since they share similar
> > syntax This makes the mapping as the original change intended.
> >
> > Signed-off-by: Richard Hubbell <richard_hubbe11@lavabit.com>
>
> Thanks.
>
> > diff --git a/gitweb.cgi.orig b/gitweb.cgi
> > index 060db27..155b238 100755
> > --- a/gitweb.cgi.orig
> > +++ b/gitweb.cgi
>
> This is not the name of the source file in git.git, so "git am"
> choked. I was able to fix it up locally, though. No need to resend.
Somehow I knew that it wouldn't be a slam dunk!
Thanks for doing what you do. FWIW maybe others can follow
this thread to know what not to do. And save you extra work.
>
> -Peff
--
"Now, in his heart, Ahab had some glimpse of this, namely; all my means
are sane, my motive and my object mad."
^ permalink raw reply
* Re: git commit/push can fail silently when clone omits ".git"
From: Jeff King @ 2012-11-08 18:56 UTC (permalink / raw)
To: Jeffrey S. Haemer; +Cc: Jens Lehmann, Heiko Voigt, Git Issues
In-Reply-To: <CAABvdFyn=_2JKHYA_jAduoNAti3U0YFHbdU94esm=m8R0s2LcA@mail.gmail.com>
On Sun, Nov 04, 2012 at 12:50:58PM -0700, Jeffrey S. Haemer wrote:
> I got bitten by what follows. Yes, it's an edge case. Yes I now understand
> why it does what it does. Yes the right answer is "Don't do that, Jeff." :-)
>
> Still, it took me a little time to figure out what I'd done wrong because
> the failure is silent, so I thought I'd document it. Perhaps there's even
> some way to issue an error message for cases like this.
>
> The attached test script shows the issue in detail, but here's the basic
> failure:
>
> $ ls
> hello.git
> $ git clone hello # *Mistake!* Succeeds, but should have cloned "hello.git"
> or into something else.
It does clone hello.git into "hello", but it sets remote.origin.url in
the cloned repository to "/path/to/hello". I.e., to itself, rather than
the correct hello.git.
The reason is that "clone" sets the config from the repo name you gave
it, not the path it finds on disk. The name you gave was not ambiguous
at the time of clone, but it became so during the clone. I am tempted to
say that we should set the config to the path we found on disk, not what
the user gave us. That includes the ugly "/.git" for non-bare repos, but
we should be able to safely strip that off without adding any ambiguity
(i.e, it is only "foo" versus "foo.git" that is ambiguous).
Unfortunately, the patch below which does that seems to make t7407 very
unhappy. It looks like the submodule test uses "git clone ." and
"git-submodule add" expects the "/." to still be at the end of the
configured URL when processing relative submodule paths. I'm not sure if
that is important, or an unnecessary brittleness in the submodule code.
Jens, Heiko?
---
diff --git a/builtin/clone.c b/builtin/clone.c
index 0d663e3..687d5c0 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -713,8 +713,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
repo_name = argv[0];
path = get_repo_path(repo_name, &is_bundle);
- if (path)
- repo = xstrdup(absolute_path(repo_name));
+ if (path) {
+ if (!suffixcmp(path, "/.git"))
+ repo = xstrndup(path, strlen(path) - 5);
+ else
+ repo = xstrdup(path);
+ }
else if (!strchr(repo_name, ':'))
die(_("repository '%s' does not exist"), repo_name);
else
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 67869b4..0eeeb2d 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -280,4 +280,20 @@ test_expect_success 'clone checking out a tag' '
test_cmp fetch.expected fetch.actual
'
+test_expect_success 'clone does not create ambiguous config' '
+ git init --bare ambiguous.git &&
+ git clone ambiguous &&
+ (
+ cd ambiguous &&
+ test_commit one &&
+ git push --all
+ ) &&
+ echo one >expect &&
+ (
+ cd ambiguous.git &&
+ git log -1 --format=%s
+ ) >actual &&
+ test_cmp expect actual
+'
+
test_done
^ permalink raw reply related
* Re: Support for a series of patches, i.e. patchset or changeset?
From: Jeff King @ 2012-11-08 19:09 UTC (permalink / raw)
To: Eric Miao; +Cc: Michael J Gruber, git
In-Reply-To: <CAMPhdO-Z3E352KbTvnrxJqCecAUGfHCwOoFRUKzObh35uLnrSw@mail.gmail.com>
On Tue, Nov 06, 2012 at 08:58:35AM +0800, Eric Miao wrote:
> > So, then the question is: What do you know/have? Is your patch the
> > output of "git format-patch", "git diff", or just some sort of diff
> > without any git information?
>
> That doesn't matter, all the info can be obtained from the SHA1 id, the
> question is: do we have a mechanism in git (or hopefully we could add)
> to record the patchset or series the patch belongs to, without people to
> guess heuristically.
>
> E.g. when we merged a series of patches:
>
> [PATCH 00/08]
> [PATCH 01/08]
> ...
> [PATCH 08/08]
>
> How do we know this whole series after merged when only one of these
> commits are known?
Others have described how you can infer this structure from the history
graph, but as you noted, the graph does not always match the series that
was sent, nor does it contain some of the meta information about the
cover letter, associated discussions, etc.
If you want to track the mapping between mailed patches (or any other
form of changeset id) to commits, you can put it in git in one of two
places:
1. In a pseudo-header at the end of the commit message. E.g., you
could use the message-id of the cover letter as a unique identifier
for the changeset, and put "Changeset: $MID" at the end of each
commit message. Then you can use "--grep" to find other entries
from the same changeset.
2. You can use git-notes to store the same information outside of the
commit message. This doesn't get pushed around automatically with
the history, but it means your commit messages are not polluted,
and you can make annotations after the commits are set in stone.
I do not use Gerrit, but I recall that they do something like (1) to
mark changesets. For git development, one of the contributors does (2)
to point notes at mailing list threads (I think he uses a script to
match up mails and commits after the fact).
But fundamentally the idea of "this is a set of logical changes" is not
represented in git's DAG. It's up to you to store changeset tokens
if you care about them.
-Peff
^ permalink raw reply
* Re: Revert option for git add --patch
From: Jeff King @ 2012-11-08 20:04 UTC (permalink / raw)
To: Nathan Broadbent; +Cc: Jonathon Mah, git@vger.kernel.org
In-Reply-To: <CAPXHQbPrjZ_Ezi3RkHny-fW0=mQCTmX9NVJfgUh7P-Xx9pgBcg@mail.gmail.com>
On Fri, Nov 09, 2012 at 08:42:33AM +1300, Nathan Broadbent wrote:
> It sounds like we want a tool that combines the functionality of 'git add
> --patch', 'git checkout --patch', and other features, so that we can
> perform various actions on 'hunks' without switching context. What do you
> think about naming this command 'git patch'?
I think it would be a slightly confusing name, given that it is not
related to the usual Unix "patch" command (git's analog to that is "git
apply").
I was thinking something like "git organize" because it is about
organizing muddled changes. Duy mentioned my earlier "git put", which is
based around the same ideas. This could be thought of as "git put
--patch", I suppose.
> I'm not sure how dropping hunks into 'buckets' would work, but the main
> idea is to be able to stage, discard or edit hunks in a single pass.
Buckets are really just patches you are building up from hunks. So I
think you are dealing with two buckets either way: a to-stage bucket and
a to-discard bucket. Adding more buckets isn't hard; you just turn the
to-stage bucket into an array of buckets.
The tricky part is figuring out how to keep the state persistent across
multiple invocations. The simplest program flow for the 2-bucket case
would be something like:
for h in hunks
do
show hunk to user
ask user what they want to do
if they want to stage
append to to-stage patch
else if they want to discard
append to to-discard patch
else
ignore the hunk
done
feed to-stage patch to "git apply --cached"
feed to-discard patch to "git apply -R"
Modifying the loop to have more buckets is easy. But the end is probably
something like:
feed to-discard patch to "git apply -R"
feed bucket[0] to "git apply --cached"
save bucket[1..n] somewhere
exit
Then the user gets control back, builds, tests, commits, whatever, and
then runs "git organize --continue" to apply bucket[1], and so on. And
since we don't know what the user did in between, we have to be ready
for conflicts in applying the buckets. Hmm, this is sounding a lot like
how "rebase --interactive" works.
One way to implement this would be to store each bucket as a commit
against HEAD (i.e., stage it into a temporary index, then run "git
write-tree" and "git commit-tree"), and then make a "rebase -i"
instruction sheet like:
pick $sha1_of_bucket_1_commit
edit
pick $sha1_of_bucket_2_commit
edit
and so forth, except that we would not want our picks to actually commit
(just pull the changes into the working tree and/or index).
That's just me thinking off the top of my head. Obviously what you are
proposing is way simpler, and it is probably sane to start with the
simple thing, and then build the more complex thing on top. But it might
be worth keeping in mind the complex case when building out the
infrastructure for the simple case.
-Peff
^ permalink raw reply
* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: Jeff King @ 2012-11-08 20:09 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <5093DC0C.5000603@drmicha.warpmail.net>
On Fri, Nov 02, 2012 at 03:43:24PM +0100, Michael J Gruber wrote:
> It seems that our fast-import is super picky with regards to author
> names. I've encountered author names like
>
> Foo Bar<foo.bar@dev.null>
> Foo Bar <foo.bar@dev.null
> foo.bar@dev.null
>
> in the self-hosting repo of some other dvcs, and the question is how to
> translate them faithfully into a git author name.
It is not just fast-import. Git's author field looks like an rfc822
address, but it's much simpler. It fundamentally does not allow angle
brackets in the "name" field, regardless of any quoting. As you noted in
your followup, we strip them out if you provide them via
GIT_AUTHOR_NAME.
I doubt this will change anytime soon due to the compatibility fallout.
So it is up to generators of fast-import streams to decide how to encode
what they get from another system (you could come up with an encoding
scheme that represents angle brackets).
> In general, we try to do
>
> fullotherdvcsname <none@none>
>
> if the other system's entry does not parse as a git author name, but
> fast-import does not accept either of
>
> Foo Bar<foo.bar@dev.null> <none@none>
> "Foo Bar<foo.bar@dev.null>" <none@none>
>
> because of the way it parses for <>. While the above could be easily
> turned into
>
> Foo Bar <foo.bar@dev.null>
>
> it would not be a faithful representation of the original commit in the
> other dvcs.
I'd think that if a remote system has names with angle brackets and
email-looking things inside them, we would do better to stick them in
the email field rather than putting in a useless <none@none>. The latter
should only be used for systems that lack the information.
But that is a quality-of-implementation issue for the import scripts
(and they may even want to have options, just like git-cvsimport allows
mapping cvs usernames into full identities).
-Peff
^ permalink raw reply
* [PATCH] custom log formats for "diff --submodule=log"
From: Jeff King @ 2012-11-08 20:29 UTC (permalink / raw)
To: git; +Cc: Jeffrey S. Haemer
An off-list discussion made me wonder if something like this would be
useful:
git log -p --submodule=log:' %m %an <%ae>: %s'
where the format could be whatever you find useful.
I do not use submodules myself, so writing the patch below was just a
fun exercise. I'm not planning on polishing it for inclusion. But I
thought I would publish it here in case anybody who is more interested
feels like picking it up. It would need documentation and tests at the
very least.
---
diff --git a/diff.c b/diff.c
index 86e5f2a..f2adaca 100644
--- a/diff.c
+++ b/diff.c
@@ -2241,7 +2241,7 @@ static void builtin_diff(const char *name_a,
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o->file, one ? one->path : two->path,
one->sha1, two->sha1, two->dirty_submodule,
- del, add, reset);
+ del, add, reset, o->submodule_log_format);
return;
}
@@ -3654,8 +3654,13 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
} else if (!strcmp(arg, "--submodule"))
DIFF_OPT_SET(options, SUBMODULE_LOG);
else if (!prefixcmp(arg, "--submodule=")) {
- if (!strcmp(arg + 12, "log"))
+ const char *v = arg + 12;
+ if (!strcmp(v, "log"))
DIFF_OPT_SET(options, SUBMODULE_LOG);
+ else if (!prefixcmp(v, "log:")) {
+ DIFF_OPT_SET(options, SUBMODULE_LOG);
+ options->submodule_log_format = xstrdup(v + 4);
+ }
}
/* misc options */
diff --git a/diff.h b/diff.h
index a658f85..9726375 100644
--- a/diff.h
+++ b/diff.h
@@ -132,6 +132,8 @@ struct diff_options {
const char *stat_sep;
long xdl_opts;
+ char *submodule_log_format;
+
int stat_width;
int stat_name_width;
int stat_graph_width;
diff --git a/submodule.c b/submodule.c
index e3e0b45..149bd87 100644
--- a/submodule.c
+++ b/submodule.c
@@ -217,12 +217,16 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,
}
static void print_submodule_summary(struct rev_info *rev, FILE *f,
- const char *del, const char *add, const char *reset)
+ const char *del, const char *add, const char *reset,
+ const char *format)
{
- static const char format[] = " %m %s";
+ static const char default_format[] = " %m %s";
struct strbuf sb = STRBUF_INIT;
struct commit *commit;
+ if (!format)
+ format = default_format;
+
while ((commit = get_revision(rev))) {
struct pretty_print_context ctx = {0};
ctx.date_mode = rev->date_mode;
@@ -259,7 +263,8 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
void show_submodule_summary(FILE *f, const char *path,
unsigned char one[20], unsigned char two[20],
unsigned dirty_submodule,
- const char *del, const char *add, const char *reset)
+ const char *del, const char *add, const char *reset,
+ const char *format)
{
struct rev_info rev;
struct commit *left = left, *right = right;
@@ -304,7 +309,7 @@ void show_submodule_summary(FILE *f, const char *path,
fwrite(sb.buf, sb.len, 1, f);
if (!message) {
- print_submodule_summary(&rev, f, del, add, reset);
+ print_submodule_summary(&rev, f, del, add, reset, format);
clear_commit_marks(left, ~0);
clear_commit_marks(right, ~0);
}
diff --git a/submodule.h b/submodule.h
index f2e8271..49a7b75 100644
--- a/submodule.h
+++ b/submodule.h
@@ -21,7 +21,8 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
void show_submodule_summary(FILE *f, const char *path,
unsigned char one[20], unsigned char two[20],
unsigned dirty_submodule,
- const char *del, const char *add, const char *reset);
+ const char *del, const char *add, const char *reset,
+ const char *format);
void set_config_fetch_recurse_submodules(int value);
void check_for_new_submodule_commits(unsigned char new_sha1[20]);
int fetch_populated_submodules(const struct argv_array *options,
^ permalink raw reply related
* Re: Long clone time after "done."
From: Jeff King @ 2012-11-08 20:33 UTC (permalink / raw)
To: Uri Moszkowicz; +Cc: git
In-Reply-To: <CAMJd5AQ24u11BH6rMAHvR95N4ys6KHfEQKD1uLzr+=TDgN_69Q@mail.gmail.com>
On Thu, Nov 08, 2012 at 11:20:29AM -0600, Uri Moszkowicz wrote:
> I tried the patch but it doesn't appear to have helped :( Clone time
> with it was ~32m.
That sounds ridiculously long.
> Do you all by any chance have a tool to obfuscate a repository?
> Probably I still wouldn't be permitted to distribute it but might make
> the option slightly more palatable. Anything else that I can do to
> help debug this problem?
I don't have anything already written. What platform are you on? If it's
Linux, can you try using "perf" to record where the time is going?
How many refs do you have? What does:
echo "heads: $(git for-each-ref refs/heads | wc -l)"
echo " tags: $(git for-each-ref refs/tags | wc -l)"
report? How long does it take to look up a tag, like:
time git cat-file tag refs/tags/some-tag
?
-Peff
^ 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