Git development
 help / color / mirror / Atom feed
* Re: Stashing individual files
From: Jeff King @ 2012-01-03 19:06 UTC (permalink / raw)
  To: Chris Leong; +Cc: git
In-Reply-To: <CAJ6vYjduoBNrRcvcvQbX_yY-3-Qm5ZbXOM0WQpWRwC1H1OCqaA@mail.gmail.com>

On Tue, Jan 03, 2012 at 10:32:02AM +1100, Chris Leong wrote:

> Thanks for making such a wonderful product. I find the stash command
> really useful, but it doesn't work very well when I just need to
> temporarily revert one or two files. I know that there is the
> interactive command, but if you have modified a large number of files,
> then it takes quite a bit of effort. Is there any way I can define an
> alias, stashfiles, so that I can just type git stashfiles file1 file2?
> Also, please consider adding such a feature into a future version.

I have sometimes wanted this, too. One problem is that the arguments in
a "stash save" get sucked into the message. I really wish it were:

  git stash save [-m <msg>] [[--] <pathspec...>]

which would match other git commands. And related, it would be nice to
have:

  git stash foo.c bar.c

but that conflicts with our safety-valve to avoid accidentally stashing
when no command is given.

For now, we could probably do it like this:

  git stash save [<message>] [-- <pathspec...>]

IOW, make the "--" a requirement for specifying filenames. The only
regression is that "--" as a single argument can no longer be used in
stash messages. So this works now:

  git stash save working on foo -- needs bar

but would be interpreted under my proposal as stashing "needs" and "bar"
with the message "working on foo". You would instead have to spell it:

  git stash save "working on foo -- needs bar"

I think that would be OK compromise, though. I'd rather not introduce a
whole new "stashfiles" command (or even a new subcommand of stash) if we
can avoid it.

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] daemon: add tests
From: Jeff King @ 2012-01-03 19:18 UTC (permalink / raw)
  To: Clemens Buchacher
  Cc: Jonathan Nieder, git, Junio C Hamano, Erik Faye-Lund,
	Ilari Liusvaara, Nguyễn Thái Ngọc Duy
In-Reply-To: <20120102194711.GA25296@ecki.lan>

On Mon, Jan 02, 2012 at 08:47:11PM +0100, Clemens Buchacher wrote:

> On Mon, Jan 02, 2012 at 03:25:08AM -0600, Jonathan Nieder wrote:
> > 
> > > [Subject: daemon: add tests]
> > 
> > Can't believe I missed this.  That seems like a worthy cause ---
> > can someone remind me why this is dropped, or if there are any
> > tweaks I can help with to get it picked up again?
> 
> We were discussing some open issues with patch 2/2, which was based
> on the tests. I later abandoned the idea for that patch. But the
> tests should be ok by themselves.

Yes, I'd like to see them included, even without the second patch. We
currently have zero tests for git-daemon, so even just verifying that
it starts and can let people fetch is an improvement.

-Peff

^ permalink raw reply

* Re: Odd remote: error: packfile ./objects/pack/pack-FOO.pack cannot be accessed
From: Jeff King @ 2012-01-03 19:33 UTC (permalink / raw)
  To: Sudarshan Wadkar; +Cc: git
In-Reply-To: <CAOoYcj0JO4q0GJRzuexQR6OAng2PdZX8gD7zNYNoOCmCCLqz4Q@mail.gmail.com>

On Mon, Jan 02, 2012 at 02:48:22PM +0530, Sudarshan Wadkar wrote:

> But when I push, I get this odd error from remote
> 
> $ git push --verbose --mirror
> ssh://wadkar@192.168.1.177:7185/~wadkar/repo/bare/myproj.git
> Pushing to ssh://wadkar@192.168.1.177:7185/~wadkar/repo/bare/myproj.git
> Counting objects: 5, done.
> Delta compression using up to 4 threads.
> Compressing objects: 100% (3/3), done.
> Writing objects: 100% (3/3), 323 bytes, done.
> Total 3 (delta 2), reused 0 (delta 0)
> remote: error: packfile
> ./objects/pack/pack-17900952dc824651db15369a341eec8d3e8f39d2.pack
> cannot be accessed
> remote: HEAD is now at 4d5a6f1 Investigate and report odd error
> To ssh://wadkar@192.168.1.177:7185/~wadkar/repo/bare/myproj.git
>    d066a2f..4d5a6f1  master -> master

Is it always the same pack? If so, have you tried looking in the
objects/pack directory to make sure it's not a permissions problem?

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] daemon: add tests
From: Junio C Hamano @ 2012-01-03 19:34 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Clemens Buchacher, git, Jeff King, Erik Faye-Lund,
	Ilari Liusvaara, Nguyễn Thái Ngọc Duy
