Sterling silver variety of thomas sabo charms
You could possibly by now be informed about your thomas sabo uk series; presented throughout 2012 your silver precious metal allure club's acceptance is constantly on the propagate all over the world. Most rings along with bracelet are generally sterling silver in the finest. According to the duration of your Jones Sabo Diamond necklace, they have a number of weight loads. Start in all-around dhs80 throughout price tag along with work up for you to dhs250. As soon as you go with a diamond necklace, you'll find silver precious metal fittings that you just select primarily based yet again in bodyweight. http://www.cheapsthomassoboshop.co.uk/ It can be just for that reason that will make sure you look at generating final decision through the variety of thomas sabo charm club supplying while this provides you with anyone the means for you to check out distinct strategies so because of this, receive the ideal discounts. It can be on the other hand highly recommended to pass through your entire listing so as to just remember to pick a qualified probable expensive jewelry that could produce the highest enjoyment along with buzz. In addition, be aware that these kind of expensive jewelry are certainly not tied to employ in bracelet by yourself, that they doubles to hold in organizations amid various other characteristics determined by the needs you have. There exists lots of Sabo expensive jewelry and even though this can be a scenario, deciding on one can possibly always be very complicated. For the reason that all are intricately along with magnificently made. In the delayed 1990s numerous Jones Sabo retail outlets ended up popped along with grew to be profitable, throughout The european countries, Japan plus the US. The stove is usually distributed by simply other stores whom usually are experts in excellent, substantial manner diamond jewelry. It can be challenging to never always be impressed with the details which in turn retreats into many of the patterns, along with the plethora of expensive jewelry, pedants and also other solutions offered. Your Sterling silver goods have a very fresh new along with full of energy experience, along with take care of numerous style along with instances. That they get your thoughts and still have the unquestionable lure. There exists a thing in the eagerness is actually Sabo along with Kolbli are generally distinguished available throughout every single part. http://www.cheapsthomassoboshop.co.uk/ Right now, a great deal of folks are inclined pertaining to Jones diamond necklace. There are several guy bracelet that will hold a new big more people. You will find there's variety of gothic bracelet adult men bring in a great deal of older people way too. http://www.cheapsthomassoboshop.co.uk/ Your thomas sabo charms uk in shape in distinct providers. These kind of allure providers occur available as bracelet, designer watches as well as rings. It can be wonderful that one could trade your current expensive jewelry from a necklace some day, for a diamond necklace in the morning in case that may be precisely what you would want to accomplish. The several providers appear in distinct price tags which in turn necessarily mean there's a new service provider that may be reasonably priced for you to anyone and will be suited for you to whatever you decide and are after. http://www.cheapsthomassoboshop.co.uk/ -- View this message in context: http://git.661346.n2.nabble.com/Sterling-silver-variety-of-thomas-sabo-charms-tp7580913.html Sent from the git mailing list archive at Nabble.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 (Mar 2013, #07; Tue, 26)
Junio C Hamano writes:
>> * tr/line-log (2013-03-23) 6 commits
>> - Speed up log -L... -M
>> - log -L: :pattern:file syntax to find by funcname
>> - Implement line-history search (git log -L)
>> - Export rewrite_parents() for 'log -L'
>> - fixup
>> - Refactor parse_loc
>>
>> Rerolled; collides with nd/magic-pathspecs.
>
> Honestly I am not sure what to make of this one. I'd say we should
> merge this down as-is to 'master', expecting that in some future we
> would fix "log --follow" to keep the refspecs per history traversal
> path, so that this can be more naturally reimplemented. Objections?
I was really hoping for something like that to happen :-) but I need to
look into at least one segfault bug in the option parser, noticed by
Antoine Pelisse. Expect a (final?) reroll soon.
--
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
[PATCH v2] Avoid loading commits twice in log with diffs
If you run a log with diffs (such as -p, --raw, --stat etc.) the
current code ends up loading many objects twice. For example, for
'log -3000 -p' my instrumentation said the objects loaded more than
once are distributed as follows:
2008 blob
2103 commit
2678 tree
Fixing blobs and trees will be harder, because those are really used
within the diff engine and need some form of caching.
However, fixing the commits is easy at least at the band-aid level.
They are triggered by log_tree_diff() invoking diff_tree_sha1() on
commits, which duly loads the specified object to dereference it to a
tree. Since log_tree_diff() knows that it works with commits and they
must have trees, we can simply pass through the trees.
We add some parse_commit() calls. The ones for the parents are
required; we do not know at this stage if they have been looked at.
The one for the commit itself is pure paranoia, but has about the same
cost as an assertion on commit->object.parsed.
This has a quite dramatic effect on log --raw, though only a
negligible impact on log -p:
Test this tree HEAD
4000.2: log --raw -3000 0.50(0.43+0.06) 0.54(0.46+0.06) +7.0%***
4000.3: log -p -3000 2.34(2.20+0.13) 2.37(2.22+0.13) +1.2%
Significance hints: '.' 0.1 '*' 0.05 '**' 0.01 '***' 0.001
Signed-off-by: Thomas Rast
Signed-off-by: Thomas Rast
---
Adjusted for the concern that the commit might not be parsed yet. I
think it's now more paranoid than the original code, since we cannot
look at commit->parents without parsing. But it's really an almost
free check.
log-tree.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 5dc45c4..8a34332 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -792,11 +792,14 @@ static int log_tree_diff(struct rev_info *opt, struct
commit *commit, struct log
{
int showed_log;
struct commit_list *parents;
- unsigned const char *sha1 = commit->object.sha1;
+ unsigned const char *sha1;
if (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS))
return 0;
+ parse_commit(commit);
+ sha1 = commit->tree->object.sha1;
+
/* Root commit? */
parents = commit->parents;
if (!parents) {
@@ -819,7 +822,9 @@ static int log_tree_diff(struct rev_info *opt, struct
commit *commit, struct log
* parent, showing summary diff of the others
* we merged _in_.
*/
- diff_tree_sha1(parents->item->object.sha1, sha1, "",
&opt->diffopt);
+ parse_commit(parents->item);
+ diff_tree_sha1(parents->item->tree->object.sha1,
+ sha1, "", &opt->diffopt);
log_tree_diff_flush(opt);
return !opt->loginfo;
}
@@ -832,7 +837,9 @@ static int log_tree_diff(struct rev_info *opt, struct
commit *commit, struct log
for (;;) {
struct commit *parent = parents->item;
- diff_tree_sha1(parent->object.sha1, sha1, "", &opt->diffopt);
+ parse_commit(parent);
+ diff_tree_sha1(parent->tree->object.sha1,
+ sha1, "", &opt->diffopt);
log_tree_diff_flush(opt);
showed_log |= !opt->loginfo;
--
1.8.2.351.g867a5da
--
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] git help config: s/insn/instruction/
On 03/28/2013 06:59 AM, Junio C Hamano wrote: Matthias Krüger writes: "insn" appears to be an in-code abbreviation and should not appear in manual/help pages. --- Thanks; sign-off? Oops, sorry. Signed-off-by: Matthias Krüger (Is this sufficient or do I have to re-send the patch with the sign-off line?) -- 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
Change the committer username
Good day, Please how can I change the committer username from system default to personalize? -- Kind Regards Eric Kom System Administrator & Programmer - Metropolitan College _ / You are scrupulously honest, frank, and \ | straightforward. Therefore you have few | \ friends./ - \ \ .--. |o_o | |:_/ | // \ \ (| Kom | ) /'\_ _/`\ \___)=(___/ 2 Hennie Van Till, White River, 1240 Tel: 013 750 2255 | Fax: 013 750 0105 | Cell: 078 879 1334 eric...@kom.za.net | eric...@metropolitancollege.co.za www.kom.za.net | www.kom.za.org | www.erickom.co.za Key fingerprint: 513E E91A C243 3020 8735 09BB 2DBC 5AD7 A9DA 1EF5 -- 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
Change the committer username
Good day, Please how can I change the committer username from system default to personalize? -- Kind Regards Eric Kom System Administrator & Programmer - Metropolitan College _ / You are scrupulously honest, frank, and \ | straightforward. Therefore you have few | \ friends./ - \ \ .--. |o_o | |:_/ | // \ \ (| Kom | ) /'\_ _/`\ \___)=(___/ 2 Hennie Van Till, White River, 1240 Tel: 013 750 2255 | Fax: 013 750 0105 | Cell: 078 879 1334 eric...@kom.za.net | eric...@metropolitancollege.co.za www.kom.za.net | www.kom.za.org | www.erickom.co.za Key fingerprint: 513E E91A C243 3020 8735 09BB 2DBC 5AD7 A9DA 1EF5 -- 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: Composing git repositories
Jens Lehmann wrote: > Unless you acknowledge that submodules are a different repo, you'll > always run into problems. I believe future enhancements will make > this less tedious, but in the end they will stay separate repos > (which is the whole point, you'd want to use a different approach > - e.g. subtree - if you want to put stuff from different upstreams > into a single repo without keeping the distinction where all that > came from). I acknowledge that it's a different repository. It's just that I think that our current design has too many seams: why do you think it's impossible to make it seamless? git-subtree is not an answer to anything. Dumping all the history into one repository has its limited usecases, but it is no solution. > What else than a commit object should that be??? Submodules are > there to have a different upstream for a part of your work tree, > and that means a commit in that repo is the only sane thing to > record in the superproject. A lot of thought has been put into > this, and it is definitely a good choice [1]. Linus argues that it shouldn't be a tree object, and I agree with that. I don't see an argument that says that the commit object is a perfect fit (probably because it's not). > How? The "submodules suck, we should try a completely different > approach" thingy comes up from time to time, but so far nobody > could provide a viable alternative to what we currently do. My argument is not "submodules suck; we should throw them out of the window, and start from scratch" at all. I'm merely questioning the fundamental assumptions that submodules make, instead of proposing that we work around everything in shell. We don't have to be married to the existing implementation of submodules and try to fix all the problems in shell. > And apart from that, let's not forget we identified some valuable > improvements to submodules in this thread: > [...] > All of those are topics I like to see materialize, and you are > welcome to tackle them. Allow me a few days to think about changing the fundamental building blocks to make our shell hackery easier. Thanks. -- 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] merge-tree: fix "same file added in subdir"
On Wed, Mar 27, 2013 at 10:57:39PM +, John Keeping wrote: > On Wed, Mar 27, 2013 at 03:42:40PM -0700, Junio C Hamano wrote: > > John Keeping writes: > > > > > When the same file is added with identical content at the top level, > > > git-merge-tree prints "added in both" with the details. But if the file > > > is added in an existing subdirectory, threeway_callback() bails out early > > > because the two trees have been modified identically. > > > > > > In order to detect this, we need to fall through and recurse into the > > > subtree in this case. > > > > > > Signed-off-by: John Keeping > > > > The rationale the above description gives is internally consistent, > > but it is rather sad to see this optimization go. The primary > > motivation behind this program, which does not use the usual > > unpack-trees machinery, is to allow us to cull the identical result > > at a shallow level of the traversal when the both sides changed (not > > added) a file deep in a subdirectory hierarchy. > > > > The patch makes me wonder if we should go the other way around, > > resolving the "both added identically" case at the top cleanly > > without complaint. > > I don't use merge-tree so I have no opinion on this, just wanted to fix > an inconsistency :-) Having re-read the manpage, I think you're right that we should just resolve the "both added identically" case cleanly, but I wonder whether some of the other cases should also be resolved cleanly. git-merge-tree(1) says: the output from the command omits entries that match the tree. so you could argue that "added in branch1", "changed in branch1, unmodified in branch2" and "removed in branch1, unchanged in branch2" should also print no output. But as I said above I don't use git-merge-tree so perhaps people who do would like to explain what they expect in these cases. -- 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: Composing git repositories
Jonathan Nieder wrote: > Do you mean that you wish you could ignore subrepository boundaries > and use commands like > > git clone --recurse-submodules http://git.zx2c4.com/cgit > cd cgit > vi git/cache.h > ... edit edit edit ... > git add --recurse-submodules git/cache.h > git commit --recurse-submodules > git push --recurse-submodules > > , possibly with configuration to allow the --recurse-submodules to be > implied, and have everything work out well? Do you realize how difficult this is to implement? We'll need to patch all the git commands to essentially do what we'd get for free if the submodule were a tree object instead of a commit object (although I'm not saying that's the Right thing to do). Some caveats: - If we maintain one global index, and try to emulate git-subtree using submodules, we've lost. It's going to take freakin' ages to stat billions of files across hundreds of nested sumodules. One major advantage of having repository boundaries is separate object stores, indexes, worktrees: little ones that git is designed to work with. - Auto-splitting commits that touch multiple submodules/ superproject at once. Although git-subtree does this, I think it's horribly ugly. - Auto-propagating commits upwards to the superproject is another big challenge. I think the current design of anchoring to a specific commit SHA-1 has its usecases, but is unwieldy when things become big. We have to fix that first. -- 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
Collective wisdom about repos on NFS accessed by concurrent clients (== corruption!?)
Hi, I'm hoping to hear some wisdom on the subject so I can decide if I'm chasing a pipe dream or if it should be expected to work and I just need to work out the kinks. Finding things like this makes it sound possible: http://permalink.gmane.org/gmane.comp.version-control.git/122670 but then again, in threads like this: http://kerneltrap.org/mailarchive/git/2010/11/14/44799 opinions are that it's just not reliable to trust. My experience so far is that I eventually get repo corruption when I stress it with concurrent read/write access from multiple hosts (beyond any sort of likely levels, but still). Maybe I'm doing something wrong, missing a configuration setting somewhere, put an unfair stress on the system, there's a bona fide bug - or, given the inherent difficulty in achieving perfect coherency between machines on what's visible on the mount, it's just impossible (?) to truly get it working under all situations. My eventual usecase is to set up a bunch of (gitolite) hosts that all are effectively identical and all seeing the same storage and clients can then be directed to any of them to get served. For the purpose of this query I assume it can be boiled down to be the same as n users working on their own workstations and accessing a central repo on an NFS share they all have mounted, using regular file paths. Correct? I have a number of load-generating test scripts (that from humble beginnings have grown to beasts), but basically, they fork a number of code pieces that proceed to hammer the repo with concurrent reading and writing. If necessary, the scripts can be started on different hosts, too. It's all about the central repo so clients will retry in various modes whenever they get an error, up to re-cloning and starting over. All in all, they can yield quite a load. On a local filesystem everything seems to be holding up fine which is expected. In the scenario with concurrency on top of shared NFS storage however, the scripts eventually fails with various problems (when the timing finally finds a hole, I guess) - possible to clone but checkouts fails, errors about refs pointing wrong and errors where the original repo is actually corrupted and can't be cloned from. Depending on test strategy, I've also had everything going fine (ops looks fine in my logs), but afterwards the repo has lost a branch or two. I've experimented with various NFS settings (e.g. turning off the attribute cache), but haven't reached a conclusion. I do suspect that it just is a fact of life with a remote filesystem to have coherency problems with high concurrency like this but I'd be happily proven wrong - I'm not an expert in either NFS or git. So, any opionions either way would be valuable, e.g. 'it should work' or 'it can never work 100%' is fine, as well as any suggestions. Obviously, the concurrency needed to make it probable to hit this seems so unlikely that maybe I just shouldn't worry... I'd be happy to share scripts and whatever if someone would like to try it out themselves. Thanks for your time, ken1 -- 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: Composing git repositories
Junio C Hamano wrote: > As I said in another thread, your top-level may be only a part in > somebody else's project, and what you consider just a part of your > project may be the whole project to somebody else. If you pick one > location to store both for the above clone, e.g. cgit/.git (it could > be cgit/.ram-git or any other name), embedding it in a yet larger > project (perhaps having both cgit and gitolite to give a one-stop > solution for hosting services) later would face the same issue as > Ram seemed to be complaining. It needs to address what happens when > that cgit/.git (or whatever name) gets in the way in the scope of > the larger project. That is why I said Ram's rant, using subjective > words like "elegant", without sound technical justification, did not > make much sense to me. I was having a lot of difficulty writing down my thoughts. Thank you for providing an illustrative example. It is terribly hard to do with our current implementation: we'd have to rewrite the "gitdir: " lines in all the .git files in the submodule trees and rebuild all the .git/modules paths. I'm thinking that we need to separate the object stores from the worktrees for good. For a project with no submodules, the object store can be present in .git/ of the toplevel directory, like it is now. The moment submodules are added, all the object stores should be relocated to a place outside the worktree. So my ~/src might look like: dotfiles.git/, auto-complete.git/, magit.git/, git-commit-mode.git/, yasnippet.git/ and dotfiles/. dotfiles/ contains lots of worktrees stitched together nicely, pointing to these object stores in ~/src. This would certainly get rid of the asymmetry for good. Now, we can focus our attention on composing git worktrees. What is a worktree? A tree object pointed to by the commit object referred to by HEAD. What we need to do is embed one tree inside another using a mediating object to establish repository boundaries, while not introducing an ugly seam. If you think about it, the mediator we've picked conveys little/ no information to the parent; it says: "there's a commit with this SHA-1 present in this submodule, but I can't tell you the commit message, tree object, branch, remote, or anything else" (obviously because the commit isn't present in the parent's object store). So, the mediator might as well have been a SHA-1 string. And we have an ugly .gitmodules conveying the remote and the branch. Why can't we stuff more information into the mediating object and get rid of .gitmodules altogether? Okay, here's a first draft of the new design. The new mediator object should look like: name = git ref = v1.7.8 The name is looked up in refs/modules/, which in turn looks like: [submodule "git"] origin = gh:artagnon/git path = git [submodule "magit"] origin = gh:magit/magit path = git/extensions/magit The ref could be 'master', 'HEAD~1', or even a commit SHA-1 (to do the current anchored-submodules). Finally, there's a .git file in the worktree, which contains a "gitdir: " line pointing to the object store, as before. This solves the two problems that I brought up earlier: - Floating submodules (which are _necessary_ if you don't want to propagate commits upwards to the root). - Initializing a nested submodule without having to initialize all the submodules in the path leading up to it. However, I suspect that we can put more information the mediator object to make life easier for the parent repository and make seams disappear. I'm currently thinking about what information git core needs to behave smoothly with submodules. -- 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/2] t5520 (pull): use test_config where appropriate
Configuration from test_config does not last beyond the end of the current test assertion, making each test easier to think about in isolation. Signed-off-by: Ramkumar Ramachandra --- Removed first hunk, as per Junio's comment. t/t5520-pull.sh | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index e5adee8..8ea 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -96,8 +96,7 @@ test_expect_success '--rebase' ' ' test_expect_success 'pull.rebase' ' git reset --hard before-rebase && - git config --bool pull.rebase true && - test_when_finished "git config --unset pull.rebase" && + test_config pull.rebase true && git pull . copy && test $(git rev-parse HEAD^) = $(git rev-parse copy) && test new = $(git show HEAD:file2) @@ -105,8 +104,7 @@ test_expect_success 'pull.rebase' ' test_expect_success 'branch.to-rebase.rebase' ' git reset --hard before-rebase && - git config --bool branch.to-rebase.rebase true && - test_when_finished "git config --unset branch.to-rebase.rebase" && + test_config branch.to-rebase.rebase true && git pull . copy && test $(git rev-parse HEAD^) = $(git rev-parse copy) && test new = $(git show HEAD:file2) @@ -114,10 +112,8 @@ test_expect_success 'branch.to-rebase.rebase' ' test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' git reset --hard before-rebase && - git config --bool pull.rebase true && - test_when_finished "git config --unset pull.rebase" && - git config --bool branch.to-rebase.rebase false && - test_when_finished "git config --unset branch.to-rebase.rebase" && + test_config pull.rebase true && + test_config branch.to-rebase.rebase false && git pull . copy && test $(git rev-parse HEAD^) != $(git rev-parse copy) && test new = $(git show HEAD:file2) @@ -171,9 +167,9 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' git update-ref refs/remotes/me/copy copy^ && COPY=$(git rev-parse --verify me/copy) && git rebase --onto $COPY copy && - git config branch.to-rebase.remote me && - git config branch.to-rebase.merge refs/heads/copy && - git config branch.to-rebase.rebase true && + test_config branch.to-rebase.remote me && + test_config branch.to-rebase.merge refs/heads/copy && + test_config branch.to-rebase.rebase true && echo dirty >> file && git add file && test_must_fail git pull && -- 1.8.2.141.g07926c6 -- 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] git-send-email.perl: implement suggestions made by perlcritic
Running perlcritic with gentle severity reports six problems. The
following lists the line numbers on which the problems occur, along
with a description of the problem. This patch fixes them all, after
carefully considering the consequences.
516: Contrary to common belief, subroutine prototypes do not enable
compile-time checks for proper arguments. They serve the singular
purpose of defining functions that behave like built-in functions, but
check_file_rev_conflict was never intended to behave like one. We
have verified that the callers of the subroutines are alright with the
change.
714, 836, 855, 1487: Returning `undef' upon failure from a subroutine
is pretty common. But if the subroutine is called in list context, an
explicit `return undef;' statement will return a one-element list
containing `(undef)'. Now if that list is subsequently put in a
boolean context to test for failure, then it evaluates to true. But
you probably wanted it to be false. The solution is to just use a
bare `return' statement whenever you want to return failure. In list
context, Perl will then give you an empty list (which is false), and
`undef' in scalar context (which is also false).
1441: The three-argument form of `open' (introduced in Perl 5.6)
prevents subtle bugs that occur when the filename starts with funny
characters like '>' or '<'. It's also more explicitly clear to define
the input mode of the file. This policy will not complain if the file
explicitly states that it is compatible with a version of Perl prior
to 5.6 via an include statement (see 'use 5.008' near the top of the
file).
Signed-off-by: Ramkumar Ramachandra
---
Junio: In future, please tell me explicitly that you're expecting a
re-roll with an updated commit message. It wasn't obvious to me at
all.
git-send-email.perl | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index c3501d9..633f447 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -513,7 +513,7 @@ if (@alias_files and $aliasfiletype and defined
$parse_alias{$aliasfiletype}) {
($sender) = expand_aliases($sender) if defined $sender;
# returns 1 if the conflict must be solved using it as a format-patch argument
-sub check_file_rev_conflict($) {
+sub check_file_rev_conflict {
return unless $repo;
my $f = shift;
try {
@@ -711,7 +711,7 @@ sub ask {
}
}
}
- return undef;
+ return;
}
my %broken_encoding;
@@ -833,7 +833,7 @@ sub extract_valid_address {
# less robust/correct than the monster regexp in Email::Valid,
# but still does a 99% job, and one less dependency
return $1 if $address =~ /($local_part_regexp\@$domain_regexp)/;
- return undef;
+ return;
}
sub extract_valid_address_or_die {
@@ -852,7 +852,7 @@ sub validate_address {
valid_re => qr/^(?:quit|q|drop|d|edit|e)/i,
default => 'q');
if (/^d/i) {
- return undef;
+ return;
} elsif (/^q/i) {
cleanup_compose_files();
exit(0);
@@ -1453,7 +1453,7 @@ sub recipients_cmd {
my $sanitized_sender = sanitize_address($sender);
my @addresses = ();
- open my $fh, "$cmd \Q$file\E |"
+ open my $fh, q{-|}, "$cmd \Q$file\E"
or die "($prefix) Could not execute '$cmd'";
while (my $address = <$fh>) {
$address =~ s/^\s*//g;
@@ -1499,7 +1499,7 @@ sub validate_patch {
return "$.: patch contains a line longer than 998
characters";
}
}
- return undef;
+ return;
}
sub file_has_nonascii {
--
1.8.2.141.g07926c6
--
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] use refnames instead of "left"/"right" in dirdiffs
Hi John.
On Wed, 2013-03-27 at 23:07 +, John Keeping wrote:
> That's not going to work well on Windows, is it?
Uhm Winwhat? No seriously... doesn't dir-diff fail ther anway? The mkdir
right now also uses mkpath with "/"... and I could read in it's
documentation that it would automatically translate this, does it?
> Anything with two dots
> in is already forbidden so we don't need to worry about that
Sure about this? I mean they're forbidden as refnames, but is this
really checked within git-difftool before?
> ; I'm not
> sure we need to remove forward slashes at all
Mhh could be... seems that the cleanup happens via completely removing
the $tmpdir path.
And for the actual diff it shouldn't matter when there's a partentdir
more.
> until we consider the
> "commit containing" syntax ':/fix nasty bug' or 'master^{/fix bug}'.
Phew... don't ask me... I'm not that into the git source code... from
the filename side, these shouldn't bother, only / an NUL is forbidden
(in POSIX,... again I haven't checked win/mac which might not adhere to
the standards).
> I'm more concerned with specifiers containing '^', '@', '{', ':' - see
> 'SPECIFYING REVISIONS' in git-rev-parse(1) for the full details of
> what's acceptable.
Mhh there are likely more problems... I just noted that $ldir/$rdir are
used in a call to system() so... that needs to be shell escaped to
prevent any bad stuff
And if perl (me not a perl guy) interprets any such characters specially
in strings, it must be handled either.
> At some point I think it may be better to fall back
> to the SHA1 of the relevant commit.
Think that would be quite a drawback...
Cheers,
Chris.
smime.p7s
Description: S/MIME cryptographic signature
Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()
Jeff King wrote: > Subject: [PATCH] t5516: drop implicit arguments from helper functions Thanks a lot for this! I just had to s/ $repo_name/ "$repo_name"/ to fix the quoting. Will post a re-roll soon. -- 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
Bug in "git rev-parse --verify"
On Junio's master, "git rev-parse --verify" accepts *any* 40-digit hexadecimal number. For example, pass it 40 "1" characters, and it accepts the argument: $ git rev-parse --verify $ echo $? 0 Obviously, my repo doesn't have an object with this hash :-) so I think this argument should be rejected. If you add or remove a digit (to make the length different than 40), it is correctly rejected: $ git rev-parse --verify 111 fatal: Needed a single revision $ echo $? 128 I believe that "git rev-parse --verify" is meant to verify that the argument is an actual object, and that it should reject fictional SHA1s. (If not then the documentation should be clarified.) The same problem also exists in 1.8.2 but I haven't checked how much older it is. The behavior presumably comes from the following clause in get_sha1_basic(): if (len == 40 && !get_sha1_hex(str, sha1)) return 0; I won't have time to pursue this. 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
[PATCH v4 0/6] Support triangular workflows
Hi, The changes in this round are: 1. Peff submitted a patch to squash into [3/6]. Since his patch essentially reverts mine, I've blamed him for the change. 2. Peff suggested a code movement in [5/6] to make things flow more naturally. 3. Jonathan suggested a better test description in [2/6]. 4. Junio suggested a minor documentation update in [6/6]. My build of git has had this feature for two weeks now (since the first iteration), and I'm very happy with it. Jeff King (1): t5516 (fetch-push): drop implicit arguments from helper functions Ramkumar Ramachandra (5): remote.c: simplify a bit of code using git_config_string() t5516 (fetch-push): update test description remote.c: introduce a way to have different remotes for fetch/push remote.c: introduce remote.pushdefault remote.c: introduce branch..pushremote Documentation/config.txt | 24 +++- builtin/push.c | 2 +- remote.c | 41 -- remote.h | 1 + t/t5516-fetch-push.sh| 316 +++ 5 files changed, 238 insertions(+), 146 deletions(-) -- 1.8.2.141.g3797f84 -- 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] remote.c: simplify a bit of code using git_config_string()
A small segment where handle_config() parses the branch.remote
configuration variable can be simplified using git_config_string().
Signed-off-by: Ramkumar Ramachandra
---
remote.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/remote.c b/remote.c
index 174e48e..02e6c4c 100644
--- a/remote.c
+++ b/remote.c
@@ -357,9 +357,8 @@ static int handle_config(const char *key, const char
*value, void *cb)
return 0;
branch = make_branch(name, subkey - name);
if (!strcmp(subkey, ".remote")) {
- if (!value)
- return config_error_nonbool(key);
- branch->remote_name = xstrdup(value);
+ if (git_config_string(&branch->remote_name, key, value))
+ return -1;
if (branch == current_branch) {
default_remote_name = branch->remote_name;
explicit_default_remote_name = 1;
--
1.8.2.141.g3797f84
--
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] t5516 (fetch-push): update test description
The file was originally created in bcdb34f (Test wildcard push/fetch, 2007-06-08), and only contained tests that exercised wildcard functionality at the time. In subsequent commits, many other tests unrelated to wildcards were added but the test description was never updated. Fix this. Helped-by: Jonathan Nieder Signed-off-by: Ramkumar Ramachandra --- t/t5516-fetch-push.sh | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 6fd125a..38f8fc0 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1,6 +1,17 @@ #!/bin/sh -test_description='fetching and pushing, with or without wildcard' +test_description='Basic fetch/push functionality. + +This test checks the following functionality: + +* command-line syntax +* refspecs +* fast-forward detection, and overriding it +* configuration +* hooks +* --porcelain output format +* hiderefs +' . ./test-lib.sh -- 1.8.2.141.g3797f84 -- 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] t5516 (fetch-push): drop implicit arguments from helper functions
From: Jeff King
Many of the tests in t5516 look like:
mk_empty &&
git push testrepo ... &&
check_push_result $commit heads/master
It's reasonably easy to see what is being tested, with the
exception that "testrepo" is a magic global name (it is
implicitly used in the helpers, but we have to name it
explicitly when calling git directly). Let's make it
explicit when call the helpers, too. This is slightly more
typing, but makes the test snippets read more naturally.
It also makes it easy for future tests to use an alternate
or multiple repositories, without a proliferation of helper
functions.
[rr: fixed sloppy quoting]
Signed-off-by: Jeff King
Signed-off-by: Ramkumar Ramachandra
---
t/t5516-fetch-push.sh | 276 ++
1 file changed, 142 insertions(+), 134 deletions(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 38f8fc0..94e0189 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -18,10 +18,11 @@ This test checks the following functionality:
D=`pwd`
mk_empty () {
- rm -fr testrepo &&
- mkdir testrepo &&
+ repo_name="$1"
+ rm -fr "$repo_name" &&
+ mkdir "$repo_name" &&
(
- cd testrepo &&
+ cd "$repo_name" &&
git init &&
git config receive.denyCurrentBranch warn &&
mv .git/hooks .git/hooks-disabled
@@ -29,16 +30,19 @@ mk_empty () {
}
mk_test () {
- mk_empty &&
+ repo_name="$1"
+ shift
+
+ mk_empty "$repo_name" &&
(
for ref in "$@"
do
- git push testrepo $the_first_commit:refs/$ref || {
+ git push "$repo_name" $the_first_commit:refs/$ref || {
echo "Oops, push refs/$ref failure"
exit 1
}
done &&
- cd testrepo &&
+ cd "$repo_name" &&
for ref in "$@"
do
r=$(git show-ref -s --verify refs/$ref) &&
@@ -52,9 +56,10 @@ mk_test () {
}
mk_test_with_hooks() {
+ repo_name=$1
mk_test "$@" &&
(
- cd testrepo &&
+ cd "$repo_name" &&
mkdir .git/hooks &&
cd .git/hooks &&
@@ -86,13 +91,16 @@ mk_test_with_hooks() {
}
mk_child() {
- rm -rf "$1" &&
- git clone testrepo "$1"
+ rm -rf "$2" &&
+ git clone "$1" "$2"
}
check_push_result () {
+ repo_name="$1"
+ shift
+
(
- cd testrepo &&
+ cd "$repo_name" &&
it="$1" &&
shift
for ref in "$@"
@@ -124,7 +132,7 @@ test_expect_success setup '
'
test_expect_success 'fetch without wildcard' '
- mk_empty &&
+ mk_empty testrepo &&
(
cd testrepo &&
git fetch .. refs/heads/master:refs/remotes/origin/master &&
@@ -137,7 +145,7 @@ test_expect_success 'fetch without wildcard' '
'
test_expect_success 'fetch with wildcard' '
- mk_empty &&
+ mk_empty testrepo &&
(
cd testrepo &&
git config remote.up.url .. &&
@@ -152,7 +160,7 @@ test_expect_success 'fetch with wildcard' '
'
test_expect_success 'fetch with insteadOf' '
- mk_empty &&
+ mk_empty testrepo &&
(
TRASH=$(pwd)/ &&
cd testrepo &&
@@ -169,7 +177,7 @@ test_expect_success 'fetch with insteadOf' '
'
test_expect_success 'fetch with pushInsteadOf (should not rewrite)' '
- mk_empty &&
+ mk_empty testrepo &&
(
TRASH=$(pwd)/ &&
cd testrepo &&
@@ -186,7 +194,7 @@ test_expect_success 'fetch with pushInsteadOf (should not
rewrite)' '
'
test_expect_success 'push without wildcard' '
- mk_empty &&
+ mk_empty testrepo &&
git push testrepo refs/heads/master:refs/remotes/origin/master &&
(
@@ -199,7 +207,7 @@ test_expect_success 'push without wildcard' '
'
test_expect_success 'push with wildcard' '
- mk_empty &&
+ mk_empty testrepo &&
git push testrepo "refs/heads/*:refs/remotes/origin/*" &&
(
@@ -212,7 +220,7 @@ test_expect_success 'push with wildcard' '
'
test_expect_success 'push with insteadOf' '
- mk_empty &&
+ mk_empty testrepo &&
TRASH="$(pwd)/" &&
git config "url.$TRASH.insteadOf" trash/ &&
git push trash/testrepo refs/heads/master:refs/remotes/origin/master &&
@@ -226,7 +234,7 @@ test_expect_success 'push with insteadOf' '
'
test_expect_success 'push with pushInsteadOf' '
- mk_empty &&
+ mk_empty testrepo &&
TRASH="$(pwd)/" &&
git config "url.$TRASH.pushInsteadOf" trash/ &&
git push trash/testrepo refs/heads/master:refs/remotes/origin/master &&
@@ -240,7 +248,7 @@ test_exp
[PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push
Currently, do_push() in push.c calls remote_get(), which gets the
configured remote for fetching and pushing. Replace this call with a
call to pushremote_get() instead, a new function that will return the
remote configured specifically for pushing. This function tries to
work with the string pushremote_name, before falling back to the
codepath of remote_get(). This patch has no visible impact, but
serves to enable future patches to introduce configuration variables
to set pushremote_name. For example, you can now do the following in
handle_config():
if (!strcmp(key, "remote.pushdefault"))
git_config_string(&pushremote_name, key, value);
Then, pushes will automatically go to the remote specified by
remote.pushdefault.
Signed-off-by: Ramkumar Ramachandra
---
builtin/push.c | 2 +-
remote.c | 25 +
remote.h | 1 +
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 42b129d..d447a80 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport,
int flags)
static int do_push(const char *repo, int flags)
{
int i, errs;
- struct remote *remote = remote_get(repo);
+ struct remote *remote = pushremote_get(repo);
const char **url;
int url_nr;
diff --git a/remote.c b/remote.c
index 02e6c4c..b733aec 100644
--- a/remote.c
+++ b/remote.c
@@ -49,6 +49,7 @@ static int branches_nr;
static struct branch *current_branch;
static const char *default_remote_name;
+static const char *pushremote_name;
static int explicit_default_remote_name;
static struct rewrites rewrites;
@@ -670,17 +671,21 @@ static int valid_remote_nick(const char *name)
return !strchr(name, '/'); /* no slash */
}
-struct remote *remote_get(const char *name)
+static struct remote *remote_get_1(const char *name, const char
*pushremote_name)
{
struct remote *ret;
int name_given = 0;
- read_config();
if (name)
name_given = 1;
else {
- name = default_remote_name;
- name_given = explicit_default_remote_name;
+ if (pushremote_name) {
+ name = pushremote_name;
+ name_given = 1;
+ } else {
+ name = default_remote_name;
+ name_given = explicit_default_remote_name;
+ }
}
ret = make_remote(name, 0);
@@ -699,6 +704,18 @@ struct remote *remote_get(const char *name)
return ret;
}
+struct remote *remote_get(const char *name)
+{
+ read_config();
+ return remote_get_1(name, NULL);
+}
+
+struct remote *pushremote_get(const char *name)
+{
+ read_config();
+ return remote_get_1(name, pushremote_name);
+}
+
int remote_is_configured(const char *name)
{
int i;
diff --git a/remote.h b/remote.h
index f7b08f1..5fe5f53 100644
--- a/remote.h
+++ b/remote.h
@@ -51,6 +51,7 @@ struct remote {
};
struct remote *remote_get(const char *name);
+struct remote *pushremote_get(const char *name);
int remote_is_configured(const char *name);
typedef int each_remote_fn(struct remote *remote, void *priv);
--
1.8.2.141.g3797f84
--
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] remote.c: introduce remote.pushdefault
This new configuration variable defines the default remote to push to,
and overrides `branch..remote` for all branches. It is useful
in the typical triangular-workflow setup, where the remote you're
fetching from is different from the remote you're pushing to.
Signed-off-by: Ramkumar Ramachandra
---
Documentation/config.txt | 13 ++---
remote.c | 7 +++
t/t5516-fetch-push.sh| 12
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c1f435f..dd78171 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -727,9 +727,12 @@ branch.autosetuprebase::
This option defaults to never.
branch..remote::
- When in branch , it tells 'git fetch' and 'git push' which
- remote to fetch from/push to. It defaults to `origin` if no remote is
- configured. `origin` is also used if you are not on any branch.
+ When on branch , it tells 'git fetch' and 'git push'
+ which remote to fetch from/push to. The remote to push to
+ may be overridden with `remote.pushdefault` (for all branches).
+ If no remote is configured, or if you are not on any branch,
+ it defaults to `origin` for fetching and `remote.pushdefault`
+ for pushing.
branch..merge::
Defines, together with branch..remote, the upstream branch
@@ -1898,6 +1901,10 @@ receive.updateserverinfo::
If set to true, git-receive-pack will run git-update-server-info
after receiving data from git-push and updating refs.
+remote.pushdefault::
+ The remote to push to by default. Overrides
+ `branch..remote` for all branches.
+
remote..url::
The URL of a remote repository. See linkgit:git-fetch[1] or
linkgit:git-push[1].
diff --git a/remote.c b/remote.c
index b733aec..49c4b8b 100644
--- a/remote.c
+++ b/remote.c
@@ -389,9 +389,16 @@ static int handle_config(const char *key, const char
*value, void *cb)
add_instead_of(rewrite, xstrdup(value));
}
}
+
if (prefixcmp(key, "remote."))
return 0;
name = key + 7;
+
+ /* Handle remote.* variables */
+ if (!strcmp(name, "pushdefault"))
+ return git_config_string(&pushremote_name, key, value);
+
+ /* Handle remote..* variables */
if (*name == '/') {
warning("Config remote shorthand cannot begin with '/': %s",
name);
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 94e0189..ac1ec9d 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -520,6 +520,18 @@ test_expect_success 'push with config remote.*.push =
HEAD' '
git config --remove-section remote.there
git config --remove-section branch.master
+test_expect_success 'push with remote.pushdefault' '
+ mk_test up_repo heads/frotz &&
+ mk_test down_repo heads/master &&
+ test_config remote.up.url up_repo &&
+ test_config remote.down.url down_repo &&
+ test_config branch.master.remote up &&
+ test_config remote.pushdefault down &&
+ git push &&
+ test_must_fail check_push_result up_repo $the_commit heads/master &&
+ check_push_result down_repo $the_commit heads/master
+'
+
test_expect_success 'push with config remote.*.pushurl' '
mk_test testrepo heads/master &&
--
1.8.2.141.g3797f84
--
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] remote.c: introduce branch..pushremote
This new configuration variable overrides `remote.pushdefault` and
`branch..remote` for pushes. When you pull from one
place (e.g. your upstream) and push to another place (e.g. your own
publishing repository), you would want to set `remote.pushdefault` to
specify the remote to push to for all branches, and use this option to
override it for a specific branch.
Signed-off-by: Ramkumar Ramachandra
---
Documentation/config.txt | 19 +++
remote.c | 4
t/t5516-fetch-push.sh| 15 +++
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index dd78171..ec579c5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -730,9 +730,19 @@ branch..remote::
When on branch , it tells 'git fetch' and 'git push'
which remote to fetch from/push to. The remote to push to
may be overridden with `remote.pushdefault` (for all branches).
- If no remote is configured, or if you are not on any branch,
- it defaults to `origin` for fetching and `remote.pushdefault`
- for pushing.
+ The remote to push to, for the current branch, may be further
+ overridden by `branch..pushremote`. If no remote is
+ configured, or if you are not on any branch, it defaults to
+ `origin` for fetching and `remote.pushdefault` for pushing.
+
+branch..pushremote::
+ When on branch , it overrides `branch..remote` for
+ pushing. It also overrides `remote.pushdefault` for pushing
+ from branch . When you pull from one place (e.g. your
+ upstream) and push to another place (e.g. your own publishing
+ repository), you would want to set `remote.pushdefault` to
+ specify the remote to push to for all branches, and use this
+ option to override it for a specific branch.
branch..merge::
Defines, together with branch..remote, the upstream branch
@@ -1903,7 +1913,8 @@ receive.updateserverinfo::
remote.pushdefault::
The remote to push to by default. Overrides
- `branch..remote` for all branches.
+ `branch..remote` for all branches, and is overridden by
+ `branch..pushremote` for specific branches.
remote..url::
The URL of a remote repository. See linkgit:git-fetch[1] or
diff --git a/remote.c b/remote.c
index 49c4b8b..e89b1b7 100644
--- a/remote.c
+++ b/remote.c
@@ -364,6 +364,10 @@ static int handle_config(const char *key, const char
*value, void *cb)
default_remote_name = branch->remote_name;
explicit_default_remote_name = 1;
}
+ } else if (!strcmp(subkey, ".pushremote")) {
+ if (branch == current_branch)
+ if (git_config_string(&pushremote_name, key,
value))
+ return -1;
} else if (!strcmp(subkey, ".merge")) {
if (!value)
return config_error_nonbool(key);
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index ac1ec9d..13028a4 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -542,6 +542,21 @@ test_expect_success 'push with config remote.*.pushurl' '
check_push_result testrepo $the_commit heads/master
'
+test_expect_success 'push with config branch.*.pushremote' '
+ mk_test up_repo heads/frotz &&
+ mk_test side_repo heads/quux &&
+ mk_test down_repo heads/master &&
+ test_config remote.up.url up_repo &&
+ test_config remote.pushdefault side_repo &&
+ test_config remote.down.url down_repo &&
+ test_config branch.master.remote up &&
+ test_config branch.master.pushremote down &&
+ git push &&
+ test_must_fail check_push_result up_repo $the_commit heads/master &&
+ test_must_fail check_push_result side_repo $the_commit heads/master &&
+ check_push_result down_repo $the_commit heads/master
+'
+
# clean up the cruft left with the previous one
git config --remove-section remote.there
--
1.8.2.141.g3797f84
--
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 subtree oddity
I built v1.8.2 last evening and found that the subtree command isn't supported. What version of git are you using? And where did you get it? SPS Sent from my iPhone On Mar 27, 2013, at 8:12 PM, Thomas Taranowski wrote: > I'd like to have the following configuration: > > /myproject.git > |__/upstream_dependency -- Points to a remote library git repo > |__/project_source -- local project source > > > I issue the following commands to pull in the upstream dependency as a > subtree of the myproject.git repo: > > git remote add upstream git://gnuradio.org/gnuradio > git fetch upstream > git checkout master > git subtree add -P upstream upstream/master > > Now, my expectation is that I would have the following: > > /myproject.git > |__/upstream_dependency -- Points to a remote library git repo > |< all the upstream files are present here, as expected > > |__/project_source < this is still intact, as expected > > |__< all the upstream files are present here. wtf?> > > My question is, why does "subtree add" pull in all the subtree files > into the root of the repo, and not just into the specified subtree > prefix? > > # > # Here's an excerpt of what I see: > # > $:~/scratch/myproject.git$ ls > AUTHORS gr-comedi gr-utils > cmake gr-digital gr-video-sdl > CMakeLists.txt gr-fcd gr-vocoder > config.h.in gr-fft gr-wavelet > COPYING gr-filter gr-wxgui > docsgr-howto-write-a-block README > dtools gr-noaa README.building-boost > gnuradio-core gr-pagerREADME.hacking > gr-analog gr-qtguiREADME-win32-mingw-short.txt > gr-atsc gr-shd upstream < the subtree directory > gr-audiogr-trellis volk > gr-blocks gruel > grc gr-uh > > # > # Also, those same files are in the upstream subtree directory as well > (as expected) > # > $:~/scratch/myproject.git$ ls upstream > AUTHORS grc gruel > cmake gr-comedi gr-uhd > CMakeLists.txt gr-digital gr-utils > config.h.in gr-fcd gr-video-sdl > COPYING gr-fft gr-vocoder > docsgr-filter gr-wavelet > dtools gr-howto-write-a-block gr-wxgui > gnuradio-core gr-noaa README > gr-analog gr-pagerREADME.building-boost > gr-atsc gr-qtguiREADME.hacking > gr-audiogr-shd README-win32-mingw-short.txt > gr-blocks gr-trellis volk > -- > 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 -- 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: propagating repo corruption across clone
On Tue, Mar 26, 2013 at 11:47 PM, Junio C Hamano wrote: > The difference between --mirror and no --mirror is a red herring. > You may want to ask Jeff Mitchell to remove the mention of it; it > only adds to the confusion without helping users. If you made > byte-for-byte copy of corrupt repository, it wouldn't make any > difference if the first "checkout" notices it. Hi, Several days ago I had actually already updated the post to indicate that my testing methodology was incorrect as a result of mixing up --no-hardlinks and --no-local, and pointed to this thread. I will say that we did see corrupted repos on the downstream mirrors. I don't have an explanation for it, but have not been able to reproduce it either. My only potential guess (untested) is that perhaps when the corruption was detected the clone aborted but left the objects already transferred locally. Again, untested -- I mention it only because it's my only potential explanation :-) > To be paranoid, you may want to set transfer.fsckObjects to true, > perhaps in your ~/.gitconfig. Interesting; I'd known about receive.fsckObjects but not transfer/fetch. Thanks for the pointer. -- 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] Avoid loading commits twice in log with diffs
On Thu, Mar 28, 2013 at 09:19:34AM +0100, Thomas Rast wrote:
> However, fixing the commits is easy at least at the band-aid level.
> They are triggered by log_tree_diff() invoking diff_tree_sha1() on
> commits, which duly loads the specified object to dereference it to a
> tree. Since log_tree_diff() knows that it works with commits and they
> must have trees, we can simply pass through the trees.
Reading this made me wonder if we could be doing the optimization at a
lower level, by re-using objects that we have in our lookup_object cache
(which would include the commit->tree peeling that you're optimizing
here).
My first instinct was to look at read_object_with_reference, but it's a
bit general; for trees, we can indeed find the buffer/size pair. But for
commits, we must infer the size from the buffer (by using strlen). And
for blobs, we do not win at all, as we do not cache the blob contents.
Another option is for diff_tree_sha1 to use parse_tree_indirect. That
will pull the commit object from the cache and avoid re-parsing it, as
well as re-use any trees (although that should not happen much in a
regular "log" traversal).
That patch looks something like:
diff --git a/tree-diff.c b/tree-diff.c
index ba01563..db454bc 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -269,27 +269,28 @@ int diff_tree_sha1(const unsigned char *old, const
unsigned char *new, const cha
int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const
char *base, struct diff_options *opt)
{
- void *tree1, *tree2;
+ struct tree *tree1, *tree2;
struct tree_desc t1, t2;
- unsigned long size1, size2;
int retval;
- tree1 = read_object_with_reference(old, tree_type, &size1, NULL);
+ tree1 = parse_tree_indirect(old);
if (!tree1)
die("unable to read source tree (%s)", sha1_to_hex(old));
- tree2 = read_object_with_reference(new, tree_type, &size2, NULL);
+ tree2 = parse_tree_indirect(new);
if (!tree2)
die("unable to read destination tree (%s)", sha1_to_hex(new));
- init_tree_desc(&t1, tree1, size1);
- init_tree_desc(&t2, tree2, size2);
+ init_tree_desc(&t1, tree1->buffer, tree1->size);
+ init_tree_desc(&t2, tree2->buffer, tree2->size);
retval = diff_tree(&t1, &t2, base, opt);
if (!*base && DIFF_OPT_TST(opt, FOLLOW_RENAMES) &&
diff_might_be_rename()) {
- init_tree_desc(&t1, tree1, size1);
- init_tree_desc(&t2, tree2, size2);
+ init_tree_desc(&t1, tree1->buffer, tree1->size);
+ init_tree_desc(&t2, tree2->buffer, tree2->size);
try_to_follow_renames(&t1, &t2, base, opt);
}
- free(tree1);
- free(tree2);
+ /*
+* Note that we are now filling up our cache with extra tree data; we
+* could potentially unparse the tree objects.
+*/
return retval;
}
but it turns out to actually be slower! The problem is that parse_object
will actually re-check the sha1 on every object it reads, which
read_object_with_reference will not do.
Using this hack:
diff --git a/object.c b/object.c
index 20703f5..361eb74 100644
--- a/object.c
+++ b/object.c
@@ -195,6 +195,7 @@ struct object *parse_object_or_die(const unsigned char
*sha1,
die(_("unable to parse object: %s"), name ? name : sha1_to_hex(sha1));
}
+int parse_object_quick = 0;
struct object *parse_object(const unsigned char *sha1)
{
unsigned long size;
@@ -221,7 +222,8 @@ struct object *parse_object(const unsigned char *sha1)
buffer = read_sha1_file(sha1, &type, &size);
if (buffer) {
- if (check_sha1_signature(repl, buffer, size, typename(type)) <
0) {
+ if (!parse_object_quick &&
+ check_sha1_signature(repl, buffer, size, typename(type)) <
0) {
free(buffer);
error("sha1 mismatch %s", sha1_to_hex(repl));
return NULL;
diff --git a/tree-diff.c b/tree-diff.c
index db454bc..4b55a1a 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -267,18 +267,23 @@ int diff_tree_sha1(const unsigned char *old, const
unsigned char *new, const cha
q->nr = 1;
}
+/* hacky */
+extern int parse_object_quick;
+
int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const
char *base, struct diff_options *opt)
{
struct tree *tree1, *tree2;
struct tree_desc t1, t2;
int retval;
+ parse_object_quick = 1;
tree1 = parse_tree_indirect(old);
if (!tree1)
die("unable to read source tree (%s)", sha1_to_hex(old));
tree2 = parse_tree_indirect(new);
if (!tree2)
die("unable to read destination tree (%s)", sha1_to_hex(new));
+ parse_object_quick = 0;
init_tree_desc(&t1, tree1->buffer, tree1->size);
init_tree_desc(&t2, tree2->buffer, tree2->size);
retv
Re: propagating repo corruption across clone
On Wed, Mar 27, 2013 at 2:51 PM, Rich Fromm wrote: > Nevertheless, I will try to contact Jeff and point him at this. My initial > reading of his blog posts definitely gave me the impression that this was a > --mirror vs. not issue, but it really sounds like his main problem was using > --local. Actually, I wasn't using --local, I just wasn't using --no-local, as I mixed up that and --no-hardlinks (which lies somewhere between, but is still not the same as using file://). It's entirely possible that you read the posts before I updated them. Also, keep in mind, if you're evaluating what you're doing, that the system we have was not set up to be a backup system. It was set up to be a mirror system. With proper sanity checking, it would have acted in a decent capacity as a backup system, but that wasn't why it was set up, nor how it was set 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: Bug in "git rev-parse --verify"
On Thu, Mar 28, 2013 at 02:04:27PM +0100, Michael Haggerty wrote: > $ git rev-parse --verify > > $ echo $? > 0 > > [...] > > I believe that "git rev-parse --verify" is meant to verify that the > argument is an actual object, and that it should reject fictional SHA1s. > (If not then the documentation should be clarified.) The same problem > also exists in 1.8.2 but I haven't checked how much older it is. I think it is only about verifying the name of the object. I.e., that you can resolve the argument to an object. It has always behaved this way; I even just tested with git v0.99. I can't think of any reason it would not be helpful to have it _also_ verify that we have the object in question. Most of the time that would be a no-op, as any ref we resolve should point to a valid object, but it would mean we could catch repository corruption (i.e., missing objects) early. Doing a has_sha1_file() check would be relatively quick. Doing a full parse_object and checking the sha1 would be more robust, but would mean that: blob=`git rev-parse --verify HEAD:some-large-file` would become much more expensive (and presumably you are about to access the object _anyway_, at which point you would notice problems, so it is redundantly expensive). -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] use refnames instead of "left"/"right" in dirdiffs
On Thu, Mar 28, 2013 at 01:46:16PM +0100, Christoph Anton Mitterer wrote:
> On Wed, 2013-03-27 at 23:07 +, John Keeping wrote:
> > That's not going to work well on Windows, is it?
>
> Uhm Winwhat? No seriously... doesn't dir-diff fail ther anway? The mkdir
> right now also uses mkpath with "/"... and I could read in it's
> documentation that it would automatically translate this, does it?
I believe Windows generally accepts '/' as a directory separator in
place of '\'. In a recent thread Johannes Sixt reported a difftool test
failure on Windows, so it does work (and we do have code to implicitly
set --no-symlinks when running on Windows).
> > Anything with two dots
> > in is already forbidden so we don't need to worry about that
>
> Sure about this? I mean they're forbidden as refnames, but is this
> really checked within git-difftool before?
We've already run git-diff by this point, and that should have
complained if the refs are invalid, causing difftool to die.
> > ; I'm not
> > sure we need to remove forward slashes at all
>
> Mhh could be... seems that the cleanup happens via completely removing
> the $tmpdir path.
> And for the actual diff it shouldn't matter when there's a partentdir
> more.
>
> > until we consider the
> > "commit containing" syntax ':/fix nasty bug' or 'master^{/fix bug}'.
>
> Phew... don't ask me... I'm not that into the git source code... from
> the filename side, these shouldn't bother, only / an NUL is forbidden
> (in POSIX,... again I haven't checked win/mac which might not adhere to
> the standards).
Filenames on Windows cannot contain any of the following[1]:
\ / : * ? " < > |
but it's also potentially annoying that '^' and '{' have special meaning
in some shells and would need escaping (although I suppose we don't
really expect users to by typing these directory names in themselves).
> > I'm more concerned with specifiers containing '^', '@', '{', ':' - see
> > 'SPECIFYING REVISIONS' in git-rev-parse(1) for the full details of
> > what's acceptable.
>
> Mhh there are likely more problems... I just noted that $ldir/$rdir are
> used in a call to system() so... that needs to be shell escaped to
> prevent any bad stuff
Are there really non-list calls to system? Providing the only calls
provide each of the arguments separately (instead of in a single string)
I think we're OK, but I'm also not a Perl expert.
> And if perl (me not a perl guy) interprets any such characters specially
> in strings, it must be handled either.
>
> > At some point I think it may be better to fall back
> > to the SHA1 of the relevant commit.
> Think that would be quite a drawback...
It depends where that happens. I suspect most people use relatively
simple ref specifiers which wouldn't get to this stage, but do you
really want to turn the following into a directory name?
origin/master@{3 weeks ago}^{/Merge branch 'jk/}^2
I admit it's something of a contrived example but I hope it illustrates
my point that sometimes naming the directory after the ref specifier may
be less useful than just using "left" or the SHA1.
[1] http://support.microsoft.com/kb/177506
John
--
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] use refnames instead of "left"/"right" in dirdiffs
On Thu, Mar 28, 2013 at 02:19:44PM +, John Keeping wrote:
> On Thu, Mar 28, 2013 at 01:46:16PM +0100, Christoph Anton Mitterer wrote:
> > On Wed, 2013-03-27 at 23:07 +, John Keeping wrote:
> > > That's not going to work well on Windows, is it?
> >
> > Uhm Winwhat? No seriously... doesn't dir-diff fail ther anway? The mkdir
> > right now also uses mkpath with "/"... and I could read in it's
> > documentation that it would automatically translate this, does it?
>
> I believe Windows generally accepts '/' as a directory separator in
> place of '\'. In a recent thread Johannes Sixt reported a difftool test
> failure on Windows, so it does work (and we do have code to implicitly
> set --no-symlinks when running on Windows).
>
> > > Anything with two dots
> > > in is already forbidden so we don't need to worry about that
> >
> > Sure about this? I mean they're forbidden as refnames, but is this
> > really checked within git-difftool before?
>
> We've already run git-diff by this point, and that should have
> complained if the refs are invalid, causing difftool to die.
>
> > > ; I'm not
> > > sure we need to remove forward slashes at all
> >
> > Mhh could be... seems that the cleanup happens via completely removing
> > the $tmpdir path.
> > And for the actual diff it shouldn't matter when there's a partentdir
> > more.
> >
> > > until we consider the
> > > "commit containing" syntax ':/fix nasty bug' or 'master^{/fix bug}'.
> >
> > Phew... don't ask me... I'm not that into the git source code... from
> > the filename side, these shouldn't bother, only / an NUL is forbidden
> > (in POSIX,... again I haven't checked win/mac which might not adhere to
> > the standards).
>
> Filenames on Windows cannot contain any of the following[1]:
>
> \ / : * ? " < > |
>
> but it's also potentially annoying that '^' and '{' have special meaning
> in some shells and would need escaping (although I suppose we don't
> really expect users to by typing these directory names in themselves).
>
> > > I'm more concerned with specifiers containing '^', '@', '{', ':' - see
> > > 'SPECIFYING REVISIONS' in git-rev-parse(1) for the full details of
> > > what's acceptable.
> >
> > Mhh there are likely more problems... I just noted that $ldir/$rdir are
> > used in a call to system() so... that needs to be shell escaped to
> > prevent any bad stuff
>
> Are there really non-list calls to system? Providing the only calls
> provide each of the arguments separately (instead of in a single string)
> I think we're OK, but I'm also not a Perl expert.
>
> > And if perl (me not a perl guy) interprets any such characters specially
> > in strings, it must be handled either.
> >
> > > At some point I think it may be better to fall back
> > > to the SHA1 of the relevant commit.
> > Think that would be quite a drawback...
>
> It depends where that happens. I suspect most people use relatively
> simple ref specifiers which wouldn't get to this stage, but do you
> really want to turn the following into a directory name?
>
> origin/master@{3 weeks ago}^{/Merge branch 'jk/}^2
>
> I admit it's something of a contrived example but I hope it illustrates
> my point that sometimes naming the directory after the ref specifier may
> be less useful than just using "left" or the SHA1.
Actually, I think I was wrong here it's probably easier to just do
something you already do, but with a whitelist, like this:
my $leftname, $rightname;
// figure out what refs these point at...
$leftname =~ s/[^a-zA-Z0-9]/_/g;
$rightname =~ s/[^a-zA-Z0-9]/_/g;
We probably want to whitelist some more characters there, but that seems
like the simplest way to tackle it. And if someone puts in a long
revision then they get a long directory name (or we truncate it at some
point).
> [1] http://support.microsoft.com/kb/177506
--
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] http-backend: respect GIT_NAMESPACE with dumb clients
John Koleszar writes: > diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh > index b5d7fbc..5a19d61 100755 > --- a/t/t5561-http-backend.sh > +++ b/t/t5561-http-backend.sh > @@ -134,6 +134,10 @@ POST /smart/repo.git/git-receive-pack HTTP/1.1 200 - > ### > GET /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 403 - > POST /smart/repo.git/git-receive-pack HTTP/1.1 403 - > + > +### namespace test > +### > +GET /smart_namespace/repo.git/info/refs HTTP/1.1 200 > EOF > test_expect_success 'server request log matches test results' ' > sed -e " > diff --git a/t/t556x_common b/t/t556x_common > index 82926cf..cb9eb9d 100755 > --- a/t/t556x_common > +++ b/t/t556x_common > @@ -120,3 +120,19 @@ test_expect_success 'http.receivepack false' ' > GET info/refs?service=git-receive-pack "403 Forbidden" && > POST git-receive-pack "403 Forbidden" > ' > +test_expect_success 'backend respects namespaces' ' > + log_div "namespace test" > + config http.uploadpack true && > + config http.getanyfile true && > + > + GIT_NAMESPACE=ns && export GIT_NAMESPACE && When other people want to enhance this test suite later, their tests may not want the namespace contaminated with the environment variable. You would need to enclose from here to the end inside a subshell or something. > + git push public master:master && > + (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && > + git for-each-ref | grep /$GIT_NAMESPACE/ >/dev/null > + ) && > + > + git ls-remote public >exp && > + curl "$HTTPD_URL/smart_namespace/repo.git/info/refs" >act && Spell out "expect" and "actual". For some unknwon reason, I am getting an HTTPD_URL at this point, causing it to fail with: curl: (3) malformed > + test_cmp exp act && > + (grep /ns/ exp && false || true) What does that last line even mean? Both false && false || true true && false || true will yield true. Leftover from your debugging session? -- 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 2/2] t5520 (pull): use test_config where appropriate
Ramkumar Ramachandra writes: > Configuration from test_config does not last beyond the end of the > current test assertion, making each test easier to think about in > isolation. > > Signed-off-by: Ramkumar Ramachandra > --- > Removed first hunk, as per Junio's comment. Thanks, but doesn't yd/use-test-config-unconfig topic already address this? -- 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: Bug in "git rev-parse --verify"
Jeff King writes: > On Thu, Mar 28, 2013 at 02:04:27PM +0100, Michael Haggerty wrote: > >> $ git rev-parse --verify >> >> $ echo $? >> 0 >> >> [...] >> >> I believe that "git rev-parse --verify" is meant to verify that the >> argument is an actual object, and that it should reject fictional SHA1s. >> (If not then the documentation should be clarified.) The same problem >> also exists in 1.8.2 but I haven't checked how much older it is. > > I think it is only about verifying the name of the object. I.e., that > you can resolve the argument to an object. It has always behaved this > way; I even just tested with git v0.99. Correct. It is about "is it well formed and something we can turn into 20-byte object name?" and nothing more. It certainly does not mean "do we have it?", as the function needs to be able to validate something we do not yet have. -- 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 2/2] t5520 (pull): use test_config where appropriate
Junio C Hamano wrote: > Thanks, but doesn't yd/use-test-config-unconfig topic already > address this? Just saw it. 9d6aa64 is identical to my patch, but misses the fourth hunk. -- 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 v4 0/6] Support triangular workflows
Ramkumar Ramachandra writes: > The changes in this round are: > > 1. Peff submitted a patch to squash into [3/6]. Since his patch >essentially reverts mine, I've blamed him for the change. > > 2. Peff suggested a code movement in [5/6] to make things flow more >naturally. > > 3. Jonathan suggested a better test description in [2/6]. > > 4. Junio suggested a minor documentation update in [6/6]. > > My build of git has had this feature for two weeks now (since the > first iteration), and I'm very happy with it. Will take a look; thanks for a reroll. We may need a bit of adjustment to the longer term plan for the push.default topic, as I expect we will have this feature a lot sooner (e.g. perhaps in 1.8.4) than we will switch the push default to "simple". The description of simple and upstream still is written around the "you push to and pull from the same place", and may need to be updated to explain "what happens if you are employing a triangular workflow?" Or it could turn out that triangular people may be better served by a push.default different from simple or upstream. Or it may turn out that we do not need any change to what is queued on the push-2.0-default-to-simple topic (I haven't thought things through). It is not urgent, but please start thinking about how you can help, now you introduced the triangular stuff. Thanks. > Jeff King (1): > t5516 (fetch-push): drop implicit arguments from helper functions > > Ramkumar Ramachandra (5): > remote.c: simplify a bit of code using git_config_string() > t5516 (fetch-push): update test description > remote.c: introduce a way to have different remotes for fetch/push > remote.c: introduce remote.pushdefault > remote.c: introduce branch..pushremote > > Documentation/config.txt | 24 +++- > builtin/push.c | 2 +- > remote.c | 41 -- > remote.h | 1 + > t/t5516-fetch-push.sh| 316 > +++ > 5 files changed, 238 insertions(+), 146 deletions(-) -- 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 2/2] t5520 (pull): use test_config where appropriate
Junio C Hamano writes: > Ramkumar Ramachandra writes: > >> Configuration from test_config does not last beyond the end of the >> current test assertion, making each test easier to think about in >> isolation. >> >> Signed-off-by: Ramkumar Ramachandra >> --- >> Removed first hunk, as per Junio's comment. > > Thanks, but doesn't yd/use-test-config-unconfig topic already > address this? The last hunk from your version was missing from 9d6aa64dc32b (t5520: use test_config to set/unset git config variables, 2013-03-24); I'll pick that part and apply as a follow up to the series. No need to resend; thanks. -- 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 and GSoC 2013
Jeff King wrote: > There was a big thread about a month ago on whether Git should do Google > Summer of Code this year[1]. Take only one or two students and get the entire community involved in learning from the GSoC experience, so we can do a bigger one next year. -- 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] gitk: Move hard-coded colors to .gitk
Gauthier Östervall wrote: > Screenshot of my current coloring setup using this patch, based on > zenburn: > http://s11.postimg.org/hozbtsfj7/gitk_zenburn.png > And the .gitk used to that end: > https://gist.github.com/fleutot/5253281 This is a really cool color theme. Would we consider shipping some themes with gitk, in contrib/ perhaps? -- 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: Bug in "git rev-parse --verify"
On 03/28/2013 02:48 PM, Junio C Hamano wrote:
> I think it has always been about "is this well formed and we can turn it
> into a raw 20-byte object name?" and never about"does it exist?"
That's surprising. The man page says
--verify
The parameter given must be usable as a single, valid object name.
Otherwise barf and abort.
"Valid", to me, implies that the parameter should be the name of an
actual object, and this also seems a more useful concept to me and more
consistent with the command's behavior when passed other arguments.
Is there a simple way to verify an object name more strictly and convert
it to an SHA1? I can only think of solutions that require two commands,
like
git cat-file -e $ARG && git rev-parse --verify $ARG
I suppose in most contexts where one wants to know whether an object
name is valid, one should also verify that the object has the type that
you expect:
test X$(git cat-file -t $ARG) = Xcommit &&
git rev-parse --verify $ARG
or (allowing tag dereferencing)
git cat-file -e $ARG^{commit} &&
git rev-parse --verify $ARG^{commit}
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: [PATCH v4 0/6] Support triangular workflows
On Thu, Mar 28, 2013 at 06:56:36PM +0530, Ramkumar Ramachandra wrote:
> Jeff King (1):
> t5516 (fetch-push): drop implicit arguments from helper functions
>
> Ramkumar Ramachandra (5):
> remote.c: simplify a bit of code using git_config_string()
> t5516 (fetch-push): update test description
> remote.c: introduce a way to have different remotes for fetch/push
> remote.c: introduce remote.pushdefault
> remote.c: introduce branch..pushremote
Thanks, this iteration looks pretty good. I have one minor nit, which is
that the tests in patches 5 and 6 do things like:
> + git push &&
> + test_must_fail check_push_result up_repo $the_commit heads/master &&
> + check_push_result down_repo $the_commit heads/master
Using test_must_fail here caught my eye, because usually it is meant to
check failure of a single git command. When it (or "!", for that matter)
is used with a compound function, you end up losing robustness in corner
cases.
For example, imagine your function is:
check_foo() {
cd "$1" &&
git foo
}
and you expect in some cases that "git foo" will succeed and in some
cases it will fail. In the affirmative case (running "check_foo"), this
is robust; if any of the steps fails, the test fails.
But if you run the negative case ("test_must_fail check_foo"), you will
also fail if any of the preparatory steps fail. I.e., you wanted to say:
cd "$1" && test_must_fail git foo
but you actually said (applying De Morgan's laws):
test_must_fail cd "$1" || test_must_fail git foo
Now we probably don't expect the "cd" to fail here, but of course the
other steps can be more complicated, too. In your case, I think the
effect you are looking for is that "heads/master != $the_commit". But
note that we would also fail if "git fsck" fails in the pushed
repository, which is not what we want.
Sometimes it's annoyingly verbose to break down a compound function. But
I think in this case, you can make your tests more robust by just
checking the affirmative that the ref is still where we expect it to be,
like:
check_push_result up_repo $the_first_commit heads/master
Sorry if that was a bit long-winded. I think that practically speaking,
it is not a likely source of problems in this case. But it's an
anti-pattern in our tests that I think is worth mentioning.
-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] push: Alias pushurl from push rewrites
Josh Triplett writes: > On Wed, Mar 27, 2013 at 05:48:45PM -0500, Rob Hoelz wrote: > ... >> The test that checked that pushInsteadOf + pushurl shouldn't work as I >> expect was actually broken; I have removed it, updated the >> documentation, and sent a new patch to the list. > > There's an argument for either behavior as valid. My original patch > specifically documented and tested for the opposite behavior, namely > that pushurl overrides any pushInsteadOf, because I intended > pushInsteadOf as a fallback if you don't have an explicit pushurl set. For only this bit. I think the test in question is this one from t5516: test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty && TRASH="$(pwd)/" && git config "url.trash2/.pushInsteadOf" trash/ && git config remote.r.url trash/wrong && git config remote.r.pushurl "$TRASH/testrepo" && git push r refs/heads/master:refs/remotes/origin/master && ( cd testrepo && r=$(git show-ref -s --verify refs/remotes/origin/master) && test "z$r" = "z$the_commit" && test 1 = $(git for-each-ref refs/remotes/origin | wc -l) ) ' It defines a remote "r", with URL "trash/wrong" (used for fetch) and pushURL "$(pwd)/testrepo" (used for push). There is a pushInsteadOf rule to rewrite anything that goes to "trash/*" to be pushed to "trash2/*" instead but that shouldn't be used to rewrite an explicit pushURL. And then the test pushes to "r" and checks if testrepo gets updated; in other words, it wants to make sure remote.r.pushURL defines the final destination, without pushInsteadOf getting in the way. But $(pwd)/testrepo does not match trash/* in the first place, so there is no chance for a pushInsteadOf to interfere; it looks to me that it is not testing what it wants to test. Perhaps we should do something like this? We tell it to push to "testrepo/" with pushURL, and set up a pushInsteadOf to rewrite "testrepo/" to "trash2/", but because for this push it comes from an explicit pushURL, it still goes to "testrepo/". As we do not have "trash2/" repository, the test not just tests the push goes to "testrepo/", but it also tests that it does not attempt to push to "trash2/", checking both sides of the coin. t/t5516-fetch-push.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index d3dc5df..b5ea32c 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -230,10 +230,9 @@ test_expect_success 'push with pushInsteadOf' ' test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty && - TRASH="$(pwd)/" && - git config "url.trash2/.pushInsteadOf" trash/ && + git config "url.trash2/.pushInsteadOf" testrepo/ && git config remote.r.url trash/wrong && - git config remote.r.pushurl "$TRASH/testrepo" && + git config remote.r.pushurl "testrepo/" && git push r refs/heads/master:refs/remotes/origin/master && ( cd testrepo && -- 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: Bug in "git rev-parse --verify"
On Thu, Mar 28, 2013 at 04:34:19PM +0100, Michael Haggerty wrote: > Is there a simple way to verify an object name more strictly and convert > it to an SHA1? I can only think of solutions that require two commands, > like > > git cat-file -e $ARG && git rev-parse --verify $ARG Is the rev-parse line doing anything there? If $ARG does not resolve to a sha1, then wouldn't cat-file have failed? -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: Bug in "git rev-parse --verify"
On 03/28/2013 04:38 PM, Jeff King wrote: > On Thu, Mar 28, 2013 at 04:34:19PM +0100, Michael Haggerty wrote: > >> Is there a simple way to verify an object name more strictly and convert >> it to an SHA1? I can only think of solutions that require two commands, >> like >> >> git cat-file -e $ARG && git rev-parse --verify $ARG > > Is the rev-parse line doing anything there? If $ARG does not resolve to > a sha1, then wouldn't cat-file have failed? It's outputting the SHA1, which cat-file seems incapable of providing in a useful way. 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: [PATCH] push: Alias pushurl from push rewrites
Jonathan Nieder writes: > Josh Triplett wrote: > >> I have a .gitconfig in my git-managed home >> directory which sets pushInsteadOf so that I can clone via git:// and >> immediately have working push. I work with a number of systems that >> don't have inbound access to each other but do have outbound access to >> the network; on some of these "satellite" boxes, I can't push changes >> directly to the server pushInsteadOf points to, so I can explicitly set >> pushurl in .git/config for that repository, which overrides the >> pushInsteadOf. This change would break that configuration. > > Would it? As long as your pushurl does not start with git://, I think > your configuration would still work fine. That is a good point, especially because it is very unlikely that git:// was used for pushURL, given that it would not have been rewritten with pushInsteadOf to an authenticated transport. > After this patch, neither pushInsteadOf nor pushUrl overrides the > other one. The rule is: > > 1. First, get the URL from the remote's configuration, based > on whether you are fetching or pushing. > > (At this step, in your setup git chooses the URL specified > with pushurl in your .git/config.) > > 2. Next, apply the most appropriate url.*.insteadOf or > url.*.pushInsteadOf rule, based on whether you are fetching > or pushing. > > (At this step, no rewrite rules apply, so the URL is used > as is.) -- 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] http-backend: respect GIT_NAMESPACE with dumb clients
On Thu, Mar 28, 2013 at 8:52 AM, John Koleszar wrote: > On Thu, Mar 28, 2013 at 7:43 AM, Junio C Hamano wrote: >> >> John Koleszar writes: >> >> > diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh >> > index b5d7fbc..5a19d61 100755 >> > --- a/t/t5561-http-backend.sh >> > +++ b/t/t5561-http-backend.sh >> > @@ -134,6 +134,10 @@ POST /smart/repo.git/git-receive-pack HTTP/1.1 200 >> > - >> > ### >> > GET /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 403 - >> > POST /smart/repo.git/git-receive-pack HTTP/1.1 403 - >> > + >> > +### namespace test >> > +### >> > +GET /smart_namespace/repo.git/info/refs HTTP/1.1 200 >> > EOF >> > test_expect_success 'server request log matches test results' ' >> > sed -e " >> > diff --git a/t/t556x_common b/t/t556x_common >> > index 82926cf..cb9eb9d 100755 >> > --- a/t/t556x_common >> > +++ b/t/t556x_common >> > @@ -120,3 +120,19 @@ test_expect_success 'http.receivepack false' ' >> > GET info/refs?service=git-receive-pack "403 Forbidden" && >> > POST git-receive-pack "403 Forbidden" >> > ' >> > +test_expect_success 'backend respects namespaces' ' >> > + log_div "namespace test" >> > + config http.uploadpack true && >> > + config http.getanyfile true && >> > + >> > + GIT_NAMESPACE=ns && export GIT_NAMESPACE && >> >> When other people want to enhance this test suite later, their tests >> may not want the namespace contaminated with the environment >> variable. You would need to enclose from here to the end inside a >> subshell or something. >> > > Ok. I'm not familiar with the test infrastructure, I had guessed that these > were already running inside a subshell. I'll make this explicit. > >> >> > + git push public master:master && >> > + (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && >> > + git for-each-ref | grep /$GIT_NAMESPACE/ >/dev/null >> > + ) && >> > + >> > + git ls-remote public >exp && >> > + curl "$HTTPD_URL/smart_namespace/repo.git/info/refs" >act && >> >> Spell out "expect" and "actual". >> > > The exiting tests are using exp and act for this. No objection if you want > me to spell it out here, but having two different files for this may be > confusing. > >> >> For some unknwon reason, I am getting an HTTPD_URL at this point, >> causing it to fail with: >> >> curl: (3) malformed >> > > Ah, my fault. I only ran t5561-http-backend.sh. Will fix. > >> > + test_cmp exp act && >> > + (grep /ns/ exp && false || true) >> >> What does that last line even mean? Both >> >> false && false || true >> true && false || true >> >> will yield true. Leftover from your debugging session? > > > Facepalm. The intent here is to invert the grep, to make sure that the /ns/ > does not appear in the output. No idea why I wrote it that way. Will fix. > -- 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] git help config: s/insn/instruction/
Matthias Krüger writes: > On 03/28/2013 06:59 AM, Junio C Hamano wrote: >> Matthias Krüger writes: >> >>> "insn" appears to be an in-code abbreviation and should not appear in >>> manual/help pages. >>> --- >> Thanks; sign-off? > Oops, sorry. > > Signed-off-by: Matthias Krüger > > (Is this sufficient or do I have to re-send the patch with the > sign-off line?) I can squash it in, so there is no need to resend. The more important thing is I won't have to for your future patches. Thanks. -- 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] push: Alias pushurl from push rewrites
On Wed, Mar 27, 2013 at 04:18:19PM -0700, Jonathan Nieder wrote: > Josh Triplett wrote: > > > I have a .gitconfig in my git-managed home > > directory which sets pushInsteadOf so that I can clone via git:// and > > immediately have working push. I work with a number of systems that > > don't have inbound access to each other but do have outbound access to > > the network; on some of these "satellite" boxes, I can't push changes > > directly to the server pushInsteadOf points to, so I can explicitly set > > pushurl in .git/config for that repository, which overrides the > > pushInsteadOf. This change would break that configuration. > > Would it? As long as your pushurl does not start with git://, I think > your configuration would still work fine. I had to think about it for a while, but I think you're right; I inferred a behavior that the patch didn't actually add or have anything to do with, namely having the result of applying pushInsteadOf to the non-push URL override the pushUrl. OK, I take it back. I *can* imagine configurations that this change would break, since it does change intentional and documented behavior, but I don't have any such configuration. The only such configuration I can imagine involves directly counting on the non-rewriting of pushUrl, by using pushInsteadOf to rewrite urls and then sometimes using pushUrl to override that and point back at the un-rewritten URL. And while supported, that does seem *odd*. Objection withdrawn; if nobody can come up with a sensible configuration that relies on the documented behavior, I don't particularly care if it changes. > After this patch, neither pushInsteadOf nor pushUrl overrides the > other one. The rule is: > > 1. First, get the URL from the remote's configuration, based > on whether you are fetching or pushing. > > (At this step, in your setup git chooses the URL specified > with pushurl in your .git/config.) > > 2. Next, apply the most appropriate url.*.insteadOf or > url.*.pushInsteadOf rule, based on whether you are fetching > or pushing. > > (At this step, no rewrite rules apply, so the URL is used > as is.) -- 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 subtree oddity
Stephen Smith writes: > I built v1.8.2 last evening and found that the subtree command > isn't supported. What version of git are you using? And where did > you get it? We have been carrying a copy of it in contrib/ but I have to say that it is in a sorry state. After the original author stopped working on it, a new maintainer of the subtree project picked it up. Some vocal users wanted to have it as a part of the git-core tarball, with a promise that it will be supported and maintained, and that is how we ended up carrying a copy of it in contrib/. We see some discussions and patches sent to the list from time to time, but I haven't been getting anything definitive from the new subtree maintainer for whatever reason (if I understand correctly, it is not his primary job; perhaps he is busy with other things) and the part of that contrib/ tree hasn't seen much activities. Also I occasionally see end-user questions here but I have a feeling that many go unanswered by area experts. I am starting to regret that I caved in and started carrying a copy of it in contrib/. It probably is a good idea to drop it from my tree and let it mature and eventually flourish outside. -- 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] push: Alias pushurl from push rewrites
On Thu, Mar 28, 2013 at 08:37:58AM -0700, Junio C Hamano wrote: > Josh Triplett writes: > > > On Wed, Mar 27, 2013 at 05:48:45PM -0500, Rob Hoelz wrote: > > ... > >> The test that checked that pushInsteadOf + pushurl shouldn't work as I > >> expect was actually broken; I have removed it, updated the > >> documentation, and sent a new patch to the list. > > > > There's an argument for either behavior as valid. My original patch > > specifically documented and tested for the opposite behavior, namely > > that pushurl overrides any pushInsteadOf, because I intended > > pushInsteadOf as a fallback if you don't have an explicit pushurl set. > > For only this bit. > > I think the test in question is this one from t5516: > > test_expect_success 'push with pushInsteadOf and explicit pushurl > (pushInsteadOf should not rewrite)' ' > mk_empty && > TRASH="$(pwd)/" && > git config "url.trash2/.pushInsteadOf" trash/ && > git config remote.r.url trash/wrong && > git config remote.r.pushurl "$TRASH/testrepo" && > git push r refs/heads/master:refs/remotes/origin/master && > ( > cd testrepo && > r=$(git show-ref -s --verify refs/remotes/origin/master) && > test "z$r" = "z$the_commit" && > > test 1 = $(git for-each-ref refs/remotes/origin | wc -l) > ) > ' > > It defines a remote "r", with URL "trash/wrong" (used for fetch) and > pushURL "$(pwd)/testrepo" (used for push). There is a pushInsteadOf > rule to rewrite anything that goes to "trash/*" to be pushed to > "trash2/*" instead but that shouldn't be used to rewrite an explicit > pushURL. > > And then the test pushes to "r" and checks if testrepo gets updated; > in other words, it wants to make sure remote.r.pushURL defines the > final destination, without pushInsteadOf getting in the way. > > But $(pwd)/testrepo does not match trash/* in the first place, so > there is no chance for a pushInsteadOf to interfere; it looks to me > that it is not testing what it wants to test. That test does actually test something important: it tests that the result of applying pushInsteadOf to url does *not* override pushurl. Not the same thing as testing that pushInsteadOf does or does not apply to pushurl. The relevant sentence of the git-config manpage (in the documentation for pushInsteadOf) says: > If a remote has an explicit pushurl, git will ignore this setting for > that remote. That really meant what I just said above: git will prefer an explicit pushurl over the pushInsteadOf rewrite of url. It says nothing about applying pushInsteadOf to rewrite pushurl. > Perhaps we should do something like this? We tell it to push to > "testrepo/" with pushURL, and set up a pushInsteadOf to rewrite > "testrepo/" to "trash2/", but because for this push it comes from an > explicit pushURL, it still goes to "testrepo/". > > As we do not have "trash2/" repository, the test not just tests the > push goes to "testrepo/", but it also tests that it does not attempt > to push to "trash2/", checking both sides of the coin. Sensible test, assuming you want to enforce that behavior. I don't strongly care either way about that one, since it only applies if your pushInsteadOf rewrites could apply to your pushurl, and I only ever use pushInsteadOf to rewrite unpushable repos to pushable ones. However... > t/t5516-fetch-push.sh | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh > index d3dc5df..b5ea32c 100755 > --- a/t/t5516-fetch-push.sh > +++ b/t/t5516-fetch-push.sh > @@ -230,10 +230,9 @@ test_expect_success 'push with pushInsteadOf' ' > > test_expect_success 'push with pushInsteadOf and explicit pushurl > (pushInsteadOf should not rewrite)' ' > mk_empty && > - TRASH="$(pwd)/" && > - git config "url.trash2/.pushInsteadOf" trash/ && > + git config "url.trash2/.pushInsteadOf" testrepo/ && > git config remote.r.url trash/wrong && > - git config remote.r.pushurl "$TRASH/testrepo" && > + git config remote.r.pushurl "testrepo/" && > git push r refs/heads/master:refs/remotes/origin/master && > ( > cd testrepo && ...the test you describe should appear in *addition* to this test, not replacing it, because as described above this test does test one critical bit of behavior, namely prefering pushurl over the pushInsteadOf rewrite of url. - Josh Triplett -- 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] push: Alias pushurl from push rewrites
Josh Triplett writes: > OK, I take it back. I *can* imagine configurations that this change > would break, since it does change intentional and documented behavior, > but I don't have any such configuration. The only such configuration I > can imagine involves directly counting on the non-rewriting of pushUrl, > by using pushInsteadOf to rewrite urls and then sometimes using pushUrl > to override that and point back at the un-rewritten URL. And while > supported, that does seem *odd*. > > Objection withdrawn; if nobody can come up with a sensible configuration > that relies on the documented behavior, I don't particularly care if it > changes. I actually do. Given the popularity of the system, "people involved in this thread cannot imagine a case that existing people may get hurt" is very different from "this is not a regression". After merging this change when people start complaining, you and Rob can hide and ignore them, but we collectively as the Git project have to have a way to help them when it happens. -- 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: which files will have conflicts between two branches?
On Wednesday, March 27, 2013 at 17:48 EDT, "J.V." wrote: > I have two local branches (tracked to remote) that are in sync (did > a git pull on both branches from their corresponding remote). > > Is this the best way to merge? > > I would be merging local/branch1 => local/branch2 (test this branch) > and then push local/branch2 => origin/branch1 (and would expect no > merge conflicts if anyone has not checked in anything. Except for maybe unusual corner cases I don't see how the merge order (branch1 into branch2 or vice versa) makes any difference. If nothing has happened with origin/branch1 since you merged from it to your local branches the push will succeed. If there have been upstream commits you'll have to update your local branch first (which might result in conflicts) before you'll be able to push. > Also with two local branches, Is there a way to get a list of files > (one line per file) of files that would have merge conflicts that > would need to be resolved? You'd have to perform an actual merge for that, perhaps with --no-commit to avoid creating the actual commit object. Inspect the "git status" output to find the files with conflicts. In a script, use "git ls-files -u" instead. -- Magnus Bäck ba...@google.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: [PATCH] http-backend: respect GIT_NAMESPACE with dumb clients
On Wed, Mar 27, 2013 at 11:02:28PM -0700, Junio C Hamano wrote:
> John Koleszar writes:
>
> > Filter the list of refs returned via the dumb HTTP protocol according
> > to the active namespace, consistent with other clients of the
> > upload-pack service.
> >
> > Signed-off-by: John Koleszar
> > ---
>
> Looks sane from a cursory read---thanks.
>
> Josh, any comments?
Looks reasonable; glad to see tests explicitly covering it, as well.
Ideally I'd like to see an additional test against that same namespaced
repository, fetching from a different URL that doesn't set
GIT_NAMESPACE, and verifying that info/refs contains the full set of
namespace-qualified refs from all namespaces. That would make sure that
particular behavior doesn't regress in the future, since one of the use
cases for namespaced repositories involves fetching the whole combined
repo, for purposes such as backups or replication.
> > http-backend.c | 8 +---
> > t/lib-httpd/apache.conf | 5 +
> > t/t5561-http-backend.sh | 4
> > t/t556x_common | 16
> > 4 files changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/http-backend.c b/http-backend.c
> > index f50e77f..b9896b0 100644
> > --- a/http-backend.c
> > +++ b/http-backend.c
> > @@ -361,17 +361,19 @@ static void run_service(const char **argv)
> > static int show_text_ref(const char *name, const unsigned char *sha1,
> > int flag, void *cb_data)
> > {
> > + const char *name_nons = strip_namespace(name);
> > struct strbuf *buf = cb_data;
> > struct object *o = parse_object(sha1);
> > if (!o)
> > return 0;
> >
> > - strbuf_addf(buf, "%s\t%s\n", sha1_to_hex(sha1), name);
> > + strbuf_addf(buf, "%s\t%s\n", sha1_to_hex(sha1), name_nons);
> > if (o->type == OBJ_TAG) {
> > o = deref_tag(o, name, 0);
> > if (!o)
> > return 0;
> > - strbuf_addf(buf, "%s\t%s^{}\n", sha1_to_hex(o->sha1), name);
> > + strbuf_addf(buf, "%s\t%s^{}\n", sha1_to_hex(o->sha1),
> > + name_nons);
> > }
> > return 0;
> > }
> > @@ -402,7 +404,7 @@ static void get_info_refs(char *arg)
> >
> > } else {
> > select_getanyfile();
> > - for_each_ref(show_text_ref, &buf);
> > + for_each_namespaced_ref(show_text_ref, &buf);
> > send_strbuf("text/plain", &buf);
> > }
> > strbuf_release(&buf);
> > diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> > index 938b4cf..ad85537 100644
> > --- a/t/lib-httpd/apache.conf
> > +++ b/t/lib-httpd/apache.conf
> > @@ -61,6 +61,11 @@ Alias /auth/dumb/ www/auth/dumb/
> > SetEnv GIT_COMMITTER_NAME "Custom User"
> > SetEnv GIT_COMMITTER_EMAIL cus...@example.com
> >
> > +
> > + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
> > + SetEnv GIT_HTTP_EXPORT_ALL
> > + SetEnv GIT_NAMESPACE ns
> > +
> > ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1
> > ScriptAlias /broken_smart/ broken-smart-http.sh/
> >
> > diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh
> > index b5d7fbc..5a19d61 100755
> > --- a/t/t5561-http-backend.sh
> > +++ b/t/t5561-http-backend.sh
> > @@ -134,6 +134,10 @@ POST /smart/repo.git/git-receive-pack HTTP/1.1 200 -
> > ###
> > GET /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 403 -
> > POST /smart/repo.git/git-receive-pack HTTP/1.1 403 -
> > +
> > +### namespace test
> > +###
> > +GET /smart_namespace/repo.git/info/refs HTTP/1.1 200
> > EOF
> > test_expect_success 'server request log matches test results' '
> > sed -e "
> > diff --git a/t/t556x_common b/t/t556x_common
> > index 82926cf..cb9eb9d 100755
> > --- a/t/t556x_common
> > +++ b/t/t556x_common
> > @@ -120,3 +120,19 @@ test_expect_success 'http.receivepack false' '
> > GET info/refs?service=git-receive-pack "403 Forbidden" &&
> > POST git-receive-pack "403 Forbidden"
> > '
> > +test_expect_success 'backend respects namespaces' '
> > + log_div "namespace test"
> > + config http.uploadpack true &&
> > + config http.getanyfile true &&
> > +
> > + GIT_NAMESPACE=ns && export GIT_NAMESPACE &&
> > + git push public master:master &&
> > + (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> > + git for-each-ref | grep /$GIT_NAMESPACE/ >/dev/null
> > + ) &&
> > +
> > + git ls-remote public >exp &&
> > + curl "$HTTPD_URL/smart_namespace/repo.git/info/refs" >act &&
> > + test_cmp exp act &&
> > + (grep /ns/ exp && false || true)
> > +'
--
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: more git weirdness (git rebase, merge conflicts
On Wednesday, March 27, 2013 at 14:51 EDT, "J.V." wrote: > I have a local/development branch tracked to origin/development. I > made no changes to local/dev and did a git pull with rebase, I did > not expect any conflicts. > > I got a conflict and was thrown onto another branch. I attempted a > merge (using IntelliJ) accepting everything from the server but a > variable definition was missing for some odd reason and the merge > was not successful (merge was resolved but the file would not > compile) so I decided to simply go back to my dev branch and figure > out how to do a git pull -f (force overwrite of all local files so > that I could get my local/dev back into sync with origin/dev. > > On my screwed up branch that I was thrust onto: > I typed: > $git rebase --skip<= I was stuck in a rebase (rebase > failed, was thrown onto a tmp branch and thought this would get me > out of there) The --skip option makes Git skip one commit in the rebase. To bail out completely and restore the original state, use --abort. > Now I have been sitting here for an hour watching "Applying: > backwards into the past one by one. What the hell is happening? > > Applying: add log information > Applying: and it goes on and on and on. You initated a rebase with an incorrect base, so Git is rebasing all past history onto a new base. Say the upstream repository looks like this, with uppercase letters denoting commits found on the upstream and lowercase letters being your local commits (if any): --C / AB--- \ DE \ --a--b In this example, you've made two commits since you started working on commit D. The expected result for a rebase is for Git to rebase the two commits you've made since D (a, b) and put them on top of E, the new baseline fetched from the upstream. However, because of how you invoke the rebase, Git actually thinks that C is the base of your local branch and computes all the commits you've made since C and finds all commits between B and b, which it now tries to rebase on top of C. > All I want to do at this point is to get back to my dev branch and > force pull from origin/dev while keeping all local files that have > not been added to my local repo. > > How do I stop this madness and get back to local dev and force pull > from origin/dev <= which is our master. Start with "git rebase --abort" to get out of the rebase loop. To delete all your local uncommitted changes made to tracked files while keeping local/untracked files, run "git reset --hard". If you haven't made any local commits a correctly invoked rebase (or pull) will be a trivial fast-forward. -- Magnus Bäck ba...@google.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 subtree oddity
> > I am starting to regret that I caved in and started carrying a copy > of it in contrib/. It probably is a good idea to drop it from my > tree and let it mature and eventually flourish outside. > that's a shame... it solves a real problem, is simple to use, and really powerfull. but unfortunately, I have sent a patch that solves a serious bug... which had already been reported and patched but had received no answer, and nobody replied to it. Is there anything that can be done to get this rolling, or a way to have the use-case it covers better handle by git-submodule ? currently the problem of a git repo in a git repo is very complicated to deal with in a clean way... Regards Jérémy -- 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: Bug in "git rev-parse --verify"
On Thu, Mar 28, 2013 at 04:52:15PM +0100, Michael Haggerty wrote: > On 03/28/2013 04:38 PM, Jeff King wrote: > > On Thu, Mar 28, 2013 at 04:34:19PM +0100, Michael Haggerty wrote: > > > >> Is there a simple way to verify an object name more strictly and convert > >> it to an SHA1? I can only think of solutions that require two commands, > >> like > >> > >> git cat-file -e $ARG && git rev-parse --verify $ARG > > > > Is the rev-parse line doing anything there? If $ARG does not resolve to > > a sha1, then wouldn't cat-file have failed? > > It's outputting the SHA1, which cat-file seems incapable of providing in > a useful way. Ah, I see; I was looking too much at your example, and not thinking about how you would want to use it in a script. -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] push: Alias pushurl from push rewrites
On Thu, Mar 28, 2013 at 09:10:59AM -0700, Junio C Hamano wrote: > Josh Triplett writes: > > > OK, I take it back. I *can* imagine configurations that this change > > would break, since it does change intentional and documented behavior, > > but I don't have any such configuration. The only such configuration I > > can imagine involves directly counting on the non-rewriting of pushUrl, > > by using pushInsteadOf to rewrite urls and then sometimes using pushUrl > > to override that and point back at the un-rewritten URL. And while > > supported, that does seem *odd*. > > > > Objection withdrawn; if nobody can come up with a sensible configuration > > that relies on the documented behavior, I don't particularly care if it > > changes. > > I actually do. > > Given the popularity of the system, "people involved in this thread > cannot imagine a case that existing people may get hurt" is very > different from "this is not a regression". After merging this > change when people start complaining, you and Rob can hide and > ignore them, but we collectively as the Git project have to have a > way to help them when it happens. I entirely agree that it represents a regression from documented behavior; I just mean that it no longer matches a specific use case I had in mind with the original change. I agree that we should hesitate to change that documented behavior. - Josh Triplett -- 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 and GSoC 2013
Christian Couder writes: > On Wed, Mar 27, 2013 at 7:52 PM, Jonathan Nieder wrote: >> Jeff King wrote: >> >>> There was a big thread about a month ago on whether Git should do Google >>> Summer of Code this year[1]. > > I think we should do it. > > It looks strange to me to say that students are great and at the same > time that we should not do it. > > Let's give them and us one more chance do to well. This is the only > way we can improve. Do you mean we should be doing the same thing over and over again and expecting different results? Einstein may not like it, and I certainly don't. What I gathered from the discussion so far is that everybody agrees that our mentoring has been suboptimal in various ways (not enough encouragement to engage with the community early, working in the cave for too long, biting too much to chew etc.). What makes you think we would do better this year? "We have a track record of being not great at mentoring, and we haven't made an effort to improve it." is a perfectly valid and humble reason to excuse ourselves from this year's GSoC. "Students are great" is immaterial. In fact, if they are great, I think it is better to give them a chance to excel by working with organizations that can mentor them better, instead of squandering their time and GSoC's money for another failure, until _we_ are ready to take great students. It is preferrable if the decision were accompanied with a concrete plan for us to prepare our mentoring capability better (if we want to participate in future GSoC, that is), but I think it is a separate issue, and I suspect that it is too late for this year's. -- 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] http-backend: respect GIT_NAMESPACE with dumb clients
John Koleszar writes: >> Facepalm. The intent here is to invert the grep, to make sure that the /ns/ >> does not appear in the output. No idea why I wrote it that way. Will fix. OK, "! grep /ns/ exp" would do. Thanks. -- 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 v10 0/5] git log -L
From: Thomas Rast This adds a bunch of fixes and failing tests for invalid -L arguments; as Antoine discovered, some variations would segfault v9. I also changed the beginning of parse_range_funcname (in patch 4/5), which now also lets you backslash-escape a : in a funcname regex. The old version was based on the assumption that there could only be a ':' in the string if we were coming from scan_range_arg, which made it a bit hard to read. Bo Yang (2): Refactor parse_loc Export rewrite_parents() for 'log -L' Thomas Rast (3): Implement line-history search (git log -L) log -L: :pattern:file syntax to find by funcname Speed up log -L... -M Documentation/blame-options.txt | 21 +- Documentation/git-blame.txt |6 +- Documentation/git-log.txt | 23 + Documentation/line-range-format.txt | 25 + Makefile|4 + builtin/blame.c | 99 +-- builtin/log.c | 31 + line-log.c | 1228 +++ line-log.h | 49 ++ line-range.c| 243 +++ line-range.h| 36 + log-tree.c |4 + revision.c | 22 +- revision.h | 16 +- t/perf/p4211-line-log.sh| 34 + t/t4211-line-log.sh | 53 ++ t/t4211/expect.beginning-of-file| 43 ++ t/t4211/expect.end-of-file | 62 ++ t/t4211/expect.move-support-f | 40 ++ t/t4211/expect.simple-f | 59 ++ t/t4211/expect.simple-f-to-main | 100 +++ t/t4211/expect.simple-main | 68 ++ t/t4211/expect.simple-main-to-end | 70 ++ t/t4211/expect.two-ranges | 102 +++ t/t4211/expect.vanishes-early | 39 ++ t/t4211/history.export | 330 ++ t/t8003-blame-corner-cases.sh |6 + 27 files changed, 2690 insertions(+), 123 deletions(-) create mode 100644 Documentation/line-range-format.txt create mode 100644 line-log.c create mode 100644 line-log.h create mode 100644 line-range.c create mode 100644 line-range.h create mode 100755 t/perf/p4211-line-log.sh create mode 100755 t/t4211-line-log.sh create mode 100644 t/t4211/expect.beginning-of-file create mode 100644 t/t4211/expect.end-of-file create mode 100644 t/t4211/expect.move-support-f create mode 100644 t/t4211/expect.simple-f create mode 100644 t/t4211/expect.simple-f-to-main create mode 100644 t/t4211/expect.simple-main create mode 100644 t/t4211/expect.simple-main-to-end create mode 100644 t/t4211/expect.two-ranges create mode 100644 t/t4211/expect.vanishes-early create mode 100644 t/t4211/history.export -- 1.8.2.446.g2b4de83 -- 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 v10 1/5] Refactor parse_loc
From: Bo Yang
We want to use the same style of -L n,m argument for 'git log -L' as
for git-blame. Refactor the argument parsing of the range arguments
from builtin/blame.c to the (new) file that will hold the 'git log -L'
logic.
To accommodate different data structures in blame and log -L, the file
contents are abstracted away; parse_range_arg takes a callback that it
uses to get the contents of a line of the (notional) file.
The new test is for a case that made me pause during debugging: the
'blame -L with invalid end' test was the only one that noticed an
outright failure to parse the end *at all*. So make a more explicit
test for that.
Signed-off-by: Bo Yang
Signed-off-by: Thomas Rast
---
Documentation/blame-options.txt | 19 +--
Documentation/line-range-format.txt | 18 +++
Makefile| 2 +
builtin/blame.c | 99 +++--
line-range.c| 92 ++
line-range.h| 24 +
t/t8003-blame-corner-cases.sh | 6 +++
7 files changed, 151 insertions(+), 109 deletions(-)
create mode 100644 Documentation/line-range-format.txt
create mode 100644 line-range.c
create mode 100644 line-range.h
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index b0d31df..6998d9f 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -13,24 +13,7 @@
Annotate only the given line range. and can take
one of these forms:
- - number
-+
-If or is a number, it specifies an
-absolute line number (lines count from 1).
-+
-
-- /regex/
-+
-This form will use the first line matching the given
-POSIX regex. If is a regex, it will search
-starting at the line given by .
-+
-
-- +offset or -offset
-+
-This is only valid for and will specify a number
-of lines before or after the line given by .
-+
+include::line-range-format.txt[]
-l::
Show long rev (Default: off).
diff --git a/Documentation/line-range-format.txt
b/Documentation/line-range-format.txt
new file mode 100644
index 000..265bc23
--- /dev/null
+++ b/Documentation/line-range-format.txt
@@ -0,0 +1,18 @@
+- number
++
+If or is a number, it specifies an
+absolute line number (lines count from 1).
++
+
+- /regex/
++
+This form will use the first line matching the given
+POSIX regex. If is a regex, it will search
+starting at the line given by .
++
+
+- +offset or -offset
++
+This is only valid for and will specify a number
+of lines before or after the line given by .
++
diff --git a/Makefile b/Makefile
index 598d631..e83f64b 100644
--- a/Makefile
+++ b/Makefile
@@ -667,6 +667,7 @@ LIB_H += help.h
LIB_H += http.h
LIB_H += kwset.h
LIB_H += levenshtein.h
+LIB_H += line-range.h
LIB_H += list-objects.h
LIB_H += ll-merge.h
LIB_H += log-tree.h
@@ -795,6 +796,7 @@ LIB_OBJS += hex.o
LIB_OBJS += ident.o
LIB_OBJS += kwset.o
LIB_OBJS += levenshtein.o
+LIB_OBJS += line-range.o
LIB_OBJS += list-objects.o
LIB_OBJS += ll-merge.o
LIB_OBJS += lockfile.o
diff --git a/builtin/blame.c b/builtin/blame.c
index 86100e9..20eb439 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -21,6 +21,7 @@
#include "parse-options.h"
#include "utf8.h"
#include "userdiff.h"
+#include "line-range.h"
static char blame_usage[] = N_("git blame [options] [rev-opts] [rev] [--]
file");
@@ -566,11 +567,16 @@ static void dup_entry(struct blame_entry *dst, struct
blame_entry *src)
dst->score = 0;
}
-static const char *nth_line(struct scoreboard *sb, int lno)
+static const char *nth_line(struct scoreboard *sb, long lno)
{
return sb->final_buf + sb->lineno[lno];
}
+static const char *nth_line_cb(void *data, long lno)
+{
+ return nth_line((struct scoreboard *)data, lno);
+}
+
/*
* It is known that lines between tlno to same came from parent, and e
* has an overlap with that range. it also is known that parent's
@@ -1927,83 +1933,6 @@ static const char *add_prefix(const char *prefix, const
char *path)
}
/*
- * Parsing of (comma separated) one item in the -L option
- */
-static const char *parse_loc(const char *spec,
-struct scoreboard *sb, long lno,
-long begin, long *ret)
-{
- char *term;
- const char *line;
- long num;
- int reg_error;
- regex_t regexp;
- regmatch_t match[1];
-
- /* Allow "-L ,+20" to mean starting at
-* for 20 lines, or "-L ,-5" for 5 lines ending at
-* .
-*/
- if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
- num = strtol(spec + 1, &term, 10);
- if (term != spec + 1) {
- if (spec[0] == '-')
- num = 0 - num;
- if (0 < num)
- *ret = begin + num - 2;
- else if (!num)
-
[PATCH v10 2/5] Export rewrite_parents() for 'log -L'
From: Bo Yang
The function rewrite_one is used to rewrite a single
parent of the current commit, and is used by rewrite_parents
to rewrite all the parents.
Decouple the dependence between them by making rewrite_one
a callback function that is passed to rewrite_parents. Then
export rewrite_parents for reuse by the line history browser.
We will use this function in line-log.c.
Signed-off-by: Bo Yang
Signed-off-by: Thomas Rast
---
revision.c | 13 -
revision.h | 10 ++
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/revision.c b/revision.c
index ef60205..46319d5 100644
--- a/revision.c
+++ b/revision.c
@@ -2173,12 +2173,6 @@ int prepare_revision_walk(struct rev_info *revs)
return 0;
}
-enum rewrite_result {
- rewrite_one_ok,
- rewrite_one_noparents,
- rewrite_one_error
-};
-
static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit
**pp)
{
struct commit_list *cache = NULL;
@@ -2200,12 +2194,13 @@ static enum rewrite_result rewrite_one(struct rev_info
*revs, struct commit **pp
}
}
-static int rewrite_parents(struct rev_info *revs, struct commit *commit)
+int rewrite_parents(struct rev_info *revs, struct commit *commit,
+ rewrite_parent_fn_t rewrite_parent)
{
struct commit_list **pp = &commit->parents;
while (*pp) {
struct commit_list *parent = *pp;
- switch (rewrite_one(revs, &parent->item)) {
+ switch (rewrite_parent(revs, &parent->item)) {
case rewrite_one_ok:
break;
case rewrite_one_noparents:
@@ -2371,7 +2366,7 @@ enum commit_action simplify_commit(struct rev_info *revs,
struct commit *commit)
if (action == commit_show &&
!revs->show_all &&
revs->prune && revs->dense && want_ancestry(revs)) {
- if (rewrite_parents(revs, commit) < 0)
+ if (rewrite_parents(revs, commit, rewrite_one) < 0)
return commit_error;
}
return action;
diff --git a/revision.h b/revision.h
index 5da09ee..640110d 100644
--- a/revision.h
+++ b/revision.h
@@ -241,4 +241,14 @@ enum commit_action {
extern enum commit_action get_commit_action(struct rev_info *revs, struct
commit *commit);
extern enum commit_action simplify_commit(struct rev_info *revs, struct commit
*commit);
+enum rewrite_result {
+ rewrite_one_ok,
+ rewrite_one_noparents,
+ rewrite_one_error
+};
+
+typedef enum rewrite_result (*rewrite_parent_fn_t)(struct rev_info *revs,
struct commit **pp);
+
+extern int rewrite_parents(struct rev_info *revs, struct commit *commit,
+ rewrite_parent_fn_t rewrite_parent);
#endif
--
1.8.2.446.g2b4de83
--
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 v10 4/5] log -L: :pattern:file syntax to find by funcname
This new syntax finds a funcname matching /pattern/, and then takes from there
up to (but not including) the next funcname. So you can say
git log -L:main:main.c
and it will dig up the main() function and show its line-log, provided
there are no other funcnames matching 'main'.
Signed-off-by: Thomas Rast
---
Documentation/blame-options.txt | 2 +-
Documentation/git-blame.txt | 6 +-
Documentation/git-log.txt | 11 +--
Documentation/line-range-format.txt | 7 ++
builtin/blame.c | 2 +-
line-log.c | 5 +-
line-range.c| 136 +++-
line-range.h| 3 +-
t/t4211-line-log.sh | 4 ++
t/t4211/expect.simple-f-to-main | 100 ++
t/t4211/expect.simple-main-to-end | 70 +++
11 files changed, 332 insertions(+), 14 deletions(-)
create mode 100644 t/t4211/expect.simple-f-to-main
create mode 100644 t/t4211/expect.simple-main-to-end
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 6998d9f..e9f984b 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -9,7 +9,7 @@
--show-stats::
Include additional statistics at the end of blame output.
--L ,::
+-L ,, -L :::
Annotate only the given line range. and can take
one of these forms:
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 9a05c2b..6cea7f1 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -8,9 +8,9 @@ git-blame - Show what revision and author last modified each
line of a file
SYNOPSIS
[verse]
-'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental] [-L n,m]
- [-S ] [-M] [-C] [-C] [-C] [--since=] [--abbrev=]
- [ | --contents | --reverse ] [--]
+'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
+ [-L n,m | -L :fn] [-S ] [-M] [-C] [-C] [-C]
[--since=]
+ [--abbrev=] [ | --contents | --reverse ] [--]
DESCRIPTION
---
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 8727c60..4850226 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -69,12 +69,13 @@ produced by --stat etc.
Note that only message is considered, if also a diff is shown
its size is not included.
--L ,:::
+-L ,:, -L
+
Trace the evolution of the line range given by ","
- within the . You may not give any pathspec limiters.
- This is currently limited to a walk starting from a single
- revision, i.e., you may only give zero or one positive
- revision arguments.
+ (or the funcname regex ) within the . You may
+ not give any pathspec limiters. This is currently limited to
+ a walk starting from a single revision, i.e., you may only
+ give zero or one positive revision arguments.
and can take one of these forms:
diff --git a/Documentation/line-range-format.txt
b/Documentation/line-range-format.txt
index 265bc23..3e7ce72 100644
--- a/Documentation/line-range-format.txt
+++ b/Documentation/line-range-format.txt
@@ -16,3 +16,10 @@ starting at the line given by .
This is only valid for and will specify a number
of lines before or after the line given by .
+
+
+- :regex
++
+If the option's argument is of the form :regex, it denotes the range
+from the first funcname line that matches , up to the next
+funcname line.
++
diff --git a/builtin/blame.c b/builtin/blame.c
index 20eb439..1c09d55 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1940,7 +1940,7 @@ static void prepare_blame_range(struct scoreboard *sb,
long lno,
long *bottom, long *top)
{
- if (parse_range_arg(bottomtop, nth_line_cb, sb, lno, bottom, top))
+ if (parse_range_arg(bottomtop, nth_line_cb, sb, lno, bottom, top,
sb->path))
usage(blame_usage);
}
diff --git a/line-log.c b/line-log.c
index 6244231..68972e2 100644
--- a/line-log.c
+++ b/line-log.c
@@ -12,6 +12,7 @@
#include "strbuf.h"
#include "log-tree.h"
#include "graph.h"
+#include "userdiff.h"
#include "line-log.h"
static void range_set_grow(struct range_set *rs, size_t extra)
@@ -438,7 +439,6 @@ static void range_set_map_across_diff(struct range_set *out,
*touched_out = touched;
}
-
static struct commit *check_single_commit(struct rev_info *revs)
{
struct object *commit = NULL;
@@ -559,7 +559,8 @@ static const char *nth_line(void *data, long line)
cb_data.line_ends = ends;
if (parse_range_arg(range_part, nth_line, &cb_data,
- lines, &begin, &end))
+ lines, &begin, &end,
+ spec->path))
[PATCH v10 5/5] Speed up log -L... -M
So far log -L only used the implicit diff filtering by pathspec. If
the user specifies -M, we cannot do that, and so we simply handed the
whole diff queue (which is approximately 'git show --raw') to
diffcore_std().
Unfortunately this is very slow. We can optimize a lot if we throw
out files that we know cannot possibly be interesting, in the same
spirit that the pathspec filtering reduces the number of files.
However, in this case, we have to be more careful. Because we want to
look out for renames, we need to keep all filepairs where something
was deleted.
This is a bit hacky and should really be replaced by equivalent
support in --follow, and just using that. However, in the meantime it
speeds up 'log -M -L' by an order of magnitude.
Signed-off-by: Thomas Rast
---
line-log.c | 56
1 file changed, 52 insertions(+), 4 deletions(-)
diff --git a/line-log.c b/line-log.c
index 68972e2..30edef4 100644
--- a/line-log.c
+++ b/line-log.c
@@ -750,7 +750,50 @@ static void move_diff_queue(struct diff_queue_struct *dst,
DIFF_QUEUE_CLEAR(src);
}
-static void queue_diffs(struct diff_options *opt,
+static void filter_diffs_for_paths(struct line_log_data *range, int
keep_deletions)
+{
+ int i;
+ struct diff_queue_struct outq;
+ DIFF_QUEUE_CLEAR(&outq);
+
+ for (i = 0; i < diff_queued_diff.nr; i++) {
+ struct diff_filepair *p = diff_queued_diff.queue[i];
+ struct line_log_data *rg = NULL;
+
+ if (!DIFF_FILE_VALID(p->two)) {
+ if (keep_deletions)
+ diff_q(&outq, p);
+ else
+ diff_free_filepair(p);
+ continue;
+ }
+ for (rg = range; rg; rg = rg->next) {
+ if (!strcmp(rg->spec->path, p->two->path))
+ break;
+ }
+ if (rg)
+ diff_q(&outq, p);
+ else
+ diff_free_filepair(p);
+ }
+ free(diff_queued_diff.queue);
+ diff_queued_diff = outq;
+}
+
+static inline int diff_might_be_rename(void)
+{
+ int i;
+ for (i = 0; i < diff_queued_diff.nr; i++)
+ if (!DIFF_FILE_VALID(diff_queued_diff.queue[i]->one)) {
+ /* fprintf(stderr, "diff_might_be_rename found creation
of: %s\n", */
+ /* diff_queued_diff.queue[i]->two->path); */
+ return 1;
+ }
+ return 0;
+}
+
+static void queue_diffs(struct line_log_data *range,
+ struct diff_options *opt,
struct diff_queue_struct *queue,
struct commit *commit, struct commit *parent)
{
@@ -766,7 +809,12 @@ static void queue_diffs(struct diff_options *opt,
DIFF_QUEUE_CLEAR(&diff_queued_diff);
diff_tree(&desc1, &desc2, "", opt);
- diffcore_std(opt);
+ if (opt->detect_rename) {
+ filter_diffs_for_paths(range, 1);
+ if (diff_might_be_rename())
+ diffcore_std(opt);
+ filter_diffs_for_paths(range, 0);
+ }
move_diff_queue(queue, &diff_queued_diff);
if (tree1)
@@ -1050,7 +1098,7 @@ static int process_ranges_ordinary_commit(struct rev_info
*rev, struct commit *c
if (commit->parents)
parent = commit->parents->item;
- queue_diffs(&rev->diffopt, &queue, commit, parent);
+ queue_diffs(range, &rev->diffopt, &queue, commit, parent);
changed = process_all_files(&parent_range, rev, &queue, range);
if (parent)
add_line_range(rev, parent, parent_range);
@@ -1075,7 +1123,7 @@ static int process_ranges_merge_commit(struct rev_info
*rev, struct commit *comm
for (i = 0; i < nparents; i++) {
parents[i] = p->item;
p = p->next;
- queue_diffs(&rev->diffopt, &diffqueues[i], commit, parents[i]);
+ queue_diffs(range, &rev->diffopt, &diffqueues[i], commit,
parents[i]);
}
for (i = 0; i < nparents; i++) {
--
1.8.2.446.g2b4de83
--
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: Bug in "git rev-parse --verify"
Jeff King writes:
> On Thu, Mar 28, 2013 at 04:34:19PM +0100, Michael Haggerty wrote:
>
>> Is there a simple way to verify an object name more strictly and convert
>> it to an SHA1? I can only think of solutions that require two commands,
>> like
>>
>> git cat-file -e $ARG && git rev-parse --verify $ARG
>
> Is the rev-parse line doing anything there? If $ARG does not resolve to
> a sha1, then wouldn't cat-file have failed?
>
> -Peff
You could force rev-parse to resolve the input to an existing
object, with something like this:
git rev-parse --verify "$ARG^{}"
It will unwrap a tag, so the output may end up pointing at a object
that is different from $ARG in such a case.
But what is the purpose of turning a random string to a random
40-hex in the first place?
--
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 v4 0/6] Support triangular workflows
Jeff King writes: > Sometimes it's annoyingly verbose to break down a compound function. But > I think in this case, you can make your tests more robust by just > checking the affirmative that the ref is still where we expect it to be, > like: > > check_push_result up_repo $the_first_commit heads/master > > Sorry if that was a bit long-winded. I think that practically speaking, > it is not a likely source of problems in this case. But it's an > anti-pattern in our tests that I think is worth mentioning. Thanks. That is one of the reasons why we do not want to see too many custom test helper functions. -- 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 and GSoC 2013
On Thu, Mar 28, 2013 at 09:45:02AM -0700, Junio C Hamano wrote: > It is preferrable if the decision were accompanied with a concrete > plan for us to prepare our mentoring capability better (if we want > to participate in future GSoC, that is), but I think it is a > separate issue, and I suspect that it is too late for this year's. I agree with this. I do think we should participate again, and I think we should try to address the issues I brought up (and you mentioned in your email) about project size and community involvement. My email was meant to be a "maybe it's not too late if people really want to work on the project ideas page". But I haven't seen anything happening there, and we really are running right up to the deadline[1]. I think at this point it makes sense to wait a year and try to approach it sooner next year. -Peff [1] I know my email didn't give much time for action; the deadline snuck up on me, 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: Composing git repositories
Ramkumar Ramachandra wrote: > Do you realize how difficult this is to implement? We'll need to > patch all the git commands to essentially do what we'd get for free if > the submodule were a tree object instead of a commit object (although > I'm not saying that's the Right thing to do). What are you talking about? Yes, of course I realize that recursing over subprojects that are managed as separate git repositories requires writing new code. That's why people have started to write such code. They seem to think it's worth it. Meanwhile others with different designs in mind have written other tools. Use cases even overlap a little, so they can compete. That is exactly as it should be. -- 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: More detailed error message for 403 forbidden.
On Wed, Mar 27, 2013 at 12:29:57PM +0900, Yi, EungJun wrote: > Currently, if user tried to access a git repository via HTTP and it > fails because the user's permission is not enough to access the > repository, git client tells that http request failed and the error > was 403 forbidden. The situations in which you'll get a 403 depend on how the server is configured. For instance, on github.com, if you successfully authenticate but are not authorized to access a repository, you get a 404 (we do this to avoid leaking information about which private repositories exist). But we do provide a 403 if you try to access the repository with a non-smart-http client. So the "403 forbidden" there is not about your account, but about the method; if git is going to give a more verbose message, it needs to be careful not to mislead the user. > It would be much better if git client shows response body which might > include an explanation of the failure. For example, > [...] > $ git clone http://localhost/foo/bar > error: The requested URL returned error: 403 while accessing > http://localhost/foo/bar > remote: User 'me' does not have enough permission to access the repository. > fatal: HTTP request failed I agree that is the best way forward, as that means the server is telling us what is going on, and we are not guessing about the meaning of the 403. One problem is that the content body sent along with the error is not necessarily appropriate for showing to the user (e.g., if it is HTML, it is probably not a good idea to show it on the terminal). So I think we would want to only show it when the server has indicated via the content-type that the message is meant to be shown to the user. I'm thinking the server would generate something like: HTTP/1.1 403 Forbidden Content-type: application/x-git-error-message User 'me' does not have enough permission to access the repository. which would produce the example you showed above. -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: More detailed error message for 403 forbidden.
Jeff King wrote: > One problem is that the content body sent along with the error is not > necessarily appropriate for showing to the user (e.g., if it is HTML, it > is probably not a good idea to show it on the terminal). So I think we > would want to only show it when the server has indicated via the > content-type that the message is meant to be shown to the user. I'm > thinking the server would generate something like: > >HTTP/1.1 403 Forbidden >Content-type: application/x-git-error-message > >User 'me' does not have enough permission to access the repository. > > which would produce the example you showed above. Would it make sense to use text/plain this way? Curious, 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: More detailed error message for 403 forbidden.
On Thu, Mar 28, 2013 at 11:41:20AM -0700, Jonathan Nieder wrote: > Jeff King wrote: > > > One problem is that the content body sent along with the error is not > > necessarily appropriate for showing to the user (e.g., if it is HTML, it > > is probably not a good idea to show it on the terminal). So I think we > > would want to only show it when the server has indicated via the > > content-type that the message is meant to be shown to the user. I'm > > thinking the server would generate something like: > > > >HTTP/1.1 403 Forbidden > >Content-type: application/x-git-error-message > > > >User 'me' does not have enough permission to access the repository. > > > > which would produce the example you showed above. > > Would it make sense to use text/plain this way? Maybe. But I would worry somewhat about sites which provide a useless and verbose text/plain message. Ideally an x-git-error-message would be no more than few lines, suitable for the error message of a terminal program. I would not want a site-branded "Your page cannot be found. Here's a complete navigation bar" page to be spewed to the terminal. Those tend to be text/html, though, so we may be safe. It's just that we're gambling on what random servers do, and if we show useless spew even some of the time, that would be a regression. -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] push: Alias pushurl from push rewrites
Josh Triplett writes: (on url.$base.pushInsteadOf) >> If a remote has an explicit pushurl, git will ignore this setting for >> that remote. > That really meant what I just said above: git will prefer an explicit > pushurl over the pushInsteadOf rewrite of url. Very correct. > It says nothing about > applying pushInsteadOf to rewrite pushurl. Incorrect, I think. If you have pushURL, pushInsteadOf is *ignored*. Of course, if you have both URL and pushURL, the ignored pushInsteadOf will not apply to _anything_. It will not apply to URL, and it will certainly not apply to pushURL. You are correct to point out that with the test we would want to make sure that for a remote with pushURL and URL, a push goes - to pushURL; - not to URL; - not to insteadOf(URL); - not to pushInsteadOf(URL); - not to insteadOf(pushURL); and - not to pushInsteadOf(pushURL). I do not think it is worth checking all of them, but I agree we should make sure it does not go to pushInsteadOf(URL) which you originally meant to check, and we should also make sure it does not go to pushInsteadOf(pushURL). >> test_expect_success 'push with pushInsteadOf and explicit pushurl >> (pushInsteadOf should not rewrite)' ' >> mk_empty && >> -TRASH="$(pwd)/" && >> -git config "url.trash2/.pushInsteadOf" trash/ && >> +git config "url.trash2/.pushInsteadOf" testrepo/ && Adding git config "url.trash3/.pusnInsteadOf" trash/wrong && here should be sufficient for that, no? If we mistakenly used URL (i.e. trash/wrong) the push would fail. If we mistakenly used pushInsteadOf(URL), that is rewritten to trash3/ and again the push would fail. pushInsteadOf(pushURL) would go to trash2/ and that would also fail. We aren't checking insteadOf(URL) and insteadOf(pushURL) but everything else is checked, I think, so we can do without replacing anything. We can just extend it, no? >> git config remote.r.url trash/wrong && >> -git config remote.r.pushurl "$TRASH/testrepo" && >> +git config remote.r.pushurl "testrepo/" && >> git push r refs/heads/master:refs/remotes/origin/master && >> ( >> cd testrepo && > > ...the test you describe should appear in *addition* to this test, not > replacing it, because as described above this test does test one > critical bit of behavior, namely prefering pushurl over the > pushInsteadOf rewrite of url. > > - Josh Triplett -- 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 1/2] fast-import: Fix an gcc -Wuninitialized warning
Jeff King wrote:
> On Tue, Mar 26, 2013 at 07:09:44PM +, Ramsay Jones wrote:
>
>> Commit cbfd5e1c ("drop some obsolete "x = x" compiler warning hacks",
>> 21-03-2013) removed a gcc hack that suppressed an "might be used
>> uninitialized" warning issued by older versions of gcc.
>>
>> However, commit 3aa99df8 ('fast-import: clarify "inline" logic in
>> file_change_m', 21-03-2013) addresses an (almost) identical issue
>> (with very similar code), but includes additional code in it's
>> resolution. The solution used by this commit, unlike that used by
>> commit cbfd5e1c, also suppresses the -Wuninitialized warning on
>> older versions of gcc.
>>
>> In order to suppress the warning (against the 'oe' symbol) in the
>> note_change_n() function, we adopt the same solution used by commit
>> 3aa99df8.
>
> Yeah, they are essentially the same piece of code, so I don't mind this
> change. It is odd to me that gcc gets it right in one case but not the
> other, but I think we are deep into the vagaries of the compiler's code
> flow analysis here, and we cannot make too many assumptions.
>
> Were you actually triggering this warning, and if so, on what version of
> gcc?
yes, with:
gcc v3.4.4 (cygwin)
gcc v4.1.2 (Linux)
msvc v15.00.30729.01 (VC/C++ 2008 express edition)
no, with:
gcc v4.4.0 (msysgit)
clang 3.2 (Linux)
tcc v0.9.26 (Linux)
[lcc can't compile git; I forget why exactly.]
> Or did the asymmetry just offend your sensibilities?
My sensibilities were, indeed, very offended! ;-)
ATB,
Ramsay Jones
--
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 2/2] cat-file: Fix an gcc -Wuninitialized warning
Jeff King wrote:
> On Tue, Mar 26, 2013 at 07:20:11PM +, Ramsay Jones wrote:
>
>> After commit cbfd5e1c ("drop some obsolete "x = x" compiler warning
>> hacks", 21-03-2013) removed a gcc specific hack, older versions of
>> gcc now issue an "'contents' might be used uninitialized" warning.
>> In order to suppress the warning, we simply initialize the variable
>> to NULL in it's declaration.
>
> I'm OK with this, if it's the direction we want to go. But I thought the
> discussion kind of ended as "we do not care about these warnings on
> ancient versions of gcc; those people should use -Wno-error=uninitialized".
Hmm, I don't recall any agreement or conclusions being reached.
I guess I missed that!
> What version of gcc are you using? If it is the most recent thing
> reasonably available on msysgit, then I am more sympathetic. But if it's
> just an antique version of gcc, I am less so.
(see previous email for compiler versions).
I suppose it depends on what you consider antique. [I recently
downloaded the "first C compiler" from github. Yes, that is an
antique compiler! ;-)] I would call some of the compilers I use
"a bit mature." :-P
Hmm, so are you saying that this patch is not acceptable because
I used a compiler that is no longer supported?
ATB,
Ramsay Jones
--
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 1/2] fast-import: Fix an gcc -Wuninitialized warning
On Thu, Mar 28, 2013 at 06:20:29PM +, Ramsay Jones wrote:
> Jeff King wrote:
> > On Tue, Mar 26, 2013 at 07:09:44PM +, Ramsay Jones wrote:
> >
> >> Commit cbfd5e1c ("drop some obsolete "x = x" compiler warning hacks",
> >> 21-03-2013) removed a gcc hack that suppressed an "might be used
> >> uninitialized" warning issued by older versions of gcc.
> >>
> >> However, commit 3aa99df8 ('fast-import: clarify "inline" logic in
> >> file_change_m', 21-03-2013) addresses an (almost) identical issue
> >> (with very similar code), but includes additional code in it's
> >> resolution. The solution used by this commit, unlike that used by
> >> commit cbfd5e1c, also suppresses the -Wuninitialized warning on
> >> older versions of gcc.
> >>
> >> In order to suppress the warning (against the 'oe' symbol) in the
> >> note_change_n() function, we adopt the same solution used by commit
> >> 3aa99df8.
> >
> > Yeah, they are essentially the same piece of code, so I don't mind this
> > change. It is odd to me that gcc gets it right in one case but not the
> > other, but I think we are deep into the vagaries of the compiler's code
> > flow analysis here, and we cannot make too many assumptions.
> >
> > Were you actually triggering this warning, and if so, on what version of
> > gcc?
>
> yes, with:
> gcc v3.4.4 (cygwin)
> gcc v4.1.2 (Linux)
> msvc v15.00.30729.01 (VC/C++ 2008 express edition)
> no, with:
> gcc v4.4.0 (msysgit)
> clang 3.2 (Linux)
> tcc v0.9.26 (Linux)
> [lcc can't compile git; I forget why exactly.]
Thanks. I do not mind this fix, as it matches the similar code, and it
is fairly straightforward. But I am also happy to let people running gcc
v3.4.4 and other ancient compilers deal with not turning on -Werror. ;)
-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 v2] git-send-email.perl: implement suggestions made by perlcritic
Ramkumar Ramachandra wrote:
> Junio: In future, please tell me explicitly that you're expecting a
> re-roll with an updated commit message. It wasn't obvious to me at
> all.
When there are questions in response to a patch, there are two
possibilities:
* temporary brainfart --- sorry for the bother
* the clarity of the patch or commit message has room for improvement
This wasn't a case of the former, so a seasoned contributor could
safely assume that it was definitely a case of the latter.
The explanation in Linux kernel's Documentation/SubmittingPatches
item 10 ("Don't get discouraged. Re-submit") has some fitting advice.
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: [PATCH 2/2] cat-file: Fix an gcc -Wuninitialized warning
On Thu, Mar 28, 2013 at 06:48:43PM +, Ramsay Jones wrote: > > I'm OK with this, if it's the direction we want to go. But I thought the > > discussion kind of ended as "we do not care about these warnings on > > ancient versions of gcc; those people should use -Wno-error=uninitialized". > > Hmm, I don't recall any agreement or conclusions being reached. > I guess I missed that! I think Jonathan said that and nobody disagreed, and I took it as a conclusion. > Hmm, so are you saying that this patch is not acceptable because > I used a compiler that is no longer supported? No, I just think we should come to a decision on how unreadable to make the code in order to suppress incorrect warnings on old compilers. I can see the point in either of the following arguments: 1. These compilers are old, and we do not need to cater to them in the code because people can just _not_ set -Werror=uninitialized (or its equivalent). It is still worth catering to bugs in modern compilers that most devs use, because being able to set -Werror is helpful. 2. The code is not made significantly less readable, especially if you put in a comment, so why not help these compilers. When we can make the code more readable _and_ help the compiler, I think it is a no-brainer. I am on the fence otherwise and don't care that much. I just think we should apply the rule consistently. -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] push: Alias pushurl from push rewrites
On Thu, Mar 28, 2013 at 11:50:08AM -0700, Junio C Hamano wrote: > Josh Triplett writes: > (on url.$base.pushInsteadOf) > >> If a remote has an explicit pushurl, git will ignore this setting for > >> that remote. > > That really meant what I just said above: git will prefer an explicit > > pushurl over the pushInsteadOf rewrite of url. > > Very correct. > > > It says nothing about > > applying pushInsteadOf to rewrite pushurl. > > Incorrect, I think. If you have pushURL, pushInsteadOf is *ignored*. > Of course, if you have both URL and pushURL, the ignored pushInsteadOf > will not apply to _anything_. It will not apply to URL, and it will > certainly not apply to pushURL. Debatable whether the documentation sentence above really says that; it certainly doesn't say it very clearly if so. But that does match the actual behavior, making the proposed change a regression from the actual behavior, whether the documentation clearly guarantees that behavior or not. > You are correct to point out that with the test we would want to > make sure that for a remote with pushURL and URL, a push goes > > - to pushURL; > - not to URL; > - not to insteadOf(URL); > - not to pushInsteadOf(URL); > - not to insteadOf(pushURL); and > - not to pushInsteadOf(pushURL). > > I do not think it is worth checking all of them, but I agree we > should make sure it does not go to pushInsteadOf(URL) which you > originally meant to check, and we should also make sure it does not > go to pushInsteadOf(pushURL). Agreed. Related to this, as a path forward, I do think it makes sense to have a setting usable as an insteadOf that only applies to pushurl, even though pushInsteadOf won't end up serving that purpose. That way, pushInsteadOf covers the "map read-only repo url to pushable repo url" case, and insteadOfPushOnly covers the "construct a magic prefix that maps to different urls when used in url or pushurl" case. > >> test_expect_success 'push with pushInsteadOf and explicit pushurl > >> (pushInsteadOf should not rewrite)' ' > >>mk_empty && > >> - TRASH="$(pwd)/" && > >> - git config "url.trash2/.pushInsteadOf" trash/ && > >> + git config "url.trash2/.pushInsteadOf" testrepo/ && > > Adding > > git config "url.trash3/.pusnInsteadOf" trash/wrong && > > here should be sufficient for that, no? If we mistakenly used URL > (i.e. trash/wrong) the push would fail. If we mistakenly used > pushInsteadOf(URL), that is rewritten to trash3/ and again the push > would fail. pushInsteadOf(pushURL) would go to trash2/ and that > would also fail. > > We aren't checking insteadOf(URL) and insteadOf(pushURL) but > everything else is checked, I think, so we can do without replacing > anything. We can just extend it, no? That sounds sensible. - Josh Triplett -- 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: More detailed error message for 403 forbidden.
Jeff King writes: > One problem is that the content body sent along with the error is not > necessarily appropriate for showing to the user (e.g., if it is HTML, it > is probably not a good idea to show it on the terminal). So I think we > would want to only show it when the server has indicated via the > content-type that the message is meant to be shown to the user. I'm > thinking the server would generate something like: > >HTTP/1.1 403 Forbidden >Content-type: application/x-git-error-message > >User 'me' does not have enough permission to access the repository. > > which would produce the example you showed above. Actually, isn't the human-readable part of the server response meant for this kind of thing? I.e. HTTP/1.1 403 User 'me' not allowed to accept the repository. -- 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] push: Alias pushurl from push rewrites
Josh Triplett wrote: > Related to this, as a path forward, I do think it makes sense to have a > setting usable as an insteadOf that only applies to pushurl, even though > pushInsteadOf won't end up serving that purpose. That way, > pushInsteadOf covers the "map read-only repo url to pushable repo url" > case, and insteadOfPushOnly covers the "construct a magic prefix that > maps to different urls when used in url or pushurl" case. I hope not. That would be making configuration even more complicated. I hope that we can fix the documentation, tests, and change description in the commit message enough to make Rob's patch a no-brainer. If that's not possible, I think the current state is livable, just confusing. I was happy to see Rob's patch because it brings git's behavior closer to following the principle of least surprise. I am not actually that excited by the use case, except the "avoiding surprise" part of it. 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: git subtree oddity
I agree that subtree solves some specific use cases I would like to support. In particular, I was hoping to use the subtree command in lieu of using the subtree merge strategy to manage and overlay changes to upstream projects, as well as other local components. At any rate, it looks like the problem I'm having is not entirely related to the subtree command, but happens when I checkout a remote into a branch ( which subtree is presumably doing in the background). It's the same setup as before. Here is the sequence of commands I'm running. git init git remote add upstream git://gnuradio.org/gnuradio fetch upstream git checkout -b upstream_tracking upstream/master Now, at this point, I expect the upstream branch to contain the contents of the gnuradio project. I also expect that my local mater branch has only the contents of my local sources, and NOT the contents of the gnuradio. However, if I 'git checkout master', I see the contents of the gnuradio project. Why, when I checkout a branch tracking upstream/master, do the changes also appear on my master branch, and not just in the remote tracking branch? As a reference, this is close to what I'm trying to accomplish. His screenshot titled 'Directory Listing in Master' shows what I expect. http://typecastexception.com/post/2013/03/16/Managing-Nested-Libraries-Using-the-GIT-Subtree-Merge-Workflow.aspx Thanks -Tom Taranowski On Thu, Mar 28, 2013 at 9:34 AM, Jeremy Rosen wrote: >> >> I am starting to regret that I caved in and started carrying a copy >> of it in contrib/. It probably is a good idea to drop it from my >> tree and let it mature and eventually flourish outside. >> > > that's a shame... it solves a real problem, is simple to use, and really > powerfull. > > but unfortunately, I have sent a patch that solves a serious bug... which had > already been reported and patched but had received no answer, and nobody > replied to it. > > Is there anything that can be done to get this rolling, or a way to have the > use-case it covers better handle by git-submodule ? > > > currently the problem of a git repo in a git repo is very complicated to deal > with in a clean way... > > > Regards > > Jérémy -- 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 2/2] cat-file: Fix an gcc -Wuninitialized warning
Jeff King wrote: > When we can make the code more readable _and_ help the compiler, I think > it is a no-brainer. Yep. :) -- 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 subtree oddity
Oh, this is odd. I can get the behavior I want by adding the '-f' flag to the remote add. So: git remote add -f upstream git://gnuradio.org/gnuradio According to the remote add help, the -f is only doing a fetch, which I was doing as a manual step after the remote add. Another interesting artifact is that I know see the "warning: no common commits" log, which I wasn't seeing in my prior process. So, my problem is 'fixed' now, but it seems like this is a bug, particularly since most of the subtree merge tuturoials I've seen online do the manual fetch step. Is there any additional information that would be useful for folks to see? -Tom On Thu, Mar 28, 2013 at 12:29 PM, Thomas Taranowski wrote: > I agree that subtree solves some specific use cases I would like to > support. In particular, I was hoping to use the subtree command in > lieu of using the subtree merge strategy to manage and overlay changes > to upstream projects, as well as other local components. > > At any rate, it looks like the problem I'm having is not entirely > related to the subtree command, but happens when I checkout a remote > into a branch ( which subtree is presumably doing in the background). > > It's the same setup as before. Here is the sequence of commands I'm running. > > git init > git remote add upstream git://gnuradio.org/gnuradio > fetch upstream > git checkout -b upstream_tracking upstream/master > > Now, at this point, I expect the upstream branch to contain the > contents of the gnuradio project. I also expect that my local mater > branch has only the contents of my local sources, and NOT the contents > of the gnuradio. However, if I 'git checkout master', I see the > contents of the gnuradio project. Why, when I checkout a branch > tracking upstream/master, do the changes also appear on my master > branch, and not just in the remote tracking branch? > > As a reference, this is close to what I'm trying to accomplish. His > screenshot titled 'Directory Listing in Master' shows what I expect. > http://typecastexception.com/post/2013/03/16/Managing-Nested-Libraries-Using-the-GIT-Subtree-Merge-Workflow.aspx > > Thanks > -Tom Taranowski > > On Thu, Mar 28, 2013 at 9:34 AM, Jeremy Rosen > wrote: >>> >>> I am starting to regret that I caved in and started carrying a copy >>> of it in contrib/. It probably is a good idea to drop it from my >>> tree and let it mature and eventually flourish outside. >>> >> >> that's a shame... it solves a real problem, is simple to use, and really >> powerfull. >> >> but unfortunately, I have sent a patch that solves a serious bug... which >> had already been reported and patched but had received no answer, and nobody >> replied to it. >> >> Is there anything that can be done to get this rolling, or a way to have the >> use-case it covers better handle by git-submodule ? >> >> >> currently the problem of a git repo in a git repo is very complicated to >> deal with in a clean way... >> >> >> Regards >> >> Jérémy -- 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/4] attr.c::path_matches(): special case paths that end with a slash
On Tue, Mar 26, 2013 at 03:05:58PM -0400, Jeff King wrote: > On Tue, Mar 26, 2013 at 11:39:30AM -0700, Junio C Hamano wrote: > > > A similar adjustment for match_pathname() might be needed, but I > > didn't look into it. > [...] > We do seem to use strncmp_icase through the rest of the function, > though, which should be OK. The one exception is that we call fnmatch at > the end. Should the allocation hack from the previous patch make its way > into an "fnmatch_icase_bytes()" function, so we can use it here, too? > And then when we have a more efficient solution, we can just plug it in > there. Hmm, yeah, there is more going on here than just that. If I add the tests below, the first one (a wildcard) passes, because you fixed the fnmatch code path. But the deep/ ones do not, as they should be going through match_pathname. I expected the deep/with/wildcard one to fail (because of the fnmatch problem I mentioned above), but not the deep/and/slashless one, which should be using strncmp. I'll see if I can track down the cause. -Peff --- diff --git a/t/t5002-archive-attr-pattern.sh b/t/t5002-archive-attr-pattern.sh index 3be809c..234a615 100755 --- a/t/t5002-archive-attr-pattern.sh +++ b/t/t5002-archive-attr-pattern.sh @@ -32,6 +32,21 @@ test_expect_success 'setup' ' git add ignored-without-slash/foo && echo ignored-without-slash export-ignore >>.git/info/attributes && + mkdir -p wildcard-without-slash && + echo "ignored without slash" >wildcard-without-slash/foo && + git add wildcard-without-slash/foo && + echo "wild*-without-slash export-ignore" >>.git/info/attributes && + + mkdir -p deep/and/slashless && + echo "ignored without slash" >deep/and/slashless/foo && + git add deep/and/slashless/foo && + echo deep/and/slashless export-ignore >>.git/info/attributes && + + mkdir -p deep/with/wildcard && + echo "ignored without slash" >deep/with/wildcard/foo && + git add deep/with/wildcard/foo && + echo "deep/*t*/wildcard export-ignore" >>.git/info/attributes && + mkdir -p one-level-lower/two-levels-lower/ignored-only-if-dir && echo ignored by ignored dir >one-level-lower/two-levels-lower/ignored-only-if-dir/ignored-by-ignored-dir && git add one-level-lower && @@ -55,6 +70,12 @@ test_expect_missing archive/ignored-without-slash/foo && test_expect_missingarchive/ignored-ony-if-dir/ignored-by-ignored-dir test_expect_missing archive/ignored-without-slash/ && test_expect_missing archive/ignored-without-slash/foo && +test_expect_missing archive/wildcard-without-slash/ +test_expect_missing archive/wildcard-without-slash/foo && +test_expect_missing archive/deep/and/slashless/ && +test_expect_missing archive/deep/and/slashless/foo && +test_expect_missing archive/deep/with/wildcard/ && +test_expect_missing archive/deep/with/wildcard/foo && test_expect_exists archive/one-level-lower/ test_expect_missing archive/one-level-lower/two-levels-lower/ignored-only-if-dir/ test_expect_missing archive/one-level-lower/two-levels-lower/ignored-ony-if-dir/ignored-by-ignored-dir -- 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: Composing git repositories
Am 28.03.2013 11:01, schrieb Ramkumar Ramachandra: > Jonathan Nieder wrote: >> Do you mean that you wish you could ignore subrepository boundaries >> and use commands like >> >> git clone --recurse-submodules http://git.zx2c4.com/cgit >> cd cgit >> vi git/cache.h >> ... edit edit edit ... >> git add --recurse-submodules git/cache.h >> git commit --recurse-submodules >> git push --recurse-submodules >> >> , possibly with configuration to allow the --recurse-submodules to be >> implied, and have everything work out well? > > Do you realize how difficult this is to implement? We'll need to > patch all the git commands to essentially do what we'd get for free if > the submodule were a tree object instead of a commit object (although > I'm not saying that's the Right thing to do). Some caveats: > > - If we maintain one global index, and try to emulate git-subtree > using submodules, we've lost. It's going to take freakin' ages to > stat billions of files across hundreds of nested sumodules. One major > advantage of having repository boundaries is separate object stores, > indexes, worktrees: little ones that git is designed to work with. Are you aware that current Git code already stats all files across all submodules recursive by default? So (again) no problem here, we do that already (unless configured otherwise). > - Auto-splitting commits that touch multiple submodules/ superproject > at once. Although git-subtree does this, I think it's horribly ugly. You don't like it, but what technical argument is hidden here I'm missing? > - Auto-propagating commits upwards to the superproject is another big > challenge. I think the current design of anchoring to a specific > commit SHA-1 has its usecases, but is unwieldy when things become big. > We have to fix that first. What??? Again there is nothing to "fix" here, "anchoring to a specific commit SHA-1" is *the* most prominent use case (think reproducibility of the whole work tree), floating submodules are the oddball here. -- 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: More detailed error message for 403 forbidden.
On Thu, Mar 28, 2013 at 12:11:55PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > One problem is that the content body sent along with the error is not > > necessarily appropriate for showing to the user (e.g., if it is HTML, it > > is probably not a good idea to show it on the terminal). So I think we > > would want to only show it when the server has indicated via the > > content-type that the message is meant to be shown to the user. I'm > > thinking the server would generate something like: > > > >HTTP/1.1 403 Forbidden > >Content-type: application/x-git-error-message > > > >User 'me' does not have enough permission to access the repository. > > > > which would produce the example you showed above. > > Actually, isn't the human-readable part of the server response meant > for this kind of thing? I.e. > > HTTP/1.1 403 User 'me' not allowed to accept the repository. In theory, yes. But I don't think that most servers make it very easy to use custom "reason phrases" (that is the rfc 2616 term for them). At least I could not easily figure out how to make Apache do so. You can do so from CGIs, but I think you'd want to customize some of this at the HTTP server level (e.g., overriding 404s with a custom message). There's much better support at that level for custom error documents (e.g., Apache's ErrorDocument). I do not configure http servers very often, though, so I could be wrong about what is normal practice, and what is easy to do. -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: Composing git repositories
Am 28.03.2013 12:48, schrieb Ramkumar Ramachandra: > Okay, here's a first draft of the new design. The new mediator object > should look like: > > name = git > ref = v1.7.8 > > The name is looked up in refs/modules/, which in turn looks like: > > [submodule "git"] > origin = gh:artagnon/git > path = git > [submodule "magit"] > origin = gh:magit/magit > path = git/extensions/magit What happens when you rename "magit" to "foo" in that branch and want to check out an older commit of the same branch? That is one of the reasons why that belongs in to a checked in .gitmodules and not someplace untracked. > This solves the two problems that I brought up earlier: > - Floating submodules (which are _necessary_ if you don't want to > propagate commits upwards to the root). If you don't want that either don't use submodules or set the ignore config so you won't be bothered with any changes to the submodules. Floating up to the submodule's tip can be easily achieved with a script (possibly checked in in the superproject). You loose the reproducibility by doing that, but that's what you asked for. No problem here. > - Initializing a nested submodule without having to initialize all the > submodules in the path leading up to it. You cannot access a nested sub-submodule without its parent telling you what submodules it has. Otherwise the first level submodule would not be self-contained, so you'll need to check it out too to access the sub-submodules. Nothing to fix here either. > However, I suspect that we can put more information the mediator > object to make life easier for the parent repository and make seams > disappear. I'm currently thinking about what information git core > needs to behave smoothly with submodules. To me your proposal is trying to fix non-issues and breaking stuff that works, so I see no improvement. -- 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] git-send-email.perl: implement suggestions made by perlcritic
Ramkumar Ramachandra writes:
> Running perlcritic with gentle severity reports six problems. The
> following lists the line numbers on which the problems occur, along
> with a description of the problem. This patch fixes them all,
Thanks.
> after
> carefully considering the consequences.
H.
> 516: Contrary to common belief, subroutine prototypes do not enable
> compile-time checks for proper arguments. They serve the singular
> purpose of defining functions that behave like built-in functions, but
> check_file_rev_conflict was never intended to behave like one. We
> have verified that the callers of the subroutines are alright with the
> change.
This, together with the "carefully considering", does not build any
confidence on the part of the reader. Subroutine prototypes are not
for compile-time checks (correct), and they were introduced to the
language only for the purpose of letting you write subroutine that
emulate built-ins like "pop @ary" (again correct),
The most important fact that is not described in the above is that
by using prototype, the subroutine enforces a non-default context at
the callsite when the arguments are evaluated. That is why you can
write an emulated "pop"; otherwise the first @ary will simply be
flattened and you won't grab an element from the array.
Saying "check_file_rev_conflict is not emulating any Perl built-in
function" is irrelevant as a justification for the change. A
subroutine that does not emulate a built-in can still be relying on
the non-default context in which its arguments are evaluated.
sub foo ($) { my ($arg) = @_; print "$arg\n"; }
sub bar { my ($arg) = @_; print "$arg\n"; }
my @baz = (100, 101, 102);
foo @baz; # says 3
bar @baz; # says 100
In general, writing subroutines without prototypes is much
preferred. I do not dispute that and would not argue against
perlcritic. But if you blindly _fix_ the above "foo" subroutine by
dropping its prototype, it changes behaviour if the callers passed
an array variable. You need to check the callers and they are not
doing something to cause the prototyped and unprototyped versions
to behave differently.
And that is what needs to be explained in the log message; not these
handwavy "carefully considering consequences" (what consequences did
you consider?), "was never intended to behave like one" (how does
that matter?) and "the callers of the subroutines are alright" (why
do you think so?).
Without that, the reviewer needs to go and check the callers. Your
log message is _not_ helping them.
Same for the remainder.
In general, you do not have to copy and paste the output from
perlcritic. Treat it more as a learning tool, and use the knowledge
you learned from its output to explain why your changes are
improvements. Not just "because perlcritic said so".
For "ask" subroutine, all its callers assign the returned value to a
single scaler variable, so the difference between "return undef" and
just "return" does not matter. If somebody starts doing
@foo = ask(...);
if (@foo) { ... we got an answer ... } else { ... we did not ... }
then "return undef;" form will break. So it is less error prone if
we dropped the explicit "undef". The same goes for extract_valid_address,
whose current callers all assign the returned value to a single scalar,
or apply "!" operator inside "while" conditional.
The change to validate_address is correct, but it is correct only
because its only caller, validate_address_list, filters out "undef"
returned from map() that calls this subroutine. By dropping the
explicit "undef" from there, it seems to me that validate_address no
longer returns "undef" so validate_address_list loses any need to
filter its return value. Seeing a patch that does not change that
caller while changing the callee makes reviewers wonder what is
going on. Perhaps even with this patch, there still is a need to
filter in validate_address_list, and if so, that needs to be
explained.
If I were doing this change, I would rather leave this subroutine
as-is. Nothing is broken and we are risking new breakages by
changing it.
> 1441: The three-argument form of `open' (introduced in Perl 5.6)
> prevents subtle bugs that occur when the filename starts with funny
> characters like '>' or '<'.
Correct, and this patch is about using the three-or-more-arg form to
take advantage of its safety. So why are we still using the shell
invocation form inherited back from the days we used two-arg form?
Again, seeing a patch that only turns "open FILEHANDLE,EXPR" into
"open FILEHANDLE,MODE,EXPR" when EXPR is not a simple command name
and not into "open FILEHANDLE,MODE,EXPR,LIST" form makes reviewers
wonder why the patch stops in the middle.
> Junio: In future, please tell me explicitly that you're expecting a
> re-roll with an updated commit message. It wasn't obvious to me at
> all.
It wasn't obvious to me, either ;-).
I said the patch was not explained well, but
Re: Git and GSoC 2013
On Thu, Mar 28, 2013 at 5:45 PM, Junio C Hamano wrote: > Christian Couder writes: > >> On Wed, Mar 27, 2013 at 7:52 PM, Jonathan Nieder wrote: >>> Jeff King wrote: >>> There was a big thread about a month ago on whether Git should do Google Summer of Code this year[1]. >> >> I think we should do it. >> >> It looks strange to me to say that students are great and at the same >> time that we should not do it. >> >> Let's give them and us one more chance do to well. This is the only >> way we can improve. > > Do you mean we should be doing the same thing over and over again > and expecting different results? Einstein may not like it, and I > certainly don't. No, I don't mean we should be doing the same. I agree that smaller projects are helpful and insisting on submitting right away on the mailing list is helpful. But if we don't even try we have no chance to see if it works. We just lose time. > What I gathered from the discussion so far is that everybody agrees > that our mentoring has been suboptimal in various ways (not enough > encouragement to engage with the community early, working in the > cave for too long, biting too much to chew etc.). What makes you > think we would do better this year? The fact that we will be more conscious that we need smaller projects and that we need to push even more for students to send their patch soon on the mailing list. If it doesn't work at all we will be set and we will know that there is not much we can do to make it work. If we don't even try we will not know soon, so not be able to improve or decide to stop. It's like software or science. If you don't test soon your hypothesis you don't progress fast. Or do you think we just stand no chance to progress? By the way we say that students should post soon to the mailing list to get a feedback soon, but it looks like we don't want to try our hypothesis around mentoring as soon as we can. Doesn't it sound strange to you? Aren't we saying "do as I say not as I do"? > "We have a track record of being not great at mentoring, and we > haven't made an effort to improve it." is a perfectly valid and > humble reason to excuse ourselves from this year's GSoC. It is also a perfectly valid justification to decide to make an effort to improve our mentoring and to try again. > "Students are great" is immaterial. "We are not great at mentoring" is as much immaterial. > In fact, if they are great, I think it is better to give them a > chance to excel by working with organizations that can mentor them > better, instead of squandering their time and GSoC's money for > another failure, until _we_ are ready to take great students. How do we know we are ready if we don't try? By waiting we just lose the experience we already have, because some mentors might not be around next year, or they will not remember well about the process. And some organizations that will perhaps be accepted, if we decide not to do it, might have no mentoring experience at all. How do you know they will mentor students better than what we have been doing? 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: Composing git repositories
Am 28.03.2013 10:16, schrieb Ramkumar Ramachandra: > Jens Lehmann wrote: >> Unless you acknowledge that submodules are a different repo, you'll >> always run into problems. I believe future enhancements will make >> this less tedious, but in the end they will stay separate repos >> (which is the whole point, you'd want to use a different approach >> - e.g. subtree - if you want to put stuff from different upstreams >> into a single repo without keeping the distinction where all that >> came from). > > I acknowledge that it's a different repository. It's just that I > think that our current design has too many seams: why do you think > it's impossible to make it seamless? > > git-subtree is not an answer to anything. Dumping all the history > into one repository has its limited usecases, but it is no solution. Guess what: submodules are the solution for a certain set of use cases, and tools like subtree are a solution for another set of use cases. There is no silver bullet. >> What else than a commit object should that be??? Submodules are >> there to have a different upstream for a part of your work tree, >> and that means a commit in that repo is the only sane thing to >> record in the superproject. A lot of thought has been put into >> this, and it is definitely a good choice [1]. > > Linus argues that it shouldn't be a tree object, and I agree with > that. I don't see an argument that says that the commit object is a > perfect fit (probably because it's not). There was discussion about what to record in the index/commit of the superproject in early submodule days (some time before I became involved in Git, seems I currently cannot find a link to that). A commit is the thing to record here because it *is* the perfect fit, as some years of submodule experience show. >> How? The "submodules suck, we should try a completely different >> approach" thingy comes up from time to time, but so far nobody >> could provide a viable alternative to what we currently do. > > My argument is not "submodules suck; we should throw them out of the > window, and start from scratch" at all. I'm merely questioning the > fundamental assumptions that submodules make, instead of proposing > that we work around everything in shell. We don't have to be married > to the existing implementation of submodules and try to fix all the > problems in shell. You cannot simply change the fundamental assumptions of submodules and expect them to be the same thing afterwards. And it doesn't matter at all if we "fix all the problems in shell" or in C-code, we'll fix the remaining problems that are fixable in whatever part of Git it makes sense. And I don't have the impression you have an idea about what submodules are good at, where they can be improved and what problems they'll probably never solve. >> And apart from that, let's not forget we identified some valuable >> improvements to submodules in this thread: >> [...] >> All of those are topics I like to see materialize, and you are >> welcome to tackle them. > > Allow me a few days to think about changing the fundamental building > blocks to make our shell hackery easier. Please go ahead, but if your goal is "to make our shell hackery easier" I'm not interested. I want to improve the user experience of submodules and don't care much in what language we achieve that. And I can't see anything fundamental being wrong with submodules but strongly believe they are a perfect match for some very important use cases (some of which I see happening at my $dayjob for some years now), so I still don't see what you are trying to "fix" here. -- 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 (Mar 2013, #07; Tue, 26)
John Keeping writes: > On Wed, Mar 27, 2013 at 03:15:44PM -0700, Junio C Hamano wrote: >> John Keeping writes: >> >> > On Wed, Mar 27, 2013 at 02:47:25PM -0700, Junio C Hamano wrote: >> >> > * jk/difftool-dir-diff-edit-fix (2013-03-14) 3 commits >> >> > (merged to 'next' on 2013-03-19 at e68014a) >> >> > + difftool --dir-diff: symlink all files matching the working tree >> >> > + difftool: avoid double slashes in symlink targets >> >> > + git-difftool(1): fix formatting of --symlink description >> >> >> >> I lost track of various discussions on "difftool" and its "symlink >> >> so that the user can edit working tree files in the tool". >> > >> > Would it be easiest if I send a new series incorporating >> > jk/difftool-dirr-diff-edit-fix and the proposed change to not overwrite >> > modified working tree files, built on top of t7800-modernize? >> >> I am somewhat reluctant to rewind a topic that has been cooking in >> 'next' for over a week (the above says 19th). Rebuilding the >> style-fixes on top of the above is fine---that topic is much >> younger. > > Sadly that's easier said than done, since it just introduces further > conflicts as jk/difftool-dir-diff-edit-fix doesn't include > da/difftool-fixes (now in master). OK, let's make it simpler then by merging jk/difftool-dir-diff-edit-fix to 'master'. The test tweaks and other work can then built on top. -- 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] attribute regression fix for maint-1.8.1 and upward
On Tue, Mar 26, 2013 at 11:39:27AM -0700, Junio C Hamano wrote: > So here is an attempt to fix the unintended regression, on top of > 9db9eecfe5c2 (attr: avoid calling find_basename() twice per path, > 2013-01-16). It consists of four patches. Here's my update to the series. I think this should fix all of the issues. And it should be very easy to drop in Duy's nwildmatch later on; it can just replace the fnmatch_icase_mem function added in patch 2 below. The main fix in this iteration is that match_pathname receives the same treatment as match_basename, which is done in patches 3 and 4 (the issues were subtly different enough that I didn't want to squash it all together; plus, gotta keep that commit count up). [1/6]: attr.c::path_matches(): the basename is part of the pathname [2/6]: dir.c::match_basename(): pay attention to the length of string parameters [3/6]: dir.c::match_pathname(): adjust patternlen when shifting pattern [4/6]: dir.c::match_pathname(): pay attention to the length of string parameters [5/6]: attr.c::path_matches(): special case paths that end with a slash [6/6]: t: check that a pattern without trailing slash matches a directory -Peff PS I followed your subject-naming convention since I was adding into your series, but it seems quite long to me. I would have just said: "match_basename: pay attention...". -- 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] attr.c::path_matches(): the basename is part of the pathname
From: Junio C Hamano
The function takes two strings (pathname and basename) as if they
are independent strings, but in reality, the latter is always
pointing into a substring in the former.
Clarify this relationship by expressing the latter as an offset into
the former.
Signed-off-by: Junio C Hamano
Signed-off-by: Jeff King
---
This is identical to the original 1/4.
attr.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/attr.c b/attr.c
index ab2aab2..4cfe0ee 100644
--- a/attr.c
+++ b/attr.c
@@ -655,7 +655,7 @@ static int path_matches(const char *pathname, int pathlen,
}
static int path_matches(const char *pathname, int pathlen,
- const char *basename,
+ int basename_offset,
const struct pattern *pat,
const char *base, int baselen)
{
@@ -667,8 +667,8 @@ static int path_matches(const char *pathname, int pathlen,
return 0;
if (pat->flags & EXC_FLAG_NODIR) {
- return match_basename(basename,
- pathlen - (basename - pathname),
+ return match_basename(pathname + basename_offset,
+ pathlen - basename_offset,
pattern, prefix,
pat->patternlen, pat->flags);
}
@@ -701,7 +701,7 @@ static int fill_one(const char *what, struct match_attr *a,
int rem)
return rem;
}
-static int fill(const char *path, int pathlen, const char *basename,
+static int fill(const char *path, int pathlen, int basename_offset,
struct attr_stack *stk, int rem)
{
int i;
@@ -711,7 +711,7 @@ static int fill(const char *path, int pathlen, const char
*basename,
struct match_attr *a = stk->attrs[i];
if (a->is_macro)
continue;
- if (path_matches(path, pathlen, basename,
+ if (path_matches(path, pathlen, basename_offset,
&a->u.pat, base, stk->originlen))
rem = fill_one("fill", a, rem);
}
@@ -750,7 +750,8 @@ static void collect_all_attrs(const char *path)
{
struct attr_stack *stk;
int i, pathlen, rem, dirlen;
- const char *basename, *cp, *last_slash = NULL;
+ const char *cp, *last_slash = NULL;
+ int basename_offset;
for (cp = path; *cp; cp++) {
if (*cp == '/' && cp[1])
@@ -758,10 +759,10 @@ static void collect_all_attrs(const char *path)
}
pathlen = cp - path;
if (last_slash) {
- basename = last_slash + 1;
+ basename_offset = last_slash + 1 - path;
dirlen = last_slash - path;
} else {
- basename = path;
+ basename_offset = 0;
dirlen = 0;
}
@@ -771,7 +772,7 @@ static void collect_all_attrs(const char *path)
rem = attr_nr;
for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
- rem = fill(path, pathlen, basename, stk, rem);
+ rem = fill(path, pathlen, basename_offset, stk, rem);
}
int git_check_attr(const char *path, int num, struct git_attr_check *check)
--
1.8.2.13.g0f18d3c
--
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] dir.c::match_basename(): pay attention to the length of string parameters
From: Junio C Hamano
The function takes two counted strings ( and
) as parameters, together with prefix (the
length of the prefix in pattern that is to be matched literally
without globbing against the basename) and EXC_* flags that tells it
how to match the pattern against the basename.
However, it did not pay attention to the length of these counted
strings. Update them to do the following:
* When the entire pattern is to be matched literally, the pattern
matches the basename only when the lengths of them are the same,
and they match up to that length.
* When the pattern is "*" followed by a string to be matched
literally, make sure that the basenamelen is equal or longer than
the "literal" part of the pattern, and the tail of the basename
string matches that literal part.
* Otherwise, use the new fnmatch_icase_mem helper to make
sure we only lookmake sure we use only look at the
counted part of the strings. Because these counted strings are
full strings most of the time, we check for termination
to avoid unnecessary allocation.
Signed-off-by: Junio C Hamano
Signed-off-by: Jeff King
---
Compared to v1:
- This factors the fnmatch bits into a helper function so we can reuse it
later. As a result, the variable names are changed a bit.
- The original did:
if (use_pat)
strbuf_release(&pat);
but AFAICT that was a useless conditional; use_pat always points to
either the incoming buffer or the strbuf. But strbuf_release will
handle both cases for us.
dir.c | 40
1 file changed, 36 insertions(+), 4 deletions(-)
diff --git a/dir.c b/dir.c
index 5a83aa7..fac82c1 100644
--- a/dir.c
+++ b/dir.c
@@ -34,6 +34,33 @@ int fnmatch_icase(const char *pattern, const char *string,
int flags)
return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD :
0));
}
+static int fnmatch_icase_mem(const char *pattern, int patternlen,
+const char *string, int stringlen,
+int flags)
+{
+ int match_status;
+ struct strbuf pat_buf = STRBUF_INIT;
+ struct strbuf str_buf = STRBUF_INIT;
+ const char *use_pat = pattern;
+ const char *use_str = string;
+
+ if (pattern[patternlen]) {
+ strbuf_add(&pat_buf, pattern, patternlen);
+ use_pat = pat_buf.buf;
+ }
+ if (string[stringlen]) {
+ strbuf_add(&str_buf, string, stringlen);
+ use_str = str_buf.buf;
+ }
+
+ match_status = fnmatch_icase(use_pat, use_str, 0);
+
+ strbuf_release(&pat_buf);
+ strbuf_release(&str_buf);
+
+ return match_status;
+}
+
static size_t common_prefix_len(const char **pathspec)
{
const char *n, *first;
@@ -537,15 +564,20 @@ int match_basename(const char *basename, int basenamelen,
int flags)
{
if (prefix == patternlen) {
- if (!strcmp_icase(pattern, basename))
+ if (patternlen == basenamelen &&
+ !strncmp_icase(pattern, basename, basenamelen))
return 1;
} else if (flags & EXC_FLAG_ENDSWITH) {
+ /* "*literal" matching against "fooliteral" */
if (patternlen - 1 <= basenamelen &&
- !strcmp_icase(pattern + 1,
- basename + basenamelen - patternlen + 1))
+ !strncmp_icase(pattern + 1,
+ basename + basenamelen - (patternlen - 1),
+ patternlen - 1))
return 1;
} else {
- if (fnmatch_icase(pattern, basename, 0) == 0)
+ if (fnmatch_icase_mem(pattern, patternlen,
+ basename, basenamelen,
+ 0) == 0)
return 1;
}
return 0;
--
1.8.2.13.g0f18d3c
--
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] dir.c::match_pathname(): adjust patternlen when shifting pattern
If we receive a pattern that starts with "/", we shift it
forward to avoid looking at the "/" part. Since the prefix
and patternlen parameters are counts of what is in the
pattern, we must decrement them as we increment the pointer.
We remembered to handle prefix, but not patternlen. This
didn't cause any bugs, though, because the patternlen
parameter is not actually used. Since it will be used in
future patches, let's correct this oversight.
Signed-off-by: Jeff King
---
New in this iteration.
dir.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/dir.c b/dir.c
index fac82c1..cc4ce8b 100644
--- a/dir.c
+++ b/dir.c
@@ -597,6 +597,7 @@ int match_pathname(const char *pathname, int pathlen,
*/
if (*pattern == '/') {
pattern++;
+ patternlen--;
prefix--;
}
--
1.8.2.13.g0f18d3c
--
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] dir.c::match_pathname(): pay attention to the length of string parameters
This function takes two counted strings: a pair and a pair. But we end up feeding the result to fnmatch, which expects NUL-terminated strings. We can fix this by calling the fnmatch_icase_mem function, which handles re-allocating into a NUL-terminated string if necessary. While we're at it, we can avoid even calling fnmatch in some cases. In addition to patternlen, we get "prefix", the size of the pattern that contains no wildcard characters. We do a straight match of the prefix part first, and then use fnmatch to cover the rest. But if there are no wildcards in the pattern at all, we do not even need to call fnmatch; we would simply be comparing two empty strings. Signed-off-by: Jeff King --- New in this iteration. dir.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/dir.c b/dir.c index cc4ce8b..3ad44c3 100644 --- a/dir.c +++ b/dir.c @@ -624,11 +624,22 @@ int match_pathname(const char *pathname, int pathlen, if (strncmp_icase(pattern, name, prefix)) return 0; pattern += prefix; + patternlen -= prefix; name+= prefix; namelen -= prefix; + + /* +* If the whole pattern did not have a wildcard, +* then our prefix match is all we need; we +* do not need to call fnmatch at all. +*/ + if (!patternlen && !namelen) + return 1; } - return fnmatch_icase(pattern, name, FNM_PATHNAME) == 0; + return fnmatch_icase_mem(pattern, patternlen, +name, namelen, +FNM_PATHNAME) == 0; } /* Scan the list and let the last match determine the fate. -- 1.8.2.13.g0f18d3c -- 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] attr.c::path_matches(): special case paths that end with a slash
From: Junio C Hamano
The function is given a string that ends with a slash to signal that
the path is a directory to make sure that a pattern that ends with a
slash (i.e. MUSTBEDIR) can tell directories and non-directories
apart. However, the pattern itself (pat->pattern and
pat->patternlen) that came from such a MUSTBEDIR pattern is
represented as a string that ends with a slash, but patternlen does
not count that trailing slash. A MUSTBEDIR pattern "element/" is
represented as a counted string <"element/", 7> and this must match
match pathname "element/".
Because match_basename() and match_pathname() want to see pathname
"element" to match against the pattern <"element/", 7>, reduce the
length of the path to exclude the trailing slash when calling
these functions.
Signed-off-by: Junio C Hamano
Signed-off-by: Jeff King
---
Tweaked since v1 to also drop the trailing slash when we pass the path
to match_pathname.
attr.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/attr.c b/attr.c
index 4cfe0ee..4d620bc 100644
--- a/attr.c
+++ b/attr.c
@@ -661,18 +661,18 @@ static int path_matches(const char *pathname, int pathlen,
{
const char *pattern = pat->pattern;
int prefix = pat->nowildcardlen;
+ int isdir = (pathlen && pathname[pathlen - 1] == '/');
- if ((pat->flags & EXC_FLAG_MUSTBEDIR) &&
- ((!pathlen) || (pathname[pathlen-1] != '/')))
+ if ((pat->flags & EXC_FLAG_MUSTBEDIR) && !isdir)
return 0;
if (pat->flags & EXC_FLAG_NODIR) {
return match_basename(pathname + basename_offset,
- pathlen - basename_offset,
+ pathlen - basename_offset - isdir,
pattern, prefix,
pat->patternlen, pat->flags);
}
- return match_pathname(pathname, pathlen,
+ return match_pathname(pathname, pathlen - isdir,
base, baselen,
pattern, prefix, pat->patternlen, pat->flags);
}
--
1.8.2.13.g0f18d3c
--
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] t: check that a pattern without trailing slash matches a directory
Prior to v1.8.1.1, with: git init echo content >foo && mkdir subdir && echo content >subdir/bar && echo "subdir export-ignore" >.gitattributes git add . && git commit -m one && git archive HEAD | tar tf - the resulting archive would contain only "foo" and ".gitattributes", not subdir. This was broken with a recent change that intended to allow "subdir/ export-ignore" to also exclude the directory, but instead ended up _requiring_ the trailing slash by mistake. A pattern "subdir" should match any path "subdir", whether it is a directory or a non-diretory. A pattern "subdir/" insists that a path "subdir" must be a directory for it to match. This patch adds test not just for this simple case, but also for deeper cross-directory cases, as well as cases with wildcards. Signed-off-by: Jeff King --- Added new tests since v1 that handle the match_pathname code path. t/t5002-archive-attr-pattern.sh | 27 +++ 1 file changed, 27 insertions(+) diff --git a/t/t5002-archive-attr-pattern.sh b/t/t5002-archive-attr-pattern.sh index 0c847fb..6667d15 100755 --- a/t/t5002-archive-attr-pattern.sh +++ b/t/t5002-archive-attr-pattern.sh @@ -27,6 +27,25 @@ test_expect_success 'setup' ' echo ignored-only-if-dir/ export-ignore >>.git/info/attributes && git add ignored-only-if-dir && + mkdir -p ignored-without-slash && + echo "ignored without slash" >ignored-without-slash/foo && + git add ignored-without-slash/foo && + echo "ignored-without-slash export-ignore" >>.git/info/attributes && + + mkdir -p wildcard-without-slash && + echo "ignored without slash" >wildcard-without-slash/foo && + git add wildcard-without-slash/foo && + echo "wild*-without-slash export-ignore" >>.git/info/attributes && + + mkdir -p deep/and/slashless && + echo "ignored without slash" >deep/and/slashless/foo && + git add deep/and/slashless/foo && + echo "deep/and/slashless export-ignore" >>.git/info/attributes && + + mkdir -p deep/with/wildcard && + echo "ignored without slash" >deep/with/wildcard/foo && + git add deep/with/wildcard/foo && + echo "deep/*t*/wildcard export-ignore" >>.git/info/attributes && mkdir -p one-level-lower/two-levels-lower/ignored-only-if-dir && echo ignored by ignored dir >one-level-lower/two-levels-lower/ignored-only-if-dir/ignored-by-ignored-dir && @@ -49,6 +68,14 @@ test_expect_missing archive/ignored-ony-if-dir/ignored-by-ignored-dir test_expect_exists archive/not-ignored-dir/ test_expect_missingarchive/ignored-only-if-dir/ test_expect_missingarchive/ignored-ony-if-dir/ignored-by-ignored-dir +test_expect_missingarchive/ignored-without-slash/ && +test_expect_missingarchive/ignored-without-slash/foo && +test_expect_missingarchive/wildcard-without-slash/ +test_expect_missingarchive/wildcard-without-slash/foo && +test_expect_missingarchive/deep/and/slashless/ && +test_expect_missingarchive/deep/and/slashless/foo && +test_expect_missingarchive/deep/with/wildcard/ && +test_expect_missingarchive/deep/with/wildcard/foo && test_expect_exists archive/one-level-lower/ test_expect_missing archive/one-level-lower/two-levels-lower/ignored-only-if-dir/ test_expect_missing archive/one-level-lower/two-levels-lower/ignored-ony-if-dir/ignored-by-ignored-dir -- 1.8.2.13.g0f18d3c -- 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] t: check that a pattern without trailing slash matches a directory
On Thu, Mar 28, 2013 at 5:50 PM, Jeff King wrote: > A pattern "subdir" should match any path "subdir", whether it is a > directory or a non-diretory. A pattern "subdir/" insists that a s/diretory/directory/ [1] > path "subdir" must be a directory for it to match. [1]: http://article.gmane.org/gmane.comp.version-control.git/219185/ -- 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] t: check that a pattern without trailing slash matches a directory
On Thu, Mar 28, 2013 at 06:21:08PM -0400, Eric Sunshine wrote: > On Thu, Mar 28, 2013 at 5:50 PM, Jeff King wrote: > > A pattern "subdir" should match any path "subdir", whether it is a > > directory or a non-diretory. A pattern "subdir/" insists that a > > s/diretory/directory/ [1] I think I've taken proofreading to a new level by missing the error I already corrected somebody else for. Thanks for noticing. :) -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 4/6] dir.c::match_pathname(): pay attention to the length of string parameters
Jeff King writes: > This function takes two counted strings: a pair > and a pair. But we end up feeding the result to > fnmatch, which expects NUL-terminated strings. > > We can fix this by calling the fnmatch_icase_mem function, which > handles re-allocating into a NUL-terminated string if necessary. > > While we're at it, we can avoid even calling fnmatch in some cases. In > addition to patternlen, we get "prefix", the size of the pattern that > contains no wildcard characters. We do a straight match of the prefix > part first, and then use fnmatch to cover the rest. But if there are > no wildcards in the pattern at all, we do not even need to call > fnmatch; we would simply be comparing two empty strings. It is not a new issue, but it would be nicer to have a comment to warn people that this part needs to be adjusted when they want to add support for using FNM_PERIOD in our codebase. Other than that, looks sane to me. Thanks. > > Signed-off-by: Jeff King > --- > New in this iteration. > > dir.c | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/dir.c b/dir.c > index cc4ce8b..3ad44c3 100644 > --- a/dir.c > +++ b/dir.c > @@ -624,11 +624,22 @@ int match_pathname(const char *pathname, int pathlen, > if (strncmp_icase(pattern, name, prefix)) > return 0; > pattern += prefix; > + patternlen -= prefix; > name+= prefix; > namelen -= prefix; > + > + /* > + * If the whole pattern did not have a wildcard, > + * then our prefix match is all we need; we > + * do not need to call fnmatch at all. > + */ > + if (!patternlen && !namelen) > + return 1; > } > > - return fnmatch_icase(pattern, name, FNM_PATHNAME) == 0; > + return fnmatch_icase_mem(pattern, patternlen, > + name, namelen, > + FNM_PATHNAME) == 0; > } > > /* Scan the list and let the last match determine the fate. -- 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
