* Re: Reference a submodule branch instead of a commit
From: Junio C Hamano @ 2016-10-04 17:31 UTC (permalink / raw)
To: Stefan Beller; +Cc: Heiko Voigt, Jeremy Morton, git@vger.kernel.org
In-Reply-To: <CAGZ79kZWtAU6YG4Qz9_Gwk2db5L2kPCCKrN+64hMYDovRjiLRw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>>
>> We already have options to support these kinds of workflows. Look at the
>> option '--remote' for 'git submodule update'.
>>
>> You then only have to commit the submodule if you do not want to see it
>> as dirty locally, but you will always get the tip of a remote tracking
>> branch when updating.
>
> I wonder if we could make that convenient for users by not tracking
> the submodule,
> i.e.
> * we have the information in the .gitmodules file
> * the path itself is in the .gitignore
> * no tree entry
>
> Then you can update to the remote latest branch, without Git reporting
> a dirty submodule locally, in fact it reports nothing for the submodule.
>
> It sounds like a hack, but maybe it's worth looking into that when
> people want to see that workflow.
It IS a hack.
But if you do not touch .git<anything> file and instead say "clone
this other project at that path yourself" in README, that would
probably be sufficient.
^ permalink raw reply
* Re: error
From: Jeff King @ 2016-10-04 17:27 UTC (permalink / raw)
To: Luciano Schillagi; +Cc: git
In-Reply-To: <C1F7B566-69F4-4A22-89EA-59B24677DF43@gmail.com>
[re-adding git@vger to the cc; please keep conversations on the list so
everybody can benefit from the answers]
On Tue, Oct 04, 2016 at 01:13:16PM -0300, Luciano Schillagi wrote:
> I ran the command
>
> Luko ~ $ git config --global --unset push.default
>
>
> and it gives me the following
>
> warning: push.default has multiple values
Ah, OK. "--unset-all" would do the trick, but it may only be one
instance that you want to get rid of...
> I can access my file .gitconfig
>
> I send attached...
> [...]
>
> [push]
> default = upstream
> default = aguas
Yep. Deleting the second line there will make your problem go away, and
presumably the first is a config setting you'd want to keep. That still
doesn't answer the question of how the "aguas" line got there, but I can
guess that at some point you might have set "push.default" instead of
"remote.pushdefault".
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 0/2] place cherry pick line below commit title
From: Junio C Hamano @ 2016-10-04 17:25 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, Christian Couder
In-Reply-To: <84f28caa-2e4b-1231-1a76-3b7e765c0b61@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> One alternative is to postpone this decision by changing sequencer
> only (and not trailer) to tolerate other lines in the trailer. This
> would make them even more divergent (sequencer supports arbitrary
> lines while trailer doesn't), but they were divergent already
> (sequencer supports "(cherry picked by" but trailer doesn't).
Given that we internally do not use the "trailers" for anything
real, anything you decide to do here would be an improvement ;-)
Before, users couldn't even get any of the examples (below, from
your message) recognized as trailer blocks.
> Signed-off-by: A <author@example.com>
> [This has nothing to do with the above line]
> Signed-off-by: B <buthor@example.com>
>
> and:
>
> Link 1: a link
> a continuation of the above
>
> and:
>
> Signed-off-by: Some body <some@body.xz> (comment
> on two lines)
So I would say it is perfectly OK if your update works only for
cases we can clearly define the semantics for. For example, we can
even start with something simple like:
* A RFC822-header like line, together with any number of whitespace
indented lines that immediately follow it, will be taken as a
single logical trailer element (with embedded LF in it if it uses
the "line folding"). For the purpose of "replace", the entire
single logical trailer element is replaced.
* A line that begins with "(cherry picked from" and "[" becomes a
single logical trailer element. No continuation of anything
fancy.
* A line with any other shape is a garbage line in a trailer
block. It is kept in its place, but because it does not even
have <token> part, it will not participate in locating with
"trailer.where", "trailer.ifexists", etc.
A block of lines that appear as the last paragraph in a commit
message is a trailer block if and only if certain number or
percentage of lines are non-garbage lines according to the above
definition.
The operations done by the codepaths in the core part of the system
are much simpler subset of what "interpret-trailers" wants to
support, namely,
- append "(cherry picked from X)" at the end.
- append "S-o-b:" at the end.
- append "S-o-b:" unless the same line appears as the last line in
the existing trailer block.
and these are quite compatible with a simplified definition of what
a logical line is illustrated in the above example, I would think.
I wonder if we can share a new helper function to do the detection
(and classification) of a trailer block and parsing the logical
lines out of a commit log message. The function signature could be
as simple as taking a single <const char *> (or a strbuf) that holds
a commit log message, and splitting it out into something like:
struct {
const char *whole;
const char *end_of_message_proper;
struct {
const char *token;
const char *contents;
} *trailer;
int alloc_trailers, nr_trailers;
};
where
- whole points at the first byte of the input, i.e. the beginning
of the commit message buffer.
- end-of-message-proper points at the first byte of the trailer
block into the buffer at "whole".
- token is a canonical header name for easy comparison for
interpret-trailers (you can use NULL for garbage lines, and made
up token like "[bracket]" and "(cherrypick)" that would not clash
with real tokens like "Signed-off-by").
- contents is the bytes on the logical line, including the header
part
E.g. an element in trailer[] array may say
{
.token = "Signed-off-by",
.contents = "Signed-Off-By: Some Body <some@body.xz>\n",
}
With something like that, you can manipulate the "insert at ...",
"replace", etc. in the trailer[] array and then produce an updated
commit message fairly easily (i.e. copy out the bytes beginning at
"whole" up to "end_of_message_proper", then iterate over trailer[]
array and show their contents field). The codepaths in the core
part only need to know
- how to check the last item in trailer[] array to see if it ends
with the same sign-off as they are trying to add.
- how to append one new element to the trailer[] array.
- reproduce an updated commit log message after the above.
Hmm?
^ permalink raw reply
* Re: Regression: git no longer works with musl libc's regex impl
From: Johannes Schindelin @ 2016-10-04 17:16 UTC (permalink / raw)
To: Rich Felker; +Cc: Jeff King, git, musl
In-Reply-To: <20161004161130.GX19318@brightrain.aerifal.cx>
Hi Rich,
On Tue, 4 Oct 2016, Rich Felker wrote:
> On Tue, Oct 04, 2016 at 06:08:33PM +0200, Johannes Schindelin wrote:
> > Hi Rich,
> >
> > On Tue, 4 Oct 2016, Rich Felker wrote:
> >
> > > On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:
> > > > On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:
> > > >
> > > > > 1. is nonzero mod page size, it just works; the remainder of the last
> > > > > page reads as zero bytes when mmapped.
> > > >
> > > > Is that a portable assumption?
> > >
> > > Yes.
> >
> > No, it is not. You quote POSIX, but the matter of the fact is that we use
> > a subset of POSIX in order to be able to keep things running on Windows.
> >
> > And quite honestly, there are lots of reasons to keep things running on
> > Windows, and even to favor Windows support over musl support. Over four
> > million reasons: the Git for Windows users.
>
> I would hope that in the future, git-for-windows users will be using
> musl, via midipix, rather than the painfully slow and awful version
> they're stuck with now...
Git for Windows actually uses the MSVC runtime, which is blazing fast.
You are probably confusing Git for Windows with Cygwin Git.
Ciao,
Johannes
^ permalink raw reply
* [ANNOUNCE] Git for Windows 2.10.1
From: Johannes Schindelin @ 2016-10-04 17:10 UTC (permalink / raw)
To: git-for-windows, git
[-- Attachment #1: Type: text/plain, Size: 1869 bytes --]
Dear Git users,
It is my pleasure to announce that Git for Windows 2.10.1 is available from:
https://git-for-windows.github.io/
Changes since Git for Windows v2.10.0 (September 3rd 2016)
New Features
• Comes with Git v2.10.1.
• Comes with Git Credential Manager v1.7.0.
• Comes with Git Flow v1.10.0.
• We now produce nice diffs for .docm and .dotm files, just as we did
for .docx files already.
Bug Fixes
• The icon in the Explorer integration ("Git Bash Here"), which was
lost by mistake in v2.10.0, is back.
• Fixed a crash when calling git diff -G<regex> on new-born files
without configured user diff drivers.
• Interactive GPG signing of commits and tags was fixed.
• Calling Git with --date=format:<invalid-format> no longer results
in an out-of-memory but reports the problem and aborts instead.
• Git Bash now opens properly even for Azure AD accounts.
• Git GUI respects the commit.gpgsign setting again.
• Upgrades the bundled OpenSSL to v1.0.2j.
Filename | SHA-256
-------- | -------
Git-2.10.1-64-bit.exe | 0fcb5a3d5795d5bfa1c0d35d1cada37f25b5e62c776a7099f27cfebc31362d95
Git-2.10.1-32-bit.exe | 66e0748cb0eb0b63506f38fb8bf1abb5c361ce647af86cb0f754bc5b6a17775b
PortableGit-2.10.1-64-bit.7z.exe | aa0634e026c70fe8b50207b8b125a18f45e259eac32cea246e068577a6546718
PortableGit-2.10.1-32-bit.7z.exe | 3ca6f426e3b2e6675a11b680f719c23affa7e4dc060e315375c6a262ed2658a5
MinGit-2.10.1-64-bit.zip | a7268f4ab447e62940347d52fe01321403cfa3e9e94b8e5cac4d6ded28962d64
MinGit-2.10.1-32-bit.zip | bcdeb7c00771f0e8e96689f704d158e8dcf67fdb4178f1ea3f388e877398a2c7
Git-2.10.1-64-bit.tar.bz2 | c0a541a60be3ea6264a269b90689b15da3e27811218e8c214359ec44593faa8e
Git-2.10.1-32-bit.tar.bz2 | 3c77a702911512708126e83673c5906af78807bcb9daad5223b0ab04ea81f4ea
Ciao,
Johannes
^ permalink raw reply
* Re: Reference a submodule branch instead of a commit
From: Stefan Beller @ 2016-10-04 17:07 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Junio C Hamano, Jeremy Morton, git@vger.kernel.org
In-Reply-To: <20161004113625.GB20309@book.hvoigt.net>
>
> We already have options to support these kinds of workflows. Look at the
> option '--remote' for 'git submodule update'.
>
> You then only have to commit the submodule if you do not want to see it
> as dirty locally, but you will always get the tip of a remote tracking
> branch when updating.
I wonder if we could make that convenient for users by not tracking
the submodule,
i.e.
* we have the information in the .gitmodules file
* the path itself is in the .gitignore
* no tree entry
Then you can update to the remote latest branch, without Git reporting
a dirty submodule locally, in fact it reports nothing for the submodule.
It sounds like a hack, but maybe it's worth looking into that when
people want to see that workflow.
^ permalink raw reply
* Re: [PATCH] l10n: de.po: translate 260 new messages
From: Ralf Thielow @ 2016-10-04 17:05 UTC (permalink / raw)
To: Matthias Rüster
Cc: git, Thomas Rast, Jan Krüger, Christian Stimming, Phillip Sz,
Magnus G
In-Reply-To: <0de216d5-8a47-bd1d-b9d6-8346a62bc489@gmail.com>
Hi Matthias,
thanks for review! Your findings and suggestions are very good and
I will integrate them in a new version.
2016-10-03 0:23 GMT+02:00 Matthias Rüster <matthias.ruester@gmail.com>:
>> #: builtin/merge.c:960
>> #, c-format
>> msgid "Bad value '%s' in environment '%s'"
>> -msgstr ""
>> +msgstr "Fehlerhafter Wert '%s' in Umgebungsvariable '%s'"
>
> I do not know the context but I would translate it to:
> "... Umgebung '%s'"
>
That was my first translation, too. Then I was wondering what environment
can Git even mean and looked at the sources. It turned out it really is
about an environment variable.
>
>> #: git-rebase--interactive.sh:147
>> msgid ""
>> "\n"
>> "Commands:\n"
>> " p, pick = use commit\n"
>> " r, reword = use commit, but edit the commit message\n"
>> " e, edit = use commit, but stop for amending\n"
>> " s, squash = use commit, but meld into previous commit\n"
>> " f, fixup = like \"squash\", but discard this commit's log message\n"
>> " x, exec = run command (the rest of the line) using shell\n"
>> " d, drop = remove commit\n"
>> "\n"
>> "These lines can be re-ordered; they are executed from top to bottom.\n"
>> msgstr ""
>> +"\n"
>> +"Befehle:\n"
>> +" p, pick = Commit verwenden\n"
>> +" r, reword = Commit verwenden, aber Commit-Beschreibung bearbeiten\n"
>> +" e, edit = Commit verwenden, aber zum Nachbessern anhalten\n"
>> +" s, squash = Commit verwenden, aber mit vorherigem Commit vereinen\n"
>> +" f, fixup = wie \"squash\", aber die Log-Nachricht des Commits verwerden\n"
>> +" x, exec = Befehl (Rest der Zeile) mittels Shell ausführen\n"
>> +" d, drop = Commit entfernen\n"
>> +"\n"
>> +"Diese Zeilen können umsortiert werden; Sie werden von oben nach unten\n"
>> +"ausgeführt.\n"
>
> In my opinion fixup needs a more detailed description here.
> Something like:
>
> "fixup: wie \"squash\", aber die Commit-Beschreibung vom Vorgänger
> verwenden\n".
>
> or:
>
> "fixup: wie \"squash\", aber diese Commit-Beschreibung verwerfen\n".
>
I think this one is the correct translation. The first can be
confusing or wrong
when there are squashes before, I think.
Thanks!
Ralf
^ permalink raw reply
* Re: [PATCH v8 00/11] Git filter protocol
From: Junio C Hamano @ 2016-10-04 16:47 UTC (permalink / raw)
To: Jeff King
Cc: Lars Schneider, Torsten Bögershausen, git, Stefan Beller,
Jakub Narębski, Martin-Louis Bright, ramsay
In-Reply-To: <20161004121111.gqilov3colsgzdft@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Oct 03, 2016 at 10:02:14AM -0700, Junio C Hamano wrote:
>
>> The timeout would be good for you to give a message "filter process
>> running the script '%s' is not exiting; I am waiting for it". The
>> user is still left with a hung Git, and can then see if that process
>> is hanging around. If it is, then we found a buggy filter. Or we
>> found a buggy Git. Either needs to be fixed. I do not think it
>> would help anybody by doing a kill(2) to sweep possible bugs under
>> the rug.
>
> I would argue that we should not even bother with such a timeout. This
> is an exceptional, buggy condition, and hanging is not at all restricted
> to this particular case. If git is hanging, then the right tools are
> "ps" or "strace" to figure out what is going on. I know that not all
> users are comfortable with those tools, but enough are in practice that
> the bugs get ironed out, without git having to carry a bunch of extra
> timing code that is essentially never exercised.
OK.
^ permalink raw reply
* [PATCH] push: change submodule default to check
From: Stefan Beller @ 2016-10-04 16:40 UTC (permalink / raw)
To: peff, gitster; +Cc: git, hvoigt, torvalds, Stefan Beller
In-Reply-To: <20161004162102.rwofudnx3g3fsyul@sigill.intra.peff.net>
When working with submodules, it is easy to forget to push the submodules.
The setting 'check', which checks if any existing submodule is present on
at least one remote of the submodule remotes, is designed to prevent this
mistake.
Flipping the default to check for submodules is safer than the current
default of ignoring submodules while pushing.
However checking for submodules requires additional work[1], which annoys
users that do not use submodules, so we turn on the check for submodules
based on a cheap heuristic, the existence of the .gitmodules file.
[1] https://public-inbox.org/git/CA+55aFyos78qODyw57V=w13Ux5-8SvBqObJFAq22K+XKPWVbAA@mail.gmail.com/
Signed-off-by: Stefan Beller <sbeller@google.com>
---
On Tue, Oct 4, 2016 at 9:21 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Oct 04, 2016 at 09:19:01AM -0700, Junio C Hamano wrote:
>>
>> Why should we even have a default different from today's? If most
>> repositories don't have submodules enabled at all, we can just let
>> those working with submodules enabled to toggle their configuration
>> and that is an very easy to understand solution, no?
>
> You will not see any complaint from me on that. I was taking for granted
> that the current default is inconvenient to submodule users, but I don't
> have any experience myself.
>
And there I was trying to help submodule users not shoot in their foot.
I think it is one of the problems that causes serious problems, that is easy
to fix from the side of Git. This patch replaces sb/push-make-submodule-check-the-default
and should be cheap enough for non-submodule users to accept, but still helping
submodule users as it seems to be an ok-ish heuristic. (It is possible to use
submodules and currently have no .gitmodules file present, because you're in
a weird state; then the heuristic fails. By weird state I mean e.g. a bare
repository, or you just checked out an ancient version that has no submodules
yet, or you deleted it locally for whatever reason.)
So how about this patch?
Thanks,
Stefan
builtin/push.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/builtin/push.c b/builtin/push.c
index 3bb9d6b..d7d664a 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -22,7 +22,7 @@ static int deleterefs;
static const char *receivepack;
static int verbosity;
static int progress = -1;
-static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int recurse_submodules;
static enum transport_family family;
static struct push_cas_option cas;
@@ -31,6 +31,14 @@ static const char **refspec;
static int refspec_nr;
static int refspec_alloc;
+static void preset_submodule_default(void)
+{
+ if (file_exists(".gitmodules"))
+ recurse_submodules = RECURSE_SUBMODULES_CHECK;
+ else
+ recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+}
+
static void add_refspec(const char *ref)
{
refspec_nr++;
@@ -552,6 +560,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
};
packet_trace_identity("push");
+ preset_submodule_default();
git_config(git_push_config, &flags);
argc = parse_options(argc, argv, prefix, options, push_usage, 0);
set_push_cert_flags(&flags, push_cert);
--
2.10.0.129.g35f6318
^ permalink raw reply related
* Re: Slow pushes on 'pu' - even when up-to-date..
From: Jeff King @ 2016-10-04 16:21 UTC (permalink / raw)
To: Junio C Hamano
Cc: Heiko Voigt, Linus Torvalds, Stefan Beller, Git Mailing List
In-Reply-To: <xmqq8tu4w0i2.fsf@gitster.mtv.corp.google.com>
On Tue, Oct 04, 2016 at 09:19:01AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > As I argued in [1], I think it's not just "this must be cheaper" but
> > "this must not be enabled if submodules are not in use at all". Most
> > repositories don't have submodules enabled at all, so anything that
> > cause any extra traversal, even of a portion of the history, is going to
> > be a net negative for a lot of people.
> >
> > I think the only sane default is going to be some kind of heuristic that
> > says "submodules are probably in use".
>
> Why should we even have a default different from today's? If most
> repositories don't have submodules enabled at all, we can just let
> those working with submodules enabled to toggle their configuration
> and that is an very easy to understand solution, no?
You will not see any complaint from me on that. I was taking for granted
that the current default is inconvenient to submodule users, but I don't
have any experience myself.
-Peff
^ permalink raw reply
* Re: Slow pushes on 'pu' - even when up-to-date..
From: Junio C Hamano @ 2016-10-04 16:19 UTC (permalink / raw)
To: Jeff King; +Cc: Heiko Voigt, Linus Torvalds, Stefan Beller, Git Mailing List
In-Reply-To: <20161004114428.4wyq54afd4td3epp@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> As I argued in [1], I think it's not just "this must be cheaper" but
> "this must not be enabled if submodules are not in use at all". Most
> repositories don't have submodules enabled at all, so anything that
> cause any extra traversal, even of a portion of the history, is going to
> be a net negative for a lot of people.
>
> I think the only sane default is going to be some kind of heuristic that
> says "submodules are probably in use".
Why should we even have a default different from today's? If most
repositories don't have submodules enabled at all, we can just let
those working with submodules enabled to toggle their configuration
and that is an very easy to understand solution, no?
^ permalink raw reply
* Re: [PATCH 1/3] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"
From: Junio C Hamano @ 2016-10-04 16:15 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8D28iq3r3O_uzjyyJT--KQunAySRgUthF3FMrb1VM6XKw@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> We don't use it internally _yet_. I need to go through all the
> external diff code and see --shift-ita should be there. The end goal
> is still changing the default behavior and getting rid of --shift-ita,
I do not agree with that endgame, and quite honestly I do not want
to waste time reviewing such a series.
^ permalink raw reply
* Re: color.diff.whitespace unused on removed lines
From: Jeff King @ 2016-10-04 16:13 UTC (permalink / raw)
To: Sandro Santilli; +Cc: git
In-Reply-To: <20161004153523.GA2798@localhost>
On Tue, Oct 04, 2016 at 05:35:23PM +0200, Sandro Santilli wrote:
> > We later did b8767f7 (diff.c: --ws-error-highlight=<kind> option,
> > 2015-05-26) to let you see them on other lines, though. I think that
> > would do what you want.
>
> Thanks, it does do what I want.
> Any chance to specify it in the config file that I want it
> always to behave in a certain way ?
No, I don't think there's currently a matching config option. You can
use an alias, or propose a patch to add a config option.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2016, #08; Tue, 27)
From: Junio C Hamano @ 2016-10-04 16:11 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kY+yB-AxRUt0rArGw6DaLDsMZDjKt2jQh9B=P-79T+qYQ@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> So how would we go about git_all_attrs then?
I think you arrived the same conclusion, but the use of
git_attr_check_elem[] in the original implementation of
git_all_attrs() does not translate to the use of struct
git_attr_check in the new world order we have been discussing. The
latter is about "here are the attributes I want to know about",
i.e. the question being asked and then we would need to separate out
the answer from it to prepare for the threaded future.
The question git_all_attrs() asks is quite different. The caller
does not even know what set of attributes it is interested in, so if
you ever want to allow more than one callers pass the same callsite
to the function at the same time, they need their own copy of both
<attr>s and <value>s. I'd expect they'd ask about a path and then
receive an array of (<attr>,<value>). This is essentially what
happens in today's "array of git_attr_check_elem[]" interface.
That is fundamentally different from the normal use of
git_check_attr(), where <attr>s are part of questions being asked.
> int git_all_attrs(const char *path, char *result_keys[], char *result_values[],
> int nr, int alloc)
Or wrap each (<attr>,<value>) in a struct and return an array of it.
I do not see a reason why the above cannot be
struct { const struct git_attr *a; const char *v; }
**git_all_attr(const char *path);
which returns an array of struct's with an sentinel element at the
end (e.g. the 'a' field set to NULL or something).
^ permalink raw reply
* Re: Regression: git no longer works with musl libc's regex impl
From: Rich Felker @ 2016-10-04 16:11 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, git, musl
In-Reply-To: <alpine.DEB.2.20.1610041802310.35196@virtualbox>
On Tue, Oct 04, 2016 at 06:08:33PM +0200, Johannes Schindelin wrote:
> Hi Rich,
>
> On Tue, 4 Oct 2016, Rich Felker wrote:
>
> > On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:
> > > On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:
> > >
> > > > 1. is nonzero mod page size, it just works; the remainder of the last
> > > > page reads as zero bytes when mmapped.
> > >
> > > Is that a portable assumption?
> >
> > Yes.
>
> No, it is not. You quote POSIX, but the matter of the fact is that we use
> a subset of POSIX in order to be able to keep things running on Windows.
>
> And quite honestly, there are lots of reasons to keep things running on
> Windows, and even to favor Windows support over musl support. Over four
> million reasons: the Git for Windows users.
I would hope that in the future, git-for-windows users will be using
musl, via midipix, rather than the painfully slow and awful version
they're stuck with now...
Rich
^ permalink raw reply
* Re: Regression: git no longer works with musl libc's regex impl
From: Johannes Schindelin @ 2016-10-04 16:08 UTC (permalink / raw)
To: Rich Felker; +Cc: Jeff King, git, musl
In-Reply-To: <20161004154045.GT19318@brightrain.aerifal.cx>
Hi Rich,
On Tue, 4 Oct 2016, Rich Felker wrote:
> On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:
> > On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:
> >
> > > 1. is nonzero mod page size, it just works; the remainder of the last
> > > page reads as zero bytes when mmapped.
> >
> > Is that a portable assumption?
>
> Yes.
No, it is not. You quote POSIX, but the matter of the fact is that we use
a subset of POSIX in order to be able to keep things running on Windows.
And quite honestly, there are lots of reasons to keep things running on
Windows, and even to favor Windows support over musl support. Over four
million reasons: the Git for Windows users.
So rather than getting into an ideological discussion about "broken"
systems, it would be good to keep things practical, realizing that those
users make up a very real chunk of all of Git's users.
As to making NO_REGEX conditional on REG_STARTEND: you are talking about
apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND
is a C preprocessor macro.
Unless you can convince the rest of the Git developers (you would not
convince me) to simulate autoconf by compiling an executable every time
`make` is run, to determine whether REG_STARTEND is defined, this is a
no-go.
However, you *can* use autoconf directly, and come up with a patch to our
configure.ac that detects the absence of REG_STARTEND and sets NO_REGEX=1.
Alternatively, you can set NO_REGEX=1 in your config.mak.
Or, if you use one of the auto-detected cases in config.mak.uname, you
could patch it to set NO_REGEX=1.
And lastly, the best alternative would be to teach musl about
REG_STARTEND, as it is rather useful a feature.
Ciao,
Johannes
^ permalink raw reply
* Re: Regression: git no longer works with musl libc's regex impl
From: Johannes Schindelin @ 2016-10-04 16:01 UTC (permalink / raw)
To: Jeff King; +Cc: Rich Felker, git, musl
In-Reply-To: <20161004152722.ex2nox43oj5ak4yi@sigill.intra.peff.net>
Hi,
On Tue, 4 Oct 2016, Jeff King wrote:
> On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:
>
> > 1. is nonzero mod page size, it just works; the remainder of the last
> > page reads as zero bytes when mmapped.
>
> Is that a portable assumption?
No.
Ciao,
Dscho
^ permalink raw reply
* Re: Bug Report: "git submodule deinit" fails right after a clone
From: Thomas Bétous @ 2016-10-04 15:46 UTC (permalink / raw)
To: Heiko Voigt; +Cc: git
In-Reply-To: <20160914202907.GD7613@sandbox>
Thank you for your answer and sorry for the delay (I was on vacation...).
I am using git 2.9.0.windows.1 (run on Windows 7 via git bash).
I tested it on this repo:
https://github.com/githubtraining/example-dependency.git
The same problem occurs.
Here a small script to reproduce the error on my PC:
#!/bin/bash
git clone https://github.com/githubtraining/example-dependency.git
cd example-dependency
git submodule deinit js
It ends with this error:
fatal: Please stage your changes to .gitmodules or stash them to proceed
Submodule work tree 'js' contains local modifications; use '-f' to discard them
Is the script working on your PC?
Thank you in advance.
Thomas
On Wed, Sep 14, 2016 at 10:29 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> On Tue, Aug 30, 2016 at 01:45:56PM +0200, Thomas Bétous wrote:
>> Are you able to reproduce this problem?
>
> No. I just did a clone and an immediate deinit afterwards and no error.
> Maybe you can provide a script to reproduce? Which System was this on?
>
> Cheers Heiko
^ permalink raw reply
* Re: Regression: git no longer works with musl libc's regex impl
From: Rich Felker @ 2016-10-04 15:40 UTC (permalink / raw)
To: Jeff King; +Cc: git, musl
In-Reply-To: <20161004152722.ex2nox43oj5ak4yi@sigill.intra.peff.net>
On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:
> On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:
>
> > This commit broke support for using git with musl libc:
> >
> > https://github.com/git/git/commit/2f8952250a84313b74f96abb7b035874854cf202
>
> Yep. The idea is that you would compile git with NO_REGEX=1, and it
> would use the included compat routines.
This is really obnoxious to have to do manually. Why can't it simply
be auto-detected based on non-definition of REG_STARDEND?
> Is there something in particular you want to get out of using musl's
> regex that is not supported in the compat library?
It's always nice not to link extra implementations of the same thing.
This comes up all the time with gratuitous gnulib replacement
functions too.
> > Rather than depending on non-portable GNU regex extensions, there is a
> > simple portable fix for the issue this code was added to work around:
> > When a text file is being mmapped for use with string functions which
> > depend on null termination, if the file size:
> >
> > 1. is nonzero mod page size, it just works; the remainder of the last
> > page reads as zero bytes when mmapped.
>
> Is that a portable assumption?
Yes. Per POSIX:
"The system shall always zero-fill any partial page at the end of an
object. Further, the system shall never write out any modified
portions of the last page of an object which are beyond its end.
References within the address range starting at pa and continuing for
len bytes to whole pages following the end of an object shall result
in delivery of a SIGBUS signal."
Source: http://pubs.opengroup.org/onlinepubs/9699919799/functions/mmap.html
The same or similar text appears at least back to SUSv2; I did not
look further back.
> > 2. if an exact multiple of the page size, then instead of directly
> > mmapping the file, first mmap a mapping 1 byte (thus 1 page) larger
> > with MAP_ANON, then use MAP_FIXED to map the file over top of all
> > but the last page. Now the mmapped buffer can safely be used as a C
> > string.
>
> I'm not sure whether all of our compat layers for mmap would be happy
> with that (e.g., see compat/win32mmap.c).
The nice thing about this approach is that if the MAP_FIXED fails due
to a broken system you can just read() into the anonymous memory.
> So it seems like any mmap-related solutions would have to be
> conditional, too. And then regexec_buf() would have to become something
> like:
>
> int regexec_buf(...)
> {
> #if defined(REG_STARTEND)
> ... set up match ...
> return regexec(..., REG_STARTEND);
> #elif defined(MMAP_ALWAYS_HAS_NUL)
> /*
> * We assume that every buffer we see is always NUL-terminated
> * eventually, either because it comes from xmallocz() or our
> * mmap layer always ensures an extra NUL.
> */
> return regexec(...);
> #else
> #error "Nope, you need either NO_REGEX or USE_MMAP_NUL"
> #endif
> }
>
> The assumption in the middle case feels pretty hacky, though. It fails
I agree. I would prefer just falling back to read() after MAP_ANON if
the MAP_FIXED fails. This would work for all systems.
> if we get a buffer from somewhere besides those two sources. It fails if
> somebody calls regexec_buf() on a subset of a string.
>
> It also doesn't handle matching past embedded NULs in the string. That's
> not something we're relying on yet, but it would be nice to support
> consistently in the long run.
This is going to be even more non-portable and
implementation-specific. There's not even a good spec for how embedded
nuls should be treated in regex.
> If there's a compelling reason, it might be worth making that tradeoff.
> But I am not sure what the compelling reason is to use musl's regex
> (aside from the obvious of "less code in the resulting executable").
The compelling reason is just being portable by default rather than
requiring manual overrides to use a replacement library with weird
extensions for something that could have been done portably to begin
with.
Rich
^ permalink raw reply
* Re: color.diff.whitespace unused on removed lines
From: Sandro Santilli @ 2016-10-04 15:35 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161004152954.74bojbyagxr2xefz@sigill.intra.peff.net>
On Tue, Oct 04, 2016 at 11:29:55AM -0400, Jeff King wrote:
> On Tue, Oct 04, 2016 at 10:14:29AM +0200, Sandro Santilli wrote:
>
> > The color.diff.whitespace configuration is not used on
> > removed lines, but only on added lines.
>
> Right. The original purpose was to warn you when you were introducing
> whitespace breakages. Getting rid of other people's whitespace breakages
> is OK.
>
> We later did b8767f7 (diff.c: --ws-error-highlight=<kind> option,
> 2015-05-26) to let you see them on other lines, though. I think that
> would do what you want.
Thanks, it does do what I want.
Any chance to specify it in the config file that I want it
always to behave in a certain way ?
--strk;
^ permalink raw reply
* Re: color.diff.whitespace unused on removed lines
From: Jeff King @ 2016-10-04 15:29 UTC (permalink / raw)
To: Sandro Santilli; +Cc: git
In-Reply-To: <20161004081429.GC17002@localhost>
On Tue, Oct 04, 2016 at 10:14:29AM +0200, Sandro Santilli wrote:
> The color.diff.whitespace configuration is not used on
> removed lines, but only on added lines.
Right. The original purpose was to warn you when you were introducing
whitespace breakages. Getting rid of other people's whitespace breakages
is OK.
We later did b8767f7 (diff.c: --ws-error-highlight=<kind> option,
2015-05-26) to let you see them on other lines, though. I think that
would do what you want.
-Peff
^ permalink raw reply
* Re: Regression: git no longer works with musl libc's regex impl
From: Jeff King @ 2016-10-04 15:27 UTC (permalink / raw)
To: Rich Felker; +Cc: git, musl
In-Reply-To: <20161004150848.GA7949@brightrain.aerifal.cx>
On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:
> This commit broke support for using git with musl libc:
>
> https://github.com/git/git/commit/2f8952250a84313b74f96abb7b035874854cf202
Yep. The idea is that you would compile git with NO_REGEX=1, and it
would use the included compat routines.
Is there something in particular you want to get out of using musl's
regex that is not supported in the compat library?
> Rather than depending on non-portable GNU regex extensions, there is a
> simple portable fix for the issue this code was added to work around:
> When a text file is being mmapped for use with string functions which
> depend on null termination, if the file size:
>
> 1. is nonzero mod page size, it just works; the remainder of the last
> page reads as zero bytes when mmapped.
Is that a portable assumption?
> 2. if an exact multiple of the page size, then instead of directly
> mmapping the file, first mmap a mapping 1 byte (thus 1 page) larger
> with MAP_ANON, then use MAP_FIXED to map the file over top of all
> but the last page. Now the mmapped buffer can safely be used as a C
> string.
I'm not sure whether all of our compat layers for mmap would be happy
with that (e.g., see compat/win32mmap.c).
So it seems like any mmap-related solutions would have to be
conditional, too. And then regexec_buf() would have to become something
like:
int regexec_buf(...)
{
#if defined(REG_STARTEND)
... set up match ...
return regexec(..., REG_STARTEND);
#elif defined(MMAP_ALWAYS_HAS_NUL)
/*
* We assume that every buffer we see is always NUL-terminated
* eventually, either because it comes from xmallocz() or our
* mmap layer always ensures an extra NUL.
*/
return regexec(...);
#else
#error "Nope, you need either NO_REGEX or USE_MMAP_NUL"
#endif
}
The assumption in the middle case feels pretty hacky, though. It fails
if we get a buffer from somewhere besides those two sources. It fails if
somebody calls regexec_buf() on a subset of a string.
It also doesn't handle matching past embedded NULs in the string. That's
not something we're relying on yet, but it would be nice to support
consistently in the long run.
If there's a compelling reason, it might be worth making that tradeoff.
But I am not sure what the compelling reason is to use musl's regex
(aside from the obvious of "less code in the resulting executable").
-Peff
^ permalink raw reply
* Regression: git no longer works with musl libc's regex impl
From: Rich Felker @ 2016-10-04 15:08 UTC (permalink / raw)
To: git; +Cc: musl
This commit broke support for using git with musl libc:
https://github.com/git/git/commit/2f8952250a84313b74f96abb7b035874854cf202
Rather than depending on non-portable GNU regex extensions, there is a
simple portable fix for the issue this code was added to work around:
When a text file is being mmapped for use with string functions which
depend on null termination, if the file size:
1. is nonzero mod page size, it just works; the remainder of the last
page reads as zero bytes when mmapped.
2. if an exact multiple of the page size, then instead of directly
mmapping the file, first mmap a mapping 1 byte (thus 1 page) larger
with MAP_ANON, then use MAP_FIXED to map the file over top of all
but the last page. Now the mmapped buffer can safely be used as a C
string.
If such a solution is acceptable I can try to prepare a patch.
Rich
^ permalink raw reply
* [PATCH v2] http: http.emptyauth should allow empty (not just NULL) usernames
From: David Turner @ 2016-10-04 14:53 UTC (permalink / raw)
To: git, sandals; +Cc: David Turner
When using Kerberos authentication with newer versions of libcurl,
CURLOPT_USERPWD must be set to a value, even if it is an empty value.
The value is never sent to the server. Previous versions of libcurl
did not require this variable to be set. One way that some users
express the empty username/password is http://:@gitserver.example.com,
which http.emptyauth was designed to support. Another, equivalent,
URL is http://@gitserver.example.com. The latter leads to a username
of zero-length, rather than a NULL username, but CURLOPT_USERPWD still
needs to be set (if http.emptyauth is set). Do so.
Signed-off-by: David Turner <dturner@twosigma.com>
---
Same patch, different message.
---
http.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/http.c b/http.c
index 82ed542..bd0dba2 100644
--- a/http.c
+++ b/http.c
@@ -351,7 +351,7 @@ static int http_options(const char *var, const char *value, void *cb)
static void init_curl_http_auth(CURL *result)
{
- if (!http_auth.username) {
+ if (!http_auth.username || !*http_auth.username) {
if (curl_empty_auth)
curl_easy_setopt(result, CURLOPT_USERPWD, ":");
return;
--
2.8.0.rc4.22.g8ae061a
^ permalink raw reply related
* Re: [PATCH 18/18] alternates: use fspathcmp to detect duplicates
From: Jeff King @ 2016-10-04 14:10 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list, René Scharfe
In-Reply-To: <CA+P7+xqhuYmp-H=b-SrNdZjN5urWGHPuNkWbeVgCBF1UuhQZKQ@mail.gmail.com>
On Mon, Oct 03, 2016 at 11:51:59PM -0700, Jacob Keller wrote:
> On Mon, Oct 3, 2016 at 1:36 PM, Jeff King <peff@peff.net> wrote:
> > On a case-insensitive filesystem, we should realize that
> > "a/objects" and "A/objects" are the same path. We already
> > use fspathcmp() to check against the main object directory,
> > but until recently we couldn't use it for comparing against
> > other alternates (because their paths were not
> > NUL-terminated strings). But now we can, so let's do so.
> >
>
> Yep, makes sense.
>
> > Note that we also need to adjust count-objects to load the
> > config, so that it can see the setting of core.ignorecase
> > (this is required by the test, but is also a general bugfix
> > for users of count-objects).
>
> Also makes sense.
BTW, I tested this on a vfat loopback device, but I was surprised to see
that quite a few other tests failed on that device.
At least one of the problems is that symlinks are not supported, but
lib-httpd.sh wants to use them for its Apache setup. I guess people on
Windows just don't run the httpd tests at all, which is not too
surprising.
Likewise, credential-cache fails because it cannot create a Unix socket
(and the flag for that is in the build, not a run-time filesystem
check).
Some of the other failures seemed to be due to lack of an executable bit
on the filesystem. I'm not sure if we could or should do better run-time
detection of that sort of thing. I think some of the checks are tied to
the build, and that's generally good enough in practice because people
don't use vfat on their Linux machines. So tracking down each of them
may just be pedantic make-work that nobody cares about.
I did wonder if there was another good filesystem to use for
case-insensitive experiments on Linux. At the time I didn't think there
was good support for making HFS+ filesystems, but it looks Debian cares
mkfs.hfs. That's probably a better choice for such experiments.
-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