In-Reply-To: <20120102092508.GA10977@elie.hsd1.il.comcast.net>

Jonathan Nieder <jrnieder@gmail.com> writes:

> (+cc: Erik, Ilari, Duy)
> Hi,
>
> Clemens Buchacher wrote:
>
>> [Subject: daemon: add tests]
>
> Can't believe I missed this.  That seems like a worthy cause ---
> can someone remind me why this is dropped, or if there are any
> tweaks I can help with to get it picked up again?

Thanks for your interest in this.

>> diff --git a/t/lib-daemon.sh b/t/lib-daemon.sh
>> new file mode 100644
>> index 0000000..30a89ea
>> --- /dev/null
>> +++ b/t/lib-daemon.sh
>> @@ -0,0 +1,52 @@
>> +#!/bin/sh
>> +
>> +if test -z "$GIT_TEST_DAEMON"
>> +then
>> +	skip_all="Daemon testing disabled (define GIT_TEST_DAEMON to enable)"
>> +	test_done
>> +fi
>> +
>> +LIB_DAEMON_PORT=${LIB_DAEMON_PORT-'8121'}

In lib-httpd.sh, LIB_HTTPD_PORT is defined in a similar way, but that is
always overridden by the users and the convention there is to use the test
numbers (cf. "git grep LIB_HTTPD_PORT t/"), which should be followed here
as well.

I am not very keen on the "lib-daemon.sh", GIT_TEST_DAEMON, etc. naming to
pretend as if "git daemon" will forever be the only daemon we will ever
ship, by the way.  We might one day want to add an inotify daemon, a
daemon for the git-pubsub protocol or somesuch.

>> +DAEMON_PID=
>> +DAEMON_DOCUMENT_ROOT_PATH="$PWD"/repo
>> +DAEMON_URL=git://127.0.0.1:$LIB_DAEMON_PORT
>> +
>> +start_daemon() {
>> +	if test -n "$DAEMON_PID"
>> +	then
>> +		error "start_daemon already called"
>> +	fi
>> +
>> +	mkdir -p "$DAEMON_DOCUMENT_ROOT_PATH"
>> +
>> +	trap 'code=$?; stop_daemon; (exit $code); die' EXIT
>> +
>> +	say >&3 "Starting git daemon ..."
>> +	git daemon --listen=127.0.0.1 --port="$LIB_DAEMON_PORT" \
>> +		--reuseaddr --verbose \
>> +		--base-path="$DAEMON_DOCUMENT_ROOT_PATH" \
>> +		"$@" "$DAEMON_DOCUMENT_ROOT_PATH" \
>> +		>&3 2>&4 &
>> +	DAEMON_PID=$!
>> +}
>> +
>> +stop_daemon() {
>> +	if test -z "$DAEMON_PID"
>> +	then
>> +		return
>> +	fi
>> +
>> +	trap 'die' EXIT
>> +
>> +	# kill git-daemon child of git
>> +	say >&3 "Stopping git daemon ..."
>> +	pkill -P "$DAEMON_PID"

How portable is this one (I usually do not trust use of pkill anywhere)?

>> +	wait "$DAEMON_PID"
>> +	ret=$?
	# Please comment what 143 is on this line.
>> +	if test $ret -ne 143
>> +	then
>> +		error "git daemon exited with status: $ret"
>> +	fi
>> +	DAEMON_PID=
>> +}
>> ...
>> +test_expect_success 'prepare pack objects' '
>> +	cp -R "$DAEMON_DOCUMENT_ROOT_PATH"/repo.git "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git &&
>> +	(cd "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git &&
>> +	 git --bare repack &&

As the later tests assume there will be only one pack, don't you want at
least "-a" and possibly "-a -d" here?

>> +	 git --bare prune-packed
>> +	)
>> +'
>> +
>> +test_expect_success 'fetch notices corrupt pack' '
>> +	cp -R "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad1.git &&
>> +	(cd "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad1.git &&
>> +	 p=`ls objects/pack/pack-*.pack` &&
>> +	 chmod u+w $p &&
>> +	 printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc
>> +	) &&
>> +	mkdir repo_bad1.git &&
>> +	(cd repo_bad1.git &&
>> +	 git --bare init &&
>> +	 test_must_fail git --bare fetch $DAEMON_URL/repo_bad1.git &&
>> +	 test 0 = `ls objects/pack/pack-*.pack | wc -l`
>> +	)
>> +'
>> +
>> +test_expect_success 'fetch notices corrupt idx' '
>> +	cp -R "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad2.git &&
>> +	(cd "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad2.git &&
>> +	 p=`ls objects/pack/pack-*.idx` &&
>> +	 chmod u+w $p &&
>> +	 printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc
>> +	) &&
>> +	mkdir repo_bad2.git &&
>> +	(cd repo_bad2.git &&
>> +	 git --bare init &&
>> +	 test_must_fail git --bare fetch $DAEMON_URL/repo_bad2.git &&
>> +	 test 0 = `ls objects/pack | wc -l`
>> +	)
>> +'
>> +
>> +test_remote_error()
>> +{
>> +	do_export=YesPlease
>> +	while test $# -gt 0
>> +	do
>> +		case $1 in
>> +		-x)
>> +			shift
>> +			chmod -X "$DAEMON_DOCUMENT_ROOT_PATH/repo.git"

