* Re: [BUG] Cannot push some grafted branches
From: Jeff King @ 2012-12-19 13:06 UTC (permalink / raw)
To: Johannes Sixt
Cc: Yann Dirson, Thomas Rast, Junio C Hamano, Andreas Schwab,
Christian Couder, Thomas Rast, git list
In-Reply-To: <50D16911.10000@viscovery.net>
On Wed, Dec 19, 2012 at 08:13:21AM +0100, Johannes Sixt wrote:
> Am 12/18/2012 17:24, schrieb Jeff King:
> > I am not really interested in pushing this forward myself, but I worked
> > up this toy that somebody might find interesting (you can "git replace
> > HEAD~20" to get dumped in an editor). It should probably handle trees,
> > and it would probably make sense to do per-object-type sanity checks
> > (e.g., call verify_tag on tags).
>
> I know it's just a throw-away patch, but I would discourage to go this
> route without also adding all the sanity checks. Otherwise, it will have
> just created a porcelain command that can generate a commit object with
> any content you want!
I think I agree with you that it would not be worth doing without sanity
checks. I am not sure if your "any content you want" statement means
"bad people can easily make bogus objects" or "it is too easy to make
arbitrary mistakes, putting your repo in a bogus state".
I would agree that the latter is compelling, but not the former. You
can already easily generate a commit with any content you want via
"hash-object -t commit", and I have frequently done this while testing
corner cases of fsck, how git behaves when given buggy data, etc. So to
me it is not about preventing intentional abuse, but about not promoting
a feature that makes it too easy to screw up.
-Peff
^ permalink raw reply
* Re: problem with BOINC repository and CR/LF
From: Toralf Förster @ 2012-12-19 10:44 UTC (permalink / raw)
To: Jeff King; +Cc: Torsten Bögershausen, Andrew Ardill, git@vger.kernel.org
In-Reply-To: <20121218164132.GC20122@sigill.intra.peff.net>
On 12/18/2012 05:41 PM, Jeff King wrote:
> I could reproduce it, too, on Linux.
>
> The reason it does not always happen is that git will not re-examine the
> file content unless the timestamp on the file is older than what's in
> the index. So it is a race condition for git to see whether the file is
> stat-dirty.
>
Ah - /me was wondering why sometimes (but rarely) I could not exactly
reproduce the problem and was really wondering if the underlying file
system (ext4) would give an extra layer of trouble or not.
Thx for that explanation.
--
MfG/Sincerely
Toralf Förster
pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
^ permalink raw reply
* Re: problem with BOINC repository and CR/LF
From: Toralf Förster @ 2012-12-19 10:43 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Andrew Ardill, git@vger.kernel.org
In-Reply-To: <50D05E62.7090605@web.de>
On 12/18/2012 01:15 PM, Torsten Bögershausen wrote:
> HTH
> /Torsten
Thx Torsten - I forwarded this answer (and all the other answers) to the
boinc alpha mailing list
- there's now a discussion about that.
--
MfG/Sincerely
Toralf Förster
pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
^ permalink raw reply
* Re: [BUG] Cannot push some grafted branches
From: Yann Dirson @ 2012-12-19 8:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andreas Schwab, Christian Couder, Thomas Rast, git list
In-Reply-To: <7vehinibpc.fsf@alter.siamese.dyndns.org>
On Tue, 18 Dec 2012 08:09:35 -0800
Junio C Hamano <gitster@pobox.com> wrote:
> Yann Dirson <dirson@bertin.fr> writes:
>
> > On Mon, 17 Dec 2012 13:14:56 -0800
> > Junio C Hamano <gitster@pobox.com> wrote:
> >
> >> Andreas Schwab <schwab@linux-m68k.org> writes:
> >>
> >> > Christian Couder <christian.couder@gmail.com> writes:
> >> >
> >> >> Yeah, at one point I wanted to have a command that created to craft a
> >> >> new commit based on an existing one.
> >> >
> >> > This isn't hard to do, you only have to resort to plumbing:
> >> >
> >> > $ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 | sed s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/ | git hash-object -t commit --stdin -w
> >> > bb45cc6356eac6c7fa432965090045306dab7026
> >>
> >> Good. I do not think an extra special-purpose command is welcome
> >> here.
> >
> > Well, I'm not sure this is intuitive enough to be useful to the average user :)
>
> I do not understand why you even want to go in the harder route in
> the first place, only to complicate things?
Although the approach you propose is elegant, it still looks like one
could not leave the worktree untouched in the case of creating a merge replace,
which the "just forge an arbitrary commit" approach handles easily.
It seems the latter would also be more powerful, in that you can create new commits with an
arbitrary number of parents, even when merge-octopus would simply refuse to help;
and it is has no special case for creating merges.
> Is this not intuitive enough?
I would say it is a nice read that can help an advanced user to earn
some XP - but well, replace refs are also meant for somewhat advanced users :)
--
Yann Dirson - Bertin Technologies
^ permalink raw reply
* Re: sys/param.h
From: Erik Faye-Lund @ 2012-12-19 7:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vobhrgupr.fsf_-_@alter.siamese.dyndns.org>
On Tue, Dec 18, 2012 at 6:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>
>>> Junio C Hamano wrote:
>>>> It could turn out that we may be able to get rid of sys/param.h
>>>> altogether, but one step at a time. Inputs from people on minority
>>>> platforms are very much appreciated---does your platform build fine
>>>> when the inclusion of the file is removed from git-compat-util.h?
>>
>> MinGW works fine with sys/param.h removed from git-compat-util.h.
>
> It seems that OpenBSD 5.2 does not mind it getting removed, either.
> Debian 5 and Debian 6 seem OK; so do Ubuntu 10.04 and 12.04. I have
> a hunch that Fedora or anything based on glibc would be fine, too.
And just to be sure; Fedora 17: OK.
^ permalink raw reply
* Re: [BUG] Cannot push some grafted branches
From: Johannes Sixt @ 2012-12-19 7:13 UTC (permalink / raw)
To: Jeff King
Cc: Yann Dirson, Thomas Rast, Junio C Hamano, Andreas Schwab,
Christian Couder, Thomas Rast, git list
In-Reply-To: <20121218162402.GA20122@sigill.intra.peff.net>
Am 12/18/2012 17:24, schrieb Jeff King:
> I am not really interested in pushing this forward myself, but I worked
> up this toy that somebody might find interesting (you can "git replace
> HEAD~20" to get dumped in an editor). It should probably handle trees,
> and it would probably make sense to do per-object-type sanity checks
> (e.g., call verify_tag on tags).
I know it's just a throw-away patch, but I would discourage to go this
route without also adding all the sanity checks. Otherwise, it will have
just created a porcelain command that can generate a commit object with
any content you want!
-- Hannes
^ permalink raw reply
* [PATCH] t0200: "locale" may not exist
From: Junio C Hamano @ 2012-12-19 6:47 UTC (permalink / raw)
To: git
On systems without "locale" installed, t0200-gettext-basic.sh leaked
error messages when checking if some test locales are available.
Hide them, as they are not very useful.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/lib-gettext.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index 0f76f6c..ae8883a 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -14,12 +14,14 @@ export GIT_TEXTDOMAINDIR GIT_PO_PATH
if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON
then
# is_IS.UTF-8 on Solaris and FreeBSD, is_IS.utf8 on Debian
- is_IS_locale=$(locale -a | sed -n '/^is_IS\.[uU][tT][fF]-*8$/{
+ is_IS_locale=$(locale -a 2>/dev/null |
+ sed -n '/^is_IS\.[uU][tT][fF]-*8$/{
p
q
}')
# is_IS.ISO8859-1 on Solaris and FreeBSD, is_IS.iso88591 on Debian
- is_IS_iso_locale=$(locale -a | sed -n '/^is_IS\.[iI][sS][oO]8859-*1$/{
+ is_IS_iso_locale=$(locale -a 2>/dev/null |
+ sed -n '/^is_IS\.[iI][sS][oO]8859-*1$/{
p
q
}')
--
1.8.1.rc2.196.g654d69e
^ permalink raw reply related
* [RFC/PATCH] tests: optionally fail passed todo immediately
From: Junio C Hamano @ 2012-12-19 5:38 UTC (permalink / raw)
To: git
Add "--fail-passed-todo" option to stop the test immediately when a
test that is expected to fail succeeds. After seeing the test stop,
the developer can go to the trash directory and inspect why it failed
to fail as expected.
I usually just insert "exit" after such test with an editor, but
an option like this might be easier to use. I dunno.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Stopping immediately after a test that is failing (and expected
to fail) and then going to the trash directory to inspect the
status of the sandbox are the first two steps I often end up
doing while fixing a bug. It may make sense to add an option to
cause the test to stop at a failure of test_expect_failure, but
that is a separate topic.
t/test-lib.sh | 3 +++
1 file changed, 3 insertions(+)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f50f834..7b7cce6 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -176,6 +176,8 @@ do
debug=t; shift ;;
-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
immediate=t; shift ;;
+ --fail-passed-todo)
+ fail_passed_todo=t; shift ;;
-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
-h|--h|--he|--hel|--help)
@@ -307,6 +309,7 @@ test_failure_ () {
test_known_broken_ok_ () {
test_fixed=$(($test_fixed+1))
say_color "" "ok $test_count - $@ # TODO known breakage"
+ test "$fail_passed_todo" = "" || { GIT_EXIT_OK=t; exit 1; }
}
test_known_broken_failure_ () {
--
1.8.1.rc2.196.g90926c8
^ permalink raw reply related
* [PATCH] t9502: do not assume GNU tar
From: Junio C Hamano @ 2012-12-19 5:03 UTC (permalink / raw)
To: git
The check_snapshot function inspects and makes sure that not cruft
outside the repository hierarchy is added to the tar archive, by
insisting that the output from "tar tf" on the resulting archive
does not contain anything that does not begin with "$prefix/".
There are two issues with this implementation:
- Traditional tar implemenations that do not understand
pax_global_header will write it out as if it is a plain file at
the top-level;
- Some implementations of tar does not add trailing slash when
showing a directory entry (i.e. the output line for the entire
archive will show "$prefix", not "$prefix/").
Fix them so that what we want to validate can be tested with
traditional tar implementations.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t9502-gitweb-standalone-parse-output.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index 731e64c..c662298 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -40,7 +40,7 @@ check_snapshot () {
echo "basename=$basename"
grep "filename=.*$basename.tar" gitweb.headers >/dev/null 2>&1 &&
"$TAR" tf gitweb.body >file_list &&
- ! grep -v "^$prefix/" file_list
+ ! grep -v -e "^$prefix$" -e "^$prefix/" -e "^pax_global_header$" file_list
}
test_expect_success setup '
--
1.8.1.rc2.196.g90926c8
^ permalink raw reply related
* [PATCH] t9020: use configured Python to run test helper
From: Junio C Hamano @ 2012-12-19 4:49 UTC (permalink / raw)
To: git
The test helper svnrdump_sim.py is used as "svnrdump" during the
execution of this test, but the arrangement had a few undesirable
things:
- it relied on symbolic links;
- unportable "export VAR=VAL" was used;
- GIT_BUILD_DIR variable was not quoted correctly;
- it assumed that the Python interpreter is in /usr/bin/ and
called "python" (i.e. not "python2.7" etc.)
Rework this by writing a small shell script that spawns the right
Python interpreter, using the right quoting.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* The analysis above counts more bugs than the number of lines that
are deleted in this section of the code...
t/t9020-remote-svn.sh | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh
index 4f2dfe0..d7be66a 100755
--- a/t/t9020-remote-svn.sh
+++ b/t/t9020-remote-svn.sh
@@ -12,9 +12,13 @@ then
test_done
fi
-# We override svnrdump by placing a symlink to the svnrdump-emulator in .
-export PATH="$HOME:$PATH"
-ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py "$HOME/svnrdump"
+# Override svnrdump with our simulator
+PATH="$HOME:$PATH"
+export PATH PYTHON_PATH GIT_BUILD_DIR
+
+write_script "$HOME/svnrdump" <<\EOF
+exec "$PYTHON_PATH" "$GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py" "$@"
+EOF
init_git () {
rm -fr .git &&
--
1.8.1.rc2.196.g90926c8
^ permalink raw reply related
* [PATCH] t4014: fix arguments to grep
From: Junio C Hamano @ 2012-12-19 3:20 UTC (permalink / raw)
To: git
These "expect-failure" tests were not looking for the right string
in the patch file. For example:
grep "^ *"S. E. Cipient" <scipient@example.com>\$" patch5
was looking for "^ *S." in three files:
"E."
"Cipient <scipient@example.com>$"
patch5
With some implementations of grep, the lack of file "E." was
reported as an error, leading to the expected failure of the test.
With other implementations of grep, the pattern "^ *S." matched what
was in patch5, without missing files diagnosed as an error, which
lead to these tests to unexpectedly pass.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t4014-format-patch.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 16a4ca1..90fd598 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -155,7 +155,7 @@ test_expect_failure 'additional command line cc (rfc822)' '
git config --replace-all format.headers "Cc: R E Cipient <rcipient@example.com>" &&
git format-patch --cc="S. E. Cipient <scipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch5 &&
grep "^Cc: R E Cipient <rcipient@example.com>,\$" patch5 &&
- grep "^ *"S. E. Cipient" <scipient@example.com>\$" patch5
+ grep "^ *\"S. E. Cipient\" <scipient@example.com>\$" patch5
'
test_expect_success 'command line headers' '
@@ -183,7 +183,7 @@ test_expect_success 'command line To: header (ascii)' '
test_expect_failure 'command line To: header (rfc822)' '
git format-patch --to="R. E. Cipient <rcipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch8 &&
- grep "^To: "R. E. Cipient" <rcipient@example.com>\$" patch8
+ grep "^To: \"R. E. Cipient\" <rcipient@example.com>\$" patch8
'
test_expect_failure 'command line To: header (rfc2047)' '
@@ -203,7 +203,7 @@ test_expect_failure 'configuration To: header (rfc822)' '
git config format.to "R. E. Cipient <rcipient@example.com>" &&
git format-patch --stdout master..side | sed -e "/^\$/q" >patch9 &&
- grep "^To: "R. E. Cipient" <rcipient@example.com>\$" patch9
+ grep "^To: \"R. E. Cipient\" <rcipient@example.com>\$" patch9
'
test_expect_failure 'configuration To: header (rfc2047)' '
--
1.8.1.rc2.196.g90926c8
^ permalink raw reply related
* [PATCH] t3600: Avoid "cp -a" which is a GNUism
From: Junio C Hamano @ 2012-12-19 2:55 UTC (permalink / raw)
To: git
With d4a7ffa (tests: "cp -a" is a GNUism, 2012-10-08), we got rid of
most of them, but a topic that was still in flight was missed.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t3600-rm.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 97254e8..324924e 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -457,7 +457,7 @@ test_expect_success 'rm of a conflicted populated submodule with a .git director
git submodule update &&
(cd submod &&
rm .git &&
- cp -a ../.git/modules/sub .git &&
+ cp -R ../.git/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
test_must_fail git merge conflict2 &&
@@ -491,7 +491,7 @@ test_expect_success 'rm of a populated submodule with a .git directory fails eve
git submodule update &&
(cd submod &&
rm .git &&
- cp -a ../.git/modules/sub .git &&
+ cp -R ../.git/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
test_must_fail git rm submod &&
@@ -589,7 +589,7 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc
git submodule update --recursive &&
(cd submod/subsubmod &&
rm .git &&
- cp -a ../../.git/modules/sub/modules/sub .git &&
+ cp -R ../../.git/modules/sub/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
test_must_fail git rm submod &&
--
1.8.1.rc2.196.g90926c8
^ permalink raw reply related
* Re: [PATCH v2] git-clean: Display more accurate delete messages
From: Junio C Hamano @ 2012-12-19 2:37 UTC (permalink / raw)
To: Zoltan Klinger; +Cc: git
In-Reply-To: <CAKJhZwRPzrsnbnW_HgRTo86T6jqmm_osznDqpYo7pKO=cUaVDA@mail.gmail.com>
Zoltan Klinger <zoltan.klinger@gmail.com> writes:
> Thanks for the feedback.
>
>> My reading of the above is that "lst" after sorting is expected to
>> have something like:
>>
>> a/
>> a/b/
>> a/b/to-be-removed
>> a/to-be-removed
>>
>> and we first show "a/", remember that prefix in "dir", not show
>> "a/b/" because it matches prefix, but still update the prefix to
>> "a/b/", not show "a/b/to-be-removed", and because "a/to-be-removed"
>> does not match the latest prefix, it is now shown. Am I confused???
>
> No, it's a bug. The correct output should be just "a/". Thanks for
> pointing it out, I'm going to fix that.
I am not sure if the approach taken by the patch is an effective
design to achieve what you are trying to do.
Imagine the code is told to "clean" (or "clean a") and is currentlly
looking at "a/b" directory. If it cannot remove some paths under
that directory, you know that you cannot abbreviate the result to
"removed a/b" and have to report a/b/<paths you managed to remove>
at that point. On the other hand, if you removed everything in that
directory, you know you have only two possible outcomes regarding
that directory in the final output:
(1) You would say "removed a/b" if you failed to remove paths that
are neighbours to that directory (e.g. "a/to-be-removed" may
not go away for some reason), because you will also list
"removed a/<other path>" next to it, and report that you
couldn't remove "a/to-be-removed". You will not report
anything about "a/b/to-be-removed" in such a case; or
(2) You would not even say "removed a/b" if you will successfully
remove all other paths under "a/".
So in either case, if you managed to remove everything in "a/b", I
do not see any reason to keep the list of successfully removed paths
annd report them upwards. They will never be used by the caller
that is looking at "a/", or its caller that is looking at the root
level, will they?
On the other hand, if you failed to remove some paths under "a/b",
before recursion leaves that directory, you know which paths to be
reported as successful or failure, which means you can start
producing output without waiting until the traversal touches the
entire tree. That can be a huge latency win, which matters a lot in
a large project.
^ permalink raw reply
* Re: Python extension commands in git - request for policy change
From: Patrick Donnelly @ 2012-12-19 2:30 UTC (permalink / raw)
To: Eric Raymond
Cc: Sitaram Chamarty, Nguyen Thai Ngoc Duy, Michael Haggerty,
Felipe Contreras, git
In-Reply-To: <20121212124306.GC25981@thyrsus.com>
On Wed, Dec 12, 2012 at 7:43 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Patrick Donnelly <batrick@batbytes.com>:
>> How would another language (e.g. Python) mitigate this?
>
> The way you mitigate this sort of problem is to have a good set of
> high-level bindings for standard services (like socket I/O) built in
> your extension language and using its abstractions, so you don't get a
> proliferation of low-level semi-custom APIs for doing the same stuff.
>
> I have elsewhere referred to this as "the harsh lesson of Perl", which
> I do not love but which was the first scripting language to get this
> right. There is a reason Tcl and a couple of earlier designs like csh
> that we would now call "scripting languages" were displaced by Python
> and Perl; this is it.
Okay, I understand what you were trying to say earlier.
I'm not going to say Lua is a silver bullet for all embedded language
needs. If you seriously need an exotic suite of libraries built into
the language, then Lua is not really going to work well for you. In
reality though, many projects that require an extension language do
not need all the system programming facilities thrown in. In fact,
many don't want them due to bloat or security considerations. So, you
take on a hyperopic viewpoint by ruling out Lua simply because it
lacks a suite of system libraries.
With Jeff's response:
> As for "interacting with the outside world", I was specifically thinking
> of stuff like git-send-email (currently in perl) and git-imap-send
> (written in C). They need to open network sockets and speak standard
> protocols. I suspect Lua would need a module or custom bindings to do
> the former at all, and certainly the code would be much simpler if we
> re-used standard modules for speaking SMTP and IMAP (which of course
>increases our dependencies again...).
I would think this can perhaps be exported into another script Lua
could exec as needed. Or luasocket may be sufficient. These
dependencies would need to be examined in detail. I wouldn't recommend
selecting a language because of one odd network protocol dependency
satisfied by an obscure built-in library (I realize Jeff's example was
exactly that, an example).
On Wed, Dec 12, 2012 at 7:43 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
>> I don't see how these languages are more appropriate based on your concerns.
>
> Your previous exchange with Jeff King indicates that you don't
> understand glue scripting very well. Your puzzlement here just
> confirms that. Trust both of us on this, it's important. And
> reread my previous three paragraphs.
What I didn't understand coming into this thread was Git's ecosystem.
I understand embedded scripting languages very well and have been
working with Lua for years.
What does puzzle me is your dismissal of Lua because it doesn't have
the library suite Python does. Lua is not a system programming
language and I could argue Python is not really an embedded language.
I came here to try to stimulate discussion about what Git actually
needs/wants from a higher level language. If a small embedded language
would fit well, the Lua should be a candidate for consideration.
--
- Patrick Donnelly
^ permalink raw reply
* Re: [PATCH v2] git-clean: Display more accurate delete messages
From: Zoltan Klinger @ 2012-12-19 0:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsj74jr2k.fsf@alter.siamese.dyndns.org>
Thanks for the feedback.
> My reading of the above is that "lst" after sorting is expected to
> have something like:
>
> a/
> a/b/
> a/b/to-be-removed
> a/to-be-removed
>
> and we first show "a/", remember that prefix in "dir", not show
> "a/b/" because it matches prefix, but still update the prefix to
> "a/b/", not show "a/b/to-be-removed", and because "a/to-be-removed"
> does not match the latest prefix, it is now shown. Am I confused???
No, it's a bug. The correct output should be just "a/". Thanks for
pointing it out, I'm going to fix that.
>
>> @@ -150,43 +170,45 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>> if (S_ISDIR(st.st_mode)) {
>> strbuf_addstr(&directory, ent->name);
>> qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
>> - if (show_only && (remove_directories ||
>> - (matches == MATCHED_EXACTLY))) {
>> - printf(_("Would remove %s\n"), qname);
>> - } else if (remove_directories ||
>> - (matches == MATCHED_EXACTLY)) {
>> - if (!quiet)
>> - printf(_("Removing %s\n"), qname);
>> - if (remove_dir_recursively(&directory,
>> - rm_flags) != 0) {
>> - warning(_("failed to remove %s"), qname);
>> - errors++;
>> - }
>> - } else if (show_only) {
>> - printf(_("Would not remove %s\n"), qname);
>> - } else {
>> - printf(_("Not removing %s\n"), qname);
>> + if (remove_directories || (matches == MATCHED_EXACTLY)) {
>> + remove_dir_recursively_with_dryrun(&directory, rm_flags, dry_run,
>> + &dels, &skips, &errs, prefix);
>> }
>
> Moving the above logic to a single helper function makes sense, but
> can we name it a bit more concisely? Also this helper feels very
> specific to "clean"---does it need to go to dir.[ch], I have to
> wonder.
Would you have a better name in mind for the
remove_dir_recursively_with_dryrun() function? I'm kinda stuck.
My thinking was that since the private function remove_dir_recurse()
in dir.c already handles the recursive removing of files and
directories and checks for nested git directories, it would be better
to modify that function rather than implement something similar but
with dels, skips and errs lists in clean.c.
> I am not very much pleased by the change to dir.[ch] in this patch,
> though.
>
>> +static void append_dir_name(struct string_list *dels, struct string_list *skips,
>> + struct string_list *errs, char *name, const char * prefix, int failed, int isdir)
>> +{
>> + struct strbuf quoted = STRBUF_INIT;
>> +
>> + quote_path_relative(name, strlen(name), "ed, prefix);
>> + if (isdir && quoted.buf[strlen(quoted.buf) -1] != '/')
>> + strbuf_addch("ed, '/');
>> +
>> + if (skips)
>> + string_list_append(skips, quoted.buf);
>> + else if (!failed && dels)
>> + string_list_append(dels, quoted.buf);
>> + else if (errs)
>> + string_list_append(errs, quoted.buf);
>> +}
>
> The three lists dels/skips/errs are mostly mutually exclusive (the
> caller knows which one to throw the element in) except that failed
> controls which one between dels or errs is used.
>
> That's an ugly interface, I have to say. I think the quote-path
> part should become a separate helper function to be used by the
> callers of this function, and the callers should stuff the path to
> the list they want to put the element in. That will eliminate the
> need for this ugliness.
Will get rid of append_dir_name() and reimplement things the way you
suggested above.
Cheers,
Zoltan
^ permalink raw reply
* Re: [PATCH v5 2/3] Makefile: detect when PYTHON_PATH changes
From: Junio C Hamano @ 2012-12-19 0:14 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: Christian Couder, git
In-Reply-To: <20121218234202.GA13151@padd.com>
Thanks.
^ permalink raw reply
* Re: [PATCH v5 2/3] Makefile: detect when PYTHON_PATH changes
From: Pete Wyckoff @ 2012-12-18 23:42 UTC (permalink / raw)
To: Christian Couder; +Cc: Junio C Hamano, git
In-Reply-To: <20121218190009.29910.39426.chriscool@tuxfamily.org>
chriscool@tuxfamily.org wrote on Tue, 18 Dec 2012 20:00 +0100:
> When make is run, the python scripts are created from *.py files that
> are changed to use the python given by PYTHON_PATH. And PYTHON_PATH
> is set by default to /usr/bin/python on Linux.
>
> This is nice except when you run make another time setting a
> different PYTHON_PATH, because, as the python scripts have already
> been created, make finds nothing to do.
>
> The goal of this patch is to detect when the PYTHON_PATH changes and
> to create the python scripts again when this happens. To do that we
> use the same trick that is done to track other variables like prefix,
> flags, tcl/tk path and shell path. We update a GIT-PYTHON-VARS file
> with the PYTHON_PATH and check if it changed.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
I played around with this a bit in the context of git-p4; and it
seems to work fine.
It's interesting that the code in git_remote_helpers/Makefile
does not work with python3, but that's not a problem to solve
here. If you get interested in looking, that approach to
installing always struck me as a bit odd. If it is the right
way, though, maybe we should try to unify the approach to git-p4
and potential future .py scripts in git.
Acked-by: Pete Wyckoff <pw@padd.com>
Thanks for fixing this bug.
-- Pete
^ permalink raw reply
* Re: Prebuilt man pages on Google code
From: Junio C Hamano @ 2012-12-18 23:03 UTC (permalink / raw)
To: John Keeping; +Cc: git
In-Reply-To: <20121216162827.GA22351@river.lan>
John Keeping <john@keeping.me.uk> writes:
> While investigating Asciidoc's quoting in this thread [1], I noticed
> that my system man pages don't display Asciidoc double quoted text
> correctly.
> ...
> I can't see any configuration option that could cause this difference,
> so I assume it must be caused by some particular tool version on the
> machine used to generate these man pages.
Yeah, Debian ships with a tad older one, and I've been using
asciidoc 8.5.2 on my primary box.
I'm experimenting a newer one from the latest tarball (asciidoc
8.6.8) and the result looks promising. If everything goes smoothly,
I'll probably do another -rc before the final 1.8.1 release goes out
with the new asciidoc, but no promises (yet).
Thanks.
^ permalink raw reply
* Re: Feature suggestion: new 'git add -i' command to discard working copy changes
From: Jeff King @ 2012-12-18 22:20 UTC (permalink / raw)
To: Evan Driscoll; +Cc: Junio C Hamano, git
In-Reply-To: <50D0E9DA.1020408@cs.wisc.edu>
On Tue, Dec 18, 2012 at 04:10:34PM -0600, Evan Driscoll wrote:
> I have two use cases of 'add -i'. The more common one is if I kind of
> want -p but don't want to do it for every file. (I guess in part this is
> my way of substituting for not knowing all the actions during -p as
> well.) But I sometimes use it if I want to stage several but not all
> files, as it's often faster for me to just choose the files I want from
> the interactive add's list than it is for me to type each of the files
> that I want (even with tab completion) -- I'm often working in a project
> with unfortunately-deep paths.
>
> What I want for my 'discard' action is more like the latter: I'd like a
> fast way to choose a file(s) to discard without having to type the path(s).
That makes sense.
> Maybe I should just investigate tig or another front end; that might
> satisfy my desire.
Yes, "tig status" will do this (use "!" to revert changes to the path).
Another trick is to stage what you want and throw away the rest, like:
$ hack hack hack
$ git add -i ;# now everything unstaged is garbage
$ git checkout .
$ test test test
$ git commit
Of course that implies that you can separate the wheat from the chaff at
that exact moment. Sometimes you are just discarding known junk to make
further work or "add -i" easier.
-Peff
^ permalink raw reply
* Re: Feature suggestion: new 'git add -i' command to discard working copy changes
From: Evan Driscoll @ 2012-12-18 22:10 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20121218215914.GB1806@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]
On 12/18/2012 03:59 PM, Jeff King wrote:
> I always assumed nobody really used the full "add -i", but maybe it is
> because I am such a command-line snob. Evan, are you after hunk
> selection (like choosing "patch" from the interactive UI), or full path
> selection?
Mostly the latter.
I have two use cases of 'add -i'. The more common one is if I kind of
want -p but don't want to do it for every file. (I guess in part this is
my way of substituting for not knowing all the actions during -p as
well.) But I sometimes use it if I want to stage several but not all
files, as it's often faster for me to just choose the files I want from
the interactive add's list than it is for me to type each of the files
that I want (even with tab completion) -- I'm often working in a project
with unfortunately-deep paths.
What I want for my 'discard' action is more like the latter: I'd like a
fast way to choose a file(s) to discard without having to type the path(s).
Maybe I should just investigate tig or another front end; that might
satisfy my desire.
Evan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 554 bytes --]
^ permalink raw reply
* [PATCH v2 2/2] Port to QNX
From: Matt Kraai @ 2012-12-18 22:03 UTC (permalink / raw)
To: git; +Cc: Matt Kraai
In-Reply-To: <1355868235-23173-1-git-send-email-kraai@ftbfs.org>
From: Matt Kraai <matt.kraai@amo.abbott.com>
Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
---
Makefile | 21 +++++++++++++++++++++
git-compat-util.h | 6 +++++-
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 2c1f04f..a39dc83 100644
--- a/Makefile
+++ b/Makefile
@@ -80,6 +80,8 @@ all::
#
# Define NO_MEMMEM if you don't have memmem.
#
+# Define NO_GETPAGESIZE if you don't have getpagesize.
+#
# Define NO_STRLCPY if you don't have strlcpy.
#
# Define NO_STRTOUMAX if you don't have both strtoimax and strtoumax in the
@@ -1446,6 +1448,22 @@ else
NO_CURL = YesPlease
endif
endif
+ifeq ($(uname_S),QNX)
+ COMPAT_CFLAGS += -DSA_RESTART=0
+ HAVE_STRINGS_H = YesPlease
+ NEEDS_SOCKET = YesPlease
+ NO_FNMATCH_CASEFOLD = YesPlease
+ NO_GETPAGESIZE = YesPlease
+ NO_ICONV = YesPlease
+ NO_MEMMEM = YesPlease
+ NO_MKDTEMP = YesPlease
+ NO_MKSTEMPS = YesPlease
+ NO_NSEC = YesPlease
+ NO_PTHREADS = YesPlease
+ NO_R_TO_GCC_LINKER = YesPlease
+ NO_STRCASESTR = YesPlease
+ NO_STRLCPY = YesPlease
+endif
-include config.mak.autogen
-include config.mak
@@ -1863,6 +1881,9 @@ ifdef NO_MEMMEM
COMPAT_CFLAGS += -DNO_MEMMEM
COMPAT_OBJS += compat/memmem.o
endif
+ifdef NO_GETPAGESIZE
+ COMPAT_CFLAGS += -DNO_GETPAGESIZE
+endif
ifdef INTERNAL_QSORT
COMPAT_CFLAGS += -DINTERNAL_QSORT
COMPAT_OBJS += compat/qsort.o
diff --git a/git-compat-util.h b/git-compat-util.h
index a88147b..610e6b7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -75,7 +75,7 @@
# endif
#elif !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(__USLC__) && \
!defined(_M_UNIX) && !defined(__sgi) && !defined(__DragonFly__) && \
- !defined(__TANDEM)
+ !defined(__TANDEM) && !defined(__QNX__)
#define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
#define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
#endif
@@ -413,6 +413,10 @@ void *gitmemmem(const void *haystack, size_t haystacklen,
const void *needle, size_t needlelen);
#endif
+#ifdef NO_GETPAGESIZE
+#define getpagesize() sysconf(_SC_PAGESIZE)
+#endif
+
#ifdef FREAD_READS_DIRECTORIES
#ifdef fopen
#undef fopen
--
1.8.0.2.8.gc42826d.dirty
^ permalink raw reply related
* [PATCH v2 1/2] Make lock local to fetch_pack
From: Matt Kraai @ 2012-12-18 22:03 UTC (permalink / raw)
To: git; +Cc: Matt Kraai
In-Reply-To: <1355868235-23173-1-git-send-email-kraai@ftbfs.org>
From: Matt Kraai <matt.kraai@amo.abbott.com>
lock is only used by fetch_pack, so move it into that function.
Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
---
builtin/fetch-pack.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index e644398..9bc10b3 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -875,8 +875,6 @@ static int fetch_pack_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
-static struct lock_file lock;
-
static void fetch_pack_setup(void)
{
static int did_setup;
@@ -1069,6 +1067,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
ref_cpy = do_fetch_pack(fd, ref, sought, pack_lockfile);
if (args.depth > 0) {
+ static struct lock_file lock;
struct cache_time mtime;
struct strbuf sb = STRBUF_INIT;
char *shallow = git_path("shallow");
--
1.8.0.2.8.gc42826d.dirty
^ permalink raw reply related
* [PATCH v2 0/2] Port to QNX
From: Matt Kraai @ 2012-12-18 22:03 UTC (permalink / raw)
To: git
This series ports Git to QNX. It differs from the previous version in
that:
* it's rebased on dm/port, so it narrows the scope of the lock
variable in builtin/fetch-pack.c instead of fetch-pack.c and uses
HAVE_STRINGS_H; and
* it disables use of Pthreads, since fork(2) doesn't work once
Pthreads are used.
The first test suite failure occurs because getcwd(3) can return paths
containing symbolic links.
^ permalink raw reply
* Re: Feature suggestion: new 'git add -i' command to discard working copy changes
From: Jeff King @ 2012-12-18 21:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Evan Driscoll, git
In-Reply-To: <7vvcbzdoy9.fsf@alter.siamese.dyndns.org>
On Tue, Dec 18, 2012 at 01:34:38PM -0800, Junio C Hamano wrote:
> Evan Driscoll <driscoll@cs.wisc.edu> writes:
>
> > It's not infrequent that I want to discard changes I've made locally to
> > files ('git checkout file.txt') and find myself wishing that this was an
> > action available from the 'git add --interactive' UI; it feels like it
> > would fit in.
>
> Hrm, not really. "git add" is about manipulating the index and its
> promise is that it won't touch working tree files. And people rely
> on that promise.
>
> I can see how it would be useful to have a UI that is more
> interactive than CLI that shows you a list of paths and lets you
> pick from the list to run "git checkout" on, but I think "git add"
> is a bad match to it.
Yeah. We already generalized "git add -p" to "git checkout -p" (and
"reset -p", etc) to do hunk selection. Nobody bothered to generalize
the rest of "git add --interactive", but logically having "git checkout
--interactive" (and "git reset --interactive") would make sense.
I always assumed nobody really used the full "add -i", but maybe it is
because I am such a command-line snob. Evan, are you after hunk
selection (like choosing "patch" from the interactive UI), or full path
selection?
-Peff
^ permalink raw reply
* Re: Feature suggestion: new 'git add -i' command to discard working copy changes
From: Junio C Hamano @ 2012-12-18 21:34 UTC (permalink / raw)
To: Evan Driscoll; +Cc: git
In-Reply-To: <50D0DA19.5030606@cs.wisc.edu>
Evan Driscoll <driscoll@cs.wisc.edu> writes:
> It's not infrequent that I want to discard changes I've made locally to
> files ('git checkout file.txt') and find myself wishing that this was an
> action available from the 'git add --interactive' UI; it feels like it
> would fit in.
Hrm, not really. "git add" is about manipulating the index and its
promise is that it won't touch working tree files. And people rely
on that promise.
I can see how it would be useful to have a UI that is more
interactive than CLI that shows you a list of paths and lets you
pick from the list to run "git checkout" on, but I think "git add"
is a bad match to it.
^ 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