* Re: [PATCH] Rename {git- => git}remote-helpers.txt
From: Jeff King @ 2013-02-01 8:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, John Keeping, git, Matthieu Moy, Max Horn
In-Reply-To: <20130201073352.GB970@sigill.intra.peff.net>
On Fri, Feb 01, 2013 at 02:33:52AM -0500, Jeff King wrote:
> I am starting to think it has grown in an unnecessarily complex
> direction, and we would be much happier just calling all of the
> "concept" documentation "git-".
>
> The steps I see are:
I am still undecided on whether it is a good idea (in some ways, I like
that "gitrevisions" signals to the user that it is not a command; but I
also recognize that it is more complex for users, and gitremote-helpers
looks silly to me). But here is what the patch looks like, for
reference. The first one is a cleanup we might want to take anyway, and
the second one is the meat.
[1/2]: Documentation/Makefile: clean up MAN*_TXT lists
[2/2]: docs: convert "concept" manpages to git-*
-Peff
^ permalink raw reply
* Re: [PATCH] Rename {git- => git}remote-helpers.txt
From: Jeff King @ 2013-02-01 7:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, John Keeping, git, Matthieu Moy, Max Horn
In-Reply-To: <7v7gmsd26o.fsf@alter.siamese.dyndns.org>
On Thu, Jan 31, 2013 at 09:37:51PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > What was the original rationale for the "gitfoo" form? Was it just to
> > visually distinguish command manpages from non-command manpages? I can't
> > remember the origins now.
>
> b27a23e (Documentation: convert tutorials to man pages, 2008-05-24)
> turns "tutorial.txt" into "gittutorial.txt". Before that, there
> were git{attributes,cli,ignore,hooks,modules}.txt. They were added
> in this order, which reveals the true reason:
>
> hooks.txt
> gitignore.txt
> gitmodules.txt
> gitcli.txt
> hooks.txt => githooks.txt
> gitatributes.txt
>
> Originally, we did not intend to name them as "git$concept.txt";
> instead, we just said "$concept.txt". Hooks are "hooks", and we
> know we are discussing hooks in the context of Git, there was no
> reason to say "githooks".
Yes, but if we are going to install a manpage, we must include "git"
somewhere in the name since that context is lost in the global manpage
database. So we need "githooks.txt" or "git-hooks.txt".
> "gitignore" was about the file format,
> and it would have been insane to call it "ignore.txt". The same
> applies to "gitmodules.txt".
Right. So we have some that must be "gitfoo", and others that do not
care. If we turned "githooks" into "git-hooks" and removed the "is it a
command?" magic from "git help", then "git help hooks" would still find
hooks. And likewise, "git help gitignore" would still find "gitignore".
But you could no longer say "git help ignore" to find "gitignore".
Which personally I am OK with. It is not a sensible thing to ask for;
the concept is not "ignore", but rather "the gitignore file".
> In other way, I think this wasn't designed, but organically grown.
Like many parts of git. :) I am starting to think it has grown in an
unnecessarily complex direction, and we would be much happier just
calling all of the "concept" documentation "git-".
The steps I see are:
1. Modify builtin/help.c, like:
diff --git a/builtin/help.c b/builtin/help.c
index 6067a61..1eca4ea 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -308,10 +308,8 @@ static const char *cmd_to_page(const char *git_cmd)
return "git";
else if (!prefixcmp(git_cmd, "git"))
return git_cmd;
- else if (is_git_command(git_cmd))
- return prepend("git-", git_cmd);
else
- return prepend("git", git_cmd);
+ return prepend("git-", git_cmd);
}
static void setup_man_path(void)
2. Rename the "concept" files like:
Documentation/{gitcli.txt => git-cli.txt} | 0
Documentation/{gitcore-tutorial.txt => git-core-tutorial.txt} | 0
Documentation/{gitcredentials.txt => git-credentials.txt} | 0
Documentation/{gitcvs-migration.txt => git-cvs-migration.txt} | 0
Documentation/{gitdiffcore.txt => git-diffcore.txt} | 0
Documentation/{gitglossary.txt => git-glossary.txt} | 0
Documentation/{githooks.txt => git-hooks.txt} | 0
Documentation/{gitnamespaces.txt => git-namespaces.txt} | 0
Documentation/{gitrepository-layout.txt => git-repository-layout.txt} | 0
Documentation/{gitrevisions.txt => git-revisions.txt} | 0
Documentation/{gittutorial-2.txt => git-tutorial-2.txt} | 0
Documentation/{gittutorial.txt => git-tutorial.txt} | 0
Documentation/{gitworkflows.txt => git-workflows.txt} | 0
3. Teach Documentation/Makefile about the new filenames. MAN1_TXT
includes git-*.txt, which is no longer right. I think we'd need to
just filter out what is in MAN5_TXT and MAN7_TXT.
4. Replace the rename "gitfoo" above with a "see git-foo..." pointer.
Users of "git help foo" would not ever see this, but people who
have trained their fingers to type "man gitfoo" would, along with
anybody following an outdated HTML link.
5. Update internal references to "linkgit:gitfoo" to point to
"git-foo".
Hmm. That really does not seem so bad. The biggest downside is the
people who have to see the redirect made in step 4.
But I have not thought about it too hard, so maybe there is something
else I'm not seeing.
-Peff
^ permalink raw reply related
* Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-02-01 7:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: jongman.heo, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <20130201063638.GD29973@sigill.intra.peff.net>
On Fri, Feb 01, 2013 at 01:36:38AM -0500, Jeff King wrote:
> On Thu, Jan 31, 2013 at 05:40:02PM -0800, Junio C Hamano wrote:
>
> > 허종만 <jongman.heo@samsung.com> writes:
> >
> > > But usually when I build upstream Linux kernel, I don't do "make
> > > clean" after git pull.. I didn't expect that I needed "make
> > > clean" for git build.
> >
> > We don't expect anybody need "make clean", either. There is
> > something wrong in the dependency.
>
> Agreed, but I cannot see what. If auto-header-dependencies is on, gcc
> should find it (it is not even a recursive dependency for
> builtin/fetch.c). And if it is not on, we should rebuild based on LIB_H,
> which includes string-list.h (and always has, as far as I can tell).
By the way, while researching this issue, I noticed this:
-- >8 --
Subject: [PATCH] Makefile: add version.h to LIB_H
This was forgotten when the file was added by 816fb46, and
not noticed because most developers are on modern systems
that support COMPUTE_HEADER_DEPENDENCIES. However, people
still relying on LIB_H for dependencies might have failed to
recompile when this file changed.
Found with "make CHECK_HEADER_DEPENDENCIES=yes".
Signed-off-by: Jeff King <peff@peff.net>
---
I don't see how this could have caused the issue at hand, but it is good
to fix nonetheless. I almost wonder if LIB_H should just be set to
$(wildcard *.h) or similar, since that is what ends up going into it.
And then we would not have to deal with manually keeping it up to date.
Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile b/Makefile
index 731b6a8..6b42f66 100644
--- a/Makefile
+++ b/Makefile
@@ -704,6 +704,7 @@ LIB_H += varint.h
LIB_H += userdiff.h
LIB_H += utf8.h
LIB_H += varint.h
+LIB_H += version.h
LIB_H += walker.h
LIB_H += wildmatch.h
LIB_H += wt-status.h
--
1.8.1.2.11.g1a2f572
^ permalink raw reply related
* Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-02-01 7:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: jongman.heo, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <20130201063638.GD29973@sigill.intra.peff.net>
On Fri, Feb 01, 2013 at 01:36:38AM -0500, Jeff King wrote:
> It seems like building each object file should depend on its dependency
> file (but only when COMPUTE_HEADER_DEPENDENCIES is on, of course), since
> otherwise we cannot know if we have the right dependencies or not.
>
> Something like this almost works, I think:
> [...]
> +$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(call dep_file, %.o)
Actually that would not work, as we do not have a rule to create
.depend/foo.o.d. We can add one, but it gets pretty hairy (and
replicates much of the normal build rule). A much simpler way is to just
find the missing dep files and force compilation of their matching
objects. Like:
diff --git a/Makefile b/Makefile
index 6b42f66..f94e8b9 100644
--- a/Makefile
+++ b/Makefile
@@ -1843,8 +1843,14 @@ dep_args = -MF $(dep_file) -MMD -MP
@mkdir -p $@
missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
+missing_dep_files := $(filter-out $(wildcard $(dep_files)),$(dep_files))
+# we want to rewrite "foo/.depend/bar.o.d" into "foo/bar.o", but
+# make's patsubst is not powerful enough to remove something from the middle of
+# a string. Hack around it by shelling out.
+obj_files_with_missing_deps := $(shell echo $(missing_dep_files:.d=) | tr ' ' '\n' | sed 's,.depend/,,')
dep_file = $(dir $@).depend/$(notdir $@).d
dep_args = -MF $(dep_file) -MMD -MP
+$(obj_files_with_missing_deps): FORCE
ifdef CHECK_HEADER_DEPENDENCIES
$(error cannot compute header dependencies outside a normal build. \
Please unset CHECK_HEADER_DEPENDENCIES and try again)
which does solve the problem, but that shell hack is nasty. It would be
much simpler if we stored the dependency for foo/bar.o as
".depend/foo/bar.o.d", rather than "foo/.depend/bar.o.d", as then we
would patsubst it away. Or maybe there is some clever way to convince
make to do what I want here. Suggestions welcome.
-Peff
^ permalink raw reply related
* Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-02-01 6:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: jongman.heo, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <7vmwvodd71.fsf@alter.siamese.dyndns.org>
On Thu, Jan 31, 2013 at 05:40:02PM -0800, Junio C Hamano wrote:
> 허종만 <jongman.heo@samsung.com> writes:
>
> > But usually when I build upstream Linux kernel, I don't do "make
> > clean" after git pull.. I didn't expect that I needed "make
> > clean" for git build.
>
> We don't expect anybody need "make clean", either. There is
> something wrong in the dependency.
Agreed, but I cannot see what. If auto-header-dependencies is on, gcc
should find it (it is not even a recursive dependency for
builtin/fetch.c). And if it is not on, we should rebuild based on LIB_H,
which includes string-list.h (and always has, as far as I can tell).
Hmm. I do notice one oddity with the computed header dependencies,
though. We build the computed dependency files as a side effect of doing
the actual compilation. So before we have run the compilation once, we
need some way to say "you _must_ build this, because we do even know the
correct dependencies". And to do that, we have each object file depend
on any missing .depend dirs, which bootstraps the whole process: we
build everything the first time because .depend is missing, and from
then on, we use the correct dependencies.
But that is not quite right. The .depend directory might exist, but be
missing the actual dependency file for a particular object. So if I do:
$ make ;# builds all objects and dependency files
$ rm builtin/.depend/fetch.o.d
$ touch string-list.h
$ make
we will fail to rebuild builtin/fetch.o properly. It does not see the
dependency on string-list (because we have no .d file), nor does it
realize that it needs to build the .d file (because it only checks that
builtin/.depend exists).
It seems like building each object file should depend on its dependency
file (but only when COMPUTE_HEADER_DEPENDENCIES is on, of course), since
otherwise we cannot know if we have the right dependencies or not.
Something like this almost works, I think:
diff --git a/Makefile b/Makefile
index 6b42f66..a329736 100644
--- a/Makefile
+++ b/Makefile
@@ -1843,8 +1843,8 @@ missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
@mkdir -p $@
missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
-dep_file = $(dir $@).depend/$(notdir $@).d
-dep_args = -MF $(dep_file) -MMD -MP
+dep_file = $(dir $1).depend/$(notdir $1).d
+dep_args = -MF $(call dep_file, $@) -MMD -MP
ifdef CHECK_HEADER_DEPENDENCIES
$(error cannot compute header dependencies outside a normal build. \
Please unset CHECK_HEADER_DEPENDENCIES and try again)
@@ -1909,9 +1909,9 @@ $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)
endif
ifndef CHECK_HEADER_DEPENDENCIES
-$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)
+$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(call dep_file, %.o)
$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
-$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)
+$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(call dep_file,%.o)
$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
endif
But not quite. The problem is that we put the dep file for foo/bar.o
into foo/.depend/bar.o. But when we call the dep_file function for the
dependency, it sees only "%.o", not "foo/bar.o", so it can't properly
split it apart. I don't think there is a way to force expansion before
calling the function.
And of course I have no idea if this was the problem that we saw,
anyway. I have no idea how one would get into this situation short of
manually removing the dependency file.
-Peff
^ permalink raw reply related
* Re: [PATCH 1/3] git-am: record full index line in the patch used while rebasing
From: Martin von Zweigbergk @ 2013-02-01 6:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Heiko Voigt
In-Reply-To: <1359693125-22357-2-git-send-email-gitster@pobox.com>
On Thu, Jan 31, 2013 at 8:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Earlier, a230949 (am --rebasing: get patch body from commit, not
> from mailbox, 2012-06-26) learned to regenerate patch body from the
> commit object while rebasing, instead of reading from the rebase-am
> front-end. While doing so, it used "git diff-tree" but without
> giving it the "--full-index" option.
>
> This does not matter for in-repository objects; during rebasing, any
> abbreviated object name should uniquely identify them.
>
> But we may be rebasing a commit that contains a change to a gitlink,
> in which case we usually should not have the object (it names a
> commit in the submodule). A full object name is necessary to later
> reconstruct a fake ancestor index for them.
>From what I can understand, this all makes sense. I didn't notice the
emails about the breakage until now, but I wouldn't have known where
to even start looking anyway, so thanks a lot for taking care of it.
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Miles Bader @ 2013-02-01 5:37 UTC (permalink / raw)
To: Jeff King
Cc: Max Horn, John Keeping, Antoine Pelisse, Junio C Hamano, git,
Johannes Sixt
In-Reply-To: <20130116175057.GB27525@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It seems a little weird to me that clang defines __GNUC__, but I
> assume there are good reasons for it.
The thing is that "gcc" is as much a language dialect these days as it
is a compiler implementation, and many other compilers, including
clang, explicitly try to implement that dialect (clang goes even
further by trying to be compatible in other ways, e.g. command-line
syntax, but that's not relevant here).
__GNUC__ is a way many programs try to detect the presence of a
compiler that implements that dialect, they have little choice but to
define it...
-Miles
--
Love, n. A temporary insanity curable by marriage or by removal of the patient
from the influences under which he incurred the disorder. This disease is
prevalent only among civilized races living under artificial conditions;
barbarous nations breathing pure air and eating simple food enjoy immunity
from its ravages. It is sometimes fatal, but more frequently to the physician
than to the patient.
^ permalink raw reply
* Re: [PATCH 1/2] docs: clarify git-branch --list behavior
From: Junio C Hamano @ 2013-02-01 5:42 UTC (permalink / raw)
To: Jeff King; +Cc: Eric Sunshine, Peter Wu, git
In-Reply-To: <20130201050637.GC29973@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Thanks. No matter how many times I proofread a doc change, I always
> manage to slip an error into the final version.
I think everybody shares that trait.
> Hooray for many eyes.
Indeed.
^ permalink raw reply
* Re: [PATCH] Rename {git- => git}remote-helpers.txt
From: Junio C Hamano @ 2013-02-01 5:37 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, John Keeping, git, Matthieu Moy, Max Horn
In-Reply-To: <20130201050343.GA29973@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> What was the original rationale for the "gitfoo" form? Was it just to
> visually distinguish command manpages from non-command manpages? I can't
> remember the origins now.
b27a23e (Documentation: convert tutorials to man pages, 2008-05-24)
turns "tutorial.txt" into "gittutorial.txt". Before that, there
were git{attributes,cli,ignore,hooks,modules}.txt. They were added
in this order, which reveals the true reason:
hooks.txt
gitignore.txt
gitmodules.txt
gitcli.txt
hooks.txt => githooks.txt
gitatributes.txt
Originally, we did not intend to name them as "git$concept.txt";
instead, we just said "$concept.txt". Hooks are "hooks", and we
know we are discussing hooks in the context of Git, there was no
reason to say "githooks". "gitignore" was about the file format,
and it would have been insane to call it "ignore.txt". The same
applies to "gitmodules.txt".
Things start to go in a strange direction when "cli" was added. We
know that is about the command line interface in the context of Git,
and it should have been "cli.txt". To make things worse, later
"hooks.txt" took an example of "gitcli.txt" in the wrong way at
a5af0e2 (Documentation: rename "hooks.txt" to "githooks.txt" and
make it a man page, 2008-05-02).
In other way, I think this wasn't designed, but organically grown.
^ permalink raw reply
* Re: [PATCH 1/2] docs: clarify git-branch --list behavior
From: Jeff King @ 2013-02-01 5:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric Sunshine, Peter Wu, git
In-Reply-To: <7v4nhweuoi.fsf@alter.siamese.dyndns.org>
On Thu, Jan 31, 2013 at 04:37:01PM -0800, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
> > On Thu, Jan 31, 2013 at 1:45 AM, Jeff King <peff@peff.net> wrote:
> >> +If `--list` is given, or if there are no non-option arguments, existing
> >> +branches are listed; the current branch will be highlighted with an
> >> +asterisk. Option `-r` causes the remote-tracking branches to be listed,
> >> +and option `-a` shows both local and remote branches. If a `<pattern>`
> >> +is given, it is used as a shell wildcard to restrict the output to
> >> +matching branches. If multiple patterns are given, a branch is shown if
> >> +any it is matched by any of the patterns.
> >
> > s/if any it is/if it is/
>
> Thanks; I'll squash this in. Peff, no need to resend.
Thanks. No matter how many times I proofread a doc change, I always
manage to slip an error into the final version. Hooray for many eyes.
-Peff
^ permalink raw reply
* Re: [PATCH] Rename {git- => git}remote-helpers.txt
From: Jeff King @ 2013-02-01 5:03 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: John Keeping, git, Junio C Hamano, Matthieu Moy, Max Horn
In-Reply-To: <20130131230455.GN27340@google.com>
On Thu, Jan 31, 2013 at 03:04:55PM -0800, Jonathan Nieder wrote:
> Jeff King wrote:
>
> > Maybe it is just me, but the fact that accessing the manpage is now:
> >
> > man gitremote-helpers
> >
> > feels weird to me. I know it technically follows our syntactic rules,
> > but having the lack of dash be significant between "git" and "remote",
> > but then having a dash later makes it hard on the eyes.
>
> Yes. I have thought for years that it should be git-remote-helpers,
> that "git help" should be tweaked to look for that, and that the
> existing gitrepository-layout and friends should be replaced with
> redirects.
What was the original rationale for the "gitfoo" form? Was it just to
visually distinguish command manpages from non-command manpages? I can't
remember the origins now. It does seem like it is causing more
hassle than it is worth, but maybe there is something I am forgetting.
-Peff
^ permalink raw reply
* [PATCH 3/3] apply: diagnose incomplete submodule object name better
From: Junio C Hamano @ 2013-02-01 4:32 UTC (permalink / raw)
To: git; +Cc: Heiko Voigt, Martin von Zweigbergk
In-Reply-To: <1359693125-22357-1-git-send-email-gitster@pobox.com>
"git am -3" uses this function to build a tree that records how the
preimage the patch was created from would have looked like. An
abbreviated object name on the index line is ordinarily sufficient
for us to figure out the object name the preimage tree would have
contained, but a change to a submodule by definition shows an object
name of a submodule commit which our repository should not have, and
get_sha1_blob() is not an appropriate way to read it (or get_sha1()
for that matter).
Use get_sha1_hex() and complain if we do not find a full object name
there.
We could read from the payload part of the patch to learn the full
object name of the commit, but the primary user "git rebase" has
been fixed to give us a full object name, so this should suffice
for now.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/apply.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index a1db7b4..1f78e2c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3606,7 +3606,11 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
if (0 < patch->is_new)
continue;
- if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
+ if (S_ISGITLINK(patch->old_mode)) {
+ if (get_sha1_hex(patch->old_sha1_prefix, sha1))
+ die("submoule change for %s without full index name",
+ name);
+ } else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
; /* ok */
} else if (!patch->lines_added && !patch->lines_deleted) {
/* mode-only change: update the current */
--
1.8.1.2.612.g09f4be5
^ permalink raw reply related
* [PATCH 2/3] apply: simplify build_fake_ancestor()
From: Junio C Hamano @ 2013-02-01 4:32 UTC (permalink / raw)
To: git; +Cc: Heiko Voigt, Martin von Zweigbergk
In-Reply-To: <1359693125-22357-1-git-send-email-gitster@pobox.com>
The local variable sha1_ptr in the build_fake_ancestor() function
used to either point at the null_sha1[] (if the ancestor did not
have the path) or at sha1[] (if we read the object name into the
local array), but 7a98869 (apply: get rid of --index-info in favor
of --build-fake-ancestor, 2007-09-17) made the "missing in the
ancestor" case unnecessary, hence sha1_ptr, when used, always points
at the local array.
Get rid of the unneeded variable, and restructure the if/else
cascade a bit to make it easier to read. There should be no
behaviour change.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/apply.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 156b3ce..a1db7b4 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3598,7 +3598,6 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
* worth showing the new sha1 prefix, but until then...
*/
for (patch = list; patch; patch = patch->next) {
- const unsigned char *sha1_ptr;
unsigned char sha1[20];
struct cache_entry *ce;
const char *name;
@@ -3606,20 +3605,19 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
name = patch->old_name ? patch->old_name : patch->new_name;
if (0 < patch->is_new)
continue;
- else if (get_sha1_blob(patch->old_sha1_prefix, sha1))
- /* git diff has no index line for mode/type changes */
- if (!patch->lines_added && !patch->lines_deleted) {
- if (get_current_sha1(patch->old_name, sha1))
- die("mode change for %s, which is not "
- "in current HEAD", name);
- sha1_ptr = sha1;
- } else
- die("sha1 information is lacking or useless "
- "(%s).", name);
- else
- sha1_ptr = sha1;
- ce = make_cache_entry(patch->old_mode, sha1_ptr, name, 0, 0);
+ if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
+ ; /* ok */
+ } else if (!patch->lines_added && !patch->lines_deleted) {
+ /* mode-only change: update the current */
+ if (get_current_sha1(patch->old_name, sha1))
+ die("mode change for %s, which is not "
+ "in current HEAD", name);
+ } else
+ die("sha1 information is lacking or useless "
+ "(%s).", name);
+
+ ce = make_cache_entry(patch->old_mode, sha1, name, 0, 0);
if (!ce)
die(_("make_cache_entry failed for path '%s'"), name);
if (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD))
--
1.8.1.2.612.g09f4be5
^ permalink raw reply related
* [PATCH 1/3] git-am: record full index line in the patch used while rebasing
From: Junio C Hamano @ 2013-02-01 4:32 UTC (permalink / raw)
To: git; +Cc: Heiko Voigt, Martin von Zweigbergk
In-Reply-To: <1359693125-22357-1-git-send-email-gitster@pobox.com>
Earlier, a230949 (am --rebasing: get patch body from commit, not
from mailbox, 2012-06-26) learned to regenerate patch body from the
commit object while rebasing, instead of reading from the rebase-am
front-end. While doing so, it used "git diff-tree" but without
giving it the "--full-index" option.
This does not matter for in-repository objects; during rebasing, any
abbreviated object name should uniquely identify them.
But we may be rebasing a commit that contains a change to a gitlink,
in which case we usually should not have the object (it names a
commit in the submodule). A full object name is necessary to later
reconstruct a fake ancestor index for them.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-am.sh | 2 +-
t/t7402-submodule-rebase.sh | 30 ++++++++++++++++++++++++++++--
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index c682d34..0e0a096 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -664,7 +664,7 @@ do
sed -e '1,/^$/d' >"$dotest/msg-clean"
echo "$commit" >"$dotest/original-commit"
get_author_ident_from_commit "$commit" >"$dotest/author-script"
- git diff-tree --root --binary "$commit" >"$dotest/patch"
+ git diff-tree --root --binary --full-index "$commit" >"$dotest/patch"
else
git mailinfo $keep $no_inbody_headers $scissors $utf8 "$dotest/msg" "$dotest/patch" \
<"$dotest/$msgnum" >"$dotest/info" ||
diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
index f919c8d..8e32f19 100755
--- a/t/t7402-submodule-rebase.sh
+++ b/t/t7402-submodule-rebase.sh
@@ -3,7 +3,7 @@
# Copyright (c) 2008 Johannes Schindelin
#
-test_description='Test rebasing and stashing with dirty submodules'
+test_description='Test rebasing, stashing, etc. with submodules'
. ./test-lib.sh
@@ -20,7 +20,8 @@ test_expect_success setup '
echo second line >> file &&
(cd submodule && git pull) &&
test_tick &&
- git commit -m file-and-submodule -a
+ git commit -m file-and-submodule -a &&
+ git branch added-submodule
'
@@ -89,4 +90,29 @@ test_expect_success 'stash with a dirty submodule' '
'
+test_expect_success 'rebasing submodule that should conflict' '
+ git reset --hard &&
+ git checkout added-submodule &&
+ git add submodule &&
+ test_tick &&
+ git commit -m third &&
+ (
+ cd submodule &&
+ git commit --allow-empty -m extra
+ ) &&
+ git add submodule &&
+ test_tick &&
+ git commit -m fourth &&
+
+ test_must_fail git rebase --onto HEAD^^ HEAD^ HEAD^0 &&
+ git ls-files -s submodule >actual &&
+ (
+ cd submodule &&
+ echo "160000 $(git rev-parse HEAD^) 1 submodule" &&
+ echo "160000 $(git rev-parse HEAD^^) 2 submodule" &&
+ echo "160000 $(git rev-parse HEAD) 3 submodule"
+ ) >expect &&
+ test_cmp expect actual
+'
+
test_done
--
1.8.1.2.612.g09f4be5
^ permalink raw reply related
* [PATCH 0/3] rebasing changes that update submodules
From: Junio C Hamano @ 2013-02-01 4:32 UTC (permalink / raw)
To: git; +Cc: Heiko Voigt, Martin von Zweigbergk
In-Reply-To: <20130130224904.GB1053@book.hvoigt.net>
"git rebase" used to use "format-patch --full-index" to generate a
series of patches and let "git am --rebase" reconstruct a fake
preimage tree by reading the object names from the "index" lines.
With a230949 (am --rebasing: get patch body from commit, not from
mailbox, 2012-06-26), we switched to use "diff-tree", but forgot
that without "--full-index", the information generated on the
"index" line is insufficient to recreate a preimage tree that
contains a submodule.
The first one is the real fix to this issue.
The other two falls into the niceties category; they are to issue a
better error messge when the machinery is fed an abbreviated object
name on the "index" line for a submodule.
Junio C Hamano (3):
git-am: record full index line in the patch used while rebasing
apply: simplify build_fake_ancestor()
apply: diagnose incomplete submodule object name better
builtin/apply.c | 30 ++++++++++++++++--------------
git-am.sh | 2 +-
t/t7402-submodule-rebase.sh | 30 ++++++++++++++++++++++++++++--
3 files changed, 45 insertions(+), 17 deletions(-)
--
1.8.1.2.612.g09f4be5
^ permalink raw reply
* Re: "sha1 information is lacking or useless" when rebasing with a submodule pointer conflict
From: Junio C Hamano @ 2013-02-01 4:01 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Michael Sims, git, Jens Lehmann, Martin von Zweigbergk
In-Reply-To: <20130130224904.GB1053@book.hvoigt.net>
Heiko Voigt <hvoigt@hvoigt.net> writes:
> Maybe Martin or Junio immediately see whats going wrong here? I would
> need to further dig into the git-am code to find out how to fix it.
Thanks. I think the minimum fix is this, but there are some nits to
pick around this area as well.
diff --git b/git-am.sh a/git-am.sh
index b4d95f5..202130f 100755
--- b/git-am.sh
+++ a/git-am.sh
@@ -664,7 +664,7 @@ do
sed -e '1,/^$/d' >"$dotest/msg-clean"
echo "$commit" >"$dotest/original-commit"
get_author_ident_from_commit "$commit" >"$dotest/author-script"
- git diff-tree --root --binary "$commit" >"$dotest/patch"
+ git diff-tree --root --binary --full-index "$commit" >"$dotest/patch"
else
git mailinfo $keep $no_inbody_headers $scissors $utf8 "$dotest/msg" "$dotest/patch" \
<"$dotest/$msgnum" >"$dotest/info" ||
^ permalink raw reply related
* Re: Segmentation fault with latest git (070c57df)
From: Junio C Hamano @ 2013-02-01 1:40 UTC (permalink / raw)
To: jongman.heo; +Cc: Jeff King, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <15540714.372461359682286477.JavaMail.weblogic@epml08>
허종만 <jongman.heo@samsung.com> writes:
> But usually when I build upstream Linux kernel, I don't do "make
> clean" after git pull.. I didn't expect that I needed "make
> clean" for git build.
We don't expect anybody need "make clean", either. There is
something wrong in the dependency.
^ permalink raw reply
* Re: Re: Segmentation fault with latest git (070c57df)
From: 허종만 @ 2013-02-01 1:31 UTC (permalink / raw)
To: Jeff King, Thomas Rast; +Cc: git, Antoine Pelisse
>
[snip]
> Good point. Unfortunately, I can't get either yours or mine to fail,
> neither with a recent version of gcc nor with gcc-4.1. But I can't
> convince git to fail, either. The only gcc-4.1 I have is Debian's
> 4.1.3 release, which is not quite what the OP has.
>
> > Or perhaps something in the build process went wrong, and fetch.c didn't
> > get the memo about the new field in the struct. Depending on stack
> > layout, the next variable might be the 'int i' right before the
> > 'string_list list' in the code, which could explain the value of 1.
>
> Yeah, that would make sense to me with respect to the behavior we are
> seeing, but that part of the Makefile should be pretty simple and
> bug-free, I'd think (and from the original report, it seems like he was
> able to reproduce it well enough to bisect). Still, trying a "make clean
> && make" might be worth it just to rule that out.
>
> Puzzled...
>
> -Peff
Hi, all,
Thomas's test code also returns "cmp is 0".
But "make clean && make" fixes my issue.
Sorry for the noise I made.
But usually when I build upstream Linux kernel, I don't do "make clean" after git pull..
I didn't expect that I needed "make clean" for git build. Thanks you guys.
Best regards,
Jongman Heo.
^ permalink raw reply
* Re: [PATCH 2/7] Undocument deprecated alias 'push.default=tracking'
From: Junio C Hamano @ 2013-02-01 1:15 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ævar Arnfjörð Bjarmason, Matthieu Moy, git,
Jeff King, Michael Haggerty
In-Reply-To: <20130131210002.GK27340@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
> [...]
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -1795,7 +1795,8 @@ push.default::
>> +
>> This is currently the default, but Git 2.0 will change the default
>> to `simple`.
>> -* `upstream` - push the current branch to its upstream branch.
>> +* `upstream` - push the current branch to its upstream branch
>> + (`tracking` is a deprecated synonym for this).
>
> I have already explained that I believe this is a bad idea and why and
> proposed an alternative. I take it that either we are
> miscommunicating or we fundamentally disagree about the role of
> documentation. :(
Whatever.
For tonight, I'll queue this version on 'pu' primarily because I do
not want to think about it anymore today and because I do not want
to see us forget that we have to fix this in some way, and this was
the only one that I can simply "git am" on this topic. It is not
because I want to say "this is the version we are going to use,
stfu!" This topic does not even deserve such inter-developer
tension, IMHO.
doc: mention tracking for pull.default
When looking at a configuration file edited long time ago, a user
may find 'pull.default = tracking' and wonder what it means, but
earlier we stopped mentioning this value, even though the code still
support it and more importantly, we have no intention to force old
timers to update their configuration files.
Instead of not mentioning it, add it to the description in a way
that makes it clear that users have no reason to add new uses of it
preferring over 'upstream', by not listing it as a separate item on
the same footing as other values but as a deprecated synonym of the
'upstream' in its description.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
^ permalink raw reply
* Re: [PATCH] The images from picon and gravatar are always used over http://, and browsers give mixed contents warning when gitweb is served over https://.
From: Junio C Hamano @ 2013-02-01 0:58 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Bryan Turner, git, Andrej E Baranov
In-Reply-To: <20130131012853.GD15680@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> Odd. "https://www.gravatar.com/" also seems to work. I've put in a
>>> technical support query to find out what the Gravatar admins prefer.
>>
>> Thanks; will hold onto Andrej's patch until we hear what the story
>> is.
>
> Good news: a kind person from Automattic answered that
> www.gravatar.com should work fine over SSL, both now and in the
> future, and promised to add updating documentation to their todo list.
>
> Thanks for your help and patience.
I'll merge Andrej's topic to 'next' in the next integration cycle.
The fix should hit 'master' no later than the beginning of next
week.
Thanks.
^ permalink raw reply
* Re: Aw: Re: [PATCH v3 3/6] Change 'git' to 'Git' whenever the whole system is referred to #2
From: Junio C Hamano @ 2013-02-01 0:46 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: git, davvid
In-Reply-To: <50804391.814945.1358931502165.JavaMail.ngmail@webmail07.arcor-online.net>
Thomas Ackermann <th.acker@arcor.de> writes:
> >
>> Thomas, I do not want to see many rounds of entire rerolls of this
>> series on the list (nobody will look at the whole series multiple
>> times with fine toothed comb). I do not think you want to do that
>> either. Can you collect remaining fixups like David's message, turn
>> them into patch form when you have collected enough to be reviewed
>> in one sitting (say, a patchfile at around 200 lines), and send them
>> over to the list to apply on top of the tree of that commit?
>>
> Sure!
I think we have waited long enough and as far as I recall we didn't
see any reports of misconversion or forgotten conversion, so I'll
squash the fixes parked on the topic branch, whose tip is at
bfb8e1eb6375afb (fixup! Change 'git' to 'Git' whenever the whole
system is referred to #4, 2013-01-22), and merge the result to
'next' sometime tomorrow.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] docs: clarify git-branch --list behavior
From: Junio C Hamano @ 2013-02-01 0:37 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Jeff King, Peter Wu, git
In-Reply-To: <CAPig+cT21Fx6BaD=1jT7KSBZSZkWu-A-0U+QsM2qO8g5gki=YQ@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Thu, Jan 31, 2013 at 1:45 AM, Jeff King <peff@peff.net> wrote:
>> +If `--list` is given, or if there are no non-option arguments, existing
>> +branches are listed; the current branch will be highlighted with an
>> +asterisk. Option `-r` causes the remote-tracking branches to be listed,
>> +and option `-a` shows both local and remote branches. If a `<pattern>`
>> +is given, it is used as a shell wildcard to restrict the output to
>> +matching branches. If multiple patterns are given, a branch is shown if
>> +any it is matched by any of the patterns.
>
> s/if any it is/if it is/
Thanks; I'll squash this in. Peff, no need to resend.
Documentation/git-branch.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 01aa87f..2635dee 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -28,7 +28,7 @@ asterisk. Option `-r` causes the remote-tracking branches to be listed,
and option `-a` shows both local and remote branches. If a `<pattern>`
is given, it is used as a shell wildcard to restrict the output to
matching branches. If multiple patterns are given, a branch is shown if
-any it is matched by any of the patterns. Note that when providing a
+it matches any of the patterns. Note that when providing a
`<pattern>`, you must use `--list`; otherwise the command is interpreted
as branch creation.
^ permalink raw reply related
* Re: [PATCH 1/2] docs: clarify git-branch --list behavior
From: Eric Sunshine @ 2013-02-01 0:20 UTC (permalink / raw)
To: Jeff King; +Cc: Peter Wu, git
In-Reply-To: <20130131064545.GA25315@sigill.intra.peff.net>
On Thu, Jan 31, 2013 at 1:45 AM, Jeff King <peff@peff.net> wrote:
> +If `--list` is given, or if there are no non-option arguments, existing
> +branches are listed; the current branch will be highlighted with an
> +asterisk. Option `-r` causes the remote-tracking branches to be listed,
> +and option `-a` shows both local and remote branches. If a `<pattern>`
> +is given, it is used as a shell wildcard to restrict the output to
> +matching branches. If multiple patterns are given, a branch is shown if
> +any it is matched by any of the patterns.
s/if any it is/if it is/
-- ES
^ permalink raw reply
* Re: [PATCH 2/3] run-command: Be more informative about what failed
From: Junio C Hamano @ 2013-01-31 23:57 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, Jeff King
In-Reply-To: <20130131223559.GC21729@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> But it is not much code, and sometimes it is surprising what information
> can be helpful when debugging, so like you, I am not opposed, just
> doubtful.
Yes, exactly my feeling.
Perhaps I should just amend the 'stdin' and friends away without
asking Stephen to reroll. In the other two I did not see any
issues. I've queued all three of them including this one but as
separate topics.
Thanks.
^ permalink raw reply
* Re: [PATCH] Rename {git- => git}remote-helpers.txt
From: Junio C Hamano @ 2013-01-31 23:49 UTC (permalink / raw)
To: John Keeping; +Cc: git, Matthieu Moy, Max Horn
In-Reply-To: <20130131225620.GQ1342@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> On Thu, Jan 31, 2013 at 02:43:20PM -0800, Junio C Hamano wrote:
>> John Keeping <john@keeping.me.uk> writes:
>>
>> > Yes. Should I change it to "git-remote-helpers.html.in" and then copy
>> > it into place? That seems like the simplest answer and means that
>> > "*.html" will continue to refer only to generated files.
>>
>> I'd like to see if we can have a way to keep its look as the default
>> css gets updated without maintainance burden.
>>
>> How about using AsciiDoc instead of "cp", perhaps like this on top
>> of your patch?
>
> I tried AsciiDoc first but didn't like the output. I think putting "See
> gitremote-helpers" in the synopsis is the magic I was missing. This
> looks good to me.
Actually I didn't mean "take this patch, it works". I've queued a
slightly different version and will push it out as part of 'pu'
later.
Thanks.
^ 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