I find the use of cap X here dubious; it makes your intention unclear.

Are you interested in the current status of 'x' bits on that directory, or
are you more interested in dropping the executable/searchable bits from
the directory no matter what its current status is (rhetorical: I fully
expect that the answer is the latter)? The same comment applies to the use
of "chmod +X" at the end of this helper function.

^ permalink raw reply

* Re: 1.7.7.3 wishlist: add --verbose option to git-tag
From: Jeff King @ 2012-01-03 19:39 UTC (permalink / raw)
  To: Jari Aalto; +Cc: git
In-Reply-To: <87d3b51vr0.fsf@cante.cante.net>

On Sat, Dec 31, 2011 at 01:32:19AM +0200, Jari Aalto wrote:

> In scripts it would be useful if "git tag" would provide option:
> 
>     --verbose
> 
> As in script:
> 
>     git tag --verbose -m "Initial import" upstream/1.0

What would the --verbose flag do?

> It would also help if all commands would use similar interface. In "git
> tag" case, this would meen relocating:
> 
>     -v      =>      -g, --verify-gpg
> 
> And reserve these:
> 
>     -v, --verbose

I agree it would be nicer. We would need to follow the usual deprecation
procedure of:

  1. introduce "-g", leave "-v" as a deprecated alias

  2. wait a long time

  3. drop "-v"

If you want to start a discussion, producing a patch for (1) is probably
the best way. If your plan is eventual consistency between options of
various commands, an even better start would be listing the existing
non-verbose uses of "-v" and making aliases for all of them.

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS
From: Junio C Hamano @ 2012-01-03 19:42 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Sven Strickroth, git, Jakub Narebski, Jeff King
In-Reply-To: <CACBZZX6iMobuU90skpbNPaGQFxYNOAjmZ6ceO4PGqfZSMkgePQ@mail.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> And how about this:
>
>     perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =
> "moo\015\012"; my ($fh, $name) = tempfile(); print $fh $str; close
> $fh; open my $in, "<:crlf", $name or die $!; my $in_str = <$in>;
> chomp(my $cin_str = $in_str); print "in_str:<$in_str>
> cin_str:<$cin_str> END\n"'
>
> It could be that there's some bug in either perl or mingw's build of
> perl where it won't turn on the :crlf IO layer by default.

I agree that PERLIO may be a solution that is supposed to work, but let's
not go there, at least not yet in this patch. The stripping of \015\012
(not \r\n) was copied from the original code and I would prefer to see the
patch focus on one primary thing it wants to enhance at a time.

Thanks.

^ permalink raw reply

* Fwd: git and github query about maintaining project
From: manoj soni @ 2012-01-03 19:42 UTC (permalink / raw)
  To: git
In-Reply-To: <CA+KSefVajgEBCH+31a0DLXPGUgb2yshxPeXV+SdmGvPFVBOBtg@mail.gmail.com>

Dear Admin,
Forwarding you below email, which I have sent to wrong email address by mistake.

Thanks,
Manoj


---------- Forwarded message ----------
From: manoj soni <manoj6891@gmail.com>
Date: Tue, Jan 3, 2012 at 2:20 PM
Subject: git and github query about maintaining project
To: git@vger.kernel.orgm, support@github.com


Dear Admin,
I am using git along with github since august 2011. I am new to it but
now I know it significantly. It is interesting and helpful.
I have started with a already existing project (say P). We were two
persons working on this project, so we forked P (on github). I got
project forked copy C1 and my friend got C2. We each have committed to
C1 and C2 independently. Now my friend merged his commits from C2 to P
and moved out of this project. I have created a branch OLD in my
repository C1 along with a already existing master branch. What I want
to do that in my project C1, OLD branch should have all of my commits
and master branch should be same as P. Please tell how should I do
this? I am trying since a week to get it done, I have attempted some
unsuccessful tries, which I can tell you if you want. If you need more
information, please tell me.

Hope to get response.
I shall be thankful.
Manoj Soni

^ permalink raw reply

* Re: [PATCH] stash: Don't fail if work dir contains file named 'HEAD'
From: Junio C Hamano @ 2012-01-03 19:44 UTC (permalink / raw)
  To: Jonathon Mah; +Cc: git
In-Reply-To: <913BB2F9-3C51-44D0-BFEC-3A49A5EC9E15@JonathonMah.com>

Jonathon Mah <me@JonathonMah.com> writes:

> When performing a plain "git stash" (without --patch), git-diff would fail
> with "fatal: ambiguous argument 'HEAD': both revision and filename". The
> output was piped into git-update-index, masking the failed exit status.
> The output is now sent to a temporary file (which is cleaned up by
> existing code), and the exit status is checked. The "HEAD" arg to the
> git-diff invocation has been disambiguated too, of course.
>
> In patch mode, "git stash -p" would fail harmlessly, leaving the working
> dir untouched.
>
> Signed-off-by: Jonathon Mah <me@JonathonMah.com>
> ---

Thanks.

^ permalink raw reply

* Re: [PATCH 1/3] expanded hook api with stdio support
From: Junio C Hamano @ 2012-01-03 19:53 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Joey Hess, git
In-Reply-To: <4EFD88CB.3050403@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> IMO, as the first step, the user of this infrastructure should only be
> required to construct the hook input as a strbuf, and receive the hook
> output, if needed, also as a strbuf.

Now you brought it up, I think I would agree. The only reason I suggested
a callback feeder approach was because I somehow was hoping that it may be
possible to share more code with the codepath for textconv that may not
want to hold too much buffer in core when we know the data is only used
sequencially and I wanted to see more things to go through streaming API
in the future.

>> +`run_hook_complex`::

Also, I think the updated interface should become the "run_hook" function;
nothing "complex" about it. The name "run_hook()" was a perfectly fine
abstraction for what it did when it used to be a static helper function
within builtin-commit.c, but its special-casing of GIT_INDEX_FILE
environment is _not_ general enough to deserve it to be called the
"run_hook" in the global scope.

