Re: [PATCH] Extend runtime prefix computation
Hello Erik, On Tue, Apr 16, 2013 at 05:18:49PM +0200, Erik Faye-Lund wrote: Support determining the binaries' installation path at runtime even if called without any path components (i.e. via search path). I think the motivation for the feature in the first place is Windows, where the installation path isn't known at build-time. In the unix-world, this is generally known (/usr/bin or something like that). What's the reason you want it on other platforms? It's part of an in-house development toolchain featuring git that I want to hand to users as a binary installation for a number of platforms but give them the choice where to install it. Secondly, once the infrastructure is in place, it's easier to do or enhance relocatability support for other platforms such as Windows. Finally: Others do it. Perl's done it and I've already needed that as well - on Mac OS X. Thanks, -- Michael Weiserscience + computing ag Senior Systems Engineer Geschaeftsstelle Duesseldorf Martinstrasse 47-55, Haus A phone: +49 211 302 708 32 D-40223 Duesseldorf fax: +49 211 302 708 50 www.science-computing.de -- Vorstandsvorsitzender/Chairman of the board of management: Gerd-Lothar Leonhart Vorstand/Board of Management: Dr. Bernd Finkbeiner, Michael Heinrichs, Dr. Arno Steitz, Dr. Ingrid Zech Vorsitzender des Aufsichtsrats/ Chairman of the Supervisory Board: Philippe Miltin Sitz/Registered Office: Tuebingen Registergericht/Registration Court: Stuttgart Registernummer/Commercial Register No.: HRB 382196 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git crash in Ubuntu 12.04
Hi, $ ulimit -c unlimited Have set the git user's crash limit to 1GB in /etc/security/limits.conf and still getting the same error when issuing gdb to the crash file. Yep, suppsedly in Ubuntu it's not that easy to just get a plain old coredump file -- see below. Got an proper dump from git this time. See whether it helps. I have setup another machine with Ubuntu 12.04 and updated only git and ported the repo there, after 200 clones no crash so far. Mostly will be moving latest repo to that server to solve this issue. gitadmin@gitserver:/var/crash/gitcash$ gdb git CoreDump GNU gdb (Ubuntu/Linaro 7.4-2012.04-0ubuntu2.1) 7.4-2012.04 Copyright (C) 2012 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type show copying and show warranty for details. This GDB was configured as x86_64-linux-gnu. For bug reporting instructions, please see: http://bugs.launchpad.net/gdb-linaro/... Reading symbols from /usr/bin/git...(no debugging symbols found)...done. [New LWP 12823] warning: Can't read pathname for load map: Input/output error. [Thread debugging using libthread_db enabled] Using host libthread_db library /lib/x86_64-linux-gnu/libthread_db.so.1. Core was generated by `git pack-objects --revs --all --stdout --progress --delta-base-offset'. Program terminated with signal 11, Segmentation fault. #0 0x0044bb77 in ?? () (gdb) bt #0 0x0044bb77 in ?? () #1 0x0044cb54 in ?? () #2 0x0044d4f1 in ?? () #3 0x00405634 in ?? () #4 0x00404a30 in ?? () #5 0x7fe2a149876d in __libc_start_main (main=0x404980, argc=7, ubp_av=0x7fffad58fb28, init=optimized out, fini=optimized out, rtld_fini=optimized out, stack_end=0x7fffad58fb18) at libc-start.c:226 #6 0x00404e65 in ?? () #7 0x7fffad58fb18 in ?? () #8 0x001c in ?? () #9 0x0007 in ?? () #10 0x7fffad591c95 in ?? () #11 0x7fffad591c99 in ?? () #12 0x7fffad591ca6 in ?? () #13 0x7fffad591cad in ?? () #14 0x7fffad591cb3 in ?? () #15 0x7fffad591cbc in ?? () #16 0x7fffad591cc7 in ?? () #17 0x in ?? () Thanks, ./Siva. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git log - crash and core dump
On Tue, Apr 16, 2013 at 10:26:40PM -0700, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: René Scharfe rene.scha...@lsrfire.ath.cx writes: How about making split_ident_line() a bit friendlier be letting it provide the epoch as default time stamp instead of NULL? Two knee-jerk concerns I have without going back to the callers: * Would that 0 ever be given to the approxidate parser, which rejects ancient dates in numbers-since-epoch format without @ prefix? * Does any existing caller use the NULL as a sign to see the input was without date and act on that information? I looked at all the callers (there aren't that many), and none of them did Do this on a person-only ident, and do that on an ident with timestamp. So for the callers that ignore timestamp, your patch will be a no-op, and for others that assume there is a timestamp, it will turn a crash/segv into output with funny timestamp. What about sane_ident_split in builtin/commit.c? It explicitly rejects a NULL date. The logic in determine_author_info is a little hard to follow (it assembles the ident line with fmt_ident and then reparses it), but I believe it should be catching a bogus line from commit -c, or from GIT_AUTHOR_DATE in the environment. As a side note, when determine_author_info sees a bogus ident, it appears to just silently ignore it, which is probably a bad thing. Shouldn't we by complaining? Or am I mis-reading the code? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git crash in Ubuntu 12.04
On Wed, Apr 17, 2013 at 4:28 PM, Sivaram Kannan siva.de...@gmail.com wrote: Got an proper dump from git this time. See whether it helps. Probably not because there are no debugging symbols. Not sure how ubuntu packages these symbols.. I have setup another machine with Ubuntu 12.04 and updated only git and ported the repo there, after 200 clones no crash so far. Any chance you could publish the repository that causes the crash? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] l10n: de.po: translate 39 new messages
Ralf Thielow ralf.thie...@gmail.com writes:
#: sequencer.c:536
#, c-format
msgid could not revert %s... %s
msgstr Konnte %s nicht zurücksetzen... %s
#: sequencer.c:1016
msgid Can't revert as initial commit
msgstr Kann nicht zu initialer Version zurücksetzen.
which I don't really like either now that you mention it -- I would
re-translate it as 'reset'. But either way they should be consistent.
I'm not sure I understand. We currently translate reset as neu
setzen/umsetzen,
which is fine if it means a branch or HEAD ('git reset'), but for commits?
What about zurücknehmen?
Sorry for the confusion -- what I meant is: given only zurücksetzen
and no context, I would probably infer that the original message related
to 'reset'. Which it doesn't, so that would be confusing.
Zurücknehmen works for me, or in the same vein you could try
widerrufen.
You can add
Acked-by: Thomas Rast tr...@inf.ethz.ch
when you reroll.
Thanks for your work!
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] l10n: de.po: translate 39 new messages
Ralf Thielow ralf.thie...@gmail.com writes:
Hi,
Thanks for review!
On Mon, Apr 15, 2013 at 9:26 PM, Christian Stimming stimm...@tuhh.dewrote:
Thanks for the regular update! This is great work.
One issue with the plural form messages, though:
Am Montag, 15. April 2013, 18:27:40 schrieb Ralf Thielow:
#: bundle.c:186
-#, fuzzy, c-format
+#, c-format
msgid The bundle contains this ref:
msgid_plural The bundle contains these %d refs:
-msgstr[0] Das Paket enthält %d Referenz
-msgstr[1] Das Paket enthält %d Referenzen
+msgstr[0] Das Paket enthält diese Referenz:
+msgstr[1] Das Paket enthält diese %d Referenzen:
The msgstr[0] must still contain a %d conversion specifier (which will be
filled with the number 1) even though the translated sentence wouldn't need
the 1 anymore. The previous msgstr[0] was correct; the English singular
msgid
is not.
That made me wonder, too. I've played around a bit with this, and it seems
to be OK as long as one of those strings contain at least one format
specifier.
C printf() only knows about the number and types of arguments from the
format string, so *ignoring* arguments is not a problem for correctness.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/33] t3210: test for spurious error messages for dangling packed refs
On 04/17/2013 01:22 AM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: would be that it expires *everything*. But in fact it seems to only expire things that are at least one second old, which doesn't seem at all useful in the real world. --expire=all is accepted without complaint but doesn't do what one would hope. Perhaps that is worth fixing, independent of this topic. Approxidate gives the current time for anything it does not understand, and that is how --expire=all is interpreted as anything older than now. For that matter, even a string now has long been interpreted as the current time with the same I do not understand it, so I'll give you the current timestamp logic, until we added an official support for now at 93cfa7c7a85e (approxidate_careful() reports errorneous date string, 2010-01-26) for entirely different reasons. A completely untested patch for your enjoyment... I enjoy it :-) But it would be better to put the the function in the date module (approxidate_expiry_careful()?) and also use it from other places where an expiry date is being parsed, like prune --expire=date reflog expire --expire=date/--expire-unreachable=date gc --prune=date (maybe there are others). And the special values all etc. would need to be documented. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git log - crash and core dump
On Wed, Apr 17, 2013 at 09:22:01AM +0400, Ivan Lyapunov wrote: I checked René Scharfe's patch and it works - no more git log crash. Also I checked a broken commits by git show and the most of them created by me. I can confirm I never used anyting else except console git commit or netbeans gui to commit, which is just git gui wrapper tool. Doesn't NetBeans use JGit[1]? That makes it a bit more than a wrapper for the Git command line tools. [1] http://eclipse.org/jgit/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Resend PATCH] t3903 (stash): add failing test for ref of form ^{/message}
So, I read through git-stash.sh a little more, and found the following: 1. Any stash that can be shown can be applied, but not necessarily popped or dropped (as the documentation indicates). The reason for this is simple: a pop/drop attempts to clear the entry in the stash reflog as well, but all stashes need to have a corresponding reflog entry (for instance, those created with 'stash create'). 2. IS_STASH_LIKE is a misnomer: all it checks is that the given rev is a merge commit. As a result, you can 'stash show' and 'stash apply' any merge commit. Should we attempt to tighten this somehow, or are we okay with the stash being just another merge commit? Check for a special commit message perhaps? Brandon Casey wrote: You can create a stash without modifying the refs/stash reflog using 'sha1=`git stash create`' and then later apply it using 'git stash apply --index $sha1'. You'll have to reset the work directory yourself though since 'git stash create' does not do so. The stash created this way is just a dangling commit so it will have a lifetime according to the gc.pruneexpire (default 2 weeks currently). Thanks, but I was worried more about reachability of the commit: if I create a ref to it in refs/stashes/* like I suggested, it wouldn't expire until that ref was gone. Then again, I suppose a ref is unnecessary for a temporary stash. Yeah, I can store the SHA-1 hex of the dangling commit in my caller's $state_dir, and apply it from there later. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking between #05 and #06
On Tue, Apr 16, 2013 at 04:52:14PM -0700, Junio C Hamano wrote: * jk/remote-helper-with-signed-tags (2013-04-15) 3 commits - transport-helper: add 'signed-tags' capability - transport-helper: pass --signed-tags=warn-strip to fast-export - fast-export: add --signed-tags=warn-strip mode There were some comments on the noisiness of the warning output, but it appears that everybody involved in the area is basically happy with the direction this series goes in, so I'll expect a reroll and then merge it to 'next'. What do you expect to change in the reroll? The only comments I've seen have been about the warning output it seems to me that we've agreed to leave that as it is. Have I missed something? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/33] repack_without_ref(): silence errors for dangling packed refs
On 04/15/2013 07:39 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Stop emitting an error message for dangling packed references found when deleting another packed reference. See the previous commit for a longer explanation of the issue. Change repack_without_ref_fn() to silently ignore dangling packed references. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Somehow this feels as if it is sweeping the problem under the rug. If you ignore a ref for which a loose ref exists when you update a packed refs file, whether the stale packed one points at an object that is still there or an object that has been garbage collected, you would not even have to check if the ref resolves to object or anything like that, no? Am I missing something? This one feels iffy in the otherwise pleasant-to-read series. The usual situation when this code would be triggered would be that the packed reference is overridden by a loose ref and points at an object that has been garbage collected. In that case it is definitely incorrect to emit an error message. But the fact that we don't explicitly verify that there is an overriding loose reference means that it is possible that the failure to resolve the packed ref comes from some kind of repository corruption, and you are correct that such a problem would be swept under the rug by my change. I've been trying to minimize the extra work that repack_without_ref() needs to do to write peeled references, to avoid stretching out the delay that can now occur when deleting a reference. Thus I was trying to save a check of loose references during this operation. But I guess I agree that a little bit more caution would be prudent. I can think of a few ways to avoid sweeping possible indications of repo corruption under the rug, in order of increasing run-time: 1. If a packed ref's SHA-1 cannot be resolved, write the packed ref to the new packed-refs file anyway with SHA-1 but without a peeled value. This would avoid having to check the loose references and avoid erasing possible evidence of corruption, but would delay an actual check for corruption until a later time. It would be a quick fix, effectively kicking the can down the road instead of sweeping it under the rug. Minor pitfall: a reference that is listed without a peeled value in a fully-peeled pack-refs file tells future readers that the corresponding SHA-1 *cannot* be peeled. IF the named object would somehow reappear in the repository (e.g., via a fetch) and IF the object is peelable and IF there is in fact no loose ref overriding the packed ref, then the final result would be that one form of corruption (reference points to non-existent object) would be converted to another form (reference falsely believed to be non-peelable). I think this is an acceptable risk because (a) it would only happen in an unlikely series of events in a repo that was already corrupt, and (b) because falsely believing a reference to be non-peelable wouldn't have terrible consequences. 2. Whenever a packed reference cannot be resolved to an object, verify that there is indeed a loose reference overriding it; if not, emit an error and in either case omit the packed ref from the output. 3. Check for an overriding loose reference *before* trying to peel a packed reference, and omit any overridden loose references from the output packed-refs file. This would be close to running pack-refs --no-prune without the is_tag_ref test and with reuse of available peeled values. This approach would tidy up the packed-refs file a bit more than (2) because it would cause the deletion of more overridden packed refs, but only as part of first peeling them, which should only happen once in a repo, and only if the first peeling occurs within repack_without_ref() as opposed to an explicit pack_refs(). So it's a negligible improvement over (2). 4. Further along the correctness spectrum, one could check for overriding loose references *every* time the packed-refs file is rewritten by repack_without_ref(), even for references whose peeled values are already known. But this would add overhead to every deletion of a packed reference, which is probably not justified. I'm worried that implementing 2-4 would introduce new race conditions of the type that Peff discovered recently, unless we fix the locking policy first (which is also on my TODO list). So my suggestion is to implement 1 now and implement 2 sometime in the future. Opinions? Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Mon, Apr 15, 2013 at 01:28:53PM -0700, Junio C Hamano wrote: Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. [...] -- [New Topics] [...] * lf/read-blob-data-from-index (2013-04-15) 3 commits (merged to 'next' on 2013-04-15 at 09f92c6) + convert.c: Remove duplicate code + Add size parameter to read_blob_data_from_index_path() + Add public function read_blob_data_from_index_path() Reduce duplicated code between convert.c and attr.c. Will merge to 'master'. Not sure if you care but the commit messages of these are all wrong now that you squashed your API fix into the first commit. They all refer to read_blob_data_from_index_path() instead of read_blob_data_from_index() and most of the details mentioned in the first commit of this series no longer apply... Just saying :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git log - crash and core dump
netbeans use some integrated git wrapper and I don't know about JGit source base or not. In Eclipse we use Egit. Also all broken commits limited to november 2012, but we still continue to use the same development environments without any problems Ivan 2013/4/17 John Keeping j...@keeping.me.uk: On Wed, Apr 17, 2013 at 09:22:01AM +0400, Ivan Lyapunov wrote: I checked René Scharfe's patch and it works - no more git log crash. Also I checked a broken commits by git show and the most of them created by me. I can confirm I never used anyting else except console git commit or netbeans gui to commit, which is just git gui wrapper tool. Doesn't NetBeans use JGit[1]? That makes it a bit more than a wrapper for the Git command line tools. [1] http://eclipse.org/jgit/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git log - crash and core dump
On Wed, 17 Apr 2013 13:14:48 +0400 Ivan Lyapunov dron...@gmail.com wrote: netbeans use some integrated git wrapper and I don't know about JGit source base or not. In Eclipse we use Egit. Also all broken commits limited to november 2012, but we still continue to use the same development environments without any problems Does the same also mean of the same version? I mean that if, say, you managed to update netbeans after November, 2012 that would explain a lot as the update might just silently pull a fix for the Git-interfacing code. The same might apply to Git itself as well. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 11/13] pretty: support padding placeholders, % % and %
On Wed, Apr 17, 2013 at 6:41 AM, Junio C Hamano gits...@pobox.com wrote:
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
+delete_trailing_dollar() {
+ sed 's/\$$//'
+}
This is what we have qz_to_tab_space for, isn't it?
Thanks! I looked for something like this, but my eyes focused on
similar sed line and forgot the tr
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Junio C Hamano gits...@pobox.com writes:
* jc/add-2.0-delete-default (2013-03-08) 3 commits
- git add pathspec... defaults to -A
(merged to 'next' on 2013-04-05 at 199442e)
+ git add: start preparing for git add pathspec... to default to -A
+ builtin/add.c: simplify boolean variables
In Git 2.0, git add pathspec will mean git add -A pathspec. If
you did this in a working tree that tracks dir/lost and dir/another:
$ rm dir/lost
$ edit dir/another
$ git add dir
The last step will not only notices and records updated
dir/another, but also notices and records the removal of dir/lost
in the index.
Start training the users for this change to say --no-all when they
want to ignore the removal to smooth the transition hump.
Will merge to 'master' the early bits and cook the rest in 'next' until Git
2.0.
The warning triggers in some cases where it shouldn't, relating to
submodules:
$ git submodule add gito...@git.csa.inf.ethz.ch:domjudge.git domjudge
Adding existing repo at 'domjudge' to the index
warning: In Git 2.0, 'git add pathspec...' will also update the
index for paths removed from the working tree that match
the given pathspec. If you want to 'add' only changed
or newly created paths, say 'git add --no-all pathspec...' instead.
It also seems to hint that the problem is with giving a 'pathspec', but
in fact in the case of a proper pathspec (that isn't an existing path)
it does *not* trigger, even though it probably should:
$ git ls-files
foo
$ rm foo
$ git add 'f*'
$ git status
# On branch master
# Changes not staged for commit:
# (use git add/rm file... to update what will be committed)
# (use git checkout -- file... to discard changes in working directory)
#
# deleted:foo
#
no changes added to commit (use git add and/or git commit -a)
$ git add -A 'f*'
$ git status
# On branch master
# Changes to be committed:
# (use git reset HEAD file... to unstage)
#
# deleted:foo
#
# Untracked files not listed (use -u option to show untracked files)
That's of course assuming that you want to unconditionally make -A the
default.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 10/13] pretty: add %C(auto) for auto-coloring
On Wed, Apr 17, 2013 at 7:33 AM, Junio C Hamano gits...@pobox.com wrote:
@@ -1005,7 +1006,15 @@ static size_t format_commit_one(struct strbuf *sb, /*
in UTF-8 */
/* these are independent of the commit */
switch (placeholder[0]) {
case 'C':
- return parse_color(sb, placeholder, c);
+ if (!prefixcmp(placeholder + 1, (auto))) {
+ c-auto_color_next = 1;
+ return 7;
+ } else {
+ int ret = parse_color(sb, placeholder, c);
+ if (ret)
+ c-auto_color_next = 0;
+ return ret;
+ }
This is to handle a corrupt input, e.g. %C(auto)%Cbleu%H where
(perhaps deprecated) %Cblue is misspelled, and parse_color()
returns 0 without consuming any byte.
Does it make sense not to turn auto off in such a case?
We don't have any mechanism to report invalid %C. Instead we let them
through as literals. If they are literals, they should not have any
side effects. So I think it makes sense not to turn off things.
Otherwise the above would become
if (!prefixcmp(placeholder + 1, (auto))) {
c-auto_color_next = 1;
return 7; /* consumed 7 bytes, C(auto) */
}
c-auto_color_next = 0;
return parse_color(sb, placeholder, c);
which may be simpler. When we see %C, previous %C(auto) is
cancelled.
If we do this, maybe we could show invalid %C with blinking. Quite
catchy and might make the user wonder why. Of course it won't work
without coloring. But who would add %C in that case.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] clone: introduce clone.submoduleGitDir to relocate $GITDIR
On Sun, Apr 14, 2013 at 5:23 AM, Ramkumar Ramachandra artag...@gmail.com wrote: This configuration variable comes into effect when 'git clone' is invoked inside an existing git repository's worktree. When set, instead of cloning the given repository as-is, it relocates the gitdir of the repository to the path specified by this variable. This setting is especially useful when working with submodules. What if I clone a repo then realize it was a mistake and remove it? With current clone, a rm -rf would do. With this, I'll need to figure out which subdir in the top .git contains the repo I want to remove. I'm not sure how git submodule handles this case though (i.e. total submodule ignorant speaking..) -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git log - crash and core dump
missed a list sorry for dup I ment the same because git changes the version too. Also netbeans/eclipse upgrade are not synced, because of different products. So the same ment only names, not versions. But in deep search another repos I found the broken commits in Eclipse repo dated by 13 march 2013. Can them produced because of previous broken commits? And probably you can be right about java git wrappers can produce this issues, I'll try to make a broken repo from scratch later. Ivan 2013/4/17 Ivan Lyapunov dron...@gmail.com: I ment the same because git changes the version too. Also netbeans/eclipse upgrade are not synced, because of different products. So the same ment only names, not versions. But in deep search another repos I found the broken commits in Eclipse repo dated by 13 march 2013. Can them produced because of previous broken commits? And probably you can be right about java git wrappers can produce this issues, I'll try to make a broken repo from scratch later. Ivan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
On Mon, Apr 8, 2013 at 7:23 AM, Jonathan Nieder jrnie...@gmail.com wrote: Ramkumar Ramachandra wrote: It's about the core object code of git parsing links, as opposed to a fringe submodule.c/ submodule.sh parsing .gitmodules. What's stopping the core object code of git parsing .gitmodules? What is the core object code? How does this compare to other metadata files like .gitattributes and .gitignore? Somewhat related to the topic. Why can't .gitattributes be used for storing what's currently in .gitmodules? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] clone: introduce clone.submoduleGitDir to relocate $GITDIR
Duy Nguyen wrote: What if I clone a repo then realize it was a mistake and remove it? With current clone, a rm -rf would do. With this, I'll need to figure out which subdir in the top .git contains the repo I want to remove. I'm not sure how git submodule handles this case though (i.e. total submodule ignorant speaking..) Currently, submodules relocate the GITDIR of submodules to .git/modules. So, my proposed patch doesn't make the situation any worse. In fact, it improves the situation because you're guaranteed that all your GITDIRs will be in ~/bare (or whatever your core.submoduleGitDir is), as opposed to a complex path in .git/modules of your containing superproject. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] clone: introduce clone.submoduleGitDir to relocate $GITDIR
On Wed, Apr 17, 2013 at 8:53 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Duy Nguyen wrote: What if I clone a repo then realize it was a mistake and remove it? With current clone, a rm -rf would do. With this, I'll need to figure out which subdir in the top .git contains the repo I want to remove. I'm not sure how git submodule handles this case though (i.e. total submodule ignorant speaking..) Currently, submodules relocate the GITDIR of submodules to .git/modules. So, my proposed patch doesn't make the situation any worse. In fact, it improves the situation because you're guaranteed that all your GITDIRs will be in ~/bare (or whatever your core.submoduleGitDir is), as opposed to a complex path in .git/modules of your containing superproject. No, submodule code does not change git clone. If I'm not mistaken, submodule will not kick in until you type git submodule something. If I turn clone.submoduleGitDir on, how can I undo my mistake in a user friendly way? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
Duy Nguyen wrote: Somewhat related to the topic. Why can't .gitattributes be used for storing what's currently in .gitmodules? It can. It's just a small syntax change from key = value attributes inside a toplevel [submodule name] section separated by newlines, to a path marked with multiple key=value attributes separated by whitespace. However, we don't want to make this change because these submodule attributes are somewhat different from .gitattributes attributes. Roughly speaking, the current .gitmodules design treats submodule directories as directories with special attributes, with two differences: these directories have a special mode in the index, and a commit object is created in the database to represent the partial state of this submodule. If you think about it, the information stored in the commit object is no less/ no more important than the path-attribute mapping in .gitmodules. I was arguing for using a special OBJ_LINK to represent the full state of the submodule, and doing away with the attributes altogether, but not everyone agrees. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] clone: introduce clone.submoduleGitDir to relocate $GITDIR
Duy Nguyen wrote: No, submodule code does not change git clone. If I'm not mistaken, submodule will not kick in until you type git submodule something. If I turn clone.submoduleGitDir on, how can I undo my mistake in a user friendly way? So, if you currently want to add a submodule, you have to 'git submodule add', which runs clone internally apart from other things. How do you undo this mistake? What I'm essentially proposing is to give the job of cloning back to clone, and the job of adding back to add, instead of creating an unnatural abstraction over them using 'git submodule add'. The point being: why would you ever _want_ to clone inside a worktree unless you intend to add a submodule? In other words, you intent for running a 'git clone' inside a worktree is exactly the same as your intent for running a 'git submodule add' inside a worktree. Ofcourse, if you have a fringe case where that was _not_ your intent, we'll provide a command-line switch to turn off the relocation for that clone. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
On Wed, Apr 17, 2013 at 9:06 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Duy Nguyen wrote: Somewhat related to the topic. Why can't .gitattributes be used for storing what's currently in .gitmodules? It can. It's just a small syntax change from key = value attributes inside a toplevel [submodule name] section separated by newlines, to a path marked with multiple key=value attributes separated by whitespace. However, we don't want to make this change because these submodule attributes are somewhat different from .gitattributes attributes. Roughly speaking, the current .gitmodules design treats submodule directories as directories with special attributes, with two differences: these directories have a special mode in the index, and a commit object is created in the database to represent the partial state of this submodule. That was my thinking. .gitmodules would break if a user moves the submodule manually (or even if .gitattributes is used) If you think about it, the information stored in the commit object is no less/ no more important than the path-attribute mapping in .gitmodules. I was arguing for using a special OBJ_LINK to represent the full state of the submodule, and doing away with the attributes altogether, but not everyone agrees. Include me to those everyone. url feels like a local thing that should not stay in object database (another way of looking at it is like an email address: the primary one fixed in stone in commits with .mailmap for future substitution). Other attributes like .update, .fetchRecursiveSubmodules... definitely should not be stored in object database. I think if they are stored in the submodule's config file, then the manual move problem above will go away. And if you're dead set on storing some submodule state in object database, why not reuse tag object with some nea header lines? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] clone: introduce clone.submoduleGitDir to relocate $GITDIR
On Wed, Apr 17, 2013 at 9:13 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Duy Nguyen wrote: No, submodule code does not change git clone. If I'm not mistaken, submodule will not kick in until you type git submodule something. If I turn clone.submoduleGitDir on, how can I undo my mistake in a user friendly way? So, if you currently want to add a submodule, you have to 'git submodule add', which runs clone internally apart from other things. How do you undo this mistake? Well, it has submodule in the command line. My first reaction would be looking for git submodule rm or something. What I'm essentially proposing is to give the job of cloning back to clone, and the job of adding back to add, instead of creating an unnatural abstraction over them using 'git submodule add'. The point being: why would you ever _want_ to clone inside a worktree unless you intend to add a submodule? In other words, you intent for running a 'git clone' inside a worktree is exactly the same as your intent for running a 'git submodule add' inside a worktree. Ofcourse, if you have a fringe case where that was _not_ your intent, we'll provide a command-line switch to turn off the relocation for that clone. No, the point is people make mistakes. What do we do in that case? Or will you introduce yet another gc command for clean up ~/bare? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
Duy Nguyen wrote: Include me to those everyone. url feels like a local thing that should not stay in object database (another way of looking at it is like an email address: the primary one fixed in stone in commits with .mailmap for future substitution). We've been over this several times in earlier emails. That's like saying that a blob should not be stored in the object database, because it is not fixed in stone (my OBJ_LINK is just a special kind of blob, as I've repeated many times already). I don't rely on what I feel, which is why I started out by posting an implementation: the implementation seems to indicate that getting an OBJ_LINK will simplify a lot of things. And that is my primary criterion for deciding: if the implementation is simple and elegant, it must clearly be doing something right. Again, I'm not saying that my approach is Correct and Final. What I'm saying is: Here's what I've done. Something interesting is going on. It's probably worth a look? Other attributes like .update, .fetchRecursiveSubmodules... definitely should not be stored in object database. Coffee and other beverages definitely should be served cold. All very nice to say, but I don't see any rationale. I think if they are stored in the submodule's config file, then the manual move problem above will go away. What? The submodule's .git/config? Why should a submodule repository know that it is being used as a submodule? What inherent properties of a git repository change if it is being used as a submodule? And if you're dead set on storing some submodule state in object database, I'm not. I'm just saying that it seems to be an interesting alternative approach. Considering that nobody else brought up a real alternative approach, and chose to just keep defending .gitmodules to the death, it's the only other approach we have. why not reuse tag object with some nea header lines? Or a unified blob, which is currently what we have. The point is to have structured parseable information that the object-parsing code of git code and easily slurp and give to the rest of git-core. Please clear your reading backlog to avoid bringing up the same points over and over again. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
On Wed, Apr 17, 2013 at 9:56 PM, Ramkumar Ramachandra artag...@gmail.com wrote: why not reuse tag object with some nea header lines? Or a unified blob, which is currently what we have. The point is to have structured parseable information that the object-parsing code of git code and easily slurp and give to the rest of git-core. I think you misunderstood. I meant instead of introducing new object type OBJ_LINK, you can reuse tag object and add new header lines for your purposes. Please clear your reading backlog to avoid bringing up the same points over and over again. Yep. I'll shut up until it's cleared. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
Duy Nguyen wrote: On Wed, Apr 17, 2013 at 9:56 PM, Ramkumar Ramachandra artag...@gmail.com wrote: why not reuse tag object with some nea header lines? Or a unified blob, which is currently what we have. The point is to have structured parseable information that the object-parsing code of git code and easily slurp and give to the rest of git-core. I think you misunderstood. I meant instead of introducing new object type OBJ_LINK, you can reuse tag object and add new header lines for your purposes. Oh, I interpreted your typo nea as neat, when you meant new. Yeah, it's worth exploring: I don't know what backward compatibility benefits it will yield yet. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH
The test cases in contrib/remote-helpers use mercurial and python.
Before the tests are run, we check if python can import
mercurial and hggit.
To run this check, python pointed out by PYTHON_PATH is used.
This may not work when different python binaries exist,
and PYTHON_PATH is not set:
Makefile sets it to the default /usr/bin/python
The PATH may point out e.g. /sw/bin/python.
When /sw/bin/python has the mercurial module installed,
but /usr/bin/python has not, the test will not be run.
Git respects PYTHON_PATH, hg does not.
Use python instead of $PYTHON_PATH to check for installed modules.
While at it, split exportX=Y into 2 lines
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
contrib/remote-helpers/test-hg-bidi.sh | 14 +-
contrib/remote-helpers/test-hg-hg-git.sh | 12 +++-
contrib/remote-helpers/test-hg.sh| 2 +-
3 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/contrib/remote-helpers/test-hg-bidi.sh
b/contrib/remote-helpers/test-hg-bidi.sh
index f368953..9f4a430 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -15,7 +15,7 @@ if ! test_have_prereq PYTHON; then
test_done
fi
-if ! $PYTHON_PATH -c 'import mercurial'; then
+if ! python -c 'import mercurial'; then
skip_all='skipping remote-hg tests; mercurial not available'
test_done
fi
@@ -68,10 +68,13 @@ setup () {
) $HOME/.hgrc
git config --global remote-hg.hg-git-compat true
- export HGEDITOR=/usr/bin/true
+ HGEDITOR=/usr/bin/true
+ export HGEDITOR
- export GIT_AUTHOR_DATE=2007-01-01 00:00:00 +0230
- export GIT_COMMITTER_DATE=$GIT_AUTHOR_DATE
+ GIT_AUTHOR_DATE=2007-01-01 00:00:00 +0230
+ GIT_COMMITTER_DATE=$GIT_AUTHOR_DATE
+
+ export GIT_AUTHOR_DATE GIT_COMMITTER_DATE
}
setup
@@ -88,7 +91,8 @@ test_expect_success 'encoding' '
git add alpha
git commit -m add älphà
- export GIT_AUTHOR_NAME=tést èncödîng
+ GIT_AUTHOR_NAME=tést èncödîng
+ export GIT_AUTHOR_NAME
echo beta beta
git add beta
git commit -m add beta
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh
b/contrib/remote-helpers/test-hg-hg-git.sh
index 253e65a..5414f81 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -15,12 +15,12 @@ if ! test_have_prereq PYTHON; then
test_done
fi
-if ! $PYTHON_PATH -c 'import mercurial'; then
+if ! python -c 'import mercurial'; then
skip_all='skipping remote-hg tests; mercurial not available'
test_done
fi
-if ! $PYTHON_PATH -c 'import hggit'; then
+if ! python -c 'import hggit'; then
skip_all='skipping remote-hg tests; hg-git not available'
test_done
fi
@@ -103,10 +103,12 @@ setup () {
git config --global receive.denycurrentbranch warn
git config --global remote-hg.hg-git-compat true
- export HGEDITOR=/usr/bin/true
+ HGEDITOR=/usr/bin/true
+ export HGEDITOR
- export GIT_AUTHOR_DATE=2007-01-01 00:00:00 +0230
- export GIT_COMMITTER_DATE=$GIT_AUTHOR_DATE
+ GIT_AUTHOR_DATE=2007-01-01 00:00:00 +0230
+ GIT_COMMITTER_DATE=$GIT_AUTHOR_DATE
+ export GIT_AUTHOR_DATE GIT_COMMITTER_DATE
}
setup
diff --git a/contrib/remote-helpers/test-hg.sh
b/contrib/remote-helpers/test-hg.sh
index d5b8730..8614fa1 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -15,7 +15,7 @@ if ! test_have_prereq PYTHON; then
test_done
fi
-if ! $PYTHON_PATH -c 'import mercurial'; then
+if ! python -c 'import mercurial'; then
skip_all='skipping remote-hg tests; mercurial not available'
test_done
fi
--
1.8.2.282.g4bc7171
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] clone: introduce clone.submoduleGitDir to relocate $GITDIR
Duy Nguyen wrote: Well, it has submodule in the command line. My first reaction would be looking for git submodule rm or something. No, 'git submodule rm' cannot remove the corresponding GITDIR. What if there are other branches that refer to the submodule? What if you want to remove it from this branch and add it to another branch? No, the point is people make mistakes. What do we do in that case? Or will you introduce yet another gc command for clean up ~/bare? So, people don't make mistakes when they use 'git submodule add', but do make mistakes when using 'git clone'? How has the problem _changed_ with my patch? It's reasonable to point it out as an existing problem, and ask for it to be fixed independent of this discussion, but that is not what you are doing. git cannot read your mind to determine if you made a mistake, if that's what you're asking. No, a gc equivalent won't work either (and there's nothing in the current submodule world), because it is impossible to determine if a workdir is attached to that GITDIR somewhere on your filesystem. You'll have to do _something_ to say that you don't want that GITDIR anymore. It's reasonable to request tooling to help with this task, but your request is entirely different. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Lukas Fleischer g...@cryptocrack.de writes: Not sure if you care but the commit messages of these are all wrong now that you squashed your API fix into the first commit. They all refer to read_blob_data_from_index_path()... Ouch; thanks for noticing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Thomas Rast tr...@inf.ethz.ch writes: The warning triggers in some cases where it shouldn't, relating to submodules: $ git submodule add gito...@git.csa.inf.ethz.ch:domjudge.git domjudge Adding existing repo at 'domjudge' to the index warning: In Git 2.0, 'git add pathspec...' will also update the index for paths removed from the working tree that match the given pathspec. If you want to 'add' only changed or newly created paths, say 'git add --no-all pathspec...' instead. Good one. So add used internally there needs to say --no-add? It also seems to hint that the problem is with giving a 'pathspec', but in fact in the case of a proper pathspec (that isn't an existing path) it does *not* trigger, even though it probably should: We have seen users who explicitly say: git add dir after removing dir/del and adding dir/ins got surprised that we do not notice removal of dir/del without add -A. And it is fairly straight-forward to check and warn for such a case. That's of course assuming that you want to unconditionally make -A the default. I thought what the warning text says is what we decided to do eventually. Not unconditionally, but with pathspec, but not only with pathspec that exactly matches an existing path. It appears that we would better discuss and decide such details further, so let's revert the warn early bits from master and kick the topic back. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 10/13] pretty: add %C(auto) for auto-coloring
Duy Nguyen pclo...@gmail.com writes: This is to handle a corrupt input, e.g. %C(auto)%Cbleu%H where (perhaps deprecated) %Cblue is misspelled, and parse_color() returns 0 without consuming any byte. Does it make sense not to turn auto off in such a case? We don't have any mechanism to report invalid %C. Instead we let them through as literals. If they are literals, they should not have any side effects. So I think it makes sense not to turn off things. Oh, you are right. Thanks for a dose of sanity. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking between #05 and #06
John Keeping j...@keeping.me.uk writes: On Tue, Apr 16, 2013 at 04:52:14PM -0700, Junio C Hamano wrote: * jk/remote-helper-with-signed-tags (2013-04-15) 3 commits - transport-helper: add 'signed-tags' capability - transport-helper: pass --signed-tags=warn-strip to fast-export - fast-export: add --signed-tags=warn-strip mode There were some comments on the noisiness of the warning output, but it appears that everybody involved in the area is basically happy with the direction this series goes in, so I'll expect a reroll and then merge it to 'next'. What do you expect to change in the reroll? The only comments I've seen have been about the warning output it seems to me that we've agreed to leave that as it is. Have I missed something? You missed the sender timestamp of the message you are responding to, and that of the discussion we later agreed there is nothing to change ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] l10n: de.po: translate 39 new messages
Am Mittwoch, 17. April 2013, 10:09:29 schrieb Thomas Rast: msgid The bundle contains this ref: msgid_plural The bundle contains these %d refs: -msgstr[0] Das Paket enthält %d Referenz -msgstr[1] Das Paket enthält %d Referenzen +msgstr[0] Das Paket enthält diese Referenz: +msgstr[1] Das Paket enthält diese %d Referenzen: The msgstr[0] must still contain a %d conversion specifier (which will be filled with the number 1) even though the translated sentence wouldn't need the 1 anymore. The previous msgstr[0] was correct; the English singular msgid is not. That made me wonder, too. I've played around a bit with this, and it seems to be OK as long as one of those strings contain at least one format specifier. C printf() only knows about the number and types of arguments from the format string, so *ignoring* arguments is not a problem for correctness. Indeed both of you are correct and I learned something new. http://stackoverflow.com/questions/3578970/passing-too-many-arguments-to- printf where the second answer quotes Online C Draft Standard (n1256), section 7.19.6.1, paragraph 2: If the format is exhausted while arguments remain, the excess arguments are evaluated (as always) but are otherwise ignored. Hence, it is indeed safe to skip unneeded conversion specifiers both in general ngettext messages and also in their respective translation. This is an explanation which has yet to be added to ngettext's documentation. Best Regards, Christian -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] clone: introduce clone.submoduleGitDir to relocate $GITDIR
Ramkumar Ramachandra wrote: 2. This ugliness complicates implementation of add/ rm/ mv, because each of them will have to know about this contrived path solution. Why is that? Can't they look at the gitfile or call some helper (that happens to be part of the same binary)? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH
Hi, Torsten Bögershausen wrote: Git respects PYTHON_PATH, hg does not. Probably more relevant is that contrib/remote-helpers/git-remote-hg has a shebang line of #!/usr/bin/env python and hence does not respect PYTHON_PATH. Use python instead of $PYTHON_PATH to check for installed modules. Makes sense to me. While at it, split exportX=Y into 2 lines Would you mind splitting this change (which also looks good) into a separate patch? It makes life debugging, cherry-picking, reading, reverting when needed, and so on easier. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Junio C Hamano gits...@pobox.com writes:
Thomas Rast tr...@inf.ethz.ch writes:
The warning triggers in some cases where it shouldn't, relating to
submodules:
$ git submodule add gito...@git.csa.inf.ethz.ch:domjudge.git domjudge
Adding existing repo at 'domjudge' to the index
warning: In Git 2.0, 'git add pathspec...' will also update
the index for paths removed from the working tree that match
the given pathspec. If you want to 'add' only changed
or newly created paths, say 'git add --no-all pathspec...' instead.
Good one. So add used internally there needs to say --no-add?
I think the logic in git-add needs to learn about submodules. The same
warning later trigger when you later say 'git add submoduledir', even
though that obviously doesn't walk inside the submodule.
It also seems to hint that the problem is with giving a 'pathspec', but
in fact in the case of a proper pathspec (that isn't an existing path)
it does *not* trigger, even though it probably should:
We have seen users who explicitly say:
git add dir
after removing dir/del and adding dir/ins got surprised that we do
not notice removal of dir/del without add -A. And it is fairly
straight-forward to check and warn for such a case.
I can see that problem, but along the same lines, why shouldn't I have
an expectation that when I say 'git add *.py' it removes stuff that I
have removed? That's what I tried to show with the f?o example.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
Duy Nguyen pclo...@gmail.com writes: Somewhat related to the topic. Why can't .gitattributes be used for storing what's currently in .gitmodules? You _could_ use gitattributes to encode, but it goes against what a gitattributes file does or is for. It is a mechanism to associate groups of paths (that may not even exist) to a set of attributes. You could list a single pattern that happens to match a single path and at the implementation level you may be able to make it work, but at the design/philosophical level, it is wrong. We need info on each submodule and we need to key it with the name of the submodule, not with its path. At any given time, a single submodule lives at (at most) one path, so you could still use path as a key in the .gitattributes, but when you need to move the submodule path, you would need to update the entry for the submodule in .gitattributes file by finding a pattern that match the old path and making it a pattern that match the new path. We have a much more suitable file format that we use to associate various values to keys: the config format. Also having a file that is only about submodules and nothing else means we could write a content-aware smart ll-merge driver that can take advantage of the knowledge that it is written in the config format and it talks about submodules. The answer to why can't question is no. No, there is no reason why you can't use it. We don't do it, because it just does not make sense. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH
Torsten Bögershausen tbo...@web.de writes:
The test cases in contrib/remote-helpers use mercurial and python.
Before the tests are run, we check if python can import
mercurial and hggit.
To run this check, python pointed out by PYTHON_PATH is used.
This may not work when different python binaries exist,
and PYTHON_PATH is not set:
Makefile sets it to the default /usr/bin/python
The PATH may point out e.g. /sw/bin/python.
When /sw/bin/python has the mercurial module installed,
but /usr/bin/python has not, the test will not be run.
Git respects PYTHON_PATH, hg does not.
The above problem analysis looks sensible.
Use python instead of $PYTHON_PATH to check for installed modules.
But I am not sure if I agree with the solution. Going back to the
analysis, I find this:
This may not work when different python binaries exist,
and PYTHON_PATH is not set:
Isn't the real problem that PYTHON_PATH is not set to point at the
instance of a python with mercurial module for it? Why not make
sure it is set and can be seen by tests correctly?
I do not offhand know where in the contrib/remote-helpers/ part the
user is expected to tweak PYTHON_PATH variable, and if the variable
is correctly arranged to be exported to the environment when running
the tests, but your problem analysis tells me that that is the part
you would want to fix. If a default-fallback value for PYTHON_PATH
is the easiest solution to the problem, you could even solve that in
the export it while running the tests logic.
Perhaps adding
PYTHON_PATH ?= python
export PYTHON_PATH
to contrib/remote-helpers/Makefile and changing nothing else would
be a starting point for a more reasonable fix to the issue?
While at it, split exportX=Y into 2 lines
That is an important portability fix, but as you said while at it,
it is orthogonal.
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
contrib/remote-helpers/test-hg-bidi.sh | 14 +-
contrib/remote-helpers/test-hg-hg-git.sh | 12 +++-
contrib/remote-helpers/test-hg.sh| 2 +-
3 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/contrib/remote-helpers/test-hg-bidi.sh
b/contrib/remote-helpers/test-hg-bidi.sh
index f368953..9f4a430 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -15,7 +15,7 @@ if ! test_have_prereq PYTHON; then
test_done
fi
-if ! $PYTHON_PATH -c 'import mercurial'; then
+if ! python -c 'import mercurial'; then
skip_all='skipping remote-hg tests; mercurial not available'
test_done
fi
@@ -68,10 +68,13 @@ setup () {
) $HOME/.hgrc
git config --global remote-hg.hg-git-compat true
- export HGEDITOR=/usr/bin/true
+ HGEDITOR=/usr/bin/true
+ export HGEDITOR
- export GIT_AUTHOR_DATE=2007-01-01 00:00:00 +0230
- export GIT_COMMITTER_DATE=$GIT_AUTHOR_DATE
+ GIT_AUTHOR_DATE=2007-01-01 00:00:00 +0230
+ GIT_COMMITTER_DATE=$GIT_AUTHOR_DATE
+
+ export GIT_AUTHOR_DATE GIT_COMMITTER_DATE
}
setup
@@ -88,7 +91,8 @@ test_expect_success 'encoding' '
git add alpha
git commit -m add älphà
- export GIT_AUTHOR_NAME=tést èncödîng
+ GIT_AUTHOR_NAME=tést èncödîng
+ export GIT_AUTHOR_NAME
echo beta beta
git add beta
git commit -m add beta
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh
b/contrib/remote-helpers/test-hg-hg-git.sh
index 253e65a..5414f81 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -15,12 +15,12 @@ if ! test_have_prereq PYTHON; then
test_done
fi
-if ! $PYTHON_PATH -c 'import mercurial'; then
+if ! python -c 'import mercurial'; then
skip_all='skipping remote-hg tests; mercurial not available'
test_done
fi
-if ! $PYTHON_PATH -c 'import hggit'; then
+if ! python -c 'import hggit'; then
skip_all='skipping remote-hg tests; hg-git not available'
test_done
fi
@@ -103,10 +103,12 @@ setup () {
git config --global receive.denycurrentbranch warn
git config --global remote-hg.hg-git-compat true
- export HGEDITOR=/usr/bin/true
+ HGEDITOR=/usr/bin/true
+ export HGEDITOR
- export GIT_AUTHOR_DATE=2007-01-01 00:00:00 +0230
- export GIT_COMMITTER_DATE=$GIT_AUTHOR_DATE
+ GIT_AUTHOR_DATE=2007-01-01 00:00:00 +0230
+ GIT_COMMITTER_DATE=$GIT_AUTHOR_DATE
+ export GIT_AUTHOR_DATE GIT_COMMITTER_DATE
}
setup
diff --git a/contrib/remote-helpers/test-hg.sh
b/contrib/remote-helpers/test-hg.sh
index d5b8730..8614fa1 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -15,7 +15,7 @@ if ! test_have_prereq PYTHON; then
test_done
fi
-if ! $PYTHON_PATH -c 'import mercurial'; then
+if ! python -c 'import mercurial'; then
skip_all='skipping remote-hg tests; mercurial not available'
test_done
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Thomas Rast tr...@inf.ethz.ch writes: I can see that problem, but along the same lines, why shouldn't I have an expectation that when I say 'git add *.py' it removes stuff that I have removed? You _should_ have that expectation. If it does not remove with the code that has been prepared for 2.0 (that is a bit beyond 'next'), then it is a big problem, but I think it does remove the removed python source without -A, as long as you give a pathspec *.py (with quotes around it) that match it. I think it is just the warning code avoiding extra complexity and overhead, if you are talking about not getting warning in the pre-2.0 step that is in 'next'. Patches are very much welcomed, especially the ones that come before I get around to it ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] clone: introduce clone.submoduleGitDir to relocate $GITDIR
Duy Nguyen pclo...@gmail.com writes: No, the point is people make mistakes. What do we do in that case? Or will you introduce yet another gc command for clean up ~/bare? I do not know if it will be a gc, but we would need a way for the user to say I no longer need the repository for this submodule kept locally here (I may have to re-clone when I check out a version that needs the submodule) to free up the .git/modules/name directories in the superproject. We might want to allow submodule deinit to also ask for it, but deinit will not be the only occasion the user might want it. It is already a problem that needs to be addressed in the current setup. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Resend PATCH] t3903 (stash): add failing test for ref of form ^{/message}
Ramkumar Ramachandra artag...@gmail.com writes: 1. Any stash that can be shown can be applied, but not necessarily popped or dropped (as the documentation indicates). The reason for this is simple: a pop/drop attempts to clear the entry in the stash reflog as well, but all stashes need to have a corresponding reflog entry (for instance, those created with 'stash create'). Correct. To show or apply, you only need a product of stash create that may not be linked anywhere in the refs or reflogs. But you can only pop or drop something on the stash reflog. 2. IS_STASH_LIKE is a misnomer: all it checks is that the given rev is a merge commit. The purpose of the logic is to reject a mistake to name stuff that is clearly not a product of stash create. The name is just being humble by not claiming I know this is definitely a product of stash-create but merely hinting This smells like such an object; I am not sure if that is a misnomer. You are free to try to think of a way to tighten the implemention to reject a random two-or-three parent merge commit that is not a product of stash create. People already have looked at this code since it was written, and didn't find a reasonable way to tighten it without triggering false negatives, so I wouldn't be surprised if anybody tried it again today and failed. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git log - crash and core dump
Jeff King p...@peff.net writes: What about sane_ident_split in builtin/commit.c? It explicitly rejects a NULL date. The logic in determine_author_info is a little hard to follow (it assembles the ident line with fmt_ident and then reparses it), but I believe it should be catching a bogus line from commit -c, or from GIT_AUTHOR_DATE in the environment. Yeah, you are of course right. I noticed that fmt_ident then parse sequence a bit funny, too. It seems to manually parse to prepare what it feeds fmt_ident. As a side note, when determine_author_info sees a bogus ident, it appears to just silently ignore it, which is probably a bad thing. True, too. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git log - crash and core dump
Am 17.04.2013 08:39, schrieb Jeff King: On Tue, Apr 16, 2013 at 10:26:40PM -0700, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: René Scharfe rene.scha...@lsrfire.ath.cx writes: How about making split_ident_line() a bit friendlier be letting it provide the epoch as default time stamp instead of NULL? Two knee-jerk concerns I have without going back to the callers: * Would that 0 ever be given to the approxidate parser, which rejects ancient dates in numbers-since-epoch format without @ prefix? * Does any existing caller use the NULL as a sign to see the input was without date and act on that information? I looked at all the callers (there aren't that many), and none of them did Do this on a person-only ident, and do that on an ident with timestamp. So for the callers that ignore timestamp, your patch will be a no-op, and for others that assume there is a timestamp, it will turn a crash/segv into output with funny timestamp. What about sane_ident_split in builtin/commit.c? It explicitly rejects a NULL date. The logic in determine_author_info is a little hard to follow (it assembles the ident line with fmt_ident and then reparses it), but I believe it should be catching a bogus line from commit -c, or from GIT_AUTHOR_DATE in the environment. Right, so let's keep the NULLs and fix the individual cases. A quick git grep -W -e date_begin -e date_end -e tz_begin -e tz_end reveals that there are only the ones we talked about: blame, pretty, commit and -- of course -- ident. And only the first two need fixing. As a side note, when determine_author_info sees a bogus ident, it appears to just silently ignore it, which is probably a bad thing. Shouldn't we by complaining? Or am I mis-reading the code? The code looks complicated, but I just tried it: fmt_ident() dies if you give it an invalid date. René -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git log - crash and core dump
On Wed, Apr 17, 2013 at 07:59:28PM +0200, René Scharfe wrote: What about sane_ident_split in builtin/commit.c? It explicitly rejects a NULL date. The logic in determine_author_info is a little hard to follow (it assembles the ident line with fmt_ident and then reparses it), but I believe it should be catching a bogus line from commit -c, or from GIT_AUTHOR_DATE in the environment. Right, so let's keep the NULLs and fix the individual cases. A quick git grep -W -e date_begin -e date_end -e tz_begin -e tz_end reveals that there are only the ones we talked about: blame, pretty, commit and -- of course -- ident. And only the first two need fixing. Yes, that matches the list I came up with in February. I think we also need to do something about git cat-file -p, which does not use the split_ident_line parser (but has its own problems with the home-grown parser). As a side note, when determine_author_info sees a bogus ident, it appears to just silently ignore it, which is probably a bad thing. Shouldn't we by complaining? Or am I mis-reading the code? The code looks complicated, but I just tried it: fmt_ident() dies if you give it an invalid date. It does seem like determine_author_info can be greatly simplified, but I haven't looked all that closely. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Junio C Hamano gits...@pobox.com writes: Thomas Rast tr...@inf.ethz.ch writes: I can see that problem, but along the same lines, why shouldn't I have an expectation that when I say 'git add *.py' it removes stuff that I have removed? You _should_ have that expectation. If it does not remove with the code that has been prepared for 2.0 (that is a bit beyond 'next'), then it is a big problem, but I think it does remove the removed python source without -A, as long as you give a pathspec *.py (with quotes around it) that match it. I think it is just the warning code avoiding extra complexity and overhead, if you are talking about not getting warning in the pre-2.0 step that is in 'next'. Patches are very much welcomed, especially the ones that come before I get around to it ;-) I took a brief look at the code, and as you said add needs to know about submodules, and the best fix looks to me to take the same approach Jonathan came up with to de-noise the add -u/-A topic. That is, to scan the working tree to actually see if we would record removals to the index in 2.0, but not remove them in this current version, and give the warning when the differences in the behaviours matter. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pretty: handle broken commit headers gracefully
Centralize the parsing of the date and time zone strings in the new
helper function show_ident_date() and make sure it checks the pointers
provided by split_ident_line() for NULL before use.
Reported-by: Ivan Lyapunov dron...@gmail.com
Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
---
The first test case passes on v1.8.1, i.e. it also showed the epoch.
The second one passes on v1.8.1 and on master because there already
is a NULL check in format_person_part().
pretty.c | 45 -
t/t4211-log-corrupt.sh | 42 ++
2 files changed, 66 insertions(+), 21 deletions(-)
create mode 100755 t/t4211-log-corrupt.sh
diff --git a/pretty.c b/pretty.c
index d3a82d2..c116248 100644
--- a/pretty.c
+++ b/pretty.c
@@ -393,6 +393,19 @@ static void add_rfc2047(struct strbuf *sb, const char
*line, size_t len,
strbuf_addstr(sb, ?=);
}
+static const char *show_ident_date(const struct ident_split *ident,
+ enum date_mode mode)
+{
+ unsigned long date = 0;
+ int tz = 0;
+
+ if (ident-date_begin ident-date_end)
+ date = strtoul(ident-date_begin, NULL, 10);
+ if (ident-tz_begin ident-tz_end)
+ tz = strtol(ident-tz_begin, NULL, 10);
+ return show_date(date, tz, mode);
+}
+
void pp_user_info(const struct pretty_print_context *pp,
const char *what, struct strbuf *sb,
const char *line, const char *encoding)
@@ -401,12 +414,10 @@ void pp_user_info(const struct pretty_print_context *pp,
struct strbuf mail;
struct ident_split ident;
int linelen;
- char *line_end, *date;
+ char *line_end;
const char *mailbuf, *namebuf;
size_t namelen, maillen;
int max_length = 78; /* per rfc2822 */
- unsigned long time;
- int tz;
if (pp-fmt == CMIT_FMT_ONELINE)
return;
@@ -438,8 +449,6 @@ void pp_user_info(const struct pretty_print_context *pp,
strbuf_add(name, namebuf, namelen);
namelen = name.len + mail.len + 3; /* ' ' + '' + '' */
- time = strtoul(ident.date_begin, date, 10);
- tz = strtol(date, NULL, 10);
if (pp-fmt == CMIT_FMT_EMAIL) {
strbuf_addstr(sb, From: );
@@ -472,13 +481,16 @@ void pp_user_info(const struct pretty_print_context *pp,
switch (pp-fmt) {
case CMIT_FMT_MEDIUM:
- strbuf_addf(sb, Date: %s\n, show_date(time, tz,
pp-date_mode));
+ strbuf_addf(sb, Date: %s\n,
+ show_ident_date(ident, pp-date_mode));
break;
case CMIT_FMT_EMAIL:
- strbuf_addf(sb, Date: %s\n, show_date(time, tz,
DATE_RFC2822));
+ strbuf_addf(sb, Date: %s\n,
+ show_ident_date(ident, DATE_RFC2822));
break;
case CMIT_FMT_FULLER:
- strbuf_addf(sb, %sDate: %s\n, what, show_date(time, tz,
pp-date_mode));
+ strbuf_addf(sb, %sDate: %s\n, what,
+ show_ident_date(ident, pp-date_mode));
break;
default:
/* notin' */
@@ -688,8 +700,6 @@ static size_t format_person_part(struct strbuf *sb, char
part,
{
/* currently all placeholders have same length */
const int placeholder_len = 2;
- int tz;
- unsigned long date = 0;
struct ident_split s;
const char *name, *mail;
size_t maillen, namelen;
@@ -716,30 +726,23 @@ static size_t format_person_part(struct strbuf *sb, char
part,
if (!s.date_begin)
goto skip;
- date = strtoul(s.date_begin, NULL, 10);
-
if (part == 't') { /* date, UNIX timestamp */
strbuf_add(sb, s.date_begin, s.date_end - s.date_begin);
return placeholder_len;
}
- /* parse tz */
- tz = strtoul(s.tz_begin + 1, NULL, 10);
- if (*s.tz_begin == '-')
- tz = -tz;
-
switch (part) {
case 'd': /* date */
- strbuf_addstr(sb, show_date(date, tz, dmode));
+ strbuf_addstr(sb, show_ident_date(s, dmode));
return placeholder_len;
case 'D': /* date, RFC2822 style */
- strbuf_addstr(sb, show_date(date, tz, DATE_RFC2822));
+ strbuf_addstr(sb, show_ident_date(s, DATE_RFC2822));
return placeholder_len;
case 'r': /* date, relative */
- strbuf_addstr(sb, show_date(date, tz, DATE_RELATIVE));
+ strbuf_addstr(sb, show_ident_date(s, DATE_RELATIVE));
return placeholder_len;
case 'i': /* date, ISO 8601 */
- strbuf_addstr(sb, show_date(date, tz, DATE_ISO8601));
+ strbuf_addstr(sb, show_ident_date(s, DATE_ISO8601));
return placeholder_len;
[PATCH] blame: handle broken commit headers gracefully
split_ident_line() can leave us with the pointers date_begin, date_end, tz_begin and tz_end all set to NULL. Check them before use and supply the same fallback values as in the case of a negative return code from split_ident_line(). The (unknown) is not actually shown in the output, though, because it will be converted to a number (zero) eventually. Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx --- Minimal patch, test case missing. It's a bit sad that the old commit parser of blame handled Ivan's specific corruption (extra - after email) gracefully because it used the spaces as cutting points instead of and . builtin/blame.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 86100e9..7770781 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -1375,10 +1375,15 @@ static void get_ac_line(const char *inbuf, const char *what, maillen = ident.mail_end - ident.mail_begin; mailbuf = ident.mail_begin; - *time = strtoul(ident.date_begin, NULL, 10); + if (ident.date_begin ident.date_end) + *time = strtoul(ident.date_begin, NULL, 10); + else + *time = 0; - len = ident.tz_end - ident.tz_begin; - strbuf_add(tz, ident.tz_begin, len); + if (ident.tz_begin ident.tz_end) + strbuf_add(tz, ident.tz_begin, ident.tz_end - ident.tz_begin); + else + strbuf_addstr(tz, (unknown)); /* * Now, convert both name and e-mail using mailmap -- 1.8.2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH
On Wed, Apr 17, 2013 at 9:10 AM, Torsten Bögershausen tbo...@web.de wrote: The test cases in contrib/remote-helpers use mercurial and python. Before the tests are run, we check if python can import mercurial and hggit. To run this check, python pointed out by PYTHON_PATH is used. This may not work when different python binaries exist, and PYTHON_PATH is not set: Makefile sets it to the default /usr/bin/python The PATH may point out e.g. /sw/bin/python. When /sw/bin/python has the mercurial module installed, but /usr/bin/python has not, the test will not be run. Git respects PYTHON_PATH, hg does not. Use python instead of $PYTHON_PATH to check for installed modules. And this would fail if the distribution doesn't have a 'python' binary, and instead has python2, python3, etc. While at it, split exportX=Y into 2 lines Do it in a separate patch. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Tue, Apr 16, 2013 at 5:45 PM, Phil Hord phil.h...@gmail.com wrote: On Tue, Apr 16, 2013 at 3:04 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Tue, Apr 16, 2013 at 4:59 AM, Thomas Rast tr...@inf.ethz.ch wrote: A cursory look^W^Wreview of the messages in fc/remote-hg: [skipping irrelevant comments] I'm sorry, did you actually hit an issue that required to look at the commit message to understand where the issue came from? No? Then I won't bother with hypotheticals. If you want to waste your time, by all means, rewrite all my commit messages with essays that nobody will ever read. I'm not going to do that for some hypothetical case that will never happen. I'm not going to waste my time. This is not a hypothetical. Almost every time I bisect a regression in git.git, I find the commit message tells me exactly why the commit did what it did and what the expected result was. I find this to be amazingly useful. Do I need to show you real instances of that happening? No. I promise it did, though. Yes please. Show me one of the instances where you hit a bisect with any of the remote-hg commits mentioned above by Thomas Rast. Of course, 99% of the commit messages may never be useful to me or anyone else. But we do not eschew them altogether. The 1% I have to rely on are nearly always helpful and clear, and that is the part I care about. And how do you know this will be part of the 1%? You don't. How many times have you tracked regressions in transport helper's import/export functionality? How many times in remote-hg? How many times has *anybody* done so? If you will not waste your time to write a decent commit message, why do you waste our time asking us to review and accept ill-defined patches? Because it *fixes a problem*. And a commit essay doesn't fix any, because nobody will ever go back in history and wonder, hey, what is up with this commit. If somebody does, then I will accept that commit essays are always a must. But it won't happen. Here, of course, I use the royal us as I do not review your patches. I do not know why that is; I suppose you patch things outside of my interests, but it may also be that your patches are simply incomprehensible by design. Yeah, but that's the thing, if you don't understand the code the patches are changing, then how can you know the commit message is sufficient to figure things out when a regression is found? You don't. You can't. Let's face the truth, you are advocating for stopping progress on the name that something might happen sometime in the feature, although most likely won't. When in reality, it just won't. And you are not saying it would be nice to have full commit essay, you are saying: without a commit essay this patch should NOT be merged, even more without a commit essay this patch should NOT be considered a cooking patch. I think the commit message is fine, you don't. So YOU go ahead and write the proper one. If you don't, all you are doing is being an impediment to progress. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git log - crash and core dump
Am 17.04.2013 20:02, schrieb Jeff King: I think we also need to do something about git cat-file -p, which does not use the split_ident_line parser (but has its own problems with the home-grown parser). Ah, while it prints commit object contents verbatim, it formats the date of tags. And it does it without help from tag.c (or ident.c), which in turn does its own parsing as well. So it looks like we have two more candidates for conversion to split_ident_line() here. René -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib/test-hg*.sh: Do not use PYTHON_PATH
On Wed, Apr 17, 2013 at 1:36 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, Apr 17, 2013 at 9:10 AM, Torsten Bögershausen tbo...@web.de wrote: The test cases in contrib/remote-helpers use mercurial and python. Before the tests are run, we check if python can import mercurial and hggit. To run this check, python pointed out by PYTHON_PATH is used. This may not work when different python binaries exist, and PYTHON_PATH is not set: Makefile sets it to the default /usr/bin/python The PATH may point out e.g. /sw/bin/python. When /sw/bin/python has the mercurial module installed, but /usr/bin/python has not, the test will not be run. Git respects PYTHON_PATH, hg does not. Use python instead of $PYTHON_PATH to check for installed modules. And this would fail if the distribution doesn't have a 'python' binary, and instead has python2, python3, etc. Also, it would fail if mercurial is installed for python2, and python is really python3. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/14] Improve git-status --ignored
Am 15.04.2013 22:25, schrieb Junio C Hamano: Karsten Blees karsten.bl...@gmail.com writes: Am 15.04.2013 21:33, schrieb Junio C Hamano: Junio C Hamano gits...@pobox.com writes: Karsten Blees karsten.bl...@gmail.com writes: This patch series addresses several bugs and performance issues in .gitignore processing. A 8-patch series ending at 5d765dc7888b (dir.c: git-status: avoid is_excluded checks for tracked files, 2013-03-18) has been cooking in 'next'; in general we won't revert and requeue a new round for a topic that has already merged to 'next'. I'm sorry to have caused such trouble. I thought you were expecting a reroll from this: Heh, that was an ancient history. My git time is scarce, so progress is slow...guess I need some performance improvements, too :-) It is not such a big deal to revert stuff from 'next'. I've tried to queue this reroll, but tentatively ejected the result from 'pu' for today's integration run, as as/check-ignore topic has an unpleasant conflict with this. I'll send fixups for #11 and #14, or you can pick the entire series rebased to pu from: https://github.com/kblees/git/tree/kb/improve-git-status-ignored-v2-pu git pull git://github.com/kblees/git.git kb/improve-git-status-ignored-v2-pu HTH, Karsten -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2-pu 11/14] dir.c: replace is_path_excluded with now equivalent is_excluded API
Signed-off-by: Karsten Blees bl...@dcon.de
---
builtin/add.c | 5 +---
builtin/check-ignore.c | 16 --
builtin/ls-files.c | 15 +++---
dir.c | 79 --
dir.h | 16 ++
unpack-trees.c | 10 +--
unpack-trees.h | 1 -
7 files changed, 21 insertions(+), 121 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index ddd5e38..050330e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -419,9 +419,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (pathspec) {
int i;
- struct path_exclude_check check;
- path_exclude_check_init(check, dir);
if (!seen)
seen = find_pathspecs_matching_against_index(pathspec);
for (i = 0; pathspec[i]; i++) {
@@ -429,7 +427,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
!file_exists(pathspec[i])) {
if (ignore_missing) {
int dtype = DT_UNKNOWN;
- if (is_path_excluded(check,
pathspec[i], -1, dtype))
+ if (is_excluded(dir, pathspec[i],
dtype))
dir_add_ignored(dir,
pathspec[i], strlen(pathspec[i]));
} else
die(_(pathspec '%s' did not match any
files),
@@ -437,7 +435,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
}
}
free(seen);
- path_exclude_check_clear(check);
}
plug_bulk_checkin();
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index c00a7d6..a4357fb 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -63,7 +63,7 @@ static void output_exclude(const char *path, struct exclude
*exclude)
}
}
-static int check_ignore(struct path_exclude_check *check,
+static int check_ignore(struct dir_struct *dir,
const char *prefix, const char **pathspec)
{
const char *path, *full_path;
@@ -91,8 +91,7 @@ static int check_ignore(struct path_exclude_check *check,
die_if_path_beyond_symlink(full_path, prefix);
exclude = NULL;
if (!seen[i]) {
- exclude = last_exclude_matching_path(check, full_path,
--1, dtype);
+ exclude = last_exclude_matching(dir, full_path, dtype);
}
if (!quiet (exclude || show_non_matching))
output_exclude(path, exclude);
@@ -104,7 +103,7 @@ static int check_ignore(struct path_exclude_check *check,
return num_ignored;
}
-static int check_ignore_stdin_paths(struct path_exclude_check *check, const
char *prefix)
+static int check_ignore_stdin_paths(struct dir_struct *dir, const char *prefix)
{
struct strbuf buf, nbuf;
char *pathspec[2] = { NULL, NULL };
@@ -121,7 +120,7 @@ static int check_ignore_stdin_paths(struct
path_exclude_check *check, const char
strbuf_swap(buf, nbuf);
}
pathspec[0] = buf.buf;
- num_ignored += check_ignore(check, prefix, (const char
**)pathspec);
+ num_ignored += check_ignore(dir, prefix, (const char
**)pathspec);
maybe_flush_or_die(stdout, check-ignore to stdout);
}
strbuf_release(buf);
@@ -133,7 +132,6 @@ int cmd_check_ignore(int argc, const char **argv, const
char *prefix)
{
int num_ignored;
struct dir_struct dir;
- struct path_exclude_check check;
git_config(git_default_config, NULL);
@@ -166,16 +164,14 @@ int cmd_check_ignore(int argc, const char **argv, const
char *prefix)
dir.flags |= DIR_COLLECT_IGNORED;
setup_standard_excludes(dir);
- path_exclude_check_init(check, dir);
if (stdin_paths) {
- num_ignored = check_ignore_stdin_paths(check, prefix);
+ num_ignored = check_ignore_stdin_paths(dir, prefix);
} else {
- num_ignored = check_ignore(check, prefix, argv);
+ num_ignored = check_ignore(dir, prefix, argv);
maybe_flush_or_die(stdout, ignore to stdout);
}
clear_directory(dir);
- path_exclude_check_clear(check);
return !num_ignored;
}
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 175e6e3..2202072 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -201,19 +201,15 @@ static void show_ru_info(void)
}
}
-static int ce_excluded(struct path_exclude_check *check, struct cache_entry
*ce)
+static int ce_excluded(struct dir_struct *dir, struct
[PATCH v2-pu 14/14] dir.c: git-status --ignored: don't scan the work tree twice
'git-status --ignored' still scans the work tree twice to collect
untracked and ignored files, respectively.
fill_directory / read_directory already supports collecting untracked and
ignored files in a single directory scan. However, the DIR_COLLECT_IGNORED
flag to enable this has some git-add specific side-effects (e.g. it
doesn't recurse into ignored directories, so listing ignored files with
--untracked=all doesn't work).
The DIR_SHOW_IGNORED flag doesn't list untracked files and returns ignored
files in dir_struct.entries[] (instead of dir_struct.ignored[] as
DIR_COLLECT_IGNORED). DIR_SHOW_IGNORED is used all throughout git.
We don't want to break the existing API, so lets introduce a new flag
DIR_SHOW_IGNORED_TOO that lists untracked as well as ignored files similar
to DIR_COLLECT_FILES, but will recurse into sub-directories based on the
other flags as DIR_SHOW_IGNORED does.
In dir.c::read_directory_recursive, add ignored files to either
dir_struct.entries[] or dir_struct.ignored[] based on the flags. Also move
the DIR_COLLECT_IGNORED case here so that filling result lists is in a
common place.
In wt-status.c::wt_status_collect_untracked, use the new flag and read
results from dir_struct.ignored[]. Remove the extra fill_directory call.
builtin/check-ignore.c doesn't call fill_directory, setting the git-add
specific DIR_COLLECT_IGNORED flag has no effect here. Remove for clarity.
Update API documentation to reflect the changes.
Performance: with this patch, 'git-status --ignored' is typically as fast
as 'git-status'.
Signed-off-by: Karsten Blees bl...@dcon.de
---
Documentation/technical/api-directory-listing.txt | 25 ---
builtin/check-ignore.c| 1 -
dir.c | 10 +
dir.h | 3 ++-
wt-status.c | 24 ++
5 files changed, 41 insertions(+), 22 deletions(-)
diff --git a/Documentation/technical/api-directory-listing.txt
b/Documentation/technical/api-directory-listing.txt
index 1f349b2..7f8e78d 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -22,12 +22,23 @@ The notable options are:
`flags`::
- A bit-field of options:
+ A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
`DIR_SHOW_IGNORED`:::
- The traversal is for finding just ignored files, not unignored
- files.
+ Return just ignored files in `entries[]`, not untracked files.
+
+`DIR_SHOW_IGNORED_TOO`:::
+
+ Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
+ in addition to untracked files in `entries[]`.
+
+`DIR_COLLECT_IGNORED`:::
+
+ Special mode for git-add. Return ignored files in `ignored[]` and
+ untracked files in `entries[]`. Only returns ignored files that match
+ pathspec exactly (no wildcards). Does not recurse into ignored
+ directories.
`DIR_SHOW_OTHER_DIRECTORIES`:::
@@ -57,6 +68,14 @@ The result of the enumeration is left in these fields:
Internal use; keeps track of allocation of `entries[]` array.
+`ignored[]`::
+
+ An array of `struct dir_entry`, used for ignored paths with the
+ `DIR_SHOW_IGNORED_TOO` and `DIR_COLLECT_IGNORED` flags.
+
+`ignored_nr`::
+
+ The number of members in `ignored[]` array.
Calling sequence
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index a4357fb..4a8fc70 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -161,7 +161,6 @@ int cmd_check_ignore(int argc, const char **argv, const
char *prefix)
die(_(index file corrupt));
memset(dir, 0, sizeof(dir));
- dir.flags |= DIR_COLLECT_IGNORED;
setup_standard_excludes(dir);
if (stdin_paths) {
diff --git a/dir.c b/dir.c
index 2088891..ee4d04d 100644
--- a/dir.c
+++ b/dir.c
@@ -1183,15 +1183,12 @@ static enum path_treatment treat_one_path(struct
dir_struct *dir,
return path_none;
exclude = is_excluded(dir, path-buf, dtype);
- if (exclude (dir-flags DIR_COLLECT_IGNORED)
-exclude_matches_pathspec(path-buf, path-len, simplify))
- dir_add_ignored(dir, path-buf, path-len);
/*
* Excluded? If we don't explicitly want to show
* ignored files, ignore it
*/
- if (exclude !(dir-flags DIR_SHOW_IGNORED))
+ if (exclude !(dir-flags (DIR_SHOW_IGNORED|DIR_SHOW_IGNORED_TOO)))
return path_excluded;
switch (dtype) {
@@ -1280,6 +1277,11 @@ static enum path_treatment
read_directory_recursive(struct dir_struct *dir,
case path_excluded:
if (dir-flags DIR_SHOW_IGNORED)
dir_add_name(dir, path.buf, path.len);
+ else if ((dir-flags
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Wed, Apr 17, 2013 at 11:14:42AM -0700, Junio C Hamano wrote: I think it is just the warning code avoiding extra complexity and overhead, if you are talking about not getting warning in the pre-2.0 step that is in 'next'. Patches are very much welcomed, especially the ones that come before I get around to it ;-) I took a brief look at the code, and as you said add needs to know about submodules, and the best fix looks to me to take the same approach Jonathan came up with to de-noise the add -u/-A topic. That is, to scan the working tree to actually see if we would record removals to the index in 2.0, but not remove them in this current version, and give the warning when the differences in the behaviours matter. Yeah, I had the same thought, as this warning has been bugging me for the last day or two. The worst part about it is that I finally trained myself to type git add . to silence the _other_ warning, and now it triggers this one. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule deinit: clarify work tree removal message
Am 17.04.2013 07:16, schrieb Junio C Hamano:
Phil Hord phil.h...@gmail.com writes:
On Mon, Apr 1, 2013 at 3:02 PM, Jens Lehmann jens.lehm...@web.de wrote:
Okay, so here is the patch for that. If someone could point out
a portable and efficient way to check if a directory is already
empty I would be happy to use that to silence the Cleaned
directory message currently printed also when deinit is run on
an already empty directory.
isemptydir() {
test -d $(find $1 -maxdepth 0 -empty)
}
Hrm, -maxdepth and -empty are not even in POSIX. Folks on GNU
platforms and BSDs (I checked NetBSD 6 and OpenBSD 5.2) should be
fine, but it makes other platforms unhappy.
Ok, that is not acceptable.
What is this check used for? To avoid running rmdir on non-empty
ones? Saying cleaning foo/ (or cleaned foo/) when foo/ is
already empty is not a crime; not omitting an empty one may actually
be a better behaviour from the point of view of repeatability and
uniformity.
It's no big issue, but 'init' issues the Submodule ... registered
for path ... message only once and is quiet on subsequent calls,
'deinit' does the same with Submodule ... unregistered for path
..., only the Cleared directory ... message appears each time
'deinit' is called, which makes it stand out. I do not believe
this little inconsistency is important enough to write a helper in
C (to have a portable way to see if the directory has already been
cleared), but this simple additional if looked easy enough. That
should have made me suspicious ;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] cat-file: print tags raw for cat-file -p
On Wed, Apr 17, 2013 at 09:06:21PM +0200, René Scharfe wrote:
Am 17.04.2013 20:02, schrieb Jeff King:
I think we also need to do something about git cat-file -p, which does
not use the split_ident_line parser (but has its own problems with the
home-grown parser).
Ah, while it prints commit object contents verbatim, it formats the date
of tags. And it does it without help from tag.c (or ident.c), which in
turn does its own parsing as well. So it looks like we have two more
candidates for conversion to split_ident_line() here.
I think we should apply the patch below to just drop the date formatting
from cat-file, along with your two patches. This is the 4/4 from the
series I posted in February:
http://thread.gmane.org/gmane.comp.version-control.git/216870/focus=217081
but there I claimed that git tag -v might be affected. Upon looking
closer, it is not; we accidentally dropped the pretty-printing of the
date from it many years ago (and nobody seemed to care).
The other patches from that series aren't necessary. The 1/4 is replaced
by your patches (which do roughly the same thing, but add nice tests and
seem to refactor a bit more). The 2/4 and 3/4 patches were about adding
new fsck checks for tags, but I think there is some refactoring
necessary there. They can wait for now.
-- 8 --
Subject: [PATCH] cat-file: print tags raw for cat-file -p
When cat-file -p prints commits, it shows them in their
raw format, since git's format is already human-readable.
For tags, however, we print the whole thing raw except for
one thing: we convert the timestamp on the tagger line into a
human-readable date.
This dates all the way back to a0f15fa (Pretty-print tagger
dates, 2006-03-01). At that time there was no other way to
pretty-print a tag. These days, however, neither of those
matters much. The normal way to pretty-print a tag is with
git show, which is much more flexible than cat-file -p.
Commit a0f15fa also built verify-tag --verbose (and
subsequently tag -v) around the cat-file -p output.
However, that behavior was lost in commit 62e09ce (Make git
tag a builtin, 2007-07-20), and we went back to printing
the raw tag contents. Nobody seems to have noticed the bug
since then (and it is arguably a saner behavior anyway, as
it shows the actual bytes for which we verified the
signature).
Let's drop the tagger-date formatting for cat-file -p. It
makes us more consistent with cat-file's commit
pretty-printer, and as a bonus, we can drop the hand-rolled
tag parsing code in cat-file (which happened to behave
inconsistently with the tag pretty-printing code elsewhere).
This is a change of output format, so it's possible that
some callers could considered this a regression. However,
the original behavior was arguably a bug (due to the
inconsistency with commits), likely nobody was relying on it
(even we do not use it ourselves these days), and anyone
relying on the -p pretty-printer should be able to expect
a change in the output format (i.e., while cat-file is
plumbing, the output format of -p was never guaranteed to
be stable).
Signed-off-by: Jeff King p...@peff.net
---
builtin/cat-file.c | 71 -
t/t1006-cat-file.sh | 5 +---
2 files changed, 1 insertion(+), 75 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 40f87b4..045cee7 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -16,73 +16,6 @@
#define BATCH 1
#define BATCH_CHECK 2
-static void pprint_tag(const unsigned char *sha1, const char *buf, unsigned
long size)
-{
- /* the parser in tag.c is useless here. */
- const char *endp = buf + size;
- const char *cp = buf;
-
- while (cp endp) {
- char c = *cp++;
- if (c != '\n')
- continue;
- if (7 = endp - cp !memcmp(tagger , cp, 7)) {
- const char *tagger = cp;
-
- /* Found the tagger line. Copy out the contents
-* of the buffer so far.
-*/
- write_or_die(1, buf, cp - buf);
-
- /*
-* Do something intelligent, like pretty-printing
-* the date.
-*/
- while (cp endp) {
- if (*cp++ == '\n') {
- /* tagger to cp is a line
-* that has ident and time.
-*/
- const char *sp = tagger;
- char *ep;
- unsigned long date;
- long tz;
- while (sp cp *sp != '')
- sp++;
- if (sp == cp) {
-
Re: [PATCH] blame: handle broken commit headers gracefully
On Wed, Apr 17, 2013 at 08:33:54PM +0200, René Scharfe wrote: Minimal patch, test case missing. It's a bit sad that the old commit parser of blame handled Ivan's specific corruption (extra - after email) gracefully because it used the spaces as cutting points instead of and . That may mean there is room for improvement in split_ident_line to be more resilient in removing cruft. With something like: Name email@host- 123456789 - it would obviously be nice to find the date timestamp there, but I wonder what the email field should return? The full broken string, or just email@host? One way is convenient for overlooking problems in broken commits, but I would worry about code paths that are using split_ident_line to verify the quality of the string (like determine_author_info). It's possible we would need a strict and a forgiving mode. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git rev-list --pretty=format: issue
git 1.8.2.1 on OSX(Mountain Lion) installed with Homebrew I am seeing an issue when trying to format the output from rev-list command. git log --pretty=format:%H - %an, %ar : %s When I attempt the above string, instead of printing to the shell, LESS is opened and the output is displayed there. Got the command from here: http://git-scm.com/book/en/Git-Basics-Viewing-the-Commit-History git log --pretty=format:%h - %an, %ar : %s The string above works fine when I change the %h to %H the issue shoes up. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking between #05 and #06
Am 17.04.2013 01:52, schrieb Junio C Hamano: * jk/submodule-subdirectory-ok (2013-04-10) 2 commits - submodule: drop the top-level requirement - rev-parse: add --prefix option Allow various subcommands of git submodule to be run not from the top of the working tree of the superproject. Waiting for comments. Any submodule users wants to weigh in? The code looked fine, but I do not heavily use it (and the repository with a submodule I have, I do not have a subdirectory ;-, so I am a bad guinea pig). I like it, as it gets rid of the top-level requirement. But from my testing it looks like we're not quite there yet. 'summary' and 'status' behave as if they were run in the toplevel directory, while a git status shows all filenames relative to the current directory. Me thinks 'summary' and 'status' (and all other submodule commands) should behave like status and print relative paths too. I'm not really sure yet how $sm_path should behave for 'foreach', but I suspect having it relative to the current directory would be the way to go (which it currently isn't). When submodule add is run with a relative path it is relative to the top-level directory, which I find confusing (and won't play well with shell completion). 'deinit .' doesn't deinit submodules above the current directory (but prints the path relative to top-level) while 'init' will initialize all submodules known to the superproject. So this is a good start, but it looks like there is some work left to do before this can hit master. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-svn: added an --include-path flag
The SVN::Fetcher module is now able to filter for inclusion as well
as exclusion (as used by --ignore-path). Also added tests, documentation
changes and git completion script.
If you have an SVN repository with many top level directories and you
only want a git-svn clone of some of them then using --ignore-path is
difficult as it requires a very long regexp. In this case it's much
easier to filter for inclusion.
Signed-off-by: Paul Walmsley pjwh...@gmail.com
---
Documentation/git-svn.txt | 12 +++
contrib/completion/git-completion.bash |2 +-
git-svn.perl |4 +
perl/Git/SVN/Fetcher.pm| 13 ++-
t/t9147-git-svn-include-paths.sh | 150
5 files changed, 179 insertions(+), 2 deletions(-)
create mode 100755 t/t9147-git-svn-include-paths.sh
--1.7.10.4
Content-Type: text/x-patch; name=0001-git-svn-added-an-include-path-flag.patch
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment;
filename=0001-git-svn-added-an-include-path-flag.patch
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 69decb1..df1f40c 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -85,6 +85,10 @@ COMMANDS
When passed to 'init' or 'clone' this regular expression will
be preserved as a config key. See 'fetch' for a description
of '--ignore-paths'.
+--include-paths=regex;;
+ When passed to 'init' or 'clone' this regular expression will
+ be preserved as a config key. See 'fetch' for a description
+ of '--include-paths'.
--no-minimize-url;;
When tracking multiple directories (using --stdlayout,
--branches, or --tags options), git svn will attempt to connect
@@ -146,6 +150,14 @@ Skip branches and tags of first level directories;;
--
+--include-paths=regex;;
+ This allows one to specify a Perl regular expression that will
+ cause the inclusion of only matching paths from checkout from SVN.
+ The '--include-paths' option should match for every 'fetch'
+ (including automatic fetches due to 'clone', 'dcommit',
+ 'rebase', etc) on a given repository. '--ignore-paths' takes
+ precedence over '--include-paths'.
+
--log-window-size=n;;
Fetch n log entries per request when scanning Subversion history.
The default is 100. For very large Subversion repositories, larger
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 39c7ff8..8b75d01 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -,7 +,7 @@ _git_svn ()
--no-metadata --use-svm-props --use-svnsync-props
--log-window-size= --no-checkout --quiet
--repack-flags --use-log-author --localtime
- --ignore-paths= $remote_opts
+ --ignore-paths= --include-paths= $remote_opts
local init_opts=
--template= --shared= --trunk= --tags=
diff --git a/git-svn.perl b/git-svn.perl
index bd5266c..6d713e1 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -127,6 +127,7 @@ my %remote_opts = ( 'username=s' =
\$Git::SVN::Prompt::_username,
'config-dir=s' = \$Git::SVN::Ra::config_dir,
'no-auth-cache' = \$Git::SVN::Prompt::_no_auth_cache,
'ignore-paths=s' = \$Git::SVN::Fetcher::_ignore_regex,
+'include-paths=s' = \$Git::SVN::Fetcher::_include_regex,
'ignore-refs=s' = \$Git::SVN::Ra::_ignore_refs_regex );
my %fc_opts = ( 'follow-parent|follow!' = \$Git::SVN::_follow_parent,
'authors-file|A=s' = \$_authors,
@@ -477,6 +478,9 @@ sub do_git_init_db {
my $ignore_paths_regex = \$Git::SVN::Fetcher::_ignore_regex;
command_noisy('config', $pfx.ignore-paths, $$ignore_paths_regex)
if defined $$ignore_paths_regex;
+ my $include_paths_regex = \$Git::SVN::Fetcher::_include_regex;
+ command_noisy('config', $pfx.include-paths, $$include_paths_regex)
+ if defined $$include_paths_regex;
my $ignore_refs_regex = \$Git::SVN::Ra::_ignore_refs_regex;
command_noisy('config', $pfx.ignore-refs, $$ignore_refs_regex)
if defined $$ignore_refs_regex;
diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
index 046a7a2..27824e7 100644
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -1,5 +1,5 @@
package Git::SVN::Fetcher;
-use vars qw/@ISA $_ignore_regex $_preserve_empty_dirs $_placeholder_filename
+use vars qw/@ISA $_ignore_regex $_include_regex $_preserve_empty_dirs
$_placeholder_filename
@deleted_gpath %added_placeholder $repo_id/;
use strict;
use warnings;
@@ -33,6 +33,10
Re: [PATCH] blame: handle broken commit headers gracefully
Jeff King p...@peff.net writes: On Wed, Apr 17, 2013 at 08:33:54PM +0200, René Scharfe wrote: Minimal patch, test case missing. It's a bit sad that the old commit parser of blame handled Ivan's specific corruption (extra - after email) gracefully because it used the spaces as cutting points instead of and . That may mean there is room for improvement in split_ident_line to be more resilient in removing cruft. With something like: Name email@host- 123456789 - it would obviously be nice to find the date timestamp there, but I wonder what the email field should return? The full broken string, or just email@host? Or you can imagine nastier input strings, like Name -email@host 123456789 - Name ema-il@host 123456789 - Name email@host~ 123456789 - I am afraid that at some point we should salvage as much as we can, which is a worthy goal, becomes a losing proposition. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/14] Improve git-status --ignored
Karsten Blees karsten.bl...@gmail.com writes: I'll send fixups for #11 and #14, or you can pick the entire series rebased to pu from: https://github.com/kblees/git/tree/kb/improve-git-status-ignored-v2-pu git pull git://github.com/kblees/git.git kb/improve-git-status-ignored-v2-pu Will take a look; very much appreciated. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git rev-list --pretty=format: issue
Forrest Galloway f.gallo...@gmail.com writes: git 1.8.2.1 on OSX(Mountain Lion) installed with Homebrew I am seeing an issue when trying to format the output from rev-list command. git log --pretty=format:%H - %an, %ar : %s When I attempt the above string, instead of printing to the shell, LESS is opened and the output is displayed there. Got the command from here: http://git-scm.com/book/en/Git-Basics-Viewing-the-Commit-History git log --pretty=format:%h - %an, %ar : %s The string above works fine when I change the %h to %H the issue shoes up. Actually, less is running in both cases. We give --quit-if-one-screen (-F) and --chop-long-lines (-S) by default when we run less, unless you have your own LESS environment variable to override our choice, if your history is shorter than one screenful *and* if your output lines are narrower than your terminal, it exits after showing the output. By passing %H instead of %h, you make the output wider, and when viewing output with --chop-long-lines, less refuses to implicitly exit with --quit-if-one-screen, because you may want to look at the RHS end of the output with right/left arrow keys, and it cannot do so if it exits after showing the last line. If you do not want pager, run it with no-pager, like this: git --no-pager log ...your other parameters... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/33] t3210: test for spurious error messages for dangling packed refs
Junio C Hamano gits...@pobox.com writes:
Do we want to use approxidate_careful here to catch other junk?
We can catch a misspelt argument or configuration value that way.
That would be a good idea.
-- 8 --
Subject: date.c: add parse_expiry_date()
git reflog --expire=all tries to expire reflog entries up to the
current second, because the approxidate() parser gives the current
timestamp for anything it does not understand (and it does not know
what time all means). When the user tells us to expire all (or
set the expiration time to now), the user wants to remove all the
reflog entries (no reflog entry should record future time).
Just set it to ULONG_MAX and to let everything that is older that
timestamp expire.
While at it, allow now to be treated the same way for callers that
parse expiry date timestamp with this function. Also use an error
reporting version of approxidate() to report misspelled date. When
the user says e.g. --expire=mnoday to delete entries two days or
older on Wednesday, we wouldn't want the unknown, default to now
logic to kick in.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* adoption to more users and tests are left as an exercise to the
reader.
builtin/reflog.c | 14 +++---
cache.h | 1 +
date.c | 22 ++
3 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index b3c9e27..44700f9 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -496,11 +496,9 @@ static int parse_expire_cfg_value(const char *var, const
char *value, unsigned l
{
if (!value)
return config_error_nonbool(var);
- if (!strcmp(value, never) || !strcmp(value, false)) {
- *expire = 0;
- return 0;
- }
- *expire = approxidate(value);
+ if (parse_expiry_date(value, expire))
+ return error(_(%s' for '%s' is not a valid timestamp),
+value, var);
return 0;
}
@@ -613,11 +611,13 @@ static int cmd_reflog_expire(int argc, const char **argv,
const char *prefix)
if (!strcmp(arg, --dry-run) || !strcmp(arg, -n))
cb.dry_run = 1;
else if (!prefixcmp(arg, --expire=)) {
- cb.expire_total = approxidate(arg + 9);
+ if (parse_expiry_date(arg + 9, cb.expire_total))
+ die(_('%s' is not a valid timestamp), arg);
explicit_expiry |= EXPIRE_TOTAL;
}
else if (!prefixcmp(arg, --expire-unreachable=)) {
- cb.expire_unreachable = approxidate(arg + 21);
+ if (parse_expiry_date(arg + 21, cb.expire_unreachable))
+ die(_('%s' is not a valid timestamp), arg);
explicit_expiry |= EXPIRE_UNREACH;
}
else if (!strcmp(arg, --stale-fix))
diff --git a/cache.h b/cache.h
index 3622e18..f43f6d9 100644
--- a/cache.h
+++ b/cache.h
@@ -878,6 +878,7 @@ void show_date_relative(unsigned long time, int tz, const
struct timeval *now,
struct strbuf *timebuf);
int parse_date(const char *date, char *buf, int bufsize);
int parse_date_basic(const char *date, unsigned long *timestamp, int *offset);
+int parse_expiry_date(const char *date, unsigned long *timestamp);
void datestamp(char *buf, int bufsize);
#define approxidate(s) approxidate_careful((s), NULL)
unsigned long approxidate_careful(const char *, int *);
diff --git a/date.c b/date.c
index 57331ed..876d679 100644
--- a/date.c
+++ b/date.c
@@ -705,6 +705,28 @@ int parse_date_basic(const char *date, unsigned long
*timestamp, int *offset)
return 0; /* success */
}
+int parse_expiry_date(const char *date, unsigned long *timestamp)
+{
+ int errors = 0;
+
+ if (!strcmp(date, never) || !strcmp(date, false))
+ *timestamp = 0;
+ else if (!strcmp(date, all) || !strcmp(date, now))
+ /*
+* We take over now here, which usually translates
+* to the current timestamp. This is because the user
+* really means to expire everything she has done in
+* the past, and by definition reflogs are the record
+* of the past, and there is nothing from the future
+* to be kept.
+*/
+ *timestamp = ULONG_MAX;
+ else
+ *timestamp = approxidate_careful(date, errors);
+
+ return errors;
+}
+
int parse_date(const char *date, char *result, int maxlen)
{
unsigned long timestamp;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] clone: introduce clone.submoduleGitDir to relocate $GITDIR
On Thu, Apr 18, 2013 at 1:02 AM, Ramkumar Ramachandra artag...@gmail.com wrote: No, the point is people make mistakes. What do we do in that case? Or will you introduce yet another gc command for clean up ~/bare? So, people don't make mistakes when they use 'git submodule add', but do make mistakes when using 'git clone'? How has the problem _changed_ with my patch? It's reasonable to point it out as an existing problem, and ask for it to be fixed independent of this discussion, but that is not what you are doing. It's the magic in git-clone that changes its behavior that I want to address. I know you agree to go with a command line option. But I think in the end there will be a switch hidden somewhere in config to make things smooth, unless you make this mode the default (*). With normal mode, rm -rf repo is enough, with the new submodule mode, it leaves some garbage behind that the user may not be aware about. Maybe this is something that should be addressed anyway even for .gitmodules mode like Junio said. But I wonder if there are any other traps that come with the config switch. (*) I don't think you can make the new mode the default though. There are repos in repos in the field that are not managed by git submodule. Switching the default will disrupt those setups. Some deprecation cycles might help, I don't know. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] l10n: de.po: translate 39 new messages
Christian Stimming stimm...@tuhh.de writes: Thanks for the regular update! This is great work. One issue with the plural form messages, though: Am Montag, 15. April 2013, 18:27:40 schrieb Ralf Thielow: #: bundle.c:186 -#, fuzzy, c-format +#, c-format msgid The bundle contains this ref: msgid_plural The bundle contains these %d refs: -msgstr[0] Das Paket enthält %d Referenz -msgstr[1] Das Paket enthält %d Referenzen +msgstr[0] Das Paket enthält diese Referenz: +msgstr[1] Das Paket enthält diese %d Referenzen: The msgstr[0] must still contain a %d conversion specifier (which will be filled with the number 1) even though the translated sentence wouldn't need the 1 anymore. The previous msgstr[0] was correct; the English singular msgid is not. Technical background: The ngettext function chooses only the string itself, which will then be fed to printf() as a second step. If the printf sees more variadic arguments than conversion specifiers in the string, it will be unhappy. At least that's what I remembered about the ngettext things... http://www.gnu.org/software/libc/manual/html_node/Advanced-gettext- functions.html This vaguely sounds familiar: http://thread.gmane.org/gmane.comp.version-control.git/218173 http://www.gnu.org/software/gettext/manual/gettext.html#Plural-forms In the English singular case, the number – always 1 – can be replaced with one: printf (ngettext (One file removed, %d files removed, n), n); This works because the ‘printf’ function discards excess arguments that are not consumed by the format string. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] help.c: add a compatibility comment to cmd_version()
From: Junio C Hamano gits...@pobox.com
Sent: Tuesday, April 16, 2013 11:35 PM
Philip Oakley philipoak...@iee.org writes:
int cmd_version(int argc, const char **argv, const char *prefix)
{
+ /*
+ * The format of this string should be kept stable for
compatibility
+ * with external projects that rely on the output of git version.
This 'tantalizes without telling', the same complaint that is given
often for over-succinct commit messages.
How about
* E.g. git gui uses the extended regular expression ^git version
[1-9]+(\.[0-9]+)+.*
* to check for an acceptable version string.
The ERE is from git-gui.sh:L958.
Shouldn't the expected format of our known external project also be
indicated?
...
printf(git version %s\n, git_version_string);
It is fairly clear from the commented code that the only guarantee
they will be getting is that it begins with a string git version .
I read the code the opposite way. It says This is the code to be
changed if you (anyone doing tweaks) want a special version string.
git_version_string[] has anything of the builder's choice. I am not
sure if there anything more to indicate.
Really, if you run
$ git version
and you get Git Source Code Management Version 3.56 from its
output, it is likely that the version is very different from what
you know, and you should not assume any your assumption would hold.
Again I am reading this from the opposite side. There would be no
assumption of difference if it _passed_ the test scripts. Unfortunately
it wouldn't be friendly to other tools (like git gui). Hence my
suggestion of the basic test that a passing git would produce a
consistent version string. It still allows the supplier's suffixes to be
added, but not the prefixes. The test suite tests that git is 'good
enough for most usages and picks up regressions. No?
Obvious inconsistent special versions would fail in many other places.
Or mention such as git gui?
I do not see what it would buy us. It is not like it is OK as long
as we upadte Git gui at the same time.
Philip
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
From: Ramkumar Ramachandra artag...@gmail.com Sent: Wednesday, April 17, 2013 12:56 PM We've been over this several times in earlier emails. [...] Again, I'm not saying that my approach is Correct and Final. What I'm saying is: Here's what I've done. Something interesting is going on. It's probably worth a look? [...] The point is to have structured parseable information that the object-parsing code of git code and easily slurp and give to the rest of git-core. Please clear your reading backlog to avoid bringing up the same points over and over again. -- Ram, The email thread is pretty long with a lot of too and fro, that would be difficult to catch up on (too much $dayjob+$family vs $sparetime). Would it be possible to summarise the key points and proposals of where the subject is now? The submodules does need 'fixing', as does agreeing the problem and abuse cases. Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Felipe Contreras felipe.contre...@gmail.com writes: And how do you know this will be part of the 1%? You don't. How many times have you tracked regressions in transport helper's import/export functionality? How many times in remote-hg? How many times has *anybody* done so? The last point makes it all the more important to have a good history [*1*]. An area that no developer rarely touches with a little user base can stay dormant for a long time, and when people do need to hunt for an ancient bug or to enhance the existing feature to support a new use case without breaking the old use case, the original author may not be around, lost interest, or no longer uses his own creation. The code left behind tells us what the author thought was the best way to solve his problem, but it does not clearly define what the problem he tried to solve was, within what constraint he had to find a solution for it, and why he thought that the solution was the best (or sometimes only) one. Log and in-code comments are to explain such things that are beyond how the code works and what it does. [Footnote] *1* In this message, I am not judging if the depth of your writing for the particular change is deep enough. It depends on how well the reader knows the area, and there is no single right answer to that question. Incidentally that is why we tend to err on the more descriptive side. The next person your commit will help may not know the area as well as you do and has to figure things out on his own. You are helping him by being descriptive. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] transport-helper: some clarifications and a fix
Hi, It seems the workings of transport-helper are anything but clear, so let's try to clarify them a bit, and after that, hopefully it would become clearer why the last patch is actually a good fix. Felipe Contreras (6): transport-helper: clarify *:* refspec transport-helper: update refspec documentation transport-helper: clarify pushing without refspecs transport-helper: warn when refspec is not used transport-helper: trivial code shuffle transport-helper: update remote helper namespace Documentation/gitremote-helpers.txt | 12 ++-- t/t5801-remote-helpers.sh | 39 ++--- transport-helper.c | 37 +++ 3 files changed, 50 insertions(+), 38 deletions(-) -- 1.8.2.1.679.g509521a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] transport-helper: clarify *:* refspec
The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/gitremote-helpers.txt | 4 ++-- t/t5801-remote-helpers.sh | 15 --- transport-helper.c | 2 +- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index f506031..0c91aba 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -174,8 +174,8 @@ ref. This capability can be advertised multiple times. The first applicable refspec takes precedence. The left-hand of refspecs advertised with this capability must cover all refs reported by -the list command. If a helper does not need a specific 'refspec' -capability then it should advertise `refspec *:*`. +the list command. If no 'refspec' capability is advertised, +there is an implied `refspec *:*`. 'bidi-import':: This modifies the 'import' capability. diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index f387027..cd1873c 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -120,21 +120,6 @@ test_expect_failure 'pushing without refspecs' ' compare_refs local2 HEAD server HEAD ' -test_expect_success 'pulling with straight refspec' ' - (cd local2 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) - compare_refs local2 HEAD server HEAD -' - -test_expect_failure 'pushing with straight refspec' ' - test_when_finished (cd local2 git reset --hard origin) - (cd local2 - echo content file - git commit -a -m eleven - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) - compare_refs local2 HEAD server HEAD -' - test_expect_success 'pulling without marks' ' (cd local2 GIT_REMOTE_TESTGIT_NO_MARKS=1 git pull) diff --git a/transport-helper.c b/transport-helper.c index dcd8d97..cea787c 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -469,7 +469,7 @@ static int fetch_with_import(struct transport *transport, * were fetching. * * (If no refspec capability was specified, for historical -* reasons we default to *:*.) +* reasons we default to the equivalent of *:*.) * * Store the result in to_fetch[i].old_sha1. Callers such * as git fetch can use the value to write feedback to the -- 1.8.2.1.679.g509521a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] transport-helper: update refspec documentation
The refspec capability is not only used by 'import', also by 'export', and it's recommend in both. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/gitremote-helpers.txt | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index 0c91aba..ba7240c 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -159,11 +159,11 @@ Miscellaneous capabilities carried out. 'refspec' refspec:: - This modifies the 'import' capability, allowing the produced - fast-import stream to modify refs in a private namespace - instead of writing to refs/heads or refs/remotes directly. - It is recommended that all importers providing the 'import' - capability use this. + For remote helpers that implement 'import' or 'export', this capability + allows the refs to be constrained to a private namespace, instead of + writing to refs/heads or refs/remotes directly. + It is recommended that all importers providing the 'import' or 'export' + capabilities use this. + A helper advertising the capability `refspec refs/heads/*:refs/svn/origin/branches/*` -- 1.8.2.1.679.g509521a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] transport-helper: clarify pushing without refspecs
This has never worked, since it's inception the code simply skips all
the refs, essentially telling fast-export to do nothing.
Let's at least tell the user what's going on.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
Documentation/gitremote-helpers.txt | 4 ++--
t/t5801-remote-helpers.sh | 6 +++---
transport-helper.c | 5 +++--
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Documentation/gitremote-helpers.txt
b/Documentation/gitremote-helpers.txt
index ba7240c..4d26e37 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -162,8 +162,8 @@ Miscellaneous capabilities
For remote helpers that implement 'import' or 'export', this capability
allows the refs to be constrained to a private namespace, instead of
writing to refs/heads or refs/remotes directly.
- It is recommended that all importers providing the 'import' or 'export'
- capabilities use this.
+ It is recommended that all importers providing the 'import'
+ capability use this. It's mandatory for 'export'.
+
A helper advertising the capability
`refspec refs/heads/*:refs/svn/origin/branches/*`
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index cd1873c..3eeb309 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -111,13 +111,13 @@ test_expect_success 'pulling without refspecs' '
compare_refs local2 HEAD server HEAD
'
-test_expect_failure 'pushing without refspecs' '
+test_expect_success 'pushing without refspecs' '
test_when_finished (cd local2 git reset --hard origin)
(cd local2
echo content file
git commit -a -m ten
- GIT_REMOTE_TESTGIT_REFSPEC= git push)
- compare_refs local2 HEAD server HEAD
+ GIT_REMOTE_TESTGIT_REFSPEC= test_must_fail git push 2 ../error)
+ grep remote-helper doesn.t support push; refspec needed error
'
test_expect_success 'pulling without marks' '
diff --git a/transport-helper.c b/transport-helper.c
index cea787c..4d98567 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport
*transport,
struct string_list revlist_args = STRING_LIST_INIT_NODUP;
struct strbuf buf = STRBUF_INIT;
+ if (!data-refspecs)
+ die(remote-helper doesn't support push; refspec needed);
+
helper = get_helper(transport);
write_constant(helper-in, export\n);
@@ -795,8 +798,6 @@ static int push_refs_with_export(struct transport
*transport,
char *private;
unsigned char sha1[20];
- if (!data-refspecs)
- continue;
private = apply_refspecs(data-refspecs, data-refspec_nr,
ref-name);
if (private !get_sha1(private, sha1)) {
strbuf_addf(buf, ^%s, private);
--
1.8.2.1.679.g509521a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] transport-helper: warn when refspec is not used
For the modes that need it. In the future we should probably error out,
instead of providing half-assed support.
The reason we want to do this is because if it's not present, the remote
helper might be updating refs/heads/*, or refs/remotes/origin/*,
directly, and in the process fetch will get confused trying to update
refs that are already updated, or older than what they should be. We
shouldn't be messing with the rest of git.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t5801-remote-helpers.sh | 6 --
transport-helper.c| 2 ++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 3eeb309..1bb7529 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -100,14 +100,16 @@ test_expect_failure 'push new branch with old:new
refspec' '
test_expect_success 'cloning without refspec' '
GIT_REMOTE_TESTGIT_REFSPEC= \
- git clone testgit::${PWD}/server local2
+ git clone testgit::${PWD}/server local2 2 error
+ grep This remote helper should implement refspec capability error
compare_refs local2 HEAD server HEAD
'
test_expect_success 'pulling without refspecs' '
(cd local2
git reset --hard
- GIT_REMOTE_TESTGIT_REFSPEC= git pull)
+ GIT_REMOTE_TESTGIT_REFSPEC= git pull 2 ../error)
+ grep This remote helper should implement refspec capability error
compare_refs local2 HEAD server HEAD
'
diff --git a/transport-helper.c b/transport-helper.c
index 4d98567..573eaf7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -215,6 +215,8 @@ static struct child_process *get_helper(struct transport
*transport)
free((char *)refspecs[i]);
}
free(refspecs);
+ } else if (data-import || data-bidi_import || data-export) {
+ warning(This remote helper should implement refspec
capability.);
}
strbuf_release(buf);
if (debug)
--
1.8.2.1.679.g509521a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] transport-helper: trivial code shuffle
Just shuffle the die() part to make it more explicit, and cleanup the
code-style.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
transport-helper.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 573eaf7..9d31f2d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -800,6 +800,9 @@ static int push_refs_with_export(struct transport
*transport,
char *private;
unsigned char sha1[20];
+ if (ref-deletion)
+ die(remote-helpers do not support ref deletion);
+
private = apply_refspecs(data-refspecs, data-refspec_nr,
ref-name);
if (private !get_sha1(private, sha1)) {
strbuf_addf(buf, ^%s, private);
@@ -807,13 +810,8 @@ static int push_refs_with_export(struct transport
*transport,
}
free(private);
- if (ref-deletion) {
- die(remote-helpers do not support ref deletion);
- }
-
if (ref-peer_ref)
string_list_append(revlist_args, ref-peer_ref-name);
-
}
if (get_exporter(transport, exporter, revlist_args))
--
1.8.2.1.679.g509521a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] transport-helper: update remote helper namespace
When pushing, the remote namespace is updated correctly
(e.g. refs/origin/master), but not the remote helper's
(e.g. refs/testgit/origin/master), which currently is only updated while
fetching.
Since the remote namespace is used to tell fast-export which commits to
avoid (because they were already imported/exported), it makes sense to
have them in sync so they don't get generated twice. If the remote
helper was implemented properly, they would be ignored, if not, they
probably would end up repeated (probably).
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t5801-remote-helpers.sh | 12
transport-helper.c| 20
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 1bb7529..097691c 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -153,4 +153,16 @@ test_expect_success 'push ref with existing object' '
compare_refs local dup server dup
'
+test_expect_success 'push update refs' '
+ (cd local
+ git checkout -b update master
+ echo update file
+ git commit -a -m update
+ git push origin update
+ git rev-parse --verify testgit/origin/heads/update expect
+ git rev-parse --verify remotes/origin/update actual
+ test_cmp expect actual
+ )
+'
+
test_done
diff --git a/transport-helper.c b/transport-helper.c
index 9d31f2d..414d6c8 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -11,6 +11,7 @@
#include thread-utils.h
#include sigchain.h
#include argv-array.h
+#include refs.h
static int debug;
@@ -620,7 +621,7 @@ static int fetch(struct transport *transport,
return -1;
}
-static void push_update_ref_status(struct strbuf *buf,
+static int push_update_ref_status(struct strbuf *buf,
struct ref **ref,
struct ref *remote_refs)
{
@@ -686,7 +687,7 @@ static void push_update_ref_status(struct strbuf *buf,
*ref = find_ref_by_name(remote_refs, refname);
if (!*ref) {
warning(helper reported unexpected status of %s, refname);
- return;
+ return 1;
}
if ((*ref)-status != REF_STATUS_NONE) {
@@ -695,11 +696,12 @@ static void push_update_ref_status(struct strbuf *buf,
* status reported by the remote helper if the latter is 'no
match'.
*/
if (status == REF_STATUS_NONE)
- return;
+ return 1;
}
(*ref)-status = status;
(*ref)-remote_status = msg;
+ return 0;
}
static void push_update_refs_status(struct helper_data *data,
@@ -708,11 +710,21 @@ static void push_update_refs_status(struct helper_data
*data,
struct strbuf buf = STRBUF_INIT;
struct ref *ref = remote_refs;
for (;;) {
+ char *private;
+
recvline(data, buf);
if (!buf.len)
break;
- push_update_ref_status(buf, ref, remote_refs);
+ if (push_update_ref_status(buf, ref, remote_refs))
+ continue;
+
+ /* propagate back the update to the remote namespace */
+ private = apply_refspecs(data-refspecs, data-refspec_nr,
ref-name);
+ if (!private)
+ continue;
+ update_ref(update by helper, private, ref-new_sha1, NULL, 0,
0);
+ free(private);
}
strbuf_release(buf);
}
--
1.8.2.1.679.g509521a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] help.c: add a compatibility comment to cmd_version()
Philip Oakley philipoak...@iee.org writes: How about * E.g. git gui uses the extended regular expression ^git version [1-9]+(\.[0-9]+)+.* * to check for an acceptable version string. The ERE is from git-gui.sh:L958. That is exactly the kind of guarantee we do _not_ want to give. ... Hence my suggestion of the basic test that a passing git would produce a consistent version string. I have been assuming that you are trying to avoid an exchange like this one we had in the past: http://thread.gmane.org/gmane.comp.version-control.git/216923/focus=217007 I also have been assuming that you are pushing to limit the possible versioning scheme, but I do not know what that extra limitation would achieve in the real world. By sticking to the pattern git gui happens to use, git gui may be able to guess that the next version of Git says it is verison 1.8.3. That is the _only_ thing your test buys. But having parsed the 1.8.3 substring out of it, git gui does not know anything about what 1.8.3 gives it. As far as it is concerned, it is a version whose git version output it has never seen (unless it has been kept up to date with the development of Git). By matching against git version [1-9]+(\.[0-9]+)+, it is accepting that future versions may break assumptions it makes for some known versions (which is warranted) and all future versions (which is unwarranted) of Git. Maybe the version 2.0 of Git adds all changes in the directory d, including removals, when you say git add d, but it may have assumed that with Git version 1.5.0 or newer, saying git add d would result in added and modified inside that directory getting updated in the index, but paths removed from the working tree will stay in the index. The only thing the scripts that read from the output of git version can reliably tell is if it is interacting with a version of Git it knows about. If it made any unwarranted assumption on the versions it hasn't seen, it has to be fixed in git gui _anyway_. Of course, we would not change the output of git version willy-nilly without good reason, but that is a different topic. So I do not know what you want to achieve in the real world with that extra limitation on the git version output format. Maybe you are proposing something else. I dunno. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] transport-helper: clarify pushing without refspecs
On Wed, Apr 17, 2013 at 5:05 PM, Felipe Contreras felipe.contre...@gmail.com wrote: This has never worked, since it's inception the code simply skips all the refs, essentially telling fast-export to do nothing. Makes sense. -- Cheers, Sverre Rabbelier -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb/INSTALL: GITWEB_CONFIG_SYSTEM is for backward compatibility
On Tue, Apr 16, 2013 at 6:26 PM, Jonathan Nieder jrnie...@gmail.com wrote:
Drew Northup wrote:
This is unobtrusive yet to the point.
I agree with the spirit.
[...]
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -55,7 +55,8 @@ following order:
then fallback system-wide configuration file (defaults to
'/etc/gitweb.conf').
Values obtained in later configuration files override values obtained
earlier
-in the above sequence.
+in the above sequence. This is different from many system-wide software
+installations and will stay this way for historical reasons.
That makes it sound like the per instance overrides common overrides
built-in cascading is what is unusual and what we need to apologize
for.
I don't think were apologizing for anything. It is helpful to say we
do some things differently here and don't plan on changing for a very
important reason.
How about something like the following? (It uses a BUGS section to
make the warning easy to notice for people tracking down confusing
behavior by searching for gitweb.conf.)
diff --git i/Documentation/gitweb.conf.txt w/Documentation/gitweb.conf.txt
index eb63631..ea0526e 100644
--- i/Documentation/gitweb.conf.txt
+++ w/Documentation/gitweb.conf.txt
@@ -857,6 +857,13 @@ adding the following lines to your gitweb configuration
file:
$known_snapshot_formats{'zip'}{'disabled'} = 1;
$known_snapshot_formats{'tgz'}{'compressor'} = ['gzip','-6'];
+BUGS
+
+Debugging would be easier if the fallback configuration file
+(`/etc/gitweb.conf`) and environment variable to override its location
+('GITWEB_CONFIG_SYSTEM') had names reflecting their fallback role.
+The current names are kept to avoid breaking working setups.
+
ENVIRONMENT
---
The location of per-instance and system-wide configuration files can be
I don't disagree with this, as some would consider the naming to be a
bug, but after having been given a good schooling on the git list a
while back as to why it is the way it is I'm hesitant to label has
history as a bug.
--
-Drew Northup
--
As opposed to vegetable or mineral error?
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Jeff King p...@peff.net writes:
Yeah, I had the same thought, as this warning has been bugging me for
the last day or two. The worst part about it is that I finally trained
myself to type git add . to silence the _other_ warning, and now it
triggers this one. :)
So here is the reworked one on top of what is in 'next'.
It introduces a bit of conflict with the add -u/-A topic, so I am
not ready to push out the integration result yet.
-- 8 --
Subject: [PATCH] git add: rework the logic to warn git add pathspec...
default change
The earlier logic to warn against git add subdir that is run
without -A or --no-all was only to check any pathspec given
exactly spells a directory name that (still) exists on the
filesystem. This had number of problems:
* git add '*dir' (note that the wildcard is hidden from the
shell) would not trigger the warning.
* git add '*.py' would behave differently between the current
version of Git and Git 2.0 for the same reason as subdir, but
would not trigger the warning.
* git add dir for a submodule dir would just update the index
entry for the submodule dir without ever recursing into it, and
use of -A or --no-all would matter. But the logic only
checks the directory-ness of dir and gives an unnecessary
warning.
Rework the logic to detect the case where the behaviour will be
different in Git 2.0, and issue a warning only when it matters.
Even with the code before this warning, git add subdir will have
to traverse the directory in order to find _new_ files the index
does not know about _anyway_, so we can do this check without adding
an extra pass to find if pathspec matches any removed file.
This essentially updates the add_files_to_cache() public API to
update_files_in_cache() API that is internal to git add, because
with the --all option, the function is no longer about adding
paths to the cache, but is also used to remove them.
There are other callers of the former from checkout (used when
checkout -m prepares the temporary tree that represents the local
modifications to be merged) and commit (commit --include that
picks up local changes in addition to what is in the index). Since
ADD_CACHE_IGNORE_ERRORS (aka --no-all) is not used by either of
them, once dust settles after Git 2.0 and the warning becomes
unnecessary, we may want to unify these two functions again.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/add.c | 64 +++
1 file changed, 38 insertions(+), 26 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index f8f6c9e..4242bce 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -26,6 +26,9 @@ static int take_worktree_changes;
struct update_callback_data {
int flags;
int add_errors;
+
+ /* only needed for 2.0 transition preparation */
+ int warn_add_would_remove;
};
static int fix_unmerged_status(struct diff_filepair *p,
@@ -49,6 +52,17 @@ static int fix_unmerged_status(struct diff_filepair *p,
return DIFF_STATUS_MODIFIED;
}
+static void warn_add_would_remove(const char *path)
+{
+ warning(_(In Git 2.0, 'git add pathspec...' will also update the\n
+ index for paths removed from the working tree that match\n
+ the given pathspec. If you want to 'add' only changed\n
+ or newly created paths, say 'git add --no-all pathspec...'
+ instead.\n\n
+ '%s' would be removed from the index without --no-all.),
+ path);
+}
+
static void update_callback(struct diff_queue_struct *q,
struct diff_options *opt, void *cbdata)
{
@@ -70,6 +84,10 @@ static void update_callback(struct diff_queue_struct *q,
}
break;
case DIFF_STATUS_DELETED:
+ if (data-warn_add_would_remove) {
+ warn_add_would_remove(path);
+ data-warn_add_would_remove = 0;
+ }
if (data-flags ADD_CACHE_IGNORE_REMOVAL)
break;
if (!(data-flags ADD_CACHE_PRETEND))
@@ -81,20 +99,27 @@ static void update_callback(struct diff_queue_struct *q,
}
}
-int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
+static void update_files_in_cache(const char *prefix, const char **pathspec,
+ struct update_callback_data *data)
{
- struct update_callback_data data;
struct rev_info rev;
init_revisions(rev, prefix);
setup_revisions(0, NULL, rev, NULL);
init_pathspec(rev.prune_data, pathspec);
rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = update_callback;
- data.flags = flags;
- data.add_errors = 0;
- rev.diffopt.format_callback_data = data;
+
Re: [PATCH] gitweb/INSTALL: GITWEB_CONFIG_SYSTEM is for backward compatibility
Drew Northup n1xim.em...@gmail.com writes:
That makes it sound like the per instance overrides common overrides
built-in cascading is what is unusual and what we need to apologize
for.
I don't think were apologizing for anything. It is helpful to say we
do some things differently here and don't plan on changing for a very
important reason.
My biggest resistance against this whole thing is that word
differently.
It really depends on who reads this document. For some, it may be
different. For others, it may not be unexpected at all. We do
things differently may help the former but I do not want the latter
to try to find difference that does not exist elsewhere and get
confused.
+BUGS
+
+Debugging would be easier if the fallback configuration file
+(`/etc/gitweb.conf`) and environment variable to override its location
+('GITWEB_CONFIG_SYSTEM') had names reflecting their fallback role.
+The current names are kept to avoid breaking working setups.
+
ENVIRONMENT
---
The location of per-instance and system-wide configuration files can be
I don't disagree with this, as some would consider the naming to be a
bug, but after having been given a good schooling on the git list a
while back as to why it is the way it is I'm hesitant to label has
history as a bug.
BUGS section being traditionally where you would throw this kind of
thing, either a bug, non-bug, or wai-but-some-may-not-agree, I think
this is a good description.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git add pathspec... defaults to -A
Make git add pathspec... notice paths that have been removed
from the working tree, i.e. a synonym to git add -A pathspec
Given that git add pathspec is to update the index with the
state of the named part of the working tree as a whole, it makes it
more intuitive, and also makes it possible to simplify the advice we
give while marking the paths the user finished resolving conflicts
with. We used to say to record removal as a resolution, remove the
path from the working tree and say 'git rm'; for all other cases,
edit the path in the working tree and say 'git add', but we can now
say update the path in the working tree and say 'git add' instead.
As promised, this merges the temporary update_files_in_cache() helper
function back to add_files_to_cache() function.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* So this comes on top of the previous update that will sit at
jc/add-2.0-delete-default~1 and replaces the one previouly at its
tip.
Documentation/git-add.txt | 18 +--
builtin/add.c | 57 ---
2 files changed, 20 insertions(+), 55 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 5c501a2..5bd0791 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -53,8 +53,14 @@ OPTIONS
Files to add content from. Fileglobs (e.g. `*.c`) can
be given to add all matching files. Also a
leading directory name (e.g. `dir` to add `dir/file1`
- and `dir/file2`) can be given to add all files in the
- directory, recursively.
+ and `dir/file2`) can be given to update the index to
+ match the current state of the directory as a whole (e.g.
+ specifying `dir` will record not just a file `dir/file1`
+ modified in the working tree, a file `dir/file2` added to
+ the working tree, but also a file `dir/file3` removed from
+ the working tree. Note that older versions of Git used
+ to ignore removed files; use `--no-all` option if you want
+ to add modified or new files but ignore removed ones.
-n::
--dry-run::
@@ -127,11 +133,9 @@ of Git, hence the form without pathspec should not be
used.
files that have been removed from the working tree. This
option is a no-op when no pathspec is used.
+
-This option is primarily to help the current users of Git, whose
-git add pathspec... ignores removed files. In future versions
-of Git, git add pathspec... will be a synonym to git add -A
-pathspec... and git add --no-all pathspec... will behave like
-today's git add pathspec..., ignoring removed files.
+This option is primarily to help users who are used to older
+versions of Git, whose git add pathspec... was a synonym
+to git add --no-all pathspec..., i.e. ignored removed files.
-N::
--intent-to-add::
diff --git a/builtin/add.c b/builtin/add.c
index 4242bce..21c685f 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -26,9 +26,6 @@ static int take_worktree_changes;
struct update_callback_data {
int flags;
int add_errors;
-
- /* only needed for 2.0 transition preparation */
- int warn_add_would_remove;
};
static int fix_unmerged_status(struct diff_filepair *p,
@@ -52,17 +49,6 @@ static int fix_unmerged_status(struct diff_filepair *p,
return DIFF_STATUS_MODIFIED;
}
-static void warn_add_would_remove(const char *path)
-{
- warning(_(In Git 2.0, 'git add pathspec...' will also update the\n
- index for paths removed from the working tree that match\n
- the given pathspec. If you want to 'add' only changed\n
- or newly created paths, say 'git add --no-all pathspec...'
- instead.\n\n
- '%s' would be removed from the index without --no-all.),
- path);
-}
-
static void update_callback(struct diff_queue_struct *q,
struct diff_options *opt, void *cbdata)
{
@@ -84,10 +70,6 @@ static void update_callback(struct diff_queue_struct *q,
}
break;
case DIFF_STATUS_DELETED:
- if (data-warn_add_would_remove) {
- warn_add_would_remove(path);
- data-warn_add_would_remove = 0;
- }
if (data-flags ADD_CACHE_IGNORE_REMOVAL)
break;
if (!(data-flags ADD_CACHE_PRETEND))
@@ -99,27 +81,20 @@ static void update_callback(struct diff_queue_struct *q,
}
}
-static void update_files_in_cache(const char *prefix, const char **pathspec,
- struct update_callback_data *data)
+int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
{
+ struct update_callback_data data;
struct rev_info rev;
init_revisions(rev, prefix);
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Wed, Apr 17, 2013 at 6:56 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: And how do you know this will be part of the 1%? You don't. How many times have you tracked regressions in transport helper's import/export functionality? How many times in remote-hg? How many times has *anybody* done so? The last point makes it all the more important to have a good history [*1*]. An area that no developer rarely touches with a little user base can stay dormant for a long time, and when people do need to hunt for an ancient bug or to enhance the existing feature to support a new use case without breaking the old use case, the original author may not be around, lost interest, or no longer uses his own creation. You are going in circles, I said such situation was *HYPOTHETICAL*, Phil Hord said it wasn't, and now you are bringing back more hypothetical examples, which I would gladly address, as soon as you accept they are HYPOTHETICAL. Now, how about you answer the questions about the *REAL* situations Phil Hord mentioned? * How many times have you tracked regressions in transport helper's import/export functionality? Hint: zero. * How many times in remote-hg? Hint: zero. * How many times has *anybody* done so? Hint: other than me, quite possibly zero. And then, before we consider this *hypothetical* situation, it might be worth noticing what commit this hypothetical person would hit if you do *not* apply this patch, and what the commit message says: --- remote-helpers: add support for an export command Signed-off-by: Junio C Hamano gits...@pobox.com --- Yeah, well, glad you didn't apply my patch, wouldn't want to mess up the code that was clearly explained by that commit message. And before you rationalize the above commit, because maybe the functionality was described in the documentation, it wasn't: transport-helper.c | 132 ++--- 1 file changed, 120 insertions(+), 12 deletions(-) If you do apply my patch, it turns out even the shortest version of my commit message already gives more information to this *hypothetical* developer person. [Footnote] *1* In this message, I am not judging if the depth of your writing for the particular change is deep enough. It depends on how well the reader knows the area, and there is no single right answer to that question. Incidentally that is why we tend to err on the more descriptive side. The next person your commit will help may not know the area as well as you do and has to figure things out on his own. You are helping him by being descriptive. I partially agree with this, but I think documenting the nuts and bolts of transport-helper would be better in done in code, documentation, tests, and mailing list analysis. And in all those respects, I believe I've done a more than adequate job. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] transport-helper: update remote helper namespace
On Wed, Apr 17, 2013 at 7:05 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -153,4 +153,16 @@ test_expect_success 'push ref with existing object' '
compare_refs local dup server dup
'
+test_expect_success 'push update refs' '
+ (cd local
+ git checkout -b update master
+ echo update file
+ git commit -a -m update
+ git push origin update
+ git rev-parse --verify testgit/origin/heads/update expect
+ git rev-parse --verify remotes/origin/update actual
+ test_cmp expect actual
Slightly confusing and a bit buggy:
- git rev-parse --verify testgit/origin/heads/update expect
- git rev-parse --verify remotes/origin/update actual
+ git rev-parse --verify remotes/origin/update expect
+ git rev-parse --verify testgit/origin/heads/update actual
static void push_update_refs_status(struct helper_data *data,
@@ -708,11 +710,21 @@ static void push_update_refs_status(struct helper_data
*data,
struct strbuf buf = STRBUF_INIT;
struct ref *ref = remote_refs;
for (;;) {
+ char *private;
+
recvline(data, buf);
if (!buf.len)
break;
- push_update_ref_status(buf, ref, remote_refs);
+ if (push_update_ref_status(buf, ref, remote_refs))
+ continue;
Actually, since this function is also used by push_with_push:
if (!data-refspecs)
continue;
I had it in my previous series but removed it.
I'll reroll.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/6] transport-helper: some clarifications and a fix
Hi, Same as before, but with a small bug fix, and minor test improvements. It seems the workings of transport-helper are anything but clear, so let's try to clarify them a bit, and after that, hopefully it would become clearer why the last patch is actually a good fix. Felipe Contreras (6): transport-helper: clarify *:* refspec transport-helper: update refspec documentation transport-helper: clarify pushing without refspecs transport-helper: warn when refspec is not used transport-helper: trivial code shuffle transport-helper: update remote helper namespace Documentation/gitremote-helpers.txt | 12 +-- t/t5801-remote-helpers.sh | 39 ++-- transport-helper.c | 40 ++--- 3 files changed, 53 insertions(+), 38 deletions(-) -- 1.8.2.1.679.g509521a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/6] transport-helper: clarify *:* refspec
The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/gitremote-helpers.txt | 4 ++-- t/t5801-remote-helpers.sh | 15 --- transport-helper.c | 2 +- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index f506031..0c91aba 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -174,8 +174,8 @@ ref. This capability can be advertised multiple times. The first applicable refspec takes precedence. The left-hand of refspecs advertised with this capability must cover all refs reported by -the list command. If a helper does not need a specific 'refspec' -capability then it should advertise `refspec *:*`. +the list command. If no 'refspec' capability is advertised, +there is an implied `refspec *:*`. 'bidi-import':: This modifies the 'import' capability. diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index f387027..cd1873c 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -120,21 +120,6 @@ test_expect_failure 'pushing without refspecs' ' compare_refs local2 HEAD server HEAD ' -test_expect_success 'pulling with straight refspec' ' - (cd local2 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) - compare_refs local2 HEAD server HEAD -' - -test_expect_failure 'pushing with straight refspec' ' - test_when_finished (cd local2 git reset --hard origin) - (cd local2 - echo content file - git commit -a -m eleven - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) - compare_refs local2 HEAD server HEAD -' - test_expect_success 'pulling without marks' ' (cd local2 GIT_REMOTE_TESTGIT_NO_MARKS=1 git pull) diff --git a/transport-helper.c b/transport-helper.c index dcd8d97..cea787c 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -469,7 +469,7 @@ static int fetch_with_import(struct transport *transport, * were fetching. * * (If no refspec capability was specified, for historical -* reasons we default to *:*.) +* reasons we default to the equivalent of *:*.) * * Store the result in to_fetch[i].old_sha1. Callers such * as git fetch can use the value to write feedback to the -- 1.8.2.1.679.g509521a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/6] transport-helper: update refspec documentation
The refspec capability is not only used by 'import', also by 'export', and it's recommend in both. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/gitremote-helpers.txt | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index 0c91aba..ba7240c 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -159,11 +159,11 @@ Miscellaneous capabilities carried out. 'refspec' refspec:: - This modifies the 'import' capability, allowing the produced - fast-import stream to modify refs in a private namespace - instead of writing to refs/heads or refs/remotes directly. - It is recommended that all importers providing the 'import' - capability use this. + For remote helpers that implement 'import' or 'export', this capability + allows the refs to be constrained to a private namespace, instead of + writing to refs/heads or refs/remotes directly. + It is recommended that all importers providing the 'import' or 'export' + capabilities use this. + A helper advertising the capability `refspec refs/heads/*:refs/svn/origin/branches/*` -- 1.8.2.1.679.g509521a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/6] transport-helper: clarify pushing without refspecs
This has never worked, since it's inception the code simply skips all
the refs, essentially telling fast-export to do nothing.
Let's at least tell the user what's going on.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
Documentation/gitremote-helpers.txt | 4 ++--
t/t5801-remote-helpers.sh | 6 +++---
transport-helper.c | 5 +++--
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Documentation/gitremote-helpers.txt
b/Documentation/gitremote-helpers.txt
index ba7240c..4d26e37 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -162,8 +162,8 @@ Miscellaneous capabilities
For remote helpers that implement 'import' or 'export', this capability
allows the refs to be constrained to a private namespace, instead of
writing to refs/heads or refs/remotes directly.
- It is recommended that all importers providing the 'import' or 'export'
- capabilities use this.
+ It is recommended that all importers providing the 'import'
+ capability use this. It's mandatory for 'export'.
+
A helper advertising the capability
`refspec refs/heads/*:refs/svn/origin/branches/*`
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index cd1873c..3eeb309 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -111,13 +111,13 @@ test_expect_success 'pulling without refspecs' '
compare_refs local2 HEAD server HEAD
'
-test_expect_failure 'pushing without refspecs' '
+test_expect_success 'pushing without refspecs' '
test_when_finished (cd local2 git reset --hard origin)
(cd local2
echo content file
git commit -a -m ten
- GIT_REMOTE_TESTGIT_REFSPEC= git push)
- compare_refs local2 HEAD server HEAD
+ GIT_REMOTE_TESTGIT_REFSPEC= test_must_fail git push 2 ../error)
+ grep remote-helper doesn.t support push; refspec needed error
'
test_expect_success 'pulling without marks' '
diff --git a/transport-helper.c b/transport-helper.c
index cea787c..4d98567 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport
*transport,
struct string_list revlist_args = STRING_LIST_INIT_NODUP;
struct strbuf buf = STRBUF_INIT;
+ if (!data-refspecs)
+ die(remote-helper doesn't support push; refspec needed);
+
helper = get_helper(transport);
write_constant(helper-in, export\n);
@@ -795,8 +798,6 @@ static int push_refs_with_export(struct transport
*transport,
char *private;
unsigned char sha1[20];
- if (!data-refspecs)
- continue;
private = apply_refspecs(data-refspecs, data-refspec_nr,
ref-name);
if (private !get_sha1(private, sha1)) {
strbuf_addf(buf, ^%s, private);
--
1.8.2.1.679.g509521a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/6] transport-helper: warn when refspec is not used
For the modes that need it. In the future we should probably error out,
instead of providing half-assed support.
The reason we want to do this is because if it's not present, the remote
helper might be updating refs/heads/*, or refs/remotes/origin/*,
directly, and in the process fetch will get confused trying to update
refs that are already updated, or older than what they should be. We
shouldn't be messing with the rest of git.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t5801-remote-helpers.sh | 6 --
transport-helper.c| 2 ++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 3eeb309..1bb7529 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -100,14 +100,16 @@ test_expect_failure 'push new branch with old:new
refspec' '
test_expect_success 'cloning without refspec' '
GIT_REMOTE_TESTGIT_REFSPEC= \
- git clone testgit::${PWD}/server local2
+ git clone testgit::${PWD}/server local2 2 error
+ grep This remote helper should implement refspec capability error
compare_refs local2 HEAD server HEAD
'
test_expect_success 'pulling without refspecs' '
(cd local2
git reset --hard
- GIT_REMOTE_TESTGIT_REFSPEC= git pull)
+ GIT_REMOTE_TESTGIT_REFSPEC= git pull 2 ../error)
+ grep This remote helper should implement refspec capability error
compare_refs local2 HEAD server HEAD
'
diff --git a/transport-helper.c b/transport-helper.c
index 4d98567..573eaf7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -215,6 +215,8 @@ static struct child_process *get_helper(struct transport
*transport)
free((char *)refspecs[i]);
}
free(refspecs);
+ } else if (data-import || data-bidi_import || data-export) {
+ warning(This remote helper should implement refspec
capability.);
}
strbuf_release(buf);
if (debug)
--
1.8.2.1.679.g509521a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/6] transport-helper: trivial code shuffle
Just shuffle the die() part to make it more explicit, and cleanup the
code-style.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
transport-helper.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 573eaf7..9d31f2d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -800,6 +800,9 @@ static int push_refs_with_export(struct transport
*transport,
char *private;
unsigned char sha1[20];
+ if (ref-deletion)
+ die(remote-helpers do not support ref deletion);
+
private = apply_refspecs(data-refspecs, data-refspec_nr,
ref-name);
if (private !get_sha1(private, sha1)) {
strbuf_addf(buf, ^%s, private);
@@ -807,13 +810,8 @@ static int push_refs_with_export(struct transport
*transport,
}
free(private);
- if (ref-deletion) {
- die(remote-helpers do not support ref deletion);
- }
-
if (ref-peer_ref)
string_list_append(revlist_args, ref-peer_ref-name);
-
}
if (get_exporter(transport, exporter, revlist_args))
--
1.8.2.1.679.g509521a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/6] transport-helper: update remote helper namespace
When pushing, the remote namespace is updated correctly
(e.g. refs/origin/master), but not the remote helper's
(e.g. refs/testgit/origin/master), which currently is only updated while
fetching.
Since the remote namespace is used to tell fast-export which commits to
avoid (because they were already imported/exported), it makes sense to
have them in sync so they don't get generated twice. If the remote
helper was implemented properly, they would be ignored, if not, they
probably would end up repeated (probably).
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t5801-remote-helpers.sh | 12
transport-helper.c| 23 +++
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 1bb7529..5466969 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -153,4 +153,16 @@ test_expect_success 'push ref with existing object' '
compare_refs local dup server dup
'
+test_expect_success 'push update refs' '
+ (cd local
+ git checkout -b update master
+ echo update file
+ git commit -a -m update
+ git push origin update
+ git rev-parse --verify remotes/origin/update expect
+ git rev-parse --verify testgit/origin/heads/update actual
+ test_cmp expect actual
+ )
+'
+
test_done
diff --git a/transport-helper.c b/transport-helper.c
index 9d31f2d..da16393 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -11,6 +11,7 @@
#include thread-utils.h
#include sigchain.h
#include argv-array.h
+#include refs.h
static int debug;
@@ -620,7 +621,7 @@ static int fetch(struct transport *transport,
return -1;
}
-static void push_update_ref_status(struct strbuf *buf,
+static int push_update_ref_status(struct strbuf *buf,
struct ref **ref,
struct ref *remote_refs)
{
@@ -686,7 +687,7 @@ static void push_update_ref_status(struct strbuf *buf,
*ref = find_ref_by_name(remote_refs, refname);
if (!*ref) {
warning(helper reported unexpected status of %s, refname);
- return;
+ return 1;
}
if ((*ref)-status != REF_STATUS_NONE) {
@@ -695,11 +696,12 @@ static void push_update_ref_status(struct strbuf *buf,
* status reported by the remote helper if the latter is 'no
match'.
*/
if (status == REF_STATUS_NONE)
- return;
+ return 1;
}
(*ref)-status = status;
(*ref)-remote_status = msg;
+ return 0;
}
static void push_update_refs_status(struct helper_data *data,
@@ -708,11 +710,24 @@ static void push_update_refs_status(struct helper_data
*data,
struct strbuf buf = STRBUF_INIT;
struct ref *ref = remote_refs;
for (;;) {
+ char *private;
+
recvline(data, buf);
if (!buf.len)
break;
- push_update_ref_status(buf, ref, remote_refs);
+ if (push_update_ref_status(buf, ref, remote_refs))
+ continue;
+
+ if (!data-refspecs)
+ continue;
+
+ /* propagate back the update to the remote namespace */
+ private = apply_refspecs(data-refspecs, data-refspec_nr,
ref-name);
+ if (!private)
+ continue;
+ update_ref(update by helper, private, ref-new_sha1, NULL, 0,
0);
+ free(private);
}
strbuf_release(buf);
}
--
1.8.2.1.679.g509521a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
put THEIR commits AFTER my commits with a single rebase command
I asked this on stackoverflow, but no reply. http://stackoverflow.com/questions/15971244/git-put-their-commits-after-my-commits-with-a-single-rebase-command Suppose master and origin/master diverged. I'm on master and I want to put the commits from origin/master after my commits and then push --force. A---B---C origin/master / D---E---F---G *master desired result: A---B---C origin/master / D---E---F---G---A'---B'---C' *master Variant 1: git branch -f tmp git reset --hard origin/master git rebase tmp This variant is bad, because 'git reset --hard' checks out some files and 'git rebase' rewrites them again before applying commits. It's a redundant job. Variant 2: git branch -f tmp origin/master git rebase --onto master master tmp git branch -f master git checkout master Too many commands. I want to do this with just one command. And I want to stay be on branch master in case of rebase conflicts. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
