Git development
 help / color / mirror / Atom feed
* [PATCH] git-mergetool: check return value from read
From: Jay Soffian @ 2011-09-19 23:40 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian, David Aguilar, Junio C Hamano
In-Reply-To: <7vaaa09skn.fsf@alter.siamese.dyndns.org>

Mostly fixed already by 6b44577 (mergetool: check return value
from read, 2011-07-01). Catch two uses it missed.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
On Mon, Sep 19, 2011 at 4:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
> We still seem to miss one "read" unchecked in resolve_symlink_merge(),
> even with this patch.
> [...]
> I suspect that it would be more consistent with 6b44577 (mergetool: check
> return value from read, 2011-07-01), which this patch is a follow-up to,
> to do:
>
> 	read answer || return 1
>
> here.

Thanks, sorry I missed that.

 git-mergetool--lib.sh |    2 +-
 git-mergetool.sh      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 8fc65d0400..ed630b208a 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -21,7 +21,7 @@ check_unchanged () {
 		do
 			echo "$MERGED seems unchanged."
 			printf "Was the merge successful? [y/n] "
-			read answer
+			read answer || return 1
 			case "$answer" in
 			y*|Y*) status=0; break ;;
 			n*|N*) status=1; break ;;
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 3c157bcd26..b6d463f0d0 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -72,7 +72,7 @@ describe_file () {
 resolve_symlink_merge () {
     while true; do
 	printf "Use (l)ocal or (r)emote, or (a)bort? "
-	read ans
+	read ans || return 1
 	case "$ans" in
 	    [lL]*)
 		git checkout-index -f --stage=2 -- "$MERGED"
-- 
1.7.7.rc2.2.gdf97720

^ permalink raw reply related

* Re: [PATCH/RFCv4 1/4] Documentation: Preparation for gitweb manpages
From: Jonathan Nieder @ 2011-09-19 23:37 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Drew Northup, John 'Warthog9' Hawley, admin
In-Reply-To: <1316352884-26193-2-git-send-email-jnareb@gmail.com>

Jakub Narebski wrote:

> Gitweb documentation currently consist of gitweb/README, gitweb/INSTALL
> and comments in gitweb source code.  This is harder to find, use and
> browse that manpages ("man gitweb" or "git help gitweb") and HTML
> documentation ("git help --web gitweb").

Language nits: s/consist/consists/; s/that manpages/than manpages/.  I
completely agree.

> The goal is to move documentation out of gitweb/README to gitweb.txt and
> gitweb.conf.txt manpages, reducing its size 10x from around 500 to
> around 50 lines (two pages), and move information not related drectly to
> building and installing gitweb out of gitweb/INSTALL there.

I guess you mean this patch prepares for or is part of a larger
project or series with that goal?  Wording nits: s/and move
information/and to move information/; s/drectly/directly/; s/ there//.
 
> The idea is to have gitweb manpage sources reside in AsciiDoc format
> in Documentation/ directory, like for gitk and git-gui.  Alternate
> solution would be to have gitweb documentation in gitweb/ directory,
> perhaps in POD format (see perlpod(1)).

Language nits: missing "the" before "Documentation/ directory" and
"gitweb manpage sources"; missing "An" before "Alternate solution".
I guess this is the most controversial aspect of the patch; your idea
seems sane enough to me.

> This patch adds infrastructure for easy generating gitweb-related
> manpages.  It adds currently empty 'gitweb-doc' target to
> Documentation/Makefile, and 'doc' proxy target to gitweb's Makefile.

Language nits: s/easy/easily/; missing "a" before "currently empty
'gitweb-doc' target" and "'doc' proxy target".

> This way to build gitweb documentation one can use
>
>   make -C gitweb doc
>
> or
>
>   cd gitweb; make doc

Language nit: a comma after "This way" would disambiguate.

Does "make -CDocumentation man html" build the gitweb documentation,
too (and "make install-doc" install it)?

[...]
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -170,6 +170,9 @@ info: git.info gitman.info
>  
>  pdf: user-manual.pdf
>  
> +GITWEB_DOC = $(filter gitweb.%,$(DOC_HTML) $(DOC_MAN1) $(DOC_MAN5) $(DOC_MAN7))
> +gitweb-doc: $(GITWEB_DOC)

Looks like no, alas.

Except for that detail, this looks good.

Thanks,
Jonathan

^ permalink raw reply

* Re: how to remove unreachable objects?
From: Jeff King @ 2011-09-19 22:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: dieter, git
In-Reply-To: <7vsjns8b6m.fsf@alter.siamese.dyndns.org>

On Mon, Sep 19, 2011 at 02:38:41PM -0700, Junio C Hamano wrote:

> Yeah, I've been thinking about making it an error to give refs to fsck, as
> I do not think the use cases for feature justifies the possible confusion
> it may cause.
>
> One possible use case might be when your repository is corrupt, and does
> not pass "git fsck" (without any argument).  In such a case, if you are
> lucky and your disk corrupted objects only reachable from a recent topic
> branch, you might find that this command:
> 
> 	$ git fsck master next ...list other topics here...
> 
> still succeeds, so that you can figure out which topic makes such a
> limited fsck fail when it is listed on the command line, judge its
> importance and resurrect what you can from there, before nuking it to
> bring the repository back in health so that you can recreate the topic.

Does that work? I had the impression from the documentation that the
arguments are purely about the reachability analysis, and that the
actual corruption/correctness checks actually look through the object db
directly, making sure each object is well-formed. Skimming cmd_fsck
seems to confirm that.

-Peff

^ permalink raw reply

* Re: gitk: 'j' and 'k' keyboard shortcuts backward
From: Klaus Robert Suetterlin @ 2011-09-19 22:05 UTC (permalink / raw)
  To: Clemens Buchacher
  Cc: Jonathan Nieder, Josh Triplett, Pat Thoyts, Paul Mackerras,
	git@vger.kernel.org
In-Reply-To: <20110919180449.GA2677@ecki>

Fine with me, too.

--Robert S.



Am 19.09.2011 um 20:04 schrieb Clemens Buchacher <drizzd@aon.at>:

> On Mon, Sep 19, 2011 at 11:49:50AM -0500, Jonathan Nieder wrote:
>> 
>> How about this patch?
>> 
>> -- >8 --
>> Subject: gitk: Make vi-style keybindings more vi-like
> 
> Fine by me.
> 
> Clemens

^ permalink raw reply

* Re: upgrading GIT from version 1.7.0.4 to 1.7.6.1.
From: Jeff King @ 2011-09-19 22:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Ericsson, Mikaël, git
In-Reply-To: <7vwrd48bq8.fsf@alter.siamese.dyndns.org>

On Mon, Sep 19, 2011 at 02:26:55PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > [2] I suspect a similar thing happened with turning on packed refs
> >     (around the v1.4.4 era?), but I didn't dig around for details.
> 
> Also when bisecting down to really ancient versions of git, you may get
> complaints from the configuration reader. E.g.
> 
>     $ rungit v1.0.0 ls-files
>     fatal: bad config file line 78 in .git/config
>     $ sed -n 78p .git/config
>     [remote "git-gui"]

Ugh, yeah. There are similar problems with some specific config options
(one I run into a lot these days is that "pager.foo" used to complain if
the value was non-boolean, but now you can put in an arbitrary command).
But those only bite you if you start using the new config options (which
you obviously are not doing until after you upgrade).

But your example will bite anyone because git generates remote config
like that by default.

Anyway, most of that is only applicable if you are crossing several
major version boundaries, or are using ancient versions of git. I'm
pretty sure that v1.7.0.x -> v1.7.6.x is pretty safe.

-Peff

^ permalink raw reply

* Re: [PATCH] git-web--browse: invoke kfmclient directly
From: Junio C Hamano @ 2011-09-19 22:28 UTC (permalink / raw)
  To: Jeff King; +Cc: Andreas Schwab, Chris Packham, git, chriscool, jepler
In-Reply-To: <20110919222325.GA4056@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Yeah. Doing:
>
>   eval '$browser_cmd'
>
> will do the whitespace-breaking we want, but it won't interpret actual
> shell magic characters, which we need in order to be compatible with
> other parts of git (which typically use "sh -c ..."). E.g.:
>
>   foo=worked
>   browser_cmd='echo $foo'

Yikes, I forgot that we do that. You are right.

^ permalink raw reply

* Re: [PATCH] git-web--browse: invoke kfmclient directly
From: Jeff King @ 2011-09-19 22:23 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Junio C Hamano, Chris Packham, git, chriscool, jepler
In-Reply-To: <m2bougtdc2.fsf@igel.home>

On Mon, Sep 19, 2011 at 11:46:37PM +0200, Andreas Schwab wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Thinking about it a bit more, I suspect that we should just let the 'eval'
> > grab value out of the $browser_cmd variable, i.e.
> >
> > 	eval '$browser_cmd "$@"'
> >
> > no?
> 
> That's a Useless Use of Eval and 100% equivalent to this:
> 
> $browser_cmd "$@"

Yeah. Doing:

  eval '$browser_cmd'

will do the whitespace-breaking we want, but it won't interpret actual
shell magic characters, which we need in order to be compatible with
other parts of git (which typically use "sh -c ..."). E.g.:

  foo=worked
  browser_cmd='echo $foo'
  # fail
  $browser_cmd
  # fail
  eval '$browser_cmd'
  # works
  eval "$browser_cmd"

-Peff

^ permalink raw reply

* Re: Where is information of "git read-tree" stored?
From: Junio C Hamano @ 2011-09-19 22:09 UTC (permalink / raw)
  To: Manuel Reimer; +Cc: git
In-Reply-To: <j586pb$emh$1@dough.gmane.org>

Manuel Reimer <Manuel.Spam@nurfuerspam.de> writes:

>> That "how to" may be badly written and this may have been unclear to you
>> but the first four steps are to be done _only once_ to set things up, and
>> after that you need to run only the fifth step whenever you want to update
>> from the Bproject. Could you suggest a better wording to update the doc?
>
> As long as I don't understand what's going on here, I can't suggest
> how to improve the documentation.
>
>> The very first "subtree merge" (the one that is recorded with the commit
>> after the read-tree) records all paths from Bproject renamed to elsewhere
>> in the merge result (you can view it with "git show -M
>> $that_merge_commit")
>
> Tried that, but the stuff, I saw on screen, doesn't make clear how GIT
> knew about what to do here.

To a certain degree, the point of a tool is that the user does not need to
know about the details, but if you are interested...

Suppose you have this tree structure in your "original" project:

        Documentation/README.txt
        hello.c
	Makefile

and then somebody else has this structure in his project (in your case, it
may happen to be stored in SVN but once it is slurped in a git repository,
it does not matter):

        goodbye.c
	Makefile

Further suppose that you would want to end up with this tree structure:

        Documentation/README.txt
	Makefile
        hello.c
        imported/Makefile
        imported/goodbye.c

i.e. you would want to move stuff that came from the other project in imported/
hierarchy.  There may be many other files, and even subdirectories, in the
other project, but they all are shifted one level down and placed in imported/
hierarchy.

The first four steps of the howto is to create such a final tree structure
and make a merge commit out of that tree.

After you update your project (which now has both the original files such
as hello.c etc., may have added new files, and may even have updated stuff
inside imported/ hierarchy) and the other side updated their project (e.g.
it may have updated goodbye.c whose change you would want to carry over to
your imported/goodbye.c, or it may have added a new file welcome.c, which
you would want to import as imported/welcome.c), you would invoke "pull -s
subtree", which in turn runs "merge -s subtree".

The subtree strategy first compares the shapes of two trees being merged,
and tries to find how much they have to be shifted to match.  Your tree
may now have:

        Documentation/README.txt
	Makefile
	hello.h (added)
        hello.c
        imported/Makefile
        imported/goodbye.c

while the other side may now have:

        goodbye.c
	Makefile
	welcome.c

The subtree strategy notices that by prefixing "imported/" in front of the
paths, the tree from the other side will match the shape of the subtree
you have under "imported/". Thus it can pair:

	their "goodbye.c" with your "imported/goodbye.c"
        their "Makefile" with your "imported/Makefile"
        their "welcome.c" with your "imported/welcome.c"

and merge the changes. The common ancestor commit of this merge will be
the initial merge you made with the first 4-step, so the three-way merge
logic would notice that there wasn't "welcome.c" in the beginning, they
added that path, while you did not do anything to the path that
corresponds to it (namely, "imported/welcome.c"), so the new "welcome.c"
file from the other project would simply be copied as "imported/welcome.c"
to your tree, and the change they made to "goodbye.c" and your changes you
made to your "imported/goodbye.c" will be merged and result is recorded in
your "imported/goodbye.c".

If "compares the shape and figures out how much to shift" makes you feel
uneasy (and it probably should), you can give an explicit directory prefix 
as the backend option "subtree" (see "git merge help" for details).

^ permalink raw reply

* Git 1.7.6: Sparse checkouts do not work with directory exclusions
From: Joshua Jensen @ 2011-09-19 22:03 UTC (permalink / raw)
  To: git@vger.kernel.org

Sometime after Git 1.7.3.2, sparse checkouts stopped working for me.  My 
sparse-checkout file looks something like:

*
!DirA/
!DirB/
DirC/

I have restored some lines of code that were removed in November 2010.  
This resolves the sparse checkout issue for me, but my guess is the 
solution is not implemented properly.

Can anyone confirm the issue and describe why those lines were removed?

Thanks.

Josh

---
  dir.c |    5 ++++-
  1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/dir.c b/dir.c
index 6e4494e..3f057b6 100644
--- a/dir.c
+++ b/dir.c
@@ -507,7 +507,10 @@ int excluded_from_list(const char *pathname,
              int to_exclude = x->to_exclude;

              if (x->flags & EXC_FLAG_MUSTBEDIR) {
-                if (*dtype == DT_UNKNOWN)
+                if (!prefixcmp(pathname, exclude) &&
+                    pathname[x->patternlen] == '/')
+                    return to_exclude;
+                else if (*dtype == DT_UNKNOWN)
                      *dtype = get_dtype(NULL, pathname, pathlen);
                  if (*dtype != DT_DIR)
                      continue;
-- 
1.7.6.msysgit.1

^ permalink raw reply related

* Re: [PATCH] git-web--browse: invoke kfmclient directly
From: Andreas Schwab @ 2011-09-19 21:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Chris Packham, git, chriscool, jepler
In-Reply-To: <7v1uvc9qhz.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Thinking about it a bit more, I suspect that we should just let the 'eval'
> grab value out of the $browser_cmd variable, i.e.
>
> 	eval '$browser_cmd "$@"'
>
> no?

That's a Useless Use of Eval and 100% equivalent to this:

$browser_cmd "$@"

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: how to remove unreachable objects?
From: Junio C Hamano @ 2011-09-19 21:38 UTC (permalink / raw)
  To: Jeff King; +Cc: dieter, git
In-Reply-To: <20110919201804.GB31930@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I was tempted to just drop this section entirely. It's mostly redundant
> with the DESCRIPTION section, and any extra details could be folded in
> there. The most useful bit is the "what do you do when there is
> corruption". But that should perhaps get its own section, if somebody
> feels like writing something more detailed (I thought we had a guide
> somewhere, but I couldn't find it).

Yeah, I've been thinking about making it an error to give refs to fsck, as
I do not think the use cases for feature justifies the possible confusion
it may cause.

One possible use case might be when your repository is corrupt, and does
not pass "git fsck" (without any argument).  In such a case, if you are
lucky and your disk corrupted objects only reachable from a recent topic
branch, you might find that this command:

	$ git fsck master next ...list other topics here...

still succeeds, so that you can figure out which topic makes such a
limited fsck fail when it is listed on the command line, judge its
importance and resurrect what you can from there, before nuking it to
bring the repository back in health so that you can recreate the topic.

^ permalink raw reply

* Re: [PATCH] git-web--browse: invoke kfmclient directly
From: Jakub Narebski @ 2011-09-19 21:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Chris Packham, git, chriscool, jepler
In-Reply-To: <7vvcso9zzi.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
> 
> > If you want to make a test, I think you would do better with something
> > like:
> >
> >   echo someurl_with_&_in_it >expect &&
> >   git config browser.custom.cmd echo &&
> >   git web--browse --browser=custom someurl_with_&_in_it >actual &&
> >   test_cmp expect actual
> >
> > That won't test that we are invoking kfmclient correctly, obviously, but
> > you can confirm at least that URLs are making it through to the browser
> > script intact.
> 
> Hmm, isn't '&' somewhat an unusual in URL? ...ah, not really, if it is in
> the query parameter part it is quite common.

In newstyle URLs the name=value pairs in CGI parameter query string
are separated with semicolons ';' rather than ampersands '&', because
of problem with & <-> &amp;.

Just FYI.
-- 
Jakub Narębski

^ permalink raw reply

* Re: upgrading GIT from version 1.7.0.4 to 1.7.6.1.
From: Junio C Hamano @ 2011-09-19 21:26 UTC (permalink / raw)
  To: Jeff King; +Cc: Andreas Ericsson, Mikaël, git
In-Reply-To: <20110919190023.GC26115@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> [2] I suspect a similar thing happened with turning on packed refs
>     (around the v1.4.4 era?), but I didn't dig around for details.

Also when bisecting down to really ancient versions of git, you may get
complaints from the configuration reader. E.g.

    $ rungit v1.0.0 ls-files
    fatal: bad config file line 78 in .git/config
    $ sed -n 78p .git/config
    [remote "git-gui"]

^ permalink raw reply

* Re: [PATCH] git-web--browse: invoke kfmclient directly
From: Junio C Hamano @ 2011-09-19 21:22 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Chris Packham, git, chriscool, jepler
In-Reply-To: <20110919204448.GA3562@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Sep 19, 2011 at 01:42:23PM -0700, Junio C Hamano wrote:
>
>> >> Yeah, I agree, and the dq around $browser_cmd is kind of important, too,
>> >> for that to work and be readable.
>> >
>> > Oops, good catch. Probably the most readable version would be:
>> >
>> >   eval "\"$browser_cmd\"" '"$@"'
>> 
>> Actually I didn't mean that double dq.
>> 
>> In fact, if browser_cmd is meant to be split as a shell snippet, I do not
>> think you want the string seen by eval to have dq around the expanded
>> version of $browser_cmd.  And I tend to prefer feeding a single string to
>> eval, so the version in your message I quoted originally looks good to me.
>> 
>> Unless I am missing something here...?
>
> Oh right. Sorry, I read your comment, thought that's what you meant, and
> that I had overlooked something. Forgetting that it was intentional to
> leave off the quotes inside.
>
> So yeah, my original is right. I just got turned around in all of the
> discussion.

Thinking about it a bit more, I suspect that we should just let the 'eval'
grab value out of the $browser_cmd variable, i.e.

	eval '$browser_cmd "$@"'

no?

^ permalink raw reply

* Re: [PATCH] git-web--browse: invoke kfmclient directly
From: Jeff King @ 2011-09-19 20:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Chris Packham, git, chriscool, jepler
In-Reply-To: <7v62ko9scw.fsf@alter.siamese.dyndns.org>

On Mon, Sep 19, 2011 at 01:42:23PM -0700, Junio C Hamano wrote:

> >> Yeah, I agree, and the dq around $browser_cmd is kind of important, too,
> >> for that to work and be readable.
> >
> > Oops, good catch. Probably the most readable version would be:
> >
> >   eval "\"$browser_cmd\"" '"$@"'
> 
> Actually I didn't mean that double dq.
> 
> In fact, if browser_cmd is meant to be split as a shell snippet, I do not
> think you want the string seen by eval to have dq around the expanded
> version of $browser_cmd.  And I tend to prefer feeding a single string to
> eval, so the version in your message I quoted originally looks good to me.
> 
> Unless I am missing something here...?

Oh right. Sorry, I read your comment, thought that's what you meant, and
that I had overlooked something. Forgetting that it was intentional to
leave off the quotes inside.

So yeah, my original is right. I just got turned around in all of the
discussion.

Sorry for the noise.

-Peff

^ permalink raw reply

* Re: [PATCH] git-web--browse: invoke kfmclient directly
From: Andreas Schwab @ 2011-09-19 20:44 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Chris Packham, git, chriscool, jepler
In-Reply-To: <20110919182049.GA26115@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Sep 19, 2011 at 10:57:37AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > Hmm. Actually, the one for custom browser commands might need it,
>> > because that one is expected to be a shell snippet. I suspect the
>> > simplest thing is to do something like:
>> >
>> >   eval "$browser_cmd \"\$@\""
>> 
>> Yeah, I agree, and the dq around $browser_cmd is kind of important, too,
>> for that to work and be readable.
>
> Oops, good catch. Probably the most readable version would be:
>
>   eval "\"$browser_cmd\"" '"$@"'

This make the use of eval even more questionable.  If $browser_cmd is
supposed to be whitespace splitted then this won't do it (unless it
contains embedded double quotes).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH] git-web--browse: invoke kfmclient directly
From: Junio C Hamano @ 2011-09-19 20:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Chris Packham, git, chriscool, jepler
In-Reply-To: <20110919182049.GA26115@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Sep 19, 2011 at 10:57:37AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > Hmm. Actually, the one for custom browser commands might need it,
>> > because that one is expected to be a shell snippet. I suspect the
>> > simplest thing is to do something like:
>> >
>> >   eval "$browser_cmd \"\$@\""
>> 
>> Yeah, I agree, and the dq around $browser_cmd is kind of important, too,
>> for that to work and be readable.
>
> Oops, good catch. Probably the most readable version would be:
>
>   eval "\"$browser_cmd\"" '"$@"'

Actually I didn't mean that double dq.

In fact, if browser_cmd is meant to be split as a shell snippet, I do not
think you want the string seen by eval to have dq around the expanded
version of $browser_cmd.  And I tend to prefer feeding a single string to
eval, so the version in your message I quoted originally looks good to me.

Unless I am missing something here...?

^ permalink raw reply

* Re: [PATCH] git-mergetool--lib.sh: make check_unchanged return 1 on invalid read
From: Junio C Hamano @ 2011-09-19 20:37 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, David Aguilar
In-Reply-To: <1316462592-27255-1-git-send-email-jaysoffian@gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> Else when the user hits ctrl-c at the "Was the merge successful?
> [y/n]" prompt, mergetool goes into an infinite loop asking
> for input.
>
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>

We still seem to miss one "read" unchecked in resolve_symlink_merge(),
even with this patch.

>  git-mergetool--lib.sh |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 8fc65d0400..0eb424484c 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -21,7 +21,11 @@ check_unchanged () {
>  		do
>  			echo "$MERGED seems unchanged."
>  			printf "Was the merge successful? [y/n] "
> -			read answer
> +			if ! read answer
> +			then
> +				status=1
> +				break
> +			fi

I suspect that it would be more consistent with 6b44577 (mergetool: check
return value from read, 2011-07-01), which this patch is a follow-up to,
to do:

	read answer || return 1

here.

Thanks.

^ permalink raw reply

* Re: how to remove unreachable objects?
From: Andreas Schwab @ 2011-09-19 20:36 UTC (permalink / raw)
  To: dieter; +Cc: git
In-Reply-To: <20110919110831.ewq03vnqos4w8cs8@webmail.edis.at>

dieter@schoen.or.at writes:

> if you look at the output of this script then you see that git knows  
> that there
> are unreachable/dangling objects, but they remain.

You also need to prune the reflog of HEAD.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: how to remove unreachable objects?
From: Jeff King @ 2011-09-19 20:18 UTC (permalink / raw)
  To: dieter; +Cc: Junio C Hamano, git
In-Reply-To: <20110919195335.GA31930@sigill.intra.peff.net>

On Mon, Sep 19, 2011 at 03:53:36PM -0400, Jeff King wrote:

> > git fsck --unreachable HEAD \
> >                     $(git for-each-ref --format="%(objectname)" refs/heads)
> 
> And this shows it as unreachable, because you are asking git to only
> look at the branch tips and HEAD (by default, it looks at all refs and
> reflogs).
> 
> I suspect you copied this straight from the git-fsck manpage. That
> advice is a bit outdated, I think. It blames (in some form) all the way
> back to the original documentation added in c64b9b8 (2005-05-05, only a
> few weeks after git was born). A few weeks later, fsck learned to
> default to looking at all refs (1024932, 2005-05-18). And then other
> sane defaults like reflogs got tacked on later (reflogs came around the
> 1.4.x era, in 2006).

So we should probably do something like this:

-- >8 --
Subject: [PATCH] docs: brush up obsolete bits of git-fsck manpage

After the description and options, the fsck manpage contains
some discussion about what it does. Over time, this
discussion has become somewhat obsolete, both in content and
formatting. In particular:

  1. There are many options now, so starting the discussion
     with "It tests..." makes it unclear whether we are
     talking about the last option, or about the tool in
     general. Let's start a new "discussion" section and
     make our antecedent more clear.

  2. It gave an example for --unreachable using for-each-ref
     to mention all of the heads, saying that it will do "a
     _lot_ of verification". This is hopelessly out-of-date,
     as giving no arguments will check much more (reflogs,
     the index, non-head refs).

  3. It goes on to mention tests "to be added" (like tree
     object sorting). We now have these tests.

Signed-off-by: Jeff King <peff@peff.net>
---
I was tempted to just drop this section entirely. It's mostly redundant
with the DESCRIPTION section, and any extra details could be folded in
there. The most useful bit is the "what do you do when there is
corruption". But that should perhaps get its own section, if somebody
feels like writing something more detailed (I thought we had a guide
somewhere, but I couldn't find it).

 Documentation/git-fsck.txt |   26 ++++++++------------------
 1 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index a2a508d..55b33d7 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -72,30 +72,20 @@ index file, all SHA1 references in .git/refs/*, and all reflogs (unless
 	a blob, the contents are written into the file, rather than
 	its object name.
 
-It tests SHA1 and general object sanity, and it does full tracking of
-the resulting reachability and everything else. It prints out any
-corruption it finds (missing or bad objects), and if you use the
-'--unreachable' flag it will also print out objects that exist but
-that aren't reachable from any of the specified head nodes.
-
-So for example
-
-	git fsck --unreachable HEAD \
-		$(git for-each-ref --format="%(objectname)" refs/heads)
+DISCUSSION
+----------
 
-will do quite a _lot_ of verification on the tree. There are a few
-extra validity tests to be added (make sure that tree objects are
-sorted properly etc), but on the whole if 'git fsck' is happy, you
-do have a valid tree.
+git-fsck tests SHA1 and general object sanity, and it does full tracking
+of the resulting reachability and everything else. It prints out any
+corruption it finds (missing or bad objects), and if you use the
+'--unreachable' flag it will also print out objects that exist but that
+aren't reachable from any of the specified head nodes (or the default
+set, as mentioned above).
 
 Any corrupt objects you will have to find in backups or other archives
 (i.e., you can just remove them and do an 'rsync' with some other site in
 the hopes that somebody else has the object you have corrupted).
 
-Of course, "valid tree" doesn't mean that it wasn't generated by some
-evil person, and the end result might be crap. git is a revision
-tracking system, not a quality assurance system ;)
-
 Extracted Diagnostics
 ---------------------
 
-- 
1.7.7.rc1.3.gb95be

^ permalink raw reply related

* Re: [PATCH] t4014: remove Message-Id/timestamp before comparing patches
From: Junio C Hamano @ 2011-09-19 20:15 UTC (permalink / raw)
  To: Jeff King; +Cc: Thomas Rast, Pang Yan Han, git
In-Reply-To: <20110919191545.GD26115@sigill.intra.peff.net>

Thanks, your fix makes a lot more sense.

^ permalink raw reply

* Re: git checkout under 1.7.6 does not properly list untracked files and aborts
From: Joshua Jensen @ 2011-09-19 20:06 UTC (permalink / raw)
  To: git@vger.kernel.org
In-Reply-To: <4E779BA4.8070109@workspacewhiz.com>

----- Original Message -----
From: Joshua Jensen
Date: 9/19/2011 1:44 PM
> ----- Original Message -----
> From: Joshua Jensen
> Date: 9/9/2011 2:04 PM
>> This may be an msysGit 1.7.6 issue, as that is what I am using.  It 
>> also occurs in msysGit 1.7.5, but I am almost certain it did not 
>> happen in msysGit 1.7.2.
>>
>> Given an untracked file in the working directory that has been added 
>> to an alternate branch, when switching to that alternate branch, 'git 
>> checkout' exits with an error code but does not print anything to the 
>> console.  It should print an untracked file error.
>>
>> I have been trying to track this down in code.  The point where the 
>> error messages are printed, display_error_msgs, is never hit.
> Okay, so I've tracked this down, but I am unsure what the correct fix is.
>
> In unpack-trees.c's unpack_trees() function, there are some lines that 
> read:
>
>     if (ce->ce_flags & CE_ADDED &&
>         verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))
>             return -1;
>
> If the 'return -1' is changed to 'goto return_failed', then a proper 
> error message appears:
>
>     error: The following untracked working tree files would be 
> overwritten by checkout:
>         one/file/listed/here.txt
>     Please move or remove them before you can switch branches.
>     Aborting
>
> The thing is, there are multiple files that would be overwritten by 
> checkout, and I believe an older version of Git showed me the entire 
> list before aborting.
While probably not correct, the following patch collects all files that 
would be overwritten and logs them to the output:

---
  unpack-trees.c |   11 +++++++----
  1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index cc616c3..e1f7263 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -993,7 +993,7 @@ static int verify_absent(struct cache_entry *, enum 
unpack_trees_error_types, st
   */
  int unpack_trees(unsigned len, struct tree_desc *t, struct 
unpack_trees_options *o)
  {
-    int i, ret;
+    int i, ret = 0;
      static struct cache_entry *dfc;
      struct exclude_list el;

@@ -1100,9 +1100,10 @@ int unpack_trees(unsigned len, struct tree_desc 
*t, struct unpack_trees_options
               * Do the real check now because we have had
               * correct CE_NEW_SKIP_WORKTREE
               */
-            if (ce->ce_flags & CE_ADDED &&
-                verify_absent(ce, 
ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))
-                    return -1;
+            if (ce->ce_flags & CE_ADDED) {
+                int result = verify_absent(ce, 
ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o);
+                ret = min(ret, result);
+            }

              if (apply_sparse_checkout(ce, o)) {
                  ret = -1;
@@ -1112,6 +1113,8 @@ int unpack_trees(unsigned len, struct tree_desc 
*t, struct unpack_trees_options
                  empty_worktree = 0;

          }
+        if (ret == -1)
+            goto return_failed;
          if (o->result.cache_nr && empty_worktree) {
              /* dubious---why should this fail??? */
              ret = unpack_failed(o, "Sparse checkout leaves no entry on 
working directory");
-- 
1.7.6.msysgit.1

^ permalink raw reply related

* [PATCH] git-mergetool--lib.sh: make check_unchanged return 1 on invalid read
From: Jay Soffian @ 2011-09-19 20:03 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian, David Aguilar, Junio C Hamano

Else when the user hits ctrl-c at the "Was the merge successful?
[y/n]" prompt, mergetool goes into an infinite loop asking
for input.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 git-mergetool--lib.sh |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 8fc65d0400..0eb424484c 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -21,7 +21,11 @@ check_unchanged () {
 		do
 			echo "$MERGED seems unchanged."
 			printf "Was the merge successful? [y/n] "
-			read answer
+			if ! read answer
+			then
+				status=1
+				break
+			fi
 			case "$answer" in
 			y*|Y*) status=0; break ;;
 			n*|N*) status=1; break ;;
-- 
1.7.7.rc2.2.gf185

^ permalink raw reply related

* Re: Where is information of "git read-tree" stored?
From: Manuel Reimer @ 2011-09-19 19:52 UTC (permalink / raw)
  To: git
In-Reply-To: <7vzki0a0yd.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> It is unclear to me what you meant by "connection" "lost" and "recreated"
> above, but I am guessing I can ignore them, as long as I take it that you
> mean by that local mirror from the subversion repository "Bproject" below.

Yes. And the reference to it isn't pushed to the remote GIT server. If I clone 
the remote repo, then "git remote -v" doesn't show the reference anymore.

> That "how to" may be badly written and this may have been unclear to you
> but the first four steps are to be done _only once_ to set things up, and
> after that you need to run only the fifth step whenever you want to update
> from the Bproject. Could you suggest a better wording to update the doc?

As long as I don't understand what's going on here, I can't suggest how to 
improve the documentation.

> The very first "subtree merge" (the one that is recorded with the commit
> after the read-tree) records all paths from Bproject renamed to elsewhere
> in the merge result (you can view it with "git show -M
> $that_merge_commit")

Tried that, but the stuff, I saw on screen, doesn't make clear how GIT knew 
about what to do here.

I also still don't know how I can restore that whole setup if anything, I have 
on my local side, gets lost. Means, that anything, that I have left, is the 
stuff, I pushed to the remote GIT and the SVN, I want to have in my own tree. 
Which steps do I have to perform to restore the setup and have it working again 
without getting conflicts?

Maybe the risk of getting problems is too high and I should copy over stuff, I 
got from SVN, from time to time manually to my GIT repo?

Yours

Manuel

^ permalink raw reply

* Re: how to remove unreachable objects?
From: Jeff King @ 2011-09-19 19:53 UTC (permalink / raw)
  To: dieter; +Cc: git
In-Reply-To: <20110919110831.ewq03vnqos4w8cs8@webmail.edis.at>

On Mon, Sep 19, 2011 at 11:08:31AM +0200, dieter@schoen.or.at wrote:

> this is my use case:
> i create a repository and produce several commits on master.
> then i go back to a certain tag and create a new branch, where i also
> commit.
> then i switch back to master and delete (-D) the other branch.
> it should now be unreachable from within git (to prove its existence,
> i remember a commit SHA1 on the dead branch).

It will still be referenced by the HEAD reflog, won't it?

> git checkout master
> git branch -D $DEAD
> git show $dead_commit
> git fsck --unreachable --full --verbose

This shows it reachable, because it is connected from the HEAD reflog.

> git fsck --unreachable HEAD \
>                     $(git for-each-ref --format="%(objectname)" refs/heads)

And this shows it as unreachable, because you are asking git to only
look at the branch tips and HEAD (by default, it looks at all refs and
reflogs).

I suspect you copied this straight from the git-fsck manpage. That
advice is a bit outdated, I think. It blames (in some form) all the way
back to the original documentation added in c64b9b8 (2005-05-05, only a
few weeks after git was born). A few weeks later, fsck learned to
default to looking at all refs (1024932, 2005-05-18). And then other
sane defaults like reflogs got tacked on later (reflogs came around the
1.4.x era, in 2006).

> git fsck --lost-found
> git prune -v $dead_commit
> git prune $(git rev-parse --all)
> git repack
> git prune-packed
> git gc --prune=now
> git gc --aggressive
> git show $dead_commit

If you really want to make it unreachable, you should expire the
reflogs, too:

  git reflog expire --expire=now --all
  # will now report unreachable
  git fsck --unreachable
  # will now actually delete objects
  git prune -v
  # gives "bad object ..."
  git show $dead_commit

git-gc will do this for you, but of course the default expiration time
is much longer (I think something like 90 days).

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox