* Re: [musl] Re: Regression: git no longer works with musl libc's regex impl
From: Rich Felker @ 2016-10-05 16:15 UTC (permalink / raw)
To: Jakub Narębski; +Cc: musl, James B, Johannes Schindelin, Jeff King, git
In-Reply-To: <bc3da1a4-4b99-737f-050e-54ef5844c402@gmail.com>
On Wed, Oct 05, 2016 at 03:11:05PM +0200, Jakub Narębski wrote:
> W dniu 05.10.2016 o 00:33, Rich Felker pisze:
> > On Wed, Oct 05, 2016 at 09:06:25AM +1100, James B wrote:
> >> On Tue, 4 Oct 2016 18:08:33 +0200 (CEST)
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >>>
> >>> 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.
> >>
> >> Wow, I don't know that Windows is a git's first-tier platform now,
> >> and Linux/POSIX second. Are we talking about the same git that was
> >> originally written in Linus Torvalds, and is used to manage Linux
> >> kernel? Are you by any chance employed by Redmond, directly or
> >> indirectly?
> >>
> >> Sorry - can't help it.
>
> Windows is one of the major platforms, yes. I think there much, much
> more people using Git on Windows, than using Git with musl. More
> users = more important.
>
> Also, working with some inconvenience (requiring compilation with
> NO_REGEX=1) is better than not working at all.
>
> In CodingGuidelines we say:
>
> - Most importantly, we never say "It's in POSIX; we'll happily
> ignore your needs should your system not conform to it."
> We live in the real world.
>
> - However, we often say "Let's stay away from that construct,
> it's not even in POSIX".
I agree wholeheartedly with these points.
>
> - In spite of the above two rules, we sometimes say "Although
> this is not in POSIX, it (is so convenient | makes the code
> much more readable | has other good characteristics) and
> practically all the platforms we care about support it, so
> let's use it".
>
> The REG_STARTEND is 3rd point,
To begin with I wasn't clear that REG_STARDEND being nonstandard was
even noticed or compatibility considered when adding the dependency on
it, but it seems such discussion did take place and most targets have
it. Perhaps this means it should be proposed for standardization in
the next issue of POSIX.
> mmap shenningans looks like 1st...
>
> ....on the other hand midipix <writeonce@midipix.org> wrote in
> http://public-inbox.org/git/20161004200057.dc30d64f61e5ec441c34ffd4f788e58e.efa66ead67.wbe@email15.godaddy.com/
> that the proposed fix should work on all Windows version we are
> interested in (I think). Test program included / attached.
>
> The above-mentioned email also explains that the problem was
> caught on MS Windows; it triggers if file end falls on the mmapped
> page boundary, which is more likely to happen with 4096 mod size
> on Windows rather than 65536 mod size on Linux.
On Linux page-size (mmap granularity) varies by arch but it's 4k on
basically all archs that people care about. I think midipix's author
was talking about real page size on Windows (4k) vs the minimum
logical page size (mmap granularity) that can be used to get
POSIX-matching semantics in midipix (which is 64k due to some
technical reasons I forget, which he could probably remind me of).
> On the other hand, while the proposed solution of "add padding as
> to not end at page boundary, if necessary" doesn't have the
> performance impact of "memcpy into NUL-terminated buffer" that
> was originally proposed in patch series, it is still extra code
> to maintain.
*nod*
Rich
^ permalink raw reply
* Re: [PATCH 1/3] add QSORT
From: Kevin Bracey @ 2016-10-05 15:00 UTC (permalink / raw)
To: René Scharfe; +Cc: GIT Mailing-list
In-Reply-To: <29d3dde0-c527-3ab8-914c-6fbdc5e81e1c@web.de>
On 04/10/2016 23:31, René Scharfe wrote:
>
> So let's summarize; here's the effect of a raw qsort(3) call:
>
> array == NULL nmemb bug QSORT following NULL check
> ------------- ----- --- ----- --------------------
> 0 0 no qsort is skipped
> 0 >0 no qsort is skipped
> 1 0 no qsort is skipped (bad!) ******
> 1 >0 yes qsort is skipped ******
>
Right - row 3 may not be a bug from the point of view of your internals,
but it means you violate the API of qsort.Therefore a fix is required.
> With the micro-optimization removed (nmemb > 0) the matrix gets simpler:
>
> array == NULL nmemb bug QSORT following NULL check
> ------------- ----- --- ----- --------------------
> 0 0 no noop is executed
> 0 >0 no qsort is skipped
> 1 0 no noop is executed
> 1 >0 yes qsort is skipped ******
>
> And with your NULL check (array != NULL) we'd get:
>
> array == NULL nmemb bug QSORT following NULL check
> ------------- ----- --- ----- --------------------
> 0 0 no qsort reuses check result
> 0 >0 no qsort reuses check result
> 1 0 no noop reuses check result
> 1 >0 yes noop reuses check result
>
> Did I get it right? AFAICS all variants (except raw qsort) are safe
> -- no useful NULL checks are removed, and buggy code should be noticed
> by segfaults in code accessing the sorted array.
I think your tables are correct.
But I disagree that you could ever call invoking the "****" lines safe.
Unless you have documentation on what limit GCC (and your other
compilers) are prepared to put on the undefined behaviour of violating
that "non-null" constraint.
Up to now dereferencing a null pointer has been implicitly (or
explicitly?) defined as simply generating SIGSEGV. And that has
naturally extended into NULL passed to library implementations. But
that's no longer true - it seems bets are somewhat off.
But, as long as you are confident you never invoke that line without a
program bug - ie an API precondition of your own QSORT is that NULL is
legal iff nmemb is zero, then I guess it's fine. Behaviour is defined,
as long as you don't violate your internal preconditions.
Kevin
^ permalink raw reply
* Re: [PATCH 06/18] t5613: clarify "too deep" recursion tests
From: Junio C Hamano @ 2016-10-05 16:14 UTC (permalink / raw)
To: Jeff King
Cc: Jakub Narębski, Stefan Beller, Jacob Keller,
Git mailing list, René Scharfe
In-Reply-To: <20161005144028.tjuvk3hkoqm3qjfd@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Oct 05, 2016 at 03:58:53PM +0200, Jakub Narębski wrote:
>
>> I would prefer the following:
>>
>> # A --> B --> C --> D --> E --> F --> G --> H
>> # 0 1 2 3 4 5 6
>
> Yeah, that is also more visually pleasing.
>
> Here's a squashable update that uses that and clarifies the points in
> the discussion with Jacob.
>
> Junio, do you mind squashing this in to jk/alt-odb-cleanup?
No, I don't.
> diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
> index b393613..62170b7 100755
> --- a/t/t5613-info-alternate.sh
> +++ b/t/t5613-info-alternate.sh
> @@ -39,13 +39,16 @@ test_expect_success 'preparing third repository' '
> )
> '
>
> -# Note: These tests depend on the hard-coded value of 5 as "too deep". We start
> -# the depth at 0 and count links, not repositories, so in a chain like:
> +# Note: These tests depend on the hard-coded value of 5 as the maximum depth
> +# we will follow recursion. We start the depth at 0 and count links, not
> +# repositories. This means that in a chain like:
> #
> -# A -> B -> C -> D -> E -> F -> G -> H
> -# 0 1 2 3 4 5 6
> +# A --> B --> C --> D --> E --> F --> G --> H
> +# 0 1 2 3 4 5 6
> #
> -# we are OK at "G", but break at "H".
> +# we are OK at "G", but break at "H", even though "H" is actually the 8th
> +# repository, not the 6th, which you might expect. Counting the links allows
> +# N+1 repositories, and counting from 0 to 5 inclusive allows 6 links.
> #
> # Note also that we must use "--bare -l" to make the link to H. The "-l"
> # ensures we do not do a connectivity check, and the "--bare" makes sure
> @@ -59,11 +62,11 @@ test_expect_success 'creating too deep nesting' '
> git clone --bare -l -s G H
> '
>
> -test_expect_success 'validity of fifth-deep repository' '
> +test_expect_success 'validity of seventh repository' '
> git -C G fsck
> '
>
> -test_expect_success 'invalidity of sixth-deep repository' '
> +test_expect_success 'invalidity of eighth repository' '
> test_must_fail git -C H fsck
> '
>
^ permalink raw reply
* Re: Reference a submodule branch instead of a commit
From: Junio C Hamano @ 2016-10-05 16:13 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Stefan Beller, Jeremy Morton, git@vger.kernel.org
In-Reply-To: <20161005141439.GD30930@book.hvoigt.net>
Heiko Voigt <hvoigt@hvoigt.net> writes:
>> It IS a hack, but having this information in .git<something> would
>> mean that it can be forced to be in machine readable form, unlike a
>> mention in README. I do not know if the .gitmodules/.gitignore
>> combination is a sensible thing to use, but it does smell like a
>> potentially useful hack.
>
> IIRC the tree entries are the reference for submodules in the code. We
> are iterating over the tree entries in many places so that change does
> not seem so easy to me.
>
> But you are right maybe we should stop arguing against this workflow and
> just let people use it until they find out whats wrong with it ;)
I didn't say that, though. I am fairly firm on _not_ changing what
the superproject records in its tree for the submodule, i.e. it must
record the exact commit, not "a branch name", for reproducibility.
I am OK if people ignored the unmatch between the recorded commit
from a submodule and what they had in the submodule directory while
they developed and tested the superproject commit. After all, it is
not an error to make a commit while having a local uncommitted
changes to tracked files, and it is equally valid to have a commit
checked out in a submodule directory that is different from what
goes in the superproject commit. But we do show "modified but not
committed" in the status output. In that light, submodule.*.ignore
may have been a mistake.
^ permalink raw reply
* Re: [musl] Re: Regression: git no longer works with musl libc's regex impl
From: Jeff King @ 2016-10-05 16:11 UTC (permalink / raw)
To: James B; +Cc: Johannes Schindelin, musl, Rich Felker, git
In-Reply-To: <20161005225934.770d73b7d491d4bf4816411d@gmail.com>
On Wed, Oct 05, 2016 at 10:59:34PM +1100, James B wrote:
> Number downloads does not make first-tier platform. You know that as
> well as everyone else.
>
> First-tier support is the decision made by the maintainers that the
> entire features of the software must be available on those first tier
> platforms. So if Windows is indeed first-tier platform for git, it
> means any features that don't work on git version of Windows must not
> be used/developed or even castrated. That's a scary thought.
Prepare to be scared, then, I guess. Ever since the msysgit project
started years ago, we have made concessions in the code to work both
with POSIX-ish systems and with the msys layer. E.g., see how git-daemon
does not fork(), but actually re-spawns itself to handle connections.
When possible we try to put our abstractions at a level where they can
be implemented in a performant way on all platforms (the git-daemon
things is probably the _most_ ugly in that respect; I think nobody has
really cared about the performance enough to add back in a forking code
path for POSIX systems).
> So this decision that "Windows is now a first-tier platform for git" -
> is your own opinion, or is this the collective opinion of *all* the
> git maintainers?
There is only one maintainer of git: Junio. However, you'll note that I
also used "we" in the paragraphs above. And that is because the approach
I am talking about is something that has been done over the course of
many years by many members of the development community.
You may disagree with that approach, but it is nothing new. The msysgit
project started in 2007.
> Well thank you for being honest. I can see now why you responded the
> way you did (and still do). By being employed by Microsoft, and
> especially paid to work on Git for Windows, you have all the
> incentives to make it work best on Windows, and to make it as its
> first-tier platform within the limitation of Windows.
Please don't insinuate that Johannes is a Microsoft shill. He has been
working on the Windows port of Git for over 9 years, and was only
employed by Microsoft this year. Furthermore, his original REG_STARTEND
patch actually did a run-time fallback of NUL-terminating the input
buffers. It was _I_ who suggested that we should simply push people
towards our compat/regex routines instead. So if you want to be mad at
somebody, be mad at me.
-Peff
^ permalink raw reply
* Re: Feature Request: user defined suffix for temp files created by git-mergetool
From: Junio C Hamano @ 2016-10-05 16:05 UTC (permalink / raw)
To: Josef Ridky; +Cc: git
In-Reply-To: <1499287628.1324571.1475653631366.JavaMail.zimbra@redhat.com>
Josef Ridky <jridky@redhat.com> writes:
> Hi,
>
> I have just realize, that my attachment has been cut off from my previous message.
> Below you can find patch with requested change.
>
> Add support for user defined suffix part of name of temporary files
> created by git mergetool
> ---
The first two paragraphs above do not look like they are meant for
the commit log for this change.
Please sign-off your patch.
> -USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'
> +USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [--local=name] [--remote=name] [--backup=name] [--base=name] [file to merge] ...'
> SUBDIRECTORY_OK=Yes
> NONGIT_OK=Yes
> OPTIONS_SPEC=
> TOOL_MODE=merge
> +
> +#optional name space convention
> +local_name=""
> +remote_name=""
> +base_name=""
> +backup_name=""
If you initialize these to LOCAL, REMOTE, etc. instead of empty,
wouldn't it make the remainder of the code a lot simpler? For
example,
> + if [ "$local_name" != "" ] && [ "$local_name" != "$remote_name" ] && [ "$local_name" != "$backup_name" ] && [ "$local_name" != "$base_name" ]
> + then
> + LOCAL="$MERGETOOL_TMPDIR/${BASE}_${local_name}_$$$ext"
> + else
> + LOCAL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext"
> + fi
This can just be made an unconditional
LOCAL="$MERGETOOL_TMPDIR/${BASE}_${local_name}_$$$ext"
without any "if" check in front. The same for all others.
The conditional you added is doing two unrelated things. It is
trying to switch between an unset $local_name and default LOCAL,
while it tries to make sure the user did not give the same string
for two different things (which is a nonsense). It is probably
better to check for nonsense just once just before all these
assuments of LOCAL, REMOTE, etc. begins, not at each point where
they are set like this patch does.
^ permalink raw reply
* Re: [PATCHv3 1/2] push: change submodule default to check when submodules exist
From: Stefan Beller @ 2016-10-05 15:54 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, git@vger.kernel.org, Heiko Voigt, Linus Torvalds
In-Reply-To: <20161005154715.qmmcwpkt2yudbc2d@sigill.intra.peff.net>
On Wed, Oct 5, 2016 at 8:47 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Oct 04, 2016 at 02:03:58PM -0700, Stefan Beller wrote:
>
>> thanks for the suggestions, both git_path(..) as well as checking the config,
>> this seems quite readable to me:
>>
>> builtin/push.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> Yeah, this seems like a good compromise to me.
>
> I did have one other thought, but I don't think it's worth pursuing now.
> We care about finding gitlinks in objects we're pushing; is there any
> process which is already looking at those objects? Genreally, the
> "pack-objects" doing the push has to. Not to actually push the objects,
> which often are sent blindly off disk, but to determine the set of
> reachable objects in the first place. So in theory we could prepare the
> list of objects to pack, and as a side effect it could say "and here are
> gitlinks referenced by those objects". But that doesn't work if bitmaps
> are in effect, because then we don't access the objects directly at all.
> I think you could solve that by extending the bitmap format to include a
> bit for gitlinks that are reachable (but not necessarily included in the
> pack).
>
> So I don't think that's worth thinking too much about now, but it might
> be an interesting optimization down the road.
>
> -Peff
That sounds like what we'd want down the road. In my imagination the
submodules of the future may live completely inside the superproject,
i.e. they do not necessarily have their own URL. They can be obtained
via fetching the superproject, that automagically also transmits the objects
of the submodules.
Thanks for the hint w.r.t. the bimaps!
Stefan
^ permalink raw reply
* Re: [PATCHv3 1/2] push: change submodule default to check when submodules exist
From: Jeff King @ 2016-10-05 15:47 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, hvoigt, torvalds
In-Reply-To: <20161004210359.15266-1-sbeller@google.com>
On Tue, Oct 04, 2016 at 02:03:58PM -0700, Stefan Beller wrote:
> thanks for the suggestions, both git_path(..) as well as checking the config,
> this seems quite readable to me:
>
> builtin/push.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
Yeah, this seems like a good compromise to me.
I did have one other thought, but I don't think it's worth pursuing now.
We care about finding gitlinks in objects we're pushing; is there any
process which is already looking at those objects? Genreally, the
"pack-objects" doing the push has to. Not to actually push the objects,
which often are sent blindly off disk, but to determine the set of
reachable objects in the first place. So in theory we could prepare the
list of objects to pack, and as a side effect it could say "and here are
gitlinks referenced by those objects". But that doesn't work if bitmaps
are in effect, because then we don't access the objects directly at all.
I think you could solve that by extending the bitmap format to include a
bit for gitlinks that are reachable (but not necessarily included in the
pack).
So I don't think that's worth thinking too much about now, but it might
be an interesting optimization down the road.
-Peff
^ permalink raw reply
* [PATCH 0/6] git-merge: a few documentation improvements
From: sorganov @ 2016-10-05 14:46 UTC (permalink / raw)
To: git; +Cc: gitster, Sergey Organov
From: Sergey Organov <sorganov@gmail.com>
Sergey Organov (6):
git-merge: clarify "usage" by adding "-m <msg>"
Documentation/git-merge.txt: remove list of options from SYNOPSIS
Documentation/git-merge.txt: fix SYNOPSIS of obsolete form to include
options
Documentation/git-merge.txt: improve short description in NAME
Documentation/git-merge.txt: improve short description in DESCRIPTION
Documentation/git-merge.txt: get rid of irrelevant references to
git-pull
Documentation/git-merge.txt | 63 ++++++++++++++++++++++-----------------------
builtin/merge.c | 2 +-
2 files changed, 32 insertions(+), 33 deletions(-)
--
2.10.0.1.g57b01a3
^ permalink raw reply
* [PATCH 4/6] Documentation/git-merge.txt: improve short description in NAME
From: sorganov @ 2016-10-05 14:46 UTC (permalink / raw)
To: git; +Cc: gitster, Sergey Organov
In-Reply-To: <cover.1475678515.git.sorganov@gmail.com>
From: Sergey Organov <sorganov@gmail.com>
Old description not only raised the question of why the tool is called
git-merge rather than git-join, but "join histories" also sounds like
very simple operation, something like what "git-merge -s ours" does.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
Documentation/git-merge.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 216d2f4..cc0329d 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -3,7 +3,8 @@ git-merge(1)
NAME
----
-git-merge - Join two or more development histories together
+
+git-merge - Merge one or more branches to the current branch
SYNOPSIS
--
2.10.0.1.g57b01a3
^ permalink raw reply related
* [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull
From: sorganov @ 2016-10-05 14:46 UTC (permalink / raw)
To: git; +Cc: gitster, Sergey Organov
In-Reply-To: <cover.1475678515.git.sorganov@gmail.com>
From: Sergey Organov <sorganov@gmail.com>
No awareness of git-pull is required to understand git-merge operation,
so leave reference to git-pull only where it actually makes sense, in
the description of fast-forward merges, and only as clarification of
when this merging behaviour is mostly useful.
Other references to git-pull are likely just a historical leftover
that are now neither required nor clarify anything. Besides, git-pull
may use rebase rather than merge, so it's also technically wrong to
say, unconditionally, that git-pull uses git-merge.
Overall, let git-pull description refer to git-merge where
appropriate, and not vice versa.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
Documentation/git-merge.txt | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 351b8fc..ba5fb0a 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -23,10 +23,6 @@ named commits and the current branch, called "merge base", is
calculated, and then net changes taken from the merge base to
the named commits are applied.
-This command is used by 'git pull' to incorporate changes from another
-repository, and can be used by hand to merge changes from one branch
-into another.
-
Assume the following history exists and the current branch is
"`master`":
@@ -119,18 +115,17 @@ of `git fetch` for merging are merged to the current branch.
PRE-MERGE CHECKS
----------------
-Before applying outside changes, you should get your own work in
-good shape and committed locally, so it will not be clobbered if
-there are conflicts. See also linkgit:git-stash[1].
-'git pull' and 'git merge' will stop without doing anything when
-local uncommitted changes overlap with files that 'git pull'/'git
-merge' may need to update.
+Before applying outside changes, you should get your own work in good
+shape and committed locally, so it will not be clobbered if there are
+conflicts. See also linkgit:git-stash[1]. 'git merge' will stop
+without doing anything when local uncommitted changes overlap with
+files that 'git merge' may need to update.
-To avoid recording unrelated changes in the merge commit,
-'git pull' and 'git merge' will also abort if there are any changes
-registered in the index relative to the `HEAD` commit. (One
-exception is when the changed index entries are in the state that
-would result from the merge already.)
+To avoid recording unrelated changes in the merge commit, 'git merge'
+will also abort if there are any changes registered in the index
+relative to the `HEAD` commit. (One exception is when the changed
+index entries are in the state that would result from the merge
+already.)
If all named commits are already ancestors of `HEAD`, 'git merge'
will exit early with the message "Already up-to-date."
@@ -138,14 +133,15 @@ will exit early with the message "Already up-to-date."
FAST-FORWARD MERGE
------------------
-Often the current branch head is an ancestor of the named commit.
+Often the current branch head is an ancestor of the named commit. In
+this case, a new commit is not needed to store the combined history;
+instead, the `HEAD` (along with the index) is updated to point at the
+named commit, without creating an extra merge commit.
+
This is the most common case especially when invoked from 'git
pull': you are tracking an upstream repository, you have committed
no local changes, and now you want to update to a newer upstream
-revision. In this case, a new commit is not needed to store the
-combined history; instead, the `HEAD` (along with the index) is
-updated to point at the named commit, without creating an extra
-merge commit.
+revision.
This behavior can be suppressed with the `--no-ff` option.
--
2.10.0.1.g57b01a3
^ permalink raw reply related
* [PATCH 1/6] git-merge: clarify "usage" by adding "-m <msg>"
From: sorganov @ 2016-10-05 14:46 UTC (permalink / raw)
To: git; +Cc: gitster, Sergey Organov
In-Reply-To: <cover.1475678515.git.sorganov@gmail.com>
From: Sergey Organov <sorganov@gmail.com>
"-m <msg>" is one of essential distinctions between obsolete
invocation form and the recent one. Add it to the "usage" returned by
'git merge -h' for more clarity.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
builtin/merge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index a8b57c7..0e367ba 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -43,7 +43,7 @@ struct strategy {
};
static const char * const builtin_merge_usage[] = {
- N_("git merge [<options>] [<commit>...]"),
+ N_("git merge [<options>] [-m <msg>] [<commit>...]"),
N_("git merge [<options>] <msg> HEAD <commit>"),
N_("git merge --abort"),
NULL
--
2.10.0.1.g57b01a3
^ permalink raw reply related
* [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION
From: sorganov @ 2016-10-05 14:46 UTC (permalink / raw)
To: git; +Cc: gitster, Sergey Organov
In-Reply-To: <cover.1475678515.git.sorganov@gmail.com>
From: Sergey Organov <sorganov@gmail.com>
Old description had a few problems:
- sounded as if commits have changes
- stated that changes are taken since some "divergence point"
that was not defined.
New description rather uses "common ancestor" and "merge base",
definitions of which are easily discoverable in the rest of GIT
documentation.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
Documentation/git-merge.txt | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index cc0329d..351b8fc 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -16,11 +16,16 @@ SYNOPSIS
DESCRIPTION
-----------
-Incorporates changes from the named commits (since the time their
-histories diverged from the current branch) into the current
-branch. This command is used by 'git pull' to incorporate changes
-from another repository and can be used by hand to merge changes
-from one branch into another.
+
+Incorporates changes that lead to the named commits into the current
+branch, and joins corresponding histories. The best common ancestor of
+named commits and the current branch, called "merge base", is
+calculated, and then net changes taken from the merge base to
+the named commits are applied.
+
+This command is used by 'git pull' to incorporate changes from another
+repository, and can be used by hand to merge changes from one branch
+into another.
Assume the following history exists and the current branch is
"`master`":
@@ -31,11 +36,11 @@ Assume the following history exists and the current branch is
D---E---F---G master
------------
-Then "`git merge topic`" will replay the changes made on the
-`topic` branch since it diverged from `master` (i.e., `E`) until
-its current commit (`C`) on top of `master`, and record the result
-in a new commit along with the names of the two parent commits and
-a log message from the user describing the changes.
+Then "`git merge topic`" will replay the changes made on the `topic`
+branch since it diverged from `master` (i.e., `E`) until its current
+commit (`C`) on top of `master`, and record the result in a new commit
+along with references to the two parent commits and a log message from
+the user describing the changes.
------------
A---B---C topic
--
2.10.0.1.g57b01a3
^ permalink raw reply related
* [PATCH 3/6] Documentation/git-merge.txt: fix SYNOPSIS of obsolete form to include options
From: sorganov @ 2016-10-05 14:46 UTC (permalink / raw)
To: git; +Cc: gitster, Sergey Organov
In-Reply-To: <cover.1475678515.git.sorganov@gmail.com>
From: Sergey Organov <sorganov@gmail.com>
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
Documentation/git-merge.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 90342eb..216d2f4 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[verse]
'git merge' [options] [-m <msg>] [<commit>...]
-'git merge' <msg> HEAD <commit>...
+'git merge' [options] <msg> HEAD <commit>...
'git merge' --abort
DESCRIPTION
--
2.10.0.1.g57b01a3
^ permalink raw reply related
* [PATCH 2/6] Documentation/git-merge.txt: remove list of options from SYNOPSIS
From: sorganov @ 2016-10-05 14:46 UTC (permalink / raw)
To: git; +Cc: gitster, Sergey Organov
In-Reply-To: <cover.1475678515.git.sorganov@gmail.com>
From: Sergey Organov <sorganov@gmail.com>
This partial list of option is confusing as it lacks a lot of
available options. It also clutters the SYNOPSIS making differences
between forms of invocation less clear.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
Documentation/git-merge.txt | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d55..90342eb 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -9,10 +9,7 @@ git-merge - Join two or more development histories together
SYNOPSIS
--------
[verse]
-'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
- [-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
- [--[no-]allow-unrelated-histories]
- [--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
+'git merge' [options] [-m <msg>] [<commit>...]
'git merge' <msg> HEAD <commit>...
'git merge' --abort
--
2.10.0.1.g57b01a3
^ permalink raw reply related
* Re: [PATCH 06/18] t5613: clarify "too deep" recursion tests
From: Jeff King @ 2016-10-05 14:40 UTC (permalink / raw)
To: Jakub Narębski
Cc: Junio C Hamano, Stefan Beller, Jacob Keller, Git mailing list,
René Scharfe
In-Reply-To: <2ea2f077-ab02-2631-4ce9-93cdd22c3c6b@gmail.com>
On Wed, Oct 05, 2016 at 03:58:53PM +0200, Jakub Narębski wrote:
> I would prefer the following:
>
> # A --> B --> C --> D --> E --> F --> G --> H
> # 0 1 2 3 4 5 6
Yeah, that is also more visually pleasing.
Here's a squashable update that uses that and clarifies the points in
the discussion with Jacob.
Junio, do you mind squashing this in to jk/alt-odb-cleanup?
diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index b393613..62170b7 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -39,13 +39,16 @@ test_expect_success 'preparing third repository' '
)
'
-# Note: These tests depend on the hard-coded value of 5 as "too deep". We start
-# the depth at 0 and count links, not repositories, so in a chain like:
+# Note: These tests depend on the hard-coded value of 5 as the maximum depth
+# we will follow recursion. We start the depth at 0 and count links, not
+# repositories. This means that in a chain like:
#
-# A -> B -> C -> D -> E -> F -> G -> H
-# 0 1 2 3 4 5 6
+# A --> B --> C --> D --> E --> F --> G --> H
+# 0 1 2 3 4 5 6
#
-# we are OK at "G", but break at "H".
+# we are OK at "G", but break at "H", even though "H" is actually the 8th
+# repository, not the 6th, which you might expect. Counting the links allows
+# N+1 repositories, and counting from 0 to 5 inclusive allows 6 links.
#
# Note also that we must use "--bare -l" to make the link to H. The "-l"
# ensures we do not do a connectivity check, and the "--bare" makes sure
@@ -59,11 +62,11 @@ test_expect_success 'creating too deep nesting' '
git clone --bare -l -s G H
'
-test_expect_success 'validity of fifth-deep repository' '
+test_expect_success 'validity of seventh repository' '
git -C G fsck
'
-test_expect_success 'invalidity of sixth-deep repository' '
+test_expect_success 'invalidity of eighth repository' '
test_must_fail git -C H fsck
'
^ permalink raw reply related
* [PATCH] clone: detect errors in normalize_path_copy
From: Jeff King @ 2016-10-05 14:29 UTC (permalink / raw)
To: git
When we are copying the alternates from the source
repository, if we find a relative path that is too deep for
the source (e.g., "../../../objects" from "/repo.git/objects"),
then normalize_path_copy will report an error and leave
trash in the buffer, which we will add to our new alternates
file. Instead, let's detect the error, print a warning, and
skip copying that alternate.
There's no need to die. The relative path is probably just
broken cruft in the source repo. If it turns out to have
been important for accessing some objects, we rely on other
parts of the clone to detect that, just as they would with a
missing object in the source repo itself (though note that
clones with "-s" are inherently local, which may do fewer
object-quality checks in the first place).
Signed-off-by: Jeff King <peff@peff.net>
---
Noticed by coverity; the recent alternates cleanups mean that all of the
other calls to normalize_path_copy() are now checked, so it realized
this one was an oddball and probably an error (I actually looked for
others with `grep` when doing that series, but somehow missed this one;
hooray for static analysis). The fix is independent of that series.
builtin/clone.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index fb75f7e..6cf3b54 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -345,8 +345,11 @@ static void copy_alternates(struct strbuf *src, struct strbuf *dst,
continue;
}
abs_path = mkpathdup("%s/objects/%s", src_repo, line.buf);
- normalize_path_copy(abs_path, abs_path);
- add_to_alternates_file(abs_path);
+ if (!normalize_path_copy(abs_path, abs_path))
+ add_to_alternates_file(abs_path);
+ else
+ warning("skipping invalid relative alternate: %s/%s",
+ src_repo, line.buf);
free(abs_path);
}
strbuf_release(&line);
--
2.10.1.506.g904834d
^ permalink raw reply related
* Re: [PATCH 16/18] count-objects: report alternates via verbose mode
From: Jakub Narębski @ 2016-10-05 14:23 UTC (permalink / raw)
To: Jeff King, git; +Cc: René Scharfe
In-Reply-To: <20161003203618.m6kxd3b6h74jbmqz@sigill.intra.peff.net>
W dniu 03.10.2016 o 22:36, Jeff King pisze:
> +test_expect_success 'count-objects shows the alternates' '
> + cat >expect <<-EOF &&
> + alternate: $(pwd)/B/.git/objects
> + alternate: $(pwd)/A/.git/objects
> + EOF
> + git -C C count-objects -v >actual &&
> + grep ^alternate: actual >actual.alternates &&
> + test_cmp expect actual.alternates
> +'
This was bit hard to grok for me without quotes around
regular expression in grep (should it be sane_grep, BTW?):
+ grep "^alternate:" actual >actual.alternates &&
But it might be just me... It's certainly not necessary.
--
Jakub Narębski
^ permalink raw reply
* Re: Reference a submodule branch instead of a commit
From: Heiko Voigt @ 2016-10-05 14:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, Jeremy Morton, git@vger.kernel.org
In-Reply-To: <xmqqlgy4szuu.fsf@gitster.mtv.corp.google.com>
On Tue, Oct 04, 2016 at 12:01:13PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Stefan Beller <sbeller@google.com> writes:
> >
> >> 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.
>
> eh,... hit send too early.
>
> It IS a hack, but having this information in .git<something> would
> mean that it can be forced to be in machine readable form, unlike a
> mention in README. I do not know if the .gitmodules/.gitignore
> combination is a sensible thing to use, but it does smell like a
> potentially useful hack.
IIRC the tree entries are the reference for submodules in the code. We
are iterating over the tree entries in many places so that change does
not seem so easy to me.
But you are right maybe we should stop arguing against this workflow and
just let people use it until they find out whats wrong with it ;)
I have another tip for Jeremy:
git config submodule.<name>.ignore all
and you will not see any changes to the submodule. Put that into your
.gitmodules and you do not see any changes to the submodules anymore.
So now the only thing missing for complete convenience is a config
option for the --remote option in 'git submodule update'.
Jeremy, does the ignore option combined with --remote what you want?
Cheers Heiko
^ permalink raw reply
* Re: [PATCH 06/18] t5613: clarify "too deep" recursion tests
From: Jakub Narębski @ 2016-10-05 13:58 UTC (permalink / raw)
To: Stefan Beller, Jeff King
Cc: Jacob Keller, Git mailing list, René Scharfe
In-Reply-To: <CAGZ79kap2ndp=FK4YdqrL4tJ8_VDuuAcSCk1dtX5X2H3aaj6kQ@mail.gmail.com>
W dniu 04.10.2016 o 22:58, Stefan Beller pisze:
> On Tue, Oct 4, 2016 at 1:55 PM, Jeff King <peff@peff.net> wrote:
>> On Tue, Oct 04, 2016 at 01:52:19PM -0700, Jacob Keller wrote:
>>
>>>>>>>> +# Note: These tests depend on the hard-coded value of 5 as "too deep". We start
>>>>>>>> +# the depth at 0 and count links, not repositories, so in a chain like:
>>>>>>>> +#
>>>>>>>> +# A -> B -> C -> D -> E -> F -> G -> H
>>>>>>>> +# 0 1 2 3 4 5 6
>>>>>>>> +#
>
> Input from a self-claimed design expert for ASCII art. ;)
> What about this?
>
> # A -0-> B -1-> C -2-> ...
I would prefer the following:
# A --> B --> C --> D --> E --> F --> G --> H
# 0 1 2 3 4 5 6
that is, the number below the middle of the arrow
(which could have been even longer)
# A ---> B ---> C ---> D ---> E ---> F ---> G ---> H
# 0 1 2 3 4 5 6
Let's paint this bikeshed _plaid_ ;-))))
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCHv3 1/2] push: change submodule default to check when submodules exist
From: Heiko Voigt @ 2016-10-05 13:53 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, peff, torvalds
In-Reply-To: <20161004210359.15266-1-sbeller@google.com>
On Tue, Oct 04, 2016 at 02:03:58PM -0700, Stefan Beller wrote:
> Jeff,
> thanks for the suggestions, both git_path(..) as well as checking the config,
> this seems quite readable to me:
When reading the discussion I thought the same: What about the
"old-style" repositories. I like this one. Checking both locations
is nice.
Cheers Heiko
^ permalink raw reply
* Re: Bug Report: "git submodule deinit" fails right after a clone
From: Heiko Voigt @ 2016-10-05 13:36 UTC (permalink / raw)
To: Thomas Bétous; +Cc: git
In-Reply-To: <CAPOqYV+xsrLk7y1hJYHZFY8OfkxVRwPcZBdqhdgrhThqdZysQA@mail.gmail.com>
Hi,
please do not top-post the conversation will otherwise get hard to
follow. Thank you.
On Tue, Oct 04, 2016 at 05:46:45PM +0200, Thomas Bétous wrote:
> 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).
My initial reaction is that this might be a problem with line endings. Did you
check whether you get any diff when you do a 'git diff' after the clone?
Maybe the variable 'core.autocrlf' is set to 'input' ? Have a look at 'git help
config'
> 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
Here I get
$ git submodule deinit js
Cleared directory 'js'
So all seems fine.
> Is the script working on your PC?
Yes. I am on Mac OS X though.
Cheers Heiko
^ permalink raw reply
* Re: [musl] Re: Regression: git no longer works with musl libc's regex impl
From: Rich Felker @ 2016-10-05 13:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, git, musl
In-Reply-To: <alpine.DEB.2.20.1610051250080.35196@virtualbox>
On Wed, Oct 05, 2016 at 01:17:49PM +0200, Johannes Schindelin wrote:
> Hi Rich,
>
> On Tue, 4 Oct 2016, Rich Felker wrote:
>
> > On Tue, Oct 04, 2016 at 06:08:33PM +0200, Johannes Schindelin wrote:
> >
> > > And lastly, the best alternative would be to teach musl about
> > > REG_STARTEND, as it is rather useful a feature.
> >
> > Maybe, but it seems fundamentally costly to support -- it's extra
> > state in the inner loops that imposes costly spill/reload on archs
> > with too few registers (x86).
>
> It is true that it could cause that.
>
> I had a brief look at the source code (you use backtracking...
Where did you get that idea? Backtracking is the most utterly
incompetent way to implement regex -- it throws away the whole
property that makes regex useful, being regular. Unfortunately, POSIX
BRE is not regular, as it contains backreferences, so any
implementation of regcomp/regexec requires at least a minimal
backtracking code path for BREs that contain backreferences.
> hopefully
> nobody uses musl to parse regular expressions from untrusted, or
On the contrary, musl's is the only system reccomp/regexec I'm aware
of that actually attempts to be safe with untrusted input -- when
using REG_EXTENDED (ERE). Other implementations provide backreferences
in ERE as an extension, making ERE unsafe just like BRE. musl
intentionally disallows them as a feature.
At least until recently, glibc also crashed on malloc failures in
regcomp, making it unsafe on untrusted input for that reason too.
Rich
^ permalink raw reply
* Re: Regression: git no longer works with musl libc's regex impl
From: Jakub Narębski @ 2016-10-05 13:11 UTC (permalink / raw)
To: musl, James B; +Cc: Johannes Schindelin, Jeff King, git
In-Reply-To: <20161004223322.GE19318@brightrain.aerifal.cx>
W dniu 05.10.2016 o 00:33, Rich Felker pisze:
> On Wed, Oct 05, 2016 at 09:06:25AM +1100, James B wrote:
>> On Tue, 4 Oct 2016 18:08:33 +0200 (CEST)
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>>>
>>> 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.
>>
>> Wow, I don't know that Windows is a git's first-tier platform now,
>> and Linux/POSIX second. Are we talking about the same git that was
>> originally written in Linus Torvalds, and is used to manage Linux
>> kernel? Are you by any chance employed by Redmond, directly or
>> indirectly?
>>
>> Sorry - can't help it.
Windows is one of the major platforms, yes. I think there much, much
more people using Git on Windows, than using Git with musl. More
users = more important.
Also, working with some inconvenience (requiring compilation with
NO_REGEX=1) is better than not working at all.
In CodingGuidelines we say:
- Most importantly, we never say "It's in POSIX; we'll happily
ignore your needs should your system not conform to it."
We live in the real world.
- However, we often say "Let's stay away from that construct,
it's not even in POSIX".
- In spite of the above two rules, we sometimes say "Although
this is not in POSIX, it (is so convenient | makes the code
much more readable | has other good characteristics) and
practically all the platforms we care about support it, so
let's use it".
The REG_STARTEND is 3rd point, mmap shenningans looks like 1st...
...on the other hand midipix <writeonce@midipix.org> wrote in
http://public-inbox.org/git/20161004200057.dc30d64f61e5ec441c34ffd4f788e58e.efa66ead67.wbe@email15.godaddy.com/
that the proposed fix should work on all Windows version we are
interested in (I think). Test program included / attached.
The above-mentioned email also explains that the problem was
caught on MS Windows; it triggers if file end falls on the mmapped
page boundary, which is more likely to happen with 4096 mod size
on Windows rather than 65536 mod size on Linux.
On the other hand, while the proposed solution of "add padding as
to not end at page boundary, if necessary" doesn't have the
performance impact of "memcpy into NUL-terminated buffer" that
was originally proposed in patch series, it is still extra code
to maintain.
>
> I don't think the hostility and sarcasm are really needed here. But
> what this does speak to is that users don't like feeling like their
> platform is being treated as a second-class target, which is what it
> feels like when you have to manually flip a switch to make git build.
You are welcome to send a patch adding to configure.ac detection
of REG_STARTEND support in standard library - setting NO_REGEX if
needed, and/or adding to Makefile uname-based defaults setting
NO_REGEX for compiling with musl.
> This is especially unfriendly when the semantics of the switch come
> across, at least to some users, as "your system regex is incomplete"
> rather than "git can't use it because git depends on nonstandard
> extensions".
Nonstandard but common extension. As 2f8952250a commit message says
https://github.com/git/git/commit/2f8952250a84313b74f96abb7b035874854cf202
Happily, there is an extension to regexec() introduced by the NetBSD
project and present in all major regex implementation including
Linux', MacOSX' and the one Git includes in compat/regex/: [...]
[...]
Since support for REG_STARTEND is so widespread by now, let's just
introduce a helper function that always uses it, and tell people
on a platform whose regex library does not support it to use the
one from our compat/regex/ directory.
Also, as Junio said, the description of NO_REGEX option in the
Makefile now explicitly says:
# Define NO_REGEX if your C library lacks regex support with REG_STARTEND
# feature.
Best,
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v2 21/25] sequencer: refactor write_message()
From: Johannes Schindelin @ 2016-10-05 13:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git, Jakub Narębski
In-Reply-To: <xmqqfup1nf3u.fsf@gitster.mtv.corp.google.com>
Hi Junio & Hannes,
On Thu, 15 Sep 2016, Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
>
> > Am 11.09.2016 um 12:55 schrieb Johannes Schindelin:
> >> -static int write_message(struct strbuf *msgbuf, const char *filename)
> >> +static int write_with_lock_file(const char *filename,
> >> + const void *buf, size_t len, int append_eol)
> >> {
> >> static struct lock_file msg_file;
> >>
> >> int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0);
> >> if (msg_fd < 0)
> >> return error_errno(_("Could not lock '%s'"), filename);
> >> - if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
> >> - return error_errno(_("Could not write to %s"), filename);
> >> - strbuf_release(msgbuf);
> >> + if (write_in_full(msg_fd, buf, len) < 0)
> >> + return error_errno(_("Could not write to '%s'"), filename);
> >> + if (append_eol && write(msg_fd, "\n", 1) < 0)
> >> + return error_errno(_("Could not write eol to '%s"), filename);
> >> if (commit_lock_file(&msg_file) < 0)
> >> return error(_("Error wrapping up %s."), filename);
> >>
> >> return 0;
> >> }
> >
> > The two error paths in the added lines should both
> >
> > rollback_lock_file(&msg_file);
> >
> > , I think. But I do notice that this is not exactly new, so...
>
> It may not be new for this step, but overall the series is aiming to
> libify the stuff, so we should fix fd and lockfile leaks like this
> as we notice them.
Makes sense, even for the final commit_lock_file().
Ciao,
Dscho
^ 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