* Re: git push tags
From: Chris Rorvick @ 2012-10-29 2:15 UTC (permalink / raw)
To: Philip Oakley; +Cc: Johannes Sixt, Angelo Borsotti, git
In-Reply-To: <4B8097A9D6854CDFA27E7CF6574B37BA@PhilipOakley>
On Sun, Oct 28, 2012 at 4:49 PM, Philip Oakley <philipoakley@iee.org> wrote:
> From: "Chris Rorvick" <chris@rorvick.com> Sent: Sunday, October 28, 2012
> 7:59 PM
>
>> On Sun, Oct 28, 2012 at 1:15 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>>>
>>> Tags are refs, just like branches. "Tags don't move" is just a
>>> convention, and git doesn't even respect it (except possibly in one
>>> place[1]). You can't reseat tags unless you use -f, which is exactly the
>>> same with branches, which you can't reseat unless you use -f.
>>>
>>> [1] By default, git fetch does not fetch tags that it already has.
>>
>>
>> Also, git checkout <tag> puts you on a detached HEAD. This seems
>> pretty significant with regard to Git respecting a "tags don't move"
>> convention.
>
>
> Surely the convention is the other way around. That is, it is branches that
> are _expected_ to move, hence unless you are checkout a branch (movable) you
> will be on a detached head at a fixed place/sha1 [aka not on a branch].
>
> The checking out of a tag action doesn't make it more or less significant.
Ah, that makes sense. Thanks. So it sounds like the solution should
be something more general than checking if the prefix is "refs/tags"
if I understand your point correctly.
Also, setting the status to REF_STATUS_REJECT_NONFASTFORWARD even when
it would be a fast-forward (does that terminology apply to non-branch
refs?) is confusing. It's already confusing that a failure to push a
tag tells me I need to fetch and merge in the changes.
Chris
^ permalink raw reply
* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Michael Haggerty @ 2012-10-29 5:10 UTC (permalink / raw)
To: David Aguilar
Cc: Junio C Hamano, Jiang Xin, Lea Wiemann, David Reiss,
Johannes Sixt, git, Lars R. Damerow, Jeff King, Marc Jordan
In-Reply-To: <CAJDDKr4ki+NjSeuZpAU6bM=YAQ_3mdHCtawstdCqe9Ewvp=arQ@mail.gmail.com>
On 10/29/2012 01:15 AM, David Aguilar wrote:
> On Sat, Oct 20, 2012 at 11:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>
>>> This patch series has the side effect that all of the directories
>>> listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to
>>> resolve any symlinks that are present in their paths. It is
>>> admittedly odd that a feature intended to avoid accessing expensive
>>> directories would now *intentionally* access directories near the
>>> expensive ones. In the above scenario this shouldn't be a problem,
>>> because /home would be the directory listed in
>>> GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be
>>> expensive.
>>
>> Interesting observation. In the last sentence, "accessing /home"
>> does not exactly mean accessing /home, but accessing / to learn
>> about "home" in it, no?
>>
>>> But there might be other scenarios for which this patch
>>> series causes a performance regression.
>>
>> Yeah, after merging this to 'next', we should ask people who care
>> about CEILING to test it sufficiently.
>>
>> Thanks for rerolling.
>
> GIT_CEILING_DIRECTORIES was always about trying to avoid
> hitting them at all; they can be (busy) NFS volumes there.
>
> Here's the description from the 1.6.0 release notes:
>
> * A new environment variable GIT_CEILING_DIRECTORIES can be used to stop
> the discovery process of the toplevel of working tree; this may be useful
> when you are working in a slow network disk and are outside any working tree,
> as bash-completion and "git help" may still need to run in these places.
>
> In 8030e44215fe8f34edd57d711a35f2f0f97a0423 Lars added
> GIT_ONE_FILESYSTEM to fix a related issue.
> Do you guys have GIT_CEILING_DIRECTORIES set too?
>
> We use GIT_CEILING_DIRECTORIES and I'm pretty sure
> we don't want every git command hitting them, so this would
> be a regression when seen from the POV of our current usage
> of this variable, which would be a bummer.
I would certainly withdraw the patch series if it causes a performance hit.
The log message of the original commit (0454dd93bf) described the
following scenario: a /home partition under which user home directories
are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid
hitting /home/.git, /home/.git/objects, and /home/objects (which would
attempt to automount those directories). I believe that this scenario
would not be slowed down by my patches.
How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a
slowdown?
> Is there another way to accomplish this without the performance hit?
> Maybe something that can be solved with configuration?
Without doing the symlink expansion there is no way for git to detect
that GIT_CEILING_DIRECTORIES contains symlinks and is therefore
ineffective. So the user has no warning about the misconfiguration
(except that git runs slowly).
On 10/29/2012 02:42 AM, Junio C Hamano wrote:
> Perhaps not canonicalize elements on the CEILING list ourselves? If
> we make it a user error to put symlinked alias in the variable, and
> document it clearly, wouldn't it suffice?
There may be no other choice. (That, and fix the test suite in another
way to tolerate a $PWD that involves symlinks.)
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: gitweb
From: Jeff King @ 2012-10-29 5:28 UTC (permalink / raw)
To: rh; +Cc: Jakub Narebski, git
In-Reply-To: <20121028165647.b79fe3fcb6784c4ae547439e@lavabit.com>
On Sun, Oct 28, 2012 at 04:56:47PM -0700, rh wrote:
> I'm not using gitweb I was thinking about using it and was looking at the
> cgi and saw this in this file:
> https://github.com/git/git/blob/master/gitweb/gitweb.perl
>
> I think I understand the intention but the outcome is wrong.
>
> our %highlight_ext = (
> # main extensions, defining name of syntax;
> # see files in /usr/share/highlight/langDefs/ directory
> map { $_ => $_ }
> qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make),
> # alternate extensions, see /etc/highlight/filetypes.conf
> 'h' => 'c',
> map { $_ => 'sh' } qw(bash zsh ksh),
> map { $_ => 'cpp' } qw(cxx c++ cc),
> map { $_ => 'php' } qw(php3 php4 php5 phps),
> map { $_ => 'pl' } qw(perl pm), # perhaps also 'cgi'
> map { $_ => 'make'} qw(mak mk),
> map { $_ => 'xml' } qw(xhtml html htm),
> );
Yeah, this is wrong. The first map will eat the rest of the list, and
you will get "h => h", "cxx => cxx", and so forth. I do not know this
chunk of code, but that does not seem like it is the likely intent.
You could fix it with extra parentheses:
our %he = (
(map { $_ => $_ } qw(py c cpp ...)),
'h' => 'c',
(map { $_ => 'sh' } qw(bash zsh ksh)),
... etc ...
);
> I think the intent is better met with this, (the print is for show)
>
> our %he = ();
> $he{'h'} = 'c';
> $he{$_} = $_ for (qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make));
> $he{$_} = 'cpp' for (qw(cxx c++ cc));
> $he{$_} = 'php' for (qw(php3 php4 php5 phps));
> $he{$_} = 'pl' for (qw(cgi perl pm));
> $he{$_} = 'make' for (qw(mak mk));
> $he{$_} = 'xml' for (qw(xhtml html htm));
> $he{$_} = 'sh' for (qw(bash zsh ksh));
That is more readable to me (though it does lose the obviousness that it
is a variable initialization).
Looks like this was broken since 592ea41 (gitweb: Refactor syntax
highlighting support, 2010-04-27). I do not use gitweb (nor highlight)
at all, but I'd guess the user-visible impact is that "*.h" files are
not correctly highlighted (unless highlight does this extension mapping
itself, but then why are we doing it here?).
Jakub, can you confirm the intent and a fix like the one above makes
things better?
-Peff
^ permalink raw reply
* Re: [PATCH v2] git-submodule add: Add -r/--record option.
From: Jeff King @ 2012-10-29 5:34 UTC (permalink / raw)
To: W. Trevor King; +Cc: Shawn Pearce, Jens Lehmann, Git, Nahor, Phil Hord
In-Reply-To: <20121028223431.GF26675@odin.tremily.us>
On Sun, Oct 28, 2012 at 06:34:31PM -0400, W. Trevor King wrote:
> On Sun, Oct 28, 2012 at 02:59:33PM -0700, Shawn Pearce wrote:
> > Looks like the Gerrit meaning is basically the same as Ævar's. Gerrit
> > updates the parent project as if you had done:
> >
> > $ git submodule foreach 'git checkout $(git config --file
> > $toplevel/.gitmodules submodule.$name.branch) && git pull'
> > $ git commit -a -m "Updated submodules"
> > $ git push
>
> Ah, good, then we *are* all using the option for the same thing.
That makes me more comfortable. Your patch adds support for setting the
variable initially. Does it need any special magic for maintenance, or
is it something that would always be updated by hand?
Right now, the variable is not an official git-submodule thing and it is
OK to say "you are on your own by setting and using it from external
tools". But as soon as we support setting it in the first place, it is
reasonable to claim it as a bug if we do not keep it up to date for
certain operations.
I'm not familiar enough with the workflows around branching submodules
to know whether any such operations actually exist.
-Peff
^ permalink raw reply
* Re: Mistake in git-reset documentation
From: Jeff King @ 2012-10-29 5:38 UTC (permalink / raw)
To: Krzysztof Mazur; +Cc: Andreas Schwab, Bojan Petrović, git
In-Reply-To: <20121028141327.GB16686@shrek.podlesie.net>
On Sun, Oct 28, 2012 at 03:13:27PM +0100, Krzysztof Mazur wrote:
> The --mixed mode is also described as second mode, and saying that --mixed
> is default earlier may save some time wasted on reading --soft
> description.
>
> There is also small inconsequence in what <mode> is, just "mixed" or
> "--mixed".
Yeah, agreed on both.
> -- >8 --
> Subject: [PATCH] doc: git-reset: make "<mode>" optional
>
> The git-reset's "<mode>" is an optional argument, however it was
> documented as required.
>
> The "<mode>" is documented as one of: --soft, --mixed, --hard, --merge
> or --keep, so "<mode>" should be used instead of "--<mode>".
>
> Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
Thanks, this looks good to me.
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Lars Damerow @ 2012-10-29 5:34 UTC (permalink / raw)
To: David Aguilar
Cc: Junio C Hamano, Michael Haggerty, Jiang Xin, Lea Wiemann,
David Reiss, Johannes Sixt, git, Jeff King, Marc Jordan
In-Reply-To: <CAJDDKr4ki+NjSeuZpAU6bM=YAQ_3mdHCtawstdCqe9Ewvp=arQ@mail.gmail.com>
>From David Aguilar <davvid@gmail.com>, Sun, Oct 28, 2012 at 05:15:29PM -0700:
>
> In 8030e44215fe8f34edd57d711a35f2f0f97a0423 Lars added
> GIT_ONE_FILESYSTEM to fix a related issue.
> Do you guys have GIT_CEILING_DIRECTORIES set too?
Once GIT_DISCOVERY_ACROSS_FILESYSTEM (the eventual name for GIT_ONE_FILESYSTEM) was accepted, we stopped setting GIT_CEILING_DIRECTORIES in our environment.
Even so, the way our filesystems work, stat() or lstat() calls on the paths that we used to have in GIT_CEILING_DIRECTORIES wouldn't have caused a problem.
I don't personally have an objection to patches that explicitly access the GIT_CEILING_DIRECTORIES paths, but it seems like other folks' configurations could be harmed by them.
-lars
--
lars r. damerow :: button pusher :: pixar animation studios
^ permalink raw reply
* Re: [PATCH] builtin/config.c: Fix a sparse warning
From: Jeff King @ 2012-10-29 5:47 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list
In-Reply-To: <508D9E15.1040105@ramsay1.demon.co.uk>
On Sun, Oct 28, 2012 at 09:05:25PM +0000, Ramsay Jones wrote:
> Sparse issues an "Using plain integer as NULL pointer" warning while
> checking a 'struct strbuf_list' initializer expression. The initial
> field of the struct has pointer type, but the initializer expression
> is given as '{0}'. In order to suppress the warning, we simply replace
> the initializer with '{NULL}'.
I slightly disagree with sparse here, as the 0 here is not about
explicitly setting the pointer, but is an idiomatic shorthand for
requesting static-like initialization (and it is purely a style thing,
not a bug, as 0 is a fine null pointer, even on platforms without
all-bits-zero null pointers).
But I think it is still worth changing in the name of
sparse-cleanliness. Thanks.
> I try to catch these types of warnings while the patches are still
> in the pu branch. I don't know how I missed this one, but I don't
> remember your 'jk/config-ignore-duplicates' branch being in pu.
>
> Sorry about that.
Not your fault. The time during which it would have been in pu was after
Junio left but before I had pushed out my pu. I did have it locally in
pu for a bit, but it ended up on next the first time it was published,
because the series had already received favorable review on the list in
the meantime.
But I forgot that some people may first see the new code via the
published pu, not the list (e.g., if they are pulling and building pu
routinely with sparse). I'll try to be better about making sure stuff
is actually published in pu for a cycle before merging to next.
-Peff
^ permalink raw reply
* Re: [PATCH] pathspec.c: Fix some sparse warnings
From: Jeff King @ 2012-10-29 5:55 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Nguyen Thai Ngoc Duy, git, GIT Mailing-list
In-Reply-To: <508D9EFF.3040900@ramsay1.demon.co.uk>
On Sun, Oct 28, 2012 at 09:09:19PM +0000, Ramsay Jones wrote:
> I asked Adam to squash this patch into his 'as/check-ignore' branch
> when he next re-rolled the branch. However, it appears that you
> resubmitted that branch instead ... :-D
>
> I don't know if this branch is ready to progress to next yet, but
> could somebody (yourself, Adam or Jeff?) please squash this into
> commit 1a88ae42 ("pathspec.c: move reusable code from builtin/add.c")
> before it hits next.
I squashed it into my version of as/check-ignore, but if we are
expecting a re-roll, please everybody remember to include it.
-Peff
^ permalink raw reply
* Re: crash on git diff-tree -Ganything <tree> for new files with textconv filter
From: Jeff King @ 2012-10-29 6:05 UTC (permalink / raw)
To: Peter Oberndorfer; +Cc: git, Junio C Hamano
In-Reply-To: <508D8DF7.7040007@arcor.de>
On Sun, Oct 28, 2012 at 08:56:39PM +0100, Peter Oberndorfer wrote:
> > The patch below should fix it. I added tests, but please try your
> > real-world test case on it to double-check.
>
> I tested your patch, but now it crashes for another reason :-)
Well, that's progress, right? :)
> i have a file with exactly 12288(0x3000) bytes in the repository.
> When the file is loaded, the data is placed luckily so the data end
> falls at a page boundary.
> Later diff_grep() calls regexec() which calls strlen() on the loaded buffer
> and ends up reading beyond the actual data into the next page
> which is not allocated and causes a pagefault.
> Or it could possibly (randomly) match the regex on data that is not
> actually part of a file...
Yuck. For the most part, we treat blob content (and generally most
object content) as a sized buffer. However, there are some spots which,
either through laziness or because a code interface expects a string, we
pass the value as a string. This works because the object-reading code
puts an extra NUL at the end of our buffer to handle just such an
instance. So we might prematurely end if the object contains embedded
NULs, but we would never read past the end.
The code to read the output of a textconv filter does not do this
explicitly. I would think it would get it for free by virtue of reading
into a strbuf, though. I'll try to investigate.
-Peff
^ permalink raw reply
* Re: crash on git diff-tree -Ganything <tree> for new files with textconv filter
From: Jeff King @ 2012-10-29 6:18 UTC (permalink / raw)
To: Peter Oberndorfer; +Cc: git, Junio C Hamano
In-Reply-To: <20121029060524.GB4457@sigill.intra.peff.net>
On Mon, Oct 29, 2012 at 02:05:24AM -0400, Jeff King wrote:
> > i have a file with exactly 12288(0x3000) bytes in the repository.
> > When the file is loaded, the data is placed luckily so the data end
> > falls at a page boundary.
> > Later diff_grep() calls regexec() which calls strlen() on the loaded buffer
> > and ends up reading beyond the actual data into the next page
> > which is not allocated and causes a pagefault.
> > Or it could possibly (randomly) match the regex on data that is not
> > actually part of a file...
>
> Yuck. For the most part, we treat blob content (and generally most
> object content) as a sized buffer. However, there are some spots which,
> either through laziness or because a code interface expects a string, we
> pass the value as a string. This works because the object-reading code
> puts an extra NUL at the end of our buffer to handle just such an
> instance. So we might prematurely end if the object contains embedded
> NULs, but we would never read past the end.
>
> The code to read the output of a textconv filter does not do this
> explicitly. I would think it would get it for free by virtue of reading
> into a strbuf, though. I'll try to investigate.
I can't seem to replicate the problem here, even under valgrind. Do you
have a minimal test case?
-Peff
^ permalink raw reply
* Re: [PATCH] Documentation: improve the example of overriding LESS via core.pager
From: Jeff King @ 2012-10-29 6:26 UTC (permalink / raw)
To: Patrick Palka; +Cc: git
In-Reply-To: <1351455166-2579-1-git-send-email-patrick@parcs.ath.cx>
On Sun, Oct 28, 2012 at 04:12:46PM -0400, Patrick Palka wrote:
> You can override an option set in the LESS variable by simply prefixing
> the command line option with `-+`. This is more robust than the previous
> example if the default LESS options are to ever change.
Yeah, the current description is quite tortured and complex. I wondered
if there might be some reason, but I think it is simply that fee7545 was
over-zealous in adopting the original text. Yours is much more sensible.
Thanks.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] gitk: simplify file filtering
From: Johannes Sixt @ 2012-10-29 6:46 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Paul Mackerras
In-Reply-To: <1351185801-18543-2-git-send-email-felipe.contreras@gmail.com>
Am 10/25/2012 19:23, schrieb Felipe Contreras:
> git diff is perfectly able to do this with '-- files', no need for
> manual filtering.
It would have been very useful if you summarized what the code did before
this change.
As for the change itself, it seems to make sense to use git diff's path
limiting instead of some home-grown method.
-- Hannes
^ permalink raw reply
* Re: [PATCH 2/2] gitk: handle --full-diff correctly
From: Johannes Sixt @ 2012-10-29 6:52 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Paul Mackerras
In-Reply-To: <1351185801-18543-3-git-send-email-felipe.contreras@gmail.com>
Am 10/25/2012 19:23, schrieb Felipe Contreras:
> Otherwise the files are missing from the diff, and the list of files.
>
> We do this by creating a limitdiffs variable specific for the view which
> can be turned of by using --full-diff.
It would be very helpful if you described the problem. "Try it yourself"
is not the answer. In particular, what does "correctly" mean in this
context? Isn't the subject more like "gitk: handle --full-diff" in the
sense that it wasn't handled at all previously and it did what it happened
to do?
The patch breaks gitk's builtin "limit to listed paths" option - with the
patch, the option is only honored at startup, but not when it is changed
during the session when --full-diff is not given. Don't know what should
happen if it is given.
-- Hannes
^ permalink raw reply
* Re: [PATCH] replace: parse revision argument for -d
From: Jeff King @ 2012-10-29 6:58 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <807340e40adb1fc97cd97161fe1fabd292bc79c3.1351258394.git.git@drmicha.warpmail.net>
On Fri, Oct 26, 2012 at 03:33:27PM +0200, Michael J Gruber wrote:
> for (p = argv; *p; p++) {
> - if (snprintf(ref, sizeof(ref), "refs/replace/%s", *p)
> + q = *p;
> + if (get_sha1(q, sha1))
> + warning("Failed to resolve '%s' as a valid ref; taking it literally.", q);
> + else
> + q = sha1_to_hex(sha1);
Doesn't get_sha1 already handle this for 40-byte sha1s (and for anything
else, it would not work anyway)?
-Peff
^ permalink raw reply
* Re: [PATCH v2] fix 'make test' for HP NonStop
From: Jeff King @ 2012-10-29 7:06 UTC (permalink / raw)
To: Joachim Schmitz; +Cc: git
In-Reply-To: <003101cdb29f$7c8d9490$75a8bdb0$@schmitz-digital.de>
On Thu, Oct 25, 2012 at 12:57:10PM +0200, Joachim Schmitz wrote:
> diff --git a/Makefile b/Makefile
> index f69979e..35380dd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1381,6 +1381,15 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> MKDIR_WO_TRAILING_SLASH = YesPlease
> # RFE 10-120912-4693 submitted to HP NonStop development.
> NO_SETITIMER = UnfortunatelyYes
> +
> + # for 'make test'
> + # some test don't work with /bin/diff, some fail with /bin/tar
> + # some need bash, and some need /usr/local/bin in PATH first
> + SHELL_PATH=/usr/local/bin/bash
> + SANE_TOOL_PATH=/usr/local/bin
I think we can drop these comments, as the reasoning really should just
go in the commit message.
> + # as of H06.25/J06.14, we might better use this
> + #SHELL_PATH=/usr/coreutils/bin/bash
> + #SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin
Is there any reason not to put both into the default SANE_TOOL_PATH? If
/usr/coreutils/bin does not exist on older versions, it will be a
harmless no-op. I guess we arestuck with picking one $SHELL_PATH,
though.
-Peff
^ permalink raw reply
* Re: [PATCH] Teach rm to remove submodules when given with a trailing '/'
From: Johannes Sixt @ 2012-10-29 7:11 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Git Mailing List, Jeff King
In-Reply-To: <508DBF92.9090200@web.de>
Am 10/29/2012 0:28, schrieb Jens Lehmann:
> + /* Remove trailing '/' from directories to find submodules in the index */
> + for (i = 0; i < argc; i++) {
> + size_t pathlen = strlen(argv[i]);
> + if (pathlen && is_directory(argv[i]) && (argv[i][pathlen - 1] == '/'))
> + argv[i] = xmemdupz(argv[i], pathlen - 1);
> + }
> +
> pathspec = get_pathspec(prefix, argv);
> refresh_index(&the_index, REFRESH_QUIET, pathspec, NULL, NULL);
That's wrong: Either move the check below get_pathspec() (which normalizes
backslashes to forward-slashes on Windows) or use is_dir_sep().
But isn't it somewhat dangerous to check pathspec for existance in the
worktree without interpreting them? Think of magic pathspec syntax (that
we do not have yet, but which may materialize sometime in the future).
-- Hannes
^ permalink raw reply
* Re: gitweb
From: Jakub Narębski @ 2012-10-29 7:12 UTC (permalink / raw)
To: Jeff King; +Cc: rh, git
In-Reply-To: <20121029052815.GA30186@sigill.intra.peff.net>
On Mon, Oct 29, 2012 at 6:28 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Oct 28, 2012 at 04:56:47PM -0700, rh wrote:
>
>> I'm not using gitweb I was thinking about using it and was looking at the
>> cgi and saw this in this file:
>> https://github.com/git/git/blob/master/gitweb/gitweb.perl
>>
>> I think I understand the intention but the outcome is wrong.
>>
>> our %highlight_ext = (
>> # main extensions, defining name of syntax;
>> # see files in /usr/share/highlight/langDefs/ directory
>> map { $_ => $_ }
>> qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make),
>> # alternate extensions, see /etc/highlight/filetypes.conf
>> 'h' => 'c',
[...]
> Yeah, this is wrong. The first map will eat the rest of the list, and
> you will get "h => h", "cxx => cxx", and so forth. I do not know this
> chunk of code, but that does not seem like it is the likely intent.
>
> You could fix it with extra parentheses:
>
> our %he = (
> (map { $_ => $_ } qw(py c cpp ...)),
> 'h' => 'c',
> (map { $_ => 'sh' } qw(bash zsh ksh)),
> ... etc ...
> );
>
>> I think the intent is better met with this, (the print is for show)
>>
>> our %he = ();
>> $he{'h'} = 'c';
>> $he{$_} = $_ for (qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make));
>> $he{$_} = 'cpp' for (qw(cxx c++ cc));
[...]
> That is more readable to me (though it does lose the obviousness that it
> is a variable initialization).
>
> Looks like this was broken since 592ea41 (gitweb: Refactor syntax
> highlighting support, 2010-04-27). I do not use gitweb (nor highlight)
> at all, but I'd guess the user-visible impact is that "*.h" files are
> not correctly highlighted
>
> Jakub, can you confirm the intent and a fix like the one above makes
> things better?
Yes, either of those makes things better.
> (unless highlight does this extension mapping
> itself, but then why are we doing it here?).
Highlight does extension mapping itself... but for that it needs file name,
and not to be feed file contents from pipe.
--
Jakub Narebski
^ permalink raw reply
* Re: git push tags
From: Angelo Borsotti @ 2012-10-29 7:13 UTC (permalink / raw)
To: Philip Oakley; +Cc: Chris Rorvick, Johannes Sixt, git
In-Reply-To: <4B8097A9D6854CDFA27E7CF6574B37BA@PhilipOakley>
Hi,
Pro Git, By Scott Chacon says:
2.6
"Like most VCSs, Git has the ability to tag specific points in
history as being important.
Generally, people use this functionality to mark release points (v1.0,
and so on)."
2.6.2:
"A [lightweight] tag is very much like a branch that does not change ..."
Clearly, tags are not the same as branches. They are there for a
different purpose. If they were exactly the same as branches, there
would be no need for them.
Of course, they share some behaviors with branches, and there are
several commands that handle them in the same way, but the key point
is that they do not change, or at least they do not do that by
default. The ability to force changes of tags is there only to correct
tags that might have set tags to the wrong points by mistake.
So, what I am telling is that this property must be preserved
consistently across all commands, including git-push.
-Angelo
^ permalink raw reply
* Re: gitweb
From: Jeff King @ 2012-10-29 7:14 UTC (permalink / raw)
To: Jakub Narębski; +Cc: rh, git
In-Reply-To: <CANQwDwd6EP94PEFkEcx8gBX1B5+-95qtjGMD6iU3ao8G+rCbLw@mail.gmail.com>
On Mon, Oct 29, 2012 at 08:12:46AM +0100, Jakub Narębski wrote:
> > Jakub, can you confirm the intent and a fix like the one above makes
> > things better?
>
> Yes, either of those makes things better.
Thanks.
> > (unless highlight does this extension mapping
> > itself, but then why are we doing it here?).
>
> Highlight does extension mapping itself... but for that it needs file name,
> and not to be feed file contents from pipe.
Ah, that makes sense.
Richard, do you want to roll a patch that fixes it?
-Peff
^ permalink raw reply
* Re: [PATCH] submodule status: remove unused orig_* variables
From: Jeff King @ 2012-10-29 7:27 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Phil Hord, Git Mailing List, phil.hord
In-Reply-To: <508DA58C.2040504@web.de>
On Sun, Oct 28, 2012 at 10:37:16PM +0100, Jens Lehmann wrote:
> When renaming orig_args to orig_flags in 98dbe63d (submodule: only
> preserve flags across recursive status/update invocations) the call site
> of the recursive cmd_status was forgotten. At that place orig_args is
> still passed into the recursion, which is always empty since then. This
> did not break anything because the orig_flags logic is not needed at all
> when a function from the submodule script is called with eval, as that
> inherits all the variables set by the option parsing done in the first
> level of the recursion.
>
> Now that we know that orig_flags and orig_args aren't needed at all,
> let's just remove them from cmd_status().
>
> Thanks-to: Phil Hord <hordp@cisco.com>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
I'll replace your earlier fix (which just fixes the s/args/flags/ and
adds a test) with Phil's patch, and this on top.
Thanks.
-Peff
^ permalink raw reply
* Re: [PATCH 0/7] Move code from builtin to libgit.a
From: Jeff King @ 2012-10-29 7:43 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1351266835-22610-1-git-send-email-pclouds@gmail.com>
On Fri, Oct 26, 2012 at 10:53:48PM +0700, Nguyen Thai Ngoc Duy wrote:
> These two are big code moves, basically all {fetch,send}-pack code.
> While I think this is good cleanup (I get surprise every time I look
> at transport.c and it leads to buitin/fetch-pack.c), this is probably
> not a good idea if we have on flight topics, which may cause lots of
> conflicts for you and Junio. Fortunately we don't at the moment.
>
> Nguyễn Thái Ngọc Duy (7):
> Move try_merge_command and checkout_fast_forward to libgit.a
> Move estimate_bisect_steps to libgit.a
> Move print_commit_list to libgit.a
> Move setup_diff_pager to libgit.a
> send-pack: move core code to libgit.a
> fetch-pack: remove global (static) configuration variable "args"
> fetch-pack: move core code to libgit.a
This does not seem to disturb anything in pu, so I am very tempted.
I tweaked the last one, which removed the inclusion of pkt-line.h from
builtin/fetch-pack.c; it is still needed by the use of packet_read_line
when reading stateless rpc refs from stdin (caught by "-Wall -Werror").
-Peff
^ permalink raw reply
* Re: [PATCH] Add -S, --gpg-sign option to manpage of "git commit"
From: Jeff King @ 2012-10-29 7:49 UTC (permalink / raw)
To: Tom Jones; +Cc: git
In-Reply-To: <20121023225101.189E57D4C@ralph.oxix.org>
On Sun, Oct 21, 2012 at 08:46:37PM +0100, Tom Jones wrote:
> git commit -S, --gpg-sign was mentioned in the program's help message,
> but not in the manpage.
>
> This adds an equivalent entry for the option in the manpage.
>
> Signed-off-by: Tom Jones <tom@oxix.org>
Thanks. This slipped through the cracks last week, but I've picked it up
now.
> DESCRIPTION
> -----------
> @@ -276,6 +276,10 @@ configuration variable documented in linkgit:git-config[1].
> commit message template when using an editor to prepare the
> default commit message.
>
> +-S[<keyid>]::
> +--gpg-sign[=<keyid>]::
> + GPG-sign commit.
> +
Potentially we could say more (like how to verify), but I will leave
that for somebody to build on top if they want.
-Peff
^ permalink raw reply
* Re: git push tags
From: Angelo Borsotti @ 2012-10-29 8:12 UTC (permalink / raw)
To: Philip Oakley; +Cc: Chris Rorvick, Johannes Sixt, git
In-Reply-To: <CAB9Jk9BmMMDLsY=kU5o-c4XF6fN3O44h_vXe3d=WF-W9HTBh_g@mail.gmail.com>
Hi,
to let the owner of a remote repository (one on which git-push
deposits objects) disallow
others to change tags, a key on its config file could be used.
An option on git-push, or environment variable, or key in config file
of the repo from which git-push takes objects do not help in enforcing
the policy not to update tags in the remote repo.
-Angelo
^ permalink raw reply
* Re: [PATCH v4 00/13] New remote-hg helper
From: Jeff King @ 2012-10-29 8:50 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
Ilari Liusvaara, Daniel Barkalow, Michael J Gruber
In-Reply-To: <1351396453-29042-1-git-send-email-felipe.contreras@gmail.com>
On Sun, Oct 28, 2012 at 04:54:00AM +0100, Felipe Contreras wrote:
> I've ported the tests from hg-git and made sure that the output from remote-hg
> matches the output of hg-git. With these extensive tests I would consider this
> one ready for wide use. Not only do the tests pass, I've compared the generated
> repos of a few projects, and the SHA-1's are exactly the same :)
Sounds cool. Unfortunately, the test script hangs for me, after starting
up xxdiff (!).
pstree reveals that it is "hg" that starts it, but I didn't investigate
beyond that.
-Peff
^ permalink raw reply
* Re: [PATCH] replace: parse revision argument for -d
From: Michael J Gruber @ 2012-10-29 9:02 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121029065836.GC5102@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 29.10.2012 07:58:
> On Fri, Oct 26, 2012 at 03:33:27PM +0200, Michael J Gruber wrote:
>
>> for (p = argv; *p; p++) {
>> - if (snprintf(ref, sizeof(ref), "refs/replace/%s", *p)
>> + q = *p;
>> + if (get_sha1(q, sha1))
>> + warning("Failed to resolve '%s' as a valid ref; taking it literally.", q);
>> + else
>> + q = sha1_to_hex(sha1);
>
> Doesn't get_sha1 already handle this for 40-byte sha1s (and for anything
> else, it would not work anyway)?
What is "this"???
So far, "git replace -d <rev>" only accepts a full sha1, because it uses
it literally as a ref name "resf/replace/<rev>" without resolving anything.
The patch makes it so that <rev> gets resolved to a sha1 even if it is
abbreviated, and then it gets used.
Or do you mean the warning?
Michael
^ 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