IOW, I am saying that we screwed up at ae98a00 (Move run_hook() from
builtin-commit.c into run-command.c (libgit), 2009-01-16.

The environment tweaking should not take a "index_file" field in the
structure, but an array "environ" that is used to tweak the environment
variables for the hook process.

^ permalink raw reply

* Re: [PATCH] Submodules always use a relative path to gitdir
From: Junio C Hamano @ 2012-01-03 19:58 UTC (permalink / raw)
  To: Phil Hord; +Cc: Antony Male, git, iveqy
In-Reply-To: <7vlipovd4n.fsf@alter.siamese.dyndns.org>

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

> ...
> And the extent of the design of
>
>     echo "gitdir: $there" >.git && git config core.worktree "$(pwd)"
>
> is to work with the locations of these two places as they are set up.
> Moving one or the other or both may or may not work without adjusting to
> what you did. If you "mv $there $newlocation" (the repository) behind
> Git's back, you may need to update .git to point at the new location of
> the repository.  If you move your working tree woth "mv", you may need to
> update core.worktree to point at the new location of the working tree.
> And until you do so things may not work. That is why we do not explicitly
> say "you can move them to arbitrary places without telling git and things
> will work"---because that is not the case.

Just to avoid any misunderstanding, I still agree with the overall goal of
the original patch to allow moving the whole superproject tree, including
its submodule repositories in its .git/modules/, and the working trees of
itself and its submodules. It is a narrow special case with a very well
defined relative relationships between the working tree of submodules and
the repositories that control them, and having them point to each other
with relative paths will make any post-move adjustments unnecessary, unlike
more general unconstrained uses of the "gitdir: $there" mechanism.

^ permalink raw reply

* Re: 1.7.7.3 wishlist: add --verbose option to git-tag
From: Junio C Hamano @ 2012-01-03 20:02 UTC (permalink / raw)
  To: Jari Aalto; +Cc: git
In-Reply-To: <87d3b51vr0.fsf@cante.cante.net>

Jari Aalto <jari.aalto@cante.net> writes:

> In scripts it would be useful if "git tag" would provide option:
>
>     --verbose
>
> As in script:
>
>     git tag --verbose -m "Initial import" upstream/1.0

"In scripts", you are expected to be capable of doing anything fancy with
"git cat-file tag", but we add things that turn out to be commonly needed.

What does the proposed "--verbose" produce that makes scripts easier to
write (i.e. avoids repeated post-processing of "git cat-file tag" output),
and how commonly would what you propose apply to various people's needs
other than yours?

^ permalink raw reply

* Re: [PATCH resend] Do not create commits whose message contains NUL
From: Jeff King @ 2012-01-03 20:03 UTC (permalink / raw)
  To: Drew Northup; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <1325435251.4752.104.camel@drew-northup.unet.maine.edu>

On Sun, Jan 01, 2012 at 11:27:31AM -0500, Drew Northup wrote:

> I had already started experimenting with automatically detecting decent
> UTF-16 a long while back so that compatible platforms could handle it
> appropriately in terms of creating diffs and dealing with newline
> munging between platforms. There is no 100% sure-fire check for UTF-16
> if you don't already suspect it is possibly UTF-16. If we really want to
> check for possible UTF-16 specifically I can scrape out the check I
> wrote up and send it along.

I also looked into this recently. You can generally detect UTF-16 by the
BOM at the beginning of the file (which will also tell you the
endian-ness). I did a simple test by integrating it into the check for
binary-ness during diffs. However, as I recall, the result wasn't
particularly useful. Some of the diff code wasn't happy with the
embedded NUL bytes (i.e., there is code that assumes that NUL is the end
of a string). Not to mention that ascii newline (0x0a) can appear as
part of other characters in a wide encoding like utf-16. And since git
outputs straight ascii for all of the diff boilerplate, you end up with
a mish-mash of utf-16 and ascii (this is OK with utf-8, of course,
because utf-8 is a superset of ascii).

If anything, I think you would want to do something like "textconv" to
convert the utf-16 into utf-8, then diff that. Git won't do it
automatically based on encoding, but if you know the filenames of the
utf-16 files in your repository, you can do something like:

  echo 'foo.txt diff=utf16' >.gitattributes
  git config diff.utf16.textconv 'iconv -f utf16 -t utf8'

and get readable diffs. Of course you couldn't use that diff to apply a
patch, though.

I strongly suspect that not many people are really using git for utf-16
files. Git treats them as binary, which makes them unpleasant for
anything except simple storage.

> The is_utf8 check was not written to detect 100% valid UTF-8 per-se. It
> seems to me that it was written as part of the "is this a binary or not"
> check in the add/commit path.

We shouldn't care about binary file content at all in the add or commit
code paths. I would guess we do only if you are using auto-crlf (but
then, I don't think we care about utf8 in that cases, only whether line
endings should be converted or not).

We do check that the commit message itself is utf8, but only to generate
a warning that you should set i81n.commitencoding.

-Peff

^ permalink raw reply

* Re: [PATCH 1/3] expanded hook api with stdio support
From: Jeff King @ 2012-01-03 20:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Joey Hess, git
In-Reply-To: <7vsjjwtvf1.fsf@alter.siamese.dyndns.org>

On Tue, Jan 03, 2012 at 11:53:22AM -0800, Junio C Hamano wrote:

> Johannes Sixt <j6t@kdbg.org> writes:
> 
> > IMO, as the first step, the user of this infrastructure should only be
> > required to construct the hook input as a strbuf, and receive the hook
> > output, if needed, also as a strbuf.
> 
> Now you brought it up, I think I would agree. The only reason I suggested
> a callback feeder approach was because I somehow was hoping that it may be
> possible to share more code with the codepath for textconv that may not
> want to hold too much buffer in core when we know the data is only used
> sequencially and I wanted to see more things to go through streaming API
> in the future.

Even if we don't make the input streaming, it would be nice to factor
the concept of "feed input to program and read its output without
deadlocking" into something independent of hooks.

The credential helper code could potentially have the same deadlock.
Possibly also clean/smudge filters.

Maybe it could even be part of the run-command interface?

-Peff

^ permalink raw reply

* Re: Stashing individual files
From: Junio C Hamano @ 2012-01-03 20:08 UTC (permalink / raw)
  To: Jeff King; +Cc: Chris Leong, git
In-Reply-To: <20120103190612.GC20926@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I think that would be OK compromise, though. I'd rather not introduce a
> whole new "stashfiles" command (or even a new subcommand of stash) if we
> can avoid it.

Why wouldn't a simple "git diff -- paths >P.diff" work?

What does such a partial stash leave in the working tree, how does the
user deal with the remaining local changes, what happens after such a
partial stash is applied/popped?

I wouldn't have worried about such a change before e0e2a9c (stash: drop
dirty worktree check on apply, 2011-04-05) but now we allow application of
stashed changes to the dirty working tree (which is a very good thing), I
am not sure how sensibly these changes in different places would interact
if we start supporting partial stashing.

^ permalink raw reply

* Re: [PATCH] fix hang in git fetch if pointed at a 0 length bundle
From: Junio C Hamano @ 2012-01-03 20:11 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Brian Harring, git, spearce
In-Reply-To: <CACsJy8B5B2bx7WK7ViziseuWCPaMgcc-avwtsw2DybRme8Vgfg@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Tue, Jan 3, 2012 at 8:13 AM, Brian Harring <ferringb@gmail.com> wrote:
>> @@ -31,7 +31,7 @@ static int strbuf_readline_fd(struct strbuf *sb, int fd)
>>        while (1) {
>>                char ch;
>>                ssize_t len = xread(fd, &ch, 1);
>> -               if (len < 0)
>> +               if (len <= 0)
>>                        return -1;
>>                strbuf_addch(sb, ch);
>>                if (ch == '\n')
>
> I think it should return 0 when len == 0 because strictly speaking eof
> is not a fault.

Even if you do not strictly speak, end of file is a perfectly normal thing
to see, no?  IOW wouldn't the original patch actively _break_ callers that
read the whole file from the file descriptor to the end?

> FWIW I went through all xread call sites. All seem to handle return
> value <= 0 correctly.

Thanks.

^ permalink raw reply

* Re: [PATCH] fix hang in git fetch if pointed at a 0 length bundle
From: Junio C Hamano @ 2012-01-03 20:13 UTC (permalink / raw)
  To: Brian Harring; +Cc: git
In-Reply-To: <20120103134603.GA31034@localhost>

Brian Harring <ferringb@gmail.com> writes:

> git-repo if interupted at the exact wrong time will generate zero
> length bundles- literal empty files.  git-repo is wrong here, but
> git fetch shouldn't effectively spin loop if pointed at a zero
> length bundle.
>
> Signed-off-by: Brian Harring <ferringb@chromium.org>
> ---

Thanks.

Also thanks to all reviewers.

^ permalink raw reply

* Re: Stashing individual files
From: Jeff King @ 2012-01-03 20:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Chris Leong, git
In-Reply-To: <7vfwfwtup7.fsf@alter.siamese.dyndns.org>

On Tue, Jan 03, 2012 at 12:08:52PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I think that would be OK compromise, though. I'd rather not introduce a
> > whole new "stashfiles" command (or even a new subcommand of stash) if we
> > can avoid it.
> 
> Why wouldn't a simple "git diff -- paths >P.diff" work?

For all the same reasons that "git diff >P.diff" is not as good as a
regular stash.

> What does such a partial stash leave in the working tree, how does the
> user deal with the remaining local changes, what happens after such a
> partial stash is applied/popped?

I would expect it to stash only the changes in the selected files,
restoring them to their state in HEAD, and leave other files untouched.

We already have partial stashing like this via "git stash -p".  This is
just a shorthand for "say yes to all of the changes in foo.c, and no to
everything else". So I don't see it as particularly new or dangerous.

-Peff

^ permalink raw reply

* Re: git and github query about maintaining project
From: Seth Robertson @ 2012-01-03 20:21 UTC (permalink / raw)
  To: manoj soni; +Cc: git
In-Reply-To: <CA+KSefW+K1hMiFkrFCP1LAVjfV9hECwFWAHz940fwGJawHuoFQ@mail.gmail.com>


In message <CA+KSefW+K1hMiFkrFCP1LAVjfV9hECwFWAHz940fwGJawHuoFQ@mail.gmail.com>, manoj soni writes:

    Forwarding you below email, which I have sent to wrong email address by mistake.

You might want to ask on the IRC #git channel for tactical support
questions, like this. irc://irc.freenode.net/git

    we forked P (on github),  I got project forked copy C1

    What I want to do that in my project C1, OLD branch should have
    all of my commits and master branch should be same as P.

Tactically, the answer to your question is as follows:
----------------------------------------------------------------------
# Create a new branch OLD from where master is (presumably containing your commits)
git checkout -b OLD master

# Share OLD with your forked C1 (if you want to)
git push origin OLD
git branch --set-upstream OLD origin/OLD

# Get access to the repository you forked from
git remote add P URL-TO-P
git fetch P

# Reset the master branch to the contents of P/master
#  Please note that any uncommitted changes WILL BE LOST
git checkout master
git reset --hard P/master

# Share your rewritten history with origin (C1)
#  Please note, rewriting publicly visible history is a BAD IDEA.
#  Anyone else who might have pulled the old history will have to do
#  special things and may hate you forever.
git push -f origin
----------------------------------------------------------------------

However, interpreting what you are really trying to do (get your
changes and C2's changes put together and uploaded to P), this is what
*I* would do:
----------------------------------------------------------------------
# Get access to the repository you forked from git remote add P
URL-TO-P git fetch P

# Merge your development with the other development that has been happening
git merge origin/P

# Share your rewritten history with origin (C1)
git push

# When you have finished testing (unless you are using the github pull request method)
git push P master
----------------------------------------------------------------------

					-Seth Robertson

^ permalink raw reply

* Re: 1.7.7.3 wishlist: add --verbose option to git-tag
From: jaalto @ 2012-01-03 20:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk458tuzq.fsf@alter.siamese.dyndns.org>

On 2012-01-03 12:02, Junio C Hamano wrote:
| Jari Aalto <jari.aalto@cante.net> writes:
| 
| > In scripts it would be useful if "git tag" would provide option:
| >
| >     --verbose
| >
| > As in script:
| >
| >     git tag --verbose -m "Initial import" upstream/1.0
| 
| What does the proposed "--verbose" produce that makes scripts easier to
| write

Not "easier write", but to display progress to user; show what is
hapening. Similarly to other --verbose options. Messgae could be:

	  "tagged: upstream/1.0 asd1234"

Jari

^ permalink raw reply

* Re: Stashing individual files
From: Junio C Hamano @ 2012-01-03 20:45 UTC (permalink / raw)
  To: Jeff King; +Cc: Chris Leong, git
In-Reply-To: <20120103201323.GA4340@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> We already have partial stashing like this via "git stash -p".  This is
> just a shorthand for "say yes to all of the changes in foo.c, and no to
> everything else". So I don't see it as particularly new or dangerous.

Ok.

^ permalink raw reply

* Re: Possible submodule or submodule documentation issue
From: Junio C Hamano @ 2012-01-03 20:48 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Bill Zaumen, git
In-Reply-To: <4F00780C.7090801@web.de>

Will queue; thanks.

^ permalink raw reply

* Re: [PATCH] git-svn: enable platform-specific authentication
From: Matthijs Kooijman @ 2012-01-03 20:44 UTC (permalink / raw)
  To: Eric Wong, Gustav Munkby, Edward Rudd, Carsten Bormann; +Cc: git
In-Reply-To: <20110809210638.GK6418@login.drsnuggles.stderr.nl>

[-- Attachment #1: Type: text/plain, Size: 4514 bytes --]

Hey folks,

I sent the below patch a few months ago, and not having it applied in
git-svn bit me again just now. Did any of you get a chance to have a
look at it?

I'm still not 100% sure if this patch is correct for all the corner
cases, but it works like a charm in the regular case.

Perhaps it should just be included as is?

Gr.

Matthijs

On Tue, Aug 09, 2011 at 11:06:38PM +0200, Matthijs Kooijman wrote:
> Hey folks,
> 
> > > Use the platform-specific authentication providers that are
> > > exposed to subversion bindings starting with subversion 1.6.
> > 
> > This came up several months ago, I understand there were some issues
> > with the SVN Perl bindings.  Cc-ing interested parties.
> I missed the CC, sorry for that.
> 
> > >  sub _auth_providers () {
> > >  	[
> > > +	  $SVN::Core::VERSION lt '1.6' ? () :
> > > +	    @{SVN::Core::auth_get_platform_specific_client_providers(
> > > +	      undef,undef)},
> > 
> > I think it needs to take into account the config from
> > SVN::Core::config_get_config, otherwise people with non-standard SVN
> > configurations could get locked out.  I seem to recall this was the
> > broken part in the SVN Perl bindings, but one of the Cc-ed parties would
> > know for sure.
> 
> Indeed, but a proposed patch by Eric for this did not work. I solved the
> problem quite some time ago, but apparently I never sent out the
> solution (I think I got distracted by trying to get a passphrase prompt
> to unlock locked keychains). I couldn't find my fixes anymore either,
> but I think I've managed to reproduce them just now.
> 
> Some basic testing shows below patch works, but I think it might need
> some more testing and work. At least the below patch allows for example
> to disable the gnome-keyring provider from a different svn config
> directory by passing --config-dir /some/path to git-svn (which is not
> possible using above patch passing undef, which will only read from
> ~/.subversion).
> 
> Using strace, I did notice that git-svn still reads stuff
> from ~/.subversion/auth/svn.ssl.server/ and
> .subversion/auth/svn.simple/, but I couldn't exactly find why this is
> right away. In any case, it also happens without this patch applied, so
> I guess it's a completely separate issue.
> 
> As for the actual patch, notice that config_get_config returns a hash
> that consists again of a "config" and "servers" patch. Previous attempts
> at this patch passed the entire hash to
> auth_get_platform_specific_client_providers, but it only wants the
> "client" part. It's a bit confusing until you realize that the
> config_get_config return value represents your ~/.subversion directory,
> which again contains a "config" and "servers" file.
> 
> I'm not 100% sure this patch is correct as it is now. I hope to get
> another look at my "automatically unlock keychain" work tomorrow,
> in case there are some hints about flaws in this patch there. In the
> meanwhile, feedback on this patch is welcome.
> 
> Gr.
> 
> Matthijs
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index da3fea8..6dc5196 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -4916,7 +4916,7 @@ BEGIN {
>  }
>  
>  sub _auth_providers () {
> -       [
> +       my @rv = (
>           SVN::Client::get_simple_provider(),
>           SVN::Client::get_ssl_server_trust_file_provider(),
>           SVN::Client::get_simple_prompt_provider(
> @@ -4932,7 +4932,23 @@ sub _auth_providers () {
>             \&Git::SVN::Prompt::ssl_server_trust),
>           SVN::Client::get_username_prompt_provider(
>             \&Git::SVN::Prompt::username, 2)
> -       ]
> +       );
> +
> +       # earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
> +       # this function
> +       if ($SVN::Core::VERSION gt '1.6.12') {
> +               my $config = SVN::Core::config_get_config($config_dir);
> +               my ($p, @a);
> +              # config_get_config returns all config files from
> +              # ~/.subversion, auth_get_platform_specific_client_providers
> +              # just wants the config "file".
> +               @a = ($config->{'config'}, undef);
> +               $p = SVN::Core::auth_get_platform_specific_client_providers(@a);
> +              # Insert the return value from
> +              # auth_get_platform_specific_providers
> +               unshift @rv, @$p;
> +       }
> +       \@rv;
>  }
>  
>  sub escape_uri_only {
> 



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH 1/3] expanded hook api with stdio support
From: Junio C Hamano @ 2012-01-03 21:44 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, Joey Hess, git
In-Reply-To: <20120103200642.GH20926@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> The credential helper code could potentially have the same deadlock.
> Possibly also clean/smudge filters.
>
> Maybe it could even be part of the run-command interface?

Hmm, that smells like the right thing to do.

^ permalink raw reply

* Re: [PATCH] Submodules always use a relative path to gitdir
From: Jens Lehmann @ 2012-01-03 22:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antony Male, git, iveqy
In-Reply-To: <7vsjjwvdyl.fsf@alter.siamese.dyndns.org>

Am 03.01.2012 19:27, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> Am 29.12.2011 23:40, schrieb Junio C Hamano:
>>> I further wonder if we can get away without using separate-git-dir option
>>> in this codepath, though. IOW using
>>>
>>>         git clone $quiet -bare ${reference:+"$reference"} "$url" "$gitdir"
>>>
>>> might be a better solution.
>>
>> A quick test shows that using a bare repo won't fly because without the
>> core.worktree setting commands that operate on the work tree can't be
>> run anymore inside submodules (starting with the initial checkout). 
> 
> Probably the right thing to do would be to restructure the flow as I
> suggested, i.e.
> 
> 	if we do not have it yet
>         then
>         	git clone --bare ...
> 	fi
> 	# now we have it, make sure they are correct
> 	git config core.bare false

Ah, I forgot to set core.bare to false when trying this. But even then
a dozen tests fail, no matter if I set core.worktree or not. A cursory
glance indicates problems with branches ... I'll have to dig deeper
here.

> 	git config core.worktree $there

Please see below.

>         echo "gitdir: $here" >$there/.git
> 
>> Yes, and the core.worktree setting also contains an absolute path. So
>> we must either make that relative too and rewrite it on every "git
>> submodule add" to record the possibly changed path there or make the
>> bare clone work with a work tree (which sounds a bit strange ;-).
> 
> Update of core.worktree has to be done regardless of the absolute/relative
> differences anyway, no?

Not if we would implement a "if no worktree is set but we came here via
a gitfile, then take the directory the gitfile was found in as worktree"
heuristic. And that heuristic looks quite sane to me, as a gitfile can
only be found in a work tree, or am I missing something obvious here?

^ permalink raw reply

* Re: [PATCH] Submodules always use a relative path to gitdir
From: Junio C Hamano @ 2012-01-03 22:17 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Antony Male, git, iveqy
In-Reply-To: <4F037CBF.9010005@web.de>

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Not if we would implement a "if no worktree is set but we came here via
> a gitfile, then take the directory the gitfile was found in as worktree"
> heuristic. And that heuristic looks quite sane to me, as a gitfile can
> only be found in a work tree, or am I missing something obvious here?

Like it wouldn't work without changes to the core side?

^ 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