Re: [PATCH v2 1/2] send-email: make annotate configurable
On Sat, Apr 6, 2013 at 9:32 PM, Junio C Hamano wrote:
> Felipe Contreras writes:
>
>> @@ -305,6 +306,7 @@ my $rc = GetOptions("h" => \$help,
>> "smtp-domain:s" => \$smtp_domain,
>> "identity=s" => \$identity,
>> "annotate" => \$annotate,
>> + "no-annotate" => \$no_annotate,
>
> Wouldn't it be much better to let GetOptions know that --no-annotate
> is allowed by saying
>
> "annotate!" => \$annotate
>
> which also let us lose an extra $no_annotate variable?
Right. Will resend.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Advice and repo setup
Michael Campbell: > So one plan is to have multiple repos, and then a mirror of those for > the remote devs. The other plan is to say "sod it" and have one local > and one remote and just suffer through possible non-requirements of > varying authorization profiles. You could also use Gerrit[1]. It's not only a code review server (and any team should have code review). It also hosts git repositories and you can write submit rules to reflect any possible write rule your company might have[2]. [1] http://en.wikipedia.org/wiki/Gerrit_%28software%29 [2] https://gerrit-review.googlesource.com/Documentation/prolog-cookbook.html Regards, Thomas Koch, http://www.koch.ro -- 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] format-patch: add format.cover-letter configuration
On Sat, Apr 6, 2013 at 9:32 PM, Junio C Hamano wrote:
> Felipe Contreras writes:
>
>> Also, add a new option: 'auto', so if there's more than one patch, the
>> cover letter is generated, otherwise it's not.
>
> Very sensible goal.
>
>> This has the slight disadvantage that a piece of code will always be run
>> even if the user doesn't want a cover letter, and thus waste a few
>> cycles.
>
> I am not sure what overhead you are referring to.
>
> We need to count how many we are emitting with or without the cover
> letter, and count is stored in "total". Even when the user said
> "auto" (which I personally think should become the default in the
> longer term, but that is a separate issue), we shouldn't have to
> spend any extra cost if you moved the code that does anything heavy
> for cover letter generation after we know what that "total" is, no?
>
> I think the reason you did not move the "find the branch for use in
> the cover letter" code down was because it uses the rev->pending
> interface, which you cannot read out of after you count the commits,
> but that logic to use rev->pending predates the introduction of a
> more modern rev->cmdline mechanism, which is already used by the
> find_branch_name() function in the same codepath, and it is not
> clobbered by prepare_revision_walk().
>
> So perhaps by moving that code down after we know what value "total"
> has, and rewriting "what was the positive commit the user gave us"
> logic using rev->cmdline, you do not have to do unnecessary work at
> all.
I did try to do that, but somehow missed a few possibilities that I
see now. Perhaps something like this (after this, it's possible to
move the find_branch_name() code):
diff --git a/builtin/log.c b/builtin/log.c
index ed89c10..32add91 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1061,15 +1061,6 @@ static char *find_branch_name(struct rev_info *rev)
if (0 <= positive) {
ref = rev->cmdline.rev[positive].name;
tip_sha1 = rev->cmdline.rev[positive].item->sha1;
- } else if (!rev->cmdline.nr && rev->pending.nr == 1 &&
- !strcmp(rev->pending.objects[0].name, "HEAD")) {
- /*
-* No actual ref from command line, but "HEAD" from
-* rev->def was added in setup_revisions()
-* e.g. format-patch --cover-letter -12
-*/
- ref = "HEAD";
- tip_sha1 = rev->pending.objects[0].item->sha1;
} else {
return NULL;
}
@@ -1299,28 +1290,41 @@ int cmd_format_patch(int argc, const char
**argv, const char *prefix)
}
if (rev.pending.nr == 1) {
+ unsigned char sha1[20];
+ const char *ref;
+ int check_head = 0;
+
if (rev.max_count < 0 && !rev.show_root_diff) {
/*
* This is traditional behaviour of "git format-patch
* origin" that prepares what the origin side still
* does not have.
*/
- unsigned char sha1[20];
- const char *ref;
-
rev.pending.objects[0].item->flags |= UNINTERESTING;
add_head_to_pending(&rev);
- ref = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
- if (ref && !prefixcmp(ref, "refs/heads/"))
- branch_name = xstrdup(ref +
strlen("refs/heads/"));
- else
- branch_name = xstrdup(""); /* no branch */
+ check_head = 1;
}
/*
* Otherwise, it is "format-patch -22 HEAD", and/or
* "format-patch --root HEAD". The user wants
* get_revision() to do the usual traversal.
*/
+
+ if (!strcmp(rev.pending.objects[0].name, "HEAD"))
+ /*
+* No actual ref from command line, but "HEAD" from
+* rev->def was added in setup_revisions()
+* e.g. format-patch --cover-letter -12
+*/
+ check_head = 1;
+
+ if (check_head) {
+ ref = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
+ if (ref && !prefixcmp(ref, "refs/heads/"))
+ branch_name = xstrdup(ref +
strlen("refs/heads/"));
+ else
+ branch_name = xstrdup(""); /* no branch */
+ }
}
/*
@@ -1329,24 +1333,6 @@ int cmd_format_patch(int argc, const char
**argv, const char *prefix)
*/
rev.show_root_diff = 1;
- /*
-* NEEDSWORK:randomly pick one positive commit to show
-* diffstat; this is often the tip and the command
-
Re: [PATCH v2 2/2] format-patch: add format.cover-letter configuration
Felipe Contreras writes: > Also, add a new option: 'auto', so if there's more than one patch, the > cover letter is generated, otherwise it's not. Very sensible goal. > This has the slight disadvantage that a piece of code will always be run > even if the user doesn't want a cover letter, and thus waste a few > cycles. I am not sure what overhead you are referring to. We need to count how many we are emitting with or without the cover letter, and count is stored in "total". Even when the user said "auto" (which I personally think should become the default in the longer term, but that is a separate issue), we shouldn't have to spend any extra cost if you moved the code that does anything heavy for cover letter generation after we know what that "total" is, no? I think the reason you did not move the "find the branch for use in the cover letter" code down was because it uses the rev->pending interface, which you cannot read out of after you count the commits, but that logic to use rev->pending predates the introduction of a more modern rev->cmdline mechanism, which is already used by the find_branch_name() function in the same codepath, and it is not clobbered by prepare_revision_walk(). So perhaps by moving that code down after we know what value "total" has, and rewriting "what was the positive commit the user gave us" logic using rev->cmdline, you do not have to do unnecessary work at all. -- 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 1/2] send-email: make annotate configurable
Felipe Contreras writes:
> @@ -305,6 +306,7 @@ my $rc = GetOptions("h" => \$help,
> "smtp-domain:s" => \$smtp_domain,
> "identity=s" => \$identity,
> "annotate" => \$annotate,
> + "no-annotate" => \$no_annotate,
Wouldn't it be much better to let GetOptions know that --no-annotate
is allowed by saying
"annotate!" => \$annotate
which also let us lose an extra $no_annotate variable?
--
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 00/13] remote-hg: general updates
Felipe Contreras writes: > On Fri, Apr 5, 2013 at 7:28 PM, Junio C Hamano wrote: > >> A tool that is in contrib/ follows the contrib/README rule. >> >> I do not maintain it. Maintenance is up to the person who asked to >> include it there. I do ask the people who propose to add something >> in contrib/ to promise that they arrange it to be maintained. > > That's true, but I meant that you are the gatekeeper. Ultimately you > decide which patches go in. If Max, or anybody else, wants a patch > into contrib, you can get it in, even if I disagree with it. In an ideal fantasy world, it could be true, but when I say "I do not maintain it, and people who put it in contrib/ is responsible for it", I really mean it. I may find the behaviour/performance of the sub-maintainer of a part in contrib/ unsatisfactory and have to take an administrative action (e.g. to remove the problematic part out of contrib/), but I would rather not to do that kind of thing, if possible. Instead I expect people who have any code in my tree (even in contrib/) to act as a responsible adult, taking and responding to constructive criticisms well, and more importantly, nudging those whose utterance you find are mostly noise into raising a more concrete and actionable issues that would be useful to you as an input. > Either way, I think if things go well, remote-hg will prove it's worth > and move out of contrib and into git's core. That was what you promised when we started carrying it in contrib/; I am still hoping to see it happen when it matures. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
Ramkumar Ramachandra writes: > So we've thought about it for some time, and I really need you to > start reviewing the code now. > > I'll just summarize what we've discussed so far: > ... I do not think we have heard anything concrete and usable about what you are trying to achieve yet. You may be proposing to discard baby with bathwater. We haven't seen an evidence that the change is really worth having. We do not even know what you are trying to change, other than "I want to add a new object type to largely replicate what is recorded in .gitmodules file". What are you trying to solve? I want to have a project for an appliance, that binds two projects, the kernel and the appliance's userspace. The usual suspects to use to implement such a project would be Git submodule, repo, or Gitslave. I want to be able to do X and Y and Z in managing such a project. If I try to use submodule, I cannot see how I could make it do X for _this thing_, and it is not a bug in the implementation but is fundamental because of _this and that_. If I try to use repo, .. the same, and the same for Gitslave. .. I propose to add a new "gitlink" object recorded in the tree and in the index, and the said cases X, Y and Z can be solved in _such and such way_. We cannot solve it without having a new "gitlink" object recorded in the tree object because of _this and that reason_. I think it is too premature to discuss _your_ code. The patches do not even tell us anything about how much more work is needed to merely make Git with your patches work properly again. For one thing, I suspect that you won't even be able to repack a repository that has OBJ_LINK only with the patches you posted. At this point the only thing that we can gain from reading your patch is that you can write C to do _something_, but that something is so fuzzily explained that we do not know what to make of that knowledge that you write good (or bad, we don't know) C. It would be much more productive to learn what these specific issues X, Y and Z are, and if the problems you are having with existing solutions are really fundamental that need changes to object layer to solve. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git instaweb - share all project files
Hello, On that first page that shows up, it shows the .git folder. It would be kind of nice if it shared out both the git repo and the actual current project files. I frequently have stuff I'd like to see in a web browser, and even requires one (i.e. Navigating to file:///home/blah/blah doesn't work; ajax requests for example) 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
Git/Mac refuses to install on OS X 10.8 (Mountain Lion)
The default security settings on OS X 10.8 (Mountain Lion) disallow the installation of unsigned packages, with no override. Git/Mac 1.8.2 is not signed and therefore will not install without changing the OS default security settings. See screenshot: http://postimg.org/image/wcb34kzy5/ -- David Foster http://dafoster.net/ -- 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] remote-hg: activate graplog extension for hg_log()
On Sat, Apr 6, 2013 at 12:50 PM, Antoine Pelisse wrote: > remote-hg: activate graplog extension for hg_log() s/graplog/graphlog/ > The hg_log() test helper uses the "--graph" parameter that is > implemented by the GraphLog extension. If the extension is not activated > by the user, the parameter is not available. Activate the extension in > setup(). > > Also changes the way we grep the output in hg_log(). The pipe operator > can hide the return code of hg command. As a matter of fact, if log > fails because it doesn't know about "--graph", it doesn't report any > failure and let's you think everything worked. > > Signed-off-by: Antoine Pelisse -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
Hi again, So we've thought about it for some time, and I really need you to start reviewing the code now. I'll just summarize what we've discussed so far: 1. The malleability argument doesn't hold, because we're proposing a link object with optional fields. 2. The local-fork argument doesn't hold, because users will be rebasing changes to the link object in exactly the same way as they currently do with the blob object .gitmodules. 3. The worktree argument doesn't hold, because we're proposing to treat the link object as nothing more than a blob object that can be parsed by git-core. It will stage and unstage just like a blob. Sure, it's not accessible directly by the filesystem: so what? What is the difference does `emacsclient .gitmodules` versus `git edit-link clayoven` make to the end-user? 4. The diff-confusion argument is just another by-the-way, but it doesn't really hold either. Currently, we see: - Subproject commit b83492 + Subproject commit 39ab2f (with diff.submodule set to log, we can actually see the log of the submodule between these two commits. With links, we will see: - checkout_rev = b83492 + checkout_rev = 39ab2f There's nothing that prevents us from respecting diff.submodule (some minor glue code will have to be written; that's all). *. There is actually one thing that .gitmodules does better than links. foreach. It's trivial to implement with .gitmodules and hard to implement with links: with .gitmodules, the paths of all the submodules are in one place. But with links, we'll have to unpack_trees() every tree in the entire repository, and dig through it to find all the link objects to initialize. Basically, inefficient and inelegant. However, I don't think this is a big problem in practice, since this is not exactly a common operation: I'd probably want to recurse-submodules once at clone time. -- 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: check-attr doesn't respect recursive definitions
Duy Nguyen wrote: > On Sat, Mar 30, 2013 at 8:45 PM, Jan Larres wrote: >> I would expect the last command to also report 'set'. I've also tried >> other patterns like 'foo/' and 'foo*', but it didn't make any >> difference. > > Try "foo/**". You need 1.8.2 though. That indeed works exactly the way I would like! Thanks a lot! Jan -- 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 0/4] remote-helpers: trivial test fixes
On 06.04.13 19:58, Felipe Contreras wrote:
> On Sat, Apr 6, 2013 at 11:45 AM, Torsten Bögershausen wrote:
>> On 06.04.13 19:29, Felipe Contreras wrote:
>>> On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen wrote:
>
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
git push
) &&
- hg -R hgrepo bookmarks | grep "devel\s\+3:"
+ hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
'
>>>
>>> I would rather use [ \t] instead.
>> That doesn't work on e.g. Mac OS.
>> [:space:] is actually portable
>
> Why wouldn't it work? This is from their manpage:
>
> A bracket expression is a list of characters enclosed by [ and ]. It
> matches any single character in
> that list; if the first character of the list is the caret ^ then it
> matches any character not in the
> list. For example, the regular expression [0123456789] matches any
> single digit.
>
It's not about the "bracket list".
It's about using \t as an abreviation for TAB.
The "backslash n" as an replacement for TAB is quite often understood by many
programs.
It is not demanded to be understood by all grep implementations, please see
below.
Instead of using \t you can use a literal TAB.
Contact your local editor how to put that inte source code ;-)
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap09.html#tag_09_03
9.3.2 BRE Ordinary Characters
An ordinary character is a BRE that matches itself: any character in the
supported character set, except for the BRE special characters listed in BRE
Special Characters.
The interpretation of an ordinary character preceded by a backslash ( '\' ) is
undefined, except for:
The characters ')', '(', '{', and '}'
The digits 1 to 9 inclusive (see BREs Matching Multiple Characters)
A character inside a bracket expression
--
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] gitremote-helpers(1): clarify refspec behaviour
In Sat, Apr 6, 2013 at 11:13 AM, John Keeping wrote: > The documentation says that "If no 'refspec' capability is advertised, > there is an implied `refspec *:*`" but this is only the case for the > "import" command. > > Since there is a comment in transport-helper.c indicating that this > default is for historical reasons, change the documentation to clarify > that a refspec should always be specified. > > Signed-off-by: John Keeping Acked-by: Sverre Rabbelier -- Cheers, Sverre Rabbelier -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gitremote-helpers(1): clarify refspec behaviour
The documentation says that "If no 'refspec' capability is advertised, there is an implied `refspec *:*`" but this is only the case for the "import" command. Since there is a comment in transport-helper.c indicating that this default is for historical reasons, change the documentation to clarify that a refspec should always be specified. Signed-off-by: John Keeping --- Documentation/gitremote-helpers.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index 0c91aba..f506031 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -174,8 +174,8 @@ ref. This capability can be advertised multiple times. The first applicable refspec takes precedence. The left-hand of refspecs advertised with this capability must cover all refs reported by -the list command. If no 'refspec' capability is advertised, -there is an implied `refspec *:*`. +the list command. If a helper does not need a specific 'refspec' +capability then it should advertise `refspec *:*`. 'bidi-import':: This modifies the 'import' capability. -- 1.8.2.692.g17a9715 -- 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: Advice and repo setup
In message , Michael Campbell writes: As a business decision we have decided to pull in some "staff augmentation". We don't want the remote developers to have direct access. Our plan is to have some sort of external repo on which they can push things, and locally we can pull those changes to our "official" repo and check it as we go. So far so good. You might want to consider using something like gitolite where you can have control over which branches users can write to. Assuming you are not trying to restrict some branches of some repos from the external users, this would be a better solution than setting up another repo with some kind of automatic mirroring scheme, though that of course will also work. Our product has several logically separate projects, which right now we have in the one big mega repo (in CVS, and migrating per checkin to Gitorious). Certainly I'd recommend using one repo per conceptual unit. There are several techniques to group repos together if you need to. Is there documentation I can refer to for this, or is there an obvious way to do these things? Any help or pointers appreciated. http://sethrobertson.github.com/GitBestPractices/ -Seth Robertson -- 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 0/4] remote-helpers: trivial test fixes
On Sat, Apr 6, 2013 at 11:45 AM, Torsten Bögershausen wrote: > On 06.04.13 19:29, Felipe Contreras wrote: >> On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen wrote: >>> --- a/contrib/remote-helpers/test-hg.sh >>> +++ b/contrib/remote-helpers/test-hg.sh >>> @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' ' >>>git push >>>) && >>> >>> - hg -R hgrepo bookmarks | grep "devel\s\+3:" >>> + hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:" >>> ' >> >> I would rather use [ \t] instead. > That doesn't work on e.g. Mac OS. > [:space:] is actually portable Why wouldn't it work? This is from their manpage: A bracket expression is a list of characters enclosed by [ and ]. It matches any single character in that list; if the first character of the list is the caret ^ then it matches any character not in the list. For example, the regular expression [0123456789] matches any single digit. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Advice and repo setup
My company is moving from CVS to git in a few weeks (and we have a training class scheduled with the github folks). That said our CI/build guys have already got gitorious set up (we get to it through ssh with ssh keys and one "git" user on the server) and we are in the process of migrating all new CVS checkins to a git repo. As a business decision we have decided to pull in some "staff augmentation". We don't want the remote developers to have direct access. Our plan is to have some sort of external repo on which they can push things, and locally we can pull those changes to our "official" repo and check it as we go. So far so good. Our product has several logically separate projects, which right now we have in the one big mega repo (in CVS, and migrating per checkin to Gitorious). So... I was wondering what the best way to split up our new repo might be - or is it best to NOT split it? One of the concerns we have is that in the one big repo we can't control access to the various projects. So far we haven't needed to but this might be because we couldn't. So one plan is to have multiple repos, and then a mirror of those for the remote devs. The other plan is to say "sod it" and have one local and one remote and just suffer through possible non-requirements of varying authorization profiles. Is there documentation I can refer to for this, or is there an obvious way to do these things? Any help or pointers appreciated. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/2] send-email: couple of improvements
Felipe Contreras writes: > First patch was already sent, I just added the --no-annotate option. Second > one > is new, it adds a configuration for --cover-letter, so it can automatically > determine when to generated: 1 patch, no cover, otherwise there is. Very good. I unconditionnally run --annotate, but do that with an alias. And indeed, I use --cover-letter almost if and only if I have several patches to send. These patches do exactly what I need! -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 0/4] remote-helpers: trivial test fixes
On 06.04.13 19:29, Felipe Contreras wrote: > On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen wrote: >> On 04.04.13 17:36, Felipe Contreras wrote: >>> Hi, >>> >>> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix >>> the >>> outsanding shell portability issue. The rest is the same. >>> >>> Felipe Contreras (4): >>> remote-bzr: avoid echo -n >>> remote-helpers: fix the run of all tests >>> remote-bzr: remove stale check code for tests >>> remote-hg: fix hg-git test-case >>> >>> contrib/remote-helpers/Makefile | 1 + >>> contrib/remote-helpers/test-bzr.sh | 16 +--- >>> contrib/remote-helpers/test-hg-hg-git.sh | 1 - >>> 3 files changed, 2 insertions(+), 16 deletions(-) >>> >> Sorry being late, now I installed bzr and hg on one of my machines >> >> One defect found: "\s" is not portable on all grep versions >> A "*" is not a "basic regular expression", so we need to use egrep >> >> >> diff --git a/contrib/remote-helpers/test-hg.sh >> b/contrib/remote-helpers/test-hg.sh >> index 5f81dfa..2e80c11 100755 >> --- a/contrib/remote-helpers/test-hg.sh >> +++ b/contrib/remote-helpers/test-hg.sh >> @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' ' >>git push >>) && >> >> - hg -R hgrepo bookmarks | grep "devel\s\+3:" >> + hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:" >> ' > > I would rather use [ \t] instead. That doesn't work on e.g. Mac OS. [:space:] is actually portable /Torsten -- 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] format-patch: add format.cover-letter configuration
Felipe Contreras writes: > +format.cover-letter:: We normally use camelCase, so format.coverLetter, not cover-letter. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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] format-patch: add format.cover-letter configuration
Felipe Contreras writes: >> Perhaps you can clarify this: Controls whether to generate a >> cover-letter when format-patch is invoked. Can be true, false, or >> auto. "auto" generates a cover-letter only when generating more than >> one patch. > > That's good, but I believe if we say it's a boolean, true and false > are implied, and then we have an extra "auto". At the moment, we don't say it's Boolean. I'm not sure it's a good idea to say Boolean here, since it would also imply that there's no value other than true and false. Also, you should mention the default value. Something like "Set to true to always generate a cover letter. Defaults to false." or so. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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] fast-export: Allow pruned-references in mark file
On Sat, Apr 6, 2013 at 11:04 AM, Antoine Pelisse wrote: > fast-export can fail because of some pruned-reference when importing a > mark file. > > The problem happens in the following scenario: > > $ git fast-export --export-marks=MARKS master > (rewrite master) > $ git prune > $ git fast-export --import-marks=MARKS master > > This might fail if some references have been removed by prune > because some marks will refer to no longer existing commits. > git-fast-export will not need these objects anyway as they were no > longer reachable. > > We still need to update last_numid so we don't change the mapping > between marks and objects for remote-helpers. > Unfortunately, the mark file should not be rewritten without lost marks > if no new objects has been exported, as we could lose track of the last > last_numid. Makes sense to me. Reviewed-by: Felipe Contreras -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/4] remote-helpers: trivial test fixes
On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen wrote: > On 04.04.13 17:36, Felipe Contreras wrote: >> Hi, >> >> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the >> outsanding shell portability issue. The rest is the same. >> >> Felipe Contreras (4): >> remote-bzr: avoid echo -n >> remote-helpers: fix the run of all tests >> remote-bzr: remove stale check code for tests >> remote-hg: fix hg-git test-case >> >> contrib/remote-helpers/Makefile | 1 + >> contrib/remote-helpers/test-bzr.sh | 16 +--- >> contrib/remote-helpers/test-hg-hg-git.sh | 1 - >> 3 files changed, 2 insertions(+), 16 deletions(-) >> > Sorry being late, now I installed bzr and hg on one of my machines > > One defect found: "\s" is not portable on all grep versions > A "*" is not a "basic regular expression", so we need to use egrep > > > diff --git a/contrib/remote-helpers/test-hg.sh > b/contrib/remote-helpers/test-hg.sh > index 5f81dfa..2e80c11 100755 > --- a/contrib/remote-helpers/test-hg.sh > +++ b/contrib/remote-helpers/test-hg.sh > @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' ' >git push >) && > > - hg -R hgrepo bookmarks | grep "devel\s\+3:" > + hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:" > ' I would rather use [ \t] instead. > 2 mninor nits: > > diff --git a/contrib/remote-helpers/test-bzr.sh > b/contrib/remote-helpers/test-bzr.sh > index 8450432..7970f9e 100755 > --- a/contrib/remote-helpers/test-bzr.sh > +++ b/contrib/remote-helpers/test-bzr.sh > @@ -13,7 +13,7 @@ if ! test_have_prereq PYTHON; then > fi > > if ! "$PYTHON_PATH" -c 'import bzrlib'; then > - skip_all='skipping remote-bzr tests; bzr not available' > + skip_all='skipping remote-bzr tests; python bzrlib not available' "python bzrlib" is meaningless to the user; bazaar provides it; basically all the code is there. > test_done > fi > > diff --git a/contrib/remote-helpers/test-hg-hg-git.sh > b/contrib/remote-helpers/test-hg-hg-git.sh > index 3f253b7..5d87282 100755 > --- a/contrib/remote-helpers/test-hg-hg-git.sh > +++ b/contrib/remote-helpers/test-hg-hg-git.sh > @@ -21,7 +21,7 @@ if ! "$PYTHON_PATH" -c 'import mercurial'; then > fi > > if ! "$PYTHON_PATH" -c 'import hggit'; then > - skip_all='skipping remote-hg tests; hg-git not available' > + skip_all='skipping remote-hg tests; python hggit not available' Same. Google for 'python hggit' and the result is hg-git; and that's what you would actually tell your package manager to install. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/4] remote-helpers: trivial test fixes
On 06.04.13 19:03, Torsten Bögershausen wrote: > On 04.04.13 17:36, Felipe Contreras wrote: >> Hi, >> >> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the >> outsanding shell portability issue. The rest is the same. >> >> Felipe Contreras (4): >> remote-bzr: avoid echo -n >> remote-helpers: fix the run of all tests >> remote-bzr: remove stale check code for tests >> remote-hg: fix hg-git test-case >> >> contrib/remote-helpers/Makefile | 1 + >> contrib/remote-helpers/test-bzr.sh | 16 +--- >> contrib/remote-helpers/test-hg-hg-git.sh | 1 - >> 3 files changed, 2 insertions(+), 16 deletions(-) >> > Sorry being late, now I installed bzr and hg on one of my machines > > One defect found: "\s" is not portable on all grep versions > A "*" is not a "basic regular expression", so we need to use egrep Sorry for confusion: A "*" is a basic regular expression, but not the "+" "+" is an extended regular expression, which is understood by some grep versions (gnu ?). Felipe: Should I send a patch, or wait for a re-roll? -- 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] fast-export: Allow pruned-references in mark file
fast-export can fail because of some pruned-reference when importing a
mark file.
The problem happens in the following scenario:
$ git fast-export --export-marks=MARKS master
(rewrite master)
$ git prune
$ git fast-export --import-marks=MARKS master
This might fail if some references have been removed by prune
because some marks will refer to no longer existing commits.
git-fast-export will not need these objects anyway as they were no
longer reachable.
We still need to update last_numid so we don't change the mapping
between marks and objects for remote-helpers.
Unfortunately, the mark file should not be rewritten without lost marks
if no new objects has been exported, as we could lose track of the last
last_numid.
Signed-off-by: Antoine Pelisse
---
Documentation/git-fast-export.txt |2 ++
builtin/fast-export.c | 11 +++
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-fast-export.txt
b/Documentation/git-fast-export.txt
index d6487e1..feab7a3 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -66,6 +66,8 @@ produced incorrect results if you gave these options.
incremental runs. As is only opened and truncated
at completion, the same path can also be safely given to
\--import-marks.
+ The file will not be written if no new object has been
+ marked/exported.
--import-marks=::
Before processing any input, load the marks specified in
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d380155..f44b76c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -618,9 +618,12 @@ static void import_marks(char *input_file)
|| *mark_end != ' ' || get_sha1(mark_end + 1, sha1))
die("corrupt mark line: %s", line);
+ if (last_idnum < mark)
+ last_idnum = mark;
+
object = parse_object(sha1);
if (!object)
- die ("Could not read blob %s", sha1_to_hex(sha1));
+ continue;
if (object->flags & SHOWN)
error("Object %s already has a mark",
sha1_to_hex(sha1));
@@ -630,8 +633,6 @@ static void import_marks(char *input_file)
continue;
mark_object(object, mark);
- if (last_idnum < mark)
- last_idnum = mark;
object->flags |= SHOWN;
}
@@ -645,6 +646,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
struct string_list extra_refs = STRING_LIST_INIT_NODUP;
struct commit *commit;
char *export_filename = NULL, *import_filename = NULL;
+ uint32_t lastimportid;
struct option options[] = {
OPT_INTEGER(0, "progress", &progress,
N_("show progress after objects")),
@@ -688,6 +690,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
if (import_filename)
import_marks(import_filename);
+ lastimportid = last_idnum;
if (import_filename && revs.prune_data.nr)
full_tree = 1;
@@ -710,7 +713,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
handle_tags_and_duplicates(&extra_refs);
- if (export_filename)
+ if (export_filename && lastimportid != last_idnum)
export_marks(export_filename);
if (use_done_feature)
--
1.7.9.5
--
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 0/4] remote-helpers: trivial test fixes
On 04.04.13 17:36, Felipe Contreras wrote: > Hi, > > A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the > outsanding shell portability issue. The rest is the same. > > Felipe Contreras (4): > remote-bzr: avoid echo -n > remote-helpers: fix the run of all tests > remote-bzr: remove stale check code for tests > remote-hg: fix hg-git test-case > > contrib/remote-helpers/Makefile | 1 + > contrib/remote-helpers/test-bzr.sh | 16 +--- > contrib/remote-helpers/test-hg-hg-git.sh | 1 - > 3 files changed, 2 insertions(+), 16 deletions(-) > Sorry being late, now I installed bzr and hg on one of my machines One defect found: "\s" is not portable on all grep versions A "*" is not a "basic regular expression", so we need to use egrep diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh index 5f81dfa..2e80c11 100755 --- a/contrib/remote-helpers/test-hg.sh +++ b/contrib/remote-helpers/test-hg.sh @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' ' git push ) && - hg -R hgrepo bookmarks | grep "devel\s\+3:" + hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:" ' 2 mninor nits: diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh index 8450432..7970f9e 100755 --- a/contrib/remote-helpers/test-bzr.sh +++ b/contrib/remote-helpers/test-bzr.sh @@ -13,7 +13,7 @@ if ! test_have_prereq PYTHON; then fi if ! "$PYTHON_PATH" -c 'import bzrlib'; then - skip_all='skipping remote-bzr tests; bzr not available' + skip_all='skipping remote-bzr tests; python bzrlib not available' test_done fi diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh index 3f253b7..5d87282 100755 --- a/contrib/remote-helpers/test-hg-hg-git.sh +++ b/contrib/remote-helpers/test-hg-hg-git.sh @@ -21,7 +21,7 @@ if ! "$PYTHON_PATH" -c 'import mercurial'; then fi if ! "$PYTHON_PATH" -c 'import hggit'; then - skip_all='skipping remote-hg tests; hg-git not available' + skip_all='skipping remote-hg tests; python hggit not available' test_done fi (And as a micro-nit: the indenting deserves some better indentation: TAB could be used for *.sh, and sub-shells could be indentented one TAB: test_expect_success 'update bookmark' ' test_when_finished "rm -rf gitrepo*" && ( cd hgrepo && hg bookmark devel /Torsten -- 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] remote-hg: activate graplog extension for hg_log()
The hg_log() test helper uses the "--graph" parameter that is
implemented by the GraphLog extension. If the extension is not activated
by the user, the parameter is not available. Activate the extension in
setup().
Also changes the way we grep the output in hg_log(). The pipe operator
can hide the return code of hg command. As a matter of fact, if log
fails because it doesn't know about "--graph", it doesn't report any
failure and let's you think everything worked.
Signed-off-by: Antoine Pelisse
---
- Updated the title to use remote-hg instead of remote-helpers
- Activate the extension instead of removing the option
- Apply the same to test-hg-hg-git
contrib/remote-helpers/test-hg-bidi.sh |5 -
contrib/remote-helpers/test-hg-hg-git.sh |4 +++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/contrib/remote-helpers/test-hg-bidi.sh
b/contrib/remote-helpers/test-hg-bidi.sh
index 1d61982..84b9228 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -50,7 +50,8 @@ hg_push () {
}
hg_log () {
- hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+ hg -R $1 log --graph --debug >log &&
+ grep -v 'tag: *default/' log
}
setup () {
@@ -62,6 +63,8 @@ setup () {
echo "commit = -d \"0 0\""
echo "debugrawcommit = -d \"0 0\""
echo "tag = -d \"0 0\""
+ echo "[extensions]"
+ echo "graphlog ="
) >> "$HOME"/.hgrc &&
git config --global remote-hg.hg-git-compat true
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh
b/contrib/remote-helpers/test-hg-hg-git.sh
index 7e3967f..17a074e 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -78,7 +78,8 @@ hg_push_hg () {
}
hg_log () {
- hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+ hg -R $1 log --graph --debug >log &&
+ grep -v 'tag: *default/' log
}
git_log () {
@@ -97,6 +98,7 @@ setup () {
echo "[extensions]"
echo "hgext.bookmarks ="
echo "hggit ="
+ echo "graphlog ="
) >> "$HOME"/.hgrc &&
git config --global receive.denycurrentbranch warn
git config --global remote-hg.hg-git-compat true
--
1.7.9.5
--
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] remote-helpers: remove --graph in hg_log()
On Sat, Apr 6, 2013 at 10:12 AM, Antoine Pelisse wrote: > On Sat, Apr 6, 2013 at 6:06 PM, Felipe Contreras > wrote: >> On Sat, Apr 6, 2013 at 10:00 AM, Antoine Pelisse wrote: >>> I'm not so confident that --graph is useless to the test. If it's really >>> necessary, it would be nice either to activate it in setup() or to use >>> it just for the command through: "--config extensions.graphlog=". >> >> I think it should be activated in the setup, it comes packaged with >> mercurial, and it's likely that many users have it enabled. > > But is it relevant to the tests ? I have the feeling that it's not > strictly necessary to both add an extension to hgrc and a command line > option. (and indeed, the tests still work for me, but maybe I'm > missing something). It's possible that the order of the commits make them look the same, but the topology is different. At least I saw a couple of cases when I was working on them, but of course, that was on test-hg-hg-git.sh, which should also have a patch. I think adding it in hgrc is the best option. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-helpers: remove --graph in hg_log()
On Sat, Apr 6, 2013 at 6:06 PM, Felipe Contreras wrote: > On Sat, Apr 6, 2013 at 10:00 AM, Antoine Pelisse wrote: >> I'm not so confident that --graph is useless to the test. If it's really >> necessary, it would be nice either to activate it in setup() or to use >> it just for the command through: "--config extensions.graphlog=". > > I think it should be activated in the setup, it comes packaged with > mercurial, and it's likely that many users have it enabled. But is it relevant to the tests ? I have the feeling that it's not strictly necessary to both add an extension to hgrc and a command line option. (and indeed, the tests still work for me, but maybe I'm missing something). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-helpers: remove --graph in hg_log()
On Sat, Apr 6, 2013 at 10:00 AM, Antoine Pelisse wrote: > The hg_log() test helper uses the "--graph" parameter that is > implemented by the GraphLog extension. If the extension is not activated > by the user, the parameter is not available. Do not use the option that > is unnecessary. > I'm not so confident that --graph is useless to the test. If it's really > necessary, it would be nice either to activate it in setup() or to use > it just for the command through: "--config extensions.graphlog=". I think it should be activated in the setup, it comes packaged with mercurial, and it's likely that many users have it enabled. > Also changes the way we grep the output in hg_log(). The pipe operator > can hide the return code of hg command. As a matter of fact, if log > fails because it doesn't know about "--graph", it doesn't report any > failure and let's you think everything worked. Yeap, that is good to have. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] format-patch: add format.cover-letter configuration
On Sat, Apr 6, 2013 at 6:43 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> Also, add a new option: 'auto', so if there's more than one patch, the >> cover letter is generated, otherwise it's not. > > Awesome! I wanted to fix this myself, but got sidetracked with the > whole submodules thing. > >> +format.cover-letter:: >> + Allows to configure the --cover-letter option of format-patch by >> + default. In addition, you can set it to 'auto' to automatically >> + determine based on the number of patches (generate if there's more >> than >> + one). >> + > > Perhaps you can clarify this: Controls whether to generate a > cover-letter when format-patch is invoked. Can be true, false, or > auto. "auto" generates a cover-letter only when generating more than > one patch. That's good, but I believe if we say it's a boolean, true and false are implied, and then we have an extra "auto". -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] remote-helpers: remove --graph in hg_log()
The hg_log() test helper uses the "--graph" parameter that is
implemented by the GraphLog extension. If the extension is not activated
by the user, the parameter is not available. Do not use the option that
is unnecessary.
Also changes the way we grep the output in hg_log(). The pipe operator
can hide the return code of hg command. As a matter of fact, if log
fails because it doesn't know about "--graph", it doesn't report any
failure and let's you think everything worked.
Signed-off-by: Antoine Pelisse
---
Hey Felipe,
I'm not so confident that --graph is useless to the test. If it's really
necessary, it would be nice either to activate it in setup() or to use
it just for the command through: "--config extensions.graphlog=".
Cheers,
contrib/remote-helpers/test-hg-bidi.sh |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/remote-helpers/test-hg-bidi.sh
b/contrib/remote-helpers/test-hg-bidi.sh
index 1d61982..96cd5a7 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -50,7 +50,8 @@ hg_push () {
}
hg_log () {
- hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+ hg -R $1 log --debug >log &&
+ grep -v 'tag: *default/' log
}
setup () {
--
1.7.9.5
--
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 00/13] remote-hg: general updates
On Sat, Apr 6, 2013 at 8:09 AM, Philip Oakley wrote: > From: "Felipe Contreras" > Sent: Saturday, April 06, 2013 1:45 AM >> Ultimately this is not about people, this is about the code. > > > In the case of helper functions this is not the case. > > The question would be better framed: > "Does this, or that, helper function make users (people) feel helped, or > frustrated (or somewhere in limbo)?". And that question is ultimately answered by code. > I've called IT help desks and often felt frustrated, and some times I've got > one of the good girls/guys who worked with me to improve my situation (often > despite official policies). I get back to those folks (even if they > 'failed'). That is not an issue when you don't have a situation that needs support, when everything just works. Having one issue with bad support is better than having 5 issues with good support. > It's not a binary black/white issue when real users need help. It's no good > keeping with the faith (e.g. the Git ideal, the coders ideal, ..) when the > users (a mixed group) environmental doctine differs. And neither Max or Jed are users, so this doesn't apply to them. As for actual users, well show me, which remote-hg users have had bad support? https://github.com/felipec/git/issues >>A sensible person that is not emotionally attached to any code, > > [I'm thinking users here, they are emotionally attached to their original > problem, and sense doesn't come into it] That's irrelevant, a sensible developer would maximize the amount of issues that do **not** hit the users. >> would simply look at the code, > > Unfortunately, even for reasonable coders, looking at the code isn't usually > the case because of lack of time, unfamiliarity with the code, extent of the > code, availablity of the code (they may be simply running a > packaged/compiled 'app'), this is not that likely to happen. We should be > thankful when folk do look. If this sensible developer doesn't have time to look at remote-hg code, he has less time to make a possibly inferior alternative work on par or better. Thus a sensible developer would either look at remote-hg code, or do not develop. > It's hard enough to get "good" bug reports from fellow coders (they are only > human / no more human than us) that tell us what _we_ want to know (rather > than what _they_ remember, or was important to them). ;-) Nobody is asking for that. > I don't use Hg, but as I read the discussion, there are incomaptibilities > between Git, and Hg. Thus neither helper can ever be perfect. The winners > will be those who solve a user need with enough documentation and error > capture to make them (their user group) feel happy. At the moment it looks > like the discussion is stratifying into various "it worked for me" camps, > each with their own problem children repos that won't respond to parental > advice, even with a --force from social services. I don't think so. The discussion has been about hypothetical, not real users. The one person that did claim was hit by a bug, had no evidence for it nor cared, so it hardly matters. The rest is in pondering such as if the user does this and that, and somebody else in the team does that, and then the user does this, and the team has that policy, they might end in a bit of a kerfuffle. Not something that has _actually_ happened. And I'm confident that with time it will be shown that in terms of real issues, remote-hg would have less, and be fixed faster. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/13] remote-hg: general updates
From: "Felipe Contreras" Sent: Saturday, April 06, 2013 1:45 AM On Fri, Apr 5, 2013 at 4:30 PM, Max Horn wrote: On 04.04.2013, at 08:42, Felipe Contreras wrote: Please consider [...] Ultimately this is not about people, this is about the code. In the case of helper functions this is not the case. The question would be better framed: "Does this, or that, helper function make users (people) feel helped, or frustrated (or somewhere in limbo)?". I've called IT help desks and often felt frustrated, and some times I've got one of the good girls/guys who worked with me to improve my situation (often despite official policies). I get back to those folks (even if they 'failed'). It's not a binary black/white issue when real users need help. It's no good keeping with the faith (e.g. the Git ideal, the coders ideal, ..) when the users (a mixed group) environmental doctine differs. A sensible person that is not emotionally attached to any code, [I'm thinking users here, they are emotionally attached to their original problem, and sense doesn't come into it] would simply look at the code, Unfortunately, even for reasonable coders, looking at the code isn't usually the case because of lack of time, unfamiliarity with the code, extent of the code, availablity of the code (they may be simply running a packaged/compiled 'app'), this is not that likely to happen. We should be thankful when folk do look. It's hard enough to get "good" bug reports from fellow coders (they are only human / no more human than us) that tell us what _we_ want to know (rather than what _they_ remember, or was important to them). ;-) I don't use Hg, but as I read the discussion, there are incomaptibilities between Git, and Hg. Thus neither helper can ever be perfect. The winners will be those who solve a user need with enough documentation and error capture to make them (their user group) feel happy. At the moment it looks like the discussion is stratifying into various "it worked for me" camps, each with their own problem children repos that won't respond to parental advice, even with a --force from social services. Philip [As they say back home: Between thee and me, ther's nowt so queer as fowk, and I ain't so sure about thee] -- 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
typo in Documentation/git-rebase.txt
Hello, please change in Documentation/git-rebase.txt, as of v1.8.2-470-g21ccebe, for --ignore-whitespace and --whitespace, line 326 from These flag are passed to the 'git apply' program to (add an 's') These flags are passed to the 'git apply' program and in the same file, under --autosqash, --no-autosquash, last paragraph If the '--autosquash' option is enabled by default using the configuration variable `rebase.autosquash`, this option can be used to override and disable this setting. please delete "by default". With "by default" comes the question, what is the default value, if rebase.autosquash is not set. I suggest as replacement text If the '--autosquash' option is enabled using the configuration variable 'rebase.autosquash', '--no-autosqash' can be used to override and disable this setting. Even "and disable" is redundant and can be skipped. Със здраве Дилян -- 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: Collective wisdom about repos on NFS accessed by concurrent clients (== corruption!?)
> -Original Message- > From: Thomas Rast > Sent: Saturday, April 06, 2013 4:12 > > Kenneth Ölwing writes: > > > On 2013-04-05 15:42, Thomas Rast wrote: > >> Can you run the same tests under strace or similar, and gather the > >> relevant outputs? Otherwise it's probably very hard to say what is > >> going wrong. In particular we've had some reports on lustre that > >> boiled down to "impossible" returns from libc functions, not git > >> issues. It's hard to say without some evidence. > > Thomas, thanks for your reply. > > > > I'm assuming I should strace the git commands as they're > issued? I'm > > already collecting regular stdout/err output in a log as I go. Is > > there any debugging things I can turn on to make the calls issue > > internal tracing of some sort? > > I don't think there's any internal debugging that helps at this point. > Usually errors pointing to corruption are caused by a chain > of syscalls failing in some way, and the final error shows > only the last one, so > strace() output is very interesting. > > > The main issue I see is that I suspect it will generate so > much data > > that it'll overflow my disk ;-). > > Well, assuming you have some automated way of detecting when > it fails, you can just overwrite the same strace output file > repeatedly; we're only interested in the last one (or all the > last ones if several gits fail concurrently). We use tmpwatch for this type of issue, especially with oracle traces. Set up a directory and tell tmpwatch to delete files older than X. This will keep the files at bay and when you detect a problem stop the clean up script. > > Fiddling with strace will unfortunately change the timings > somewhat (causing a bunch of extra context switches per > syscall), but I hope that you can still get it to reproduce. -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- - - - Jason Pyeron PD Inc. http://www.pdinc.us - - Principal Consultant 10 West 24th Street #100- - +1 (443) 269-1555 x333Baltimore, Maryland 21218 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- This message is copyright PD Inc, subject to license 20080407P00. -- 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] format-patch: add format.cover-letter configuration
Felipe Contreras wrote: > Also, add a new option: 'auto', so if there's more than one patch, the > cover letter is generated, otherwise it's not. Awesome! I wanted to fix this myself, but got sidetracked with the whole submodules thing. > +format.cover-letter:: > + Allows to configure the --cover-letter option of format-patch by > + default. In addition, you can set it to 'auto' to automatically > + determine based on the number of patches (generate if there's more > than > + one). > + Perhaps you can clarify this: Controls whether to generate a cover-letter when format-patch is invoked. Can be true, false, or auto. "auto" generates a cover-letter only when generating more than one patch. 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 v2 1/2] perl: redirect stderr to /dev/null instead of closing
On Sat, Apr 06, 2013 at 10:07:40AM +0200, Thomas Rast wrote: > The manpage for dup2 does, however, say > >If newfd was open, any errors that would have been reported at >close(2) time are lost. A careful programmer will not use dup2() or >dup3() without closing newfd first. > > which is probably what you were referring to. Yes, that's probably one reason why I had this stuck in my mind (though, how often does anyone bother to detect errors on close()...? ;-). Funnily enough, POSIX.2008 specifies that if closing newfd would fail, dup2() reports EIO and newfd is not closed, eliminating this problem. The manpage does not cover this; well, that's fair enough as Linux just doesn't care and never does that if I didn't miss anything in the code. -- Petr "Pasky who might even send a patch, but the matter is oh so obscure" Baudis -- 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] format-patch: add format.cover-letter configuration
Also, add a new option: 'auto', so if there's more than one patch, the
cover letter is generated, otherwise it's not.
This has the slight disadvantage that a piece of code will always be run
even if the user doesn't want a cover letter, and thus waste a few
cycles. But the convenience is well worth it.
Signed-off-by: Felipe Contreras
---
Documentation/config.txt | 6 +
Documentation/git-format-patch.txt | 5 ++--
builtin/log.c | 55 +++---
3 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c8e2178..c10195c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1092,6 +1092,12 @@ format.signoff::
the rights to submit this work under the same open source license.
Please see the 'SubmittingPatches' document for further discussion.
+format.cover-letter::
+ Allows to configure the --cover-letter option of format-patch by
+ default. In addition, you can set it to 'auto' to automatically
+ determine based on the number of patches (generate if there's more than
+ one).
+
filter..clean::
The command which is used to convert the content of a worktree
file to a blob upon checkin. See linkgit:gitattributes[5] for
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 3a62f50..e1f5730 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -20,7 +20,7 @@ SYNOPSIS
[--ignore-if-in-upstream]
[--subject-prefix=Subject-Prefix] [(--reroll-count|-v) ]
[--to=] [--cc=]
- [--cover-letter] [--quiet] [--notes[=]]
+ [--[no-]cover-letter] [--quiet] [--notes[=]]
[]
[ | ]
@@ -195,7 +195,7 @@ will want to ensure that threading is disabled for `git
send-email`.
`Cc:`, and custom) headers added so far from config or command
line.
---cover-letter::
+--[no-]cover-letter::
In addition to the patches, generate a cover letter file
containing the shortlog and the overall diffstat. You can
fill in a description in the file before sending it out.
@@ -260,6 +260,7 @@ attachments, and sign off patches with configuration
variables.
cc =
attach [ = mime-boundary-string ]
signoff = true
+ cover-letter = auto
diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..ed89c10 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -618,6 +618,7 @@ static void add_header(const char *value)
#define THREAD_DEEP 2
static int thread;
static int do_signoff;
+static int cover_letter;
static const char *signature = git_version_string;
static int git_format_config(const char *var, const char *value, void *cb)
@@ -680,6 +681,17 @@ static int git_format_config(const char *var, const char
*value, void *cb)
}
if (!strcmp(var, "format.signature"))
return git_config_string(&signature, var, value);
+ if (!strcmp(var, "format.cover-letter")) {
+ if (cover_letter != -1)
+ /* user overrode it */
+ return 0;
+ if (value && !strcasecmp(value, "auto")) {
+ cover_letter = -1;
+ return 0;
+ }
+ cover_letter = git_config_bool(var, value);
+ return 0;
+ }
return git_log_config(var, value, cb);
}
@@ -1080,7 +1092,6 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
int start_number = -1;
int just_numbers = 0;
int ignore_if_in_upstream = 0;
- int cover_letter = 0;
int boundary_count = 0;
int no_binary_diff = 0;
struct commit *origin = NULL, *head = NULL;
@@ -1318,28 +1329,26 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
*/
rev.show_root_diff = 1;
- if (cover_letter) {
- /*
-* NEEDSWORK:randomly pick one positive commit to show
-* diffstat; this is often the tip and the command
-* happens to do the right thing in most cases, but a
-* complex command like "--cover-letter a b c ^bottom"
-* picks "c" and shows diffstat between bottom..c
-* which may not match what the series represents at
-* all and totally broken.
-*/
- int i;
- for (i = 0; i < rev.pending.nr; i++) {
- struct object *o = rev.pending.objects[i].item;
- if (!(o->flags & UNINTERESTING))
- head = (struct commit *)o;
- }
- /* There is nothing to show; it is not an error, though. */
- if (!head)
-
[PATCH v2 1/2] send-email: make annotate configurable
Some people always do --annotate, lets not force them to always type
that.
Signed-off-by: Felipe Contreras
---
Documentation/config.txt | 1 +
Documentation/git-send-email.txt | 5 +++--
git-send-email.perl | 12 +---
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bbba728..c8e2178 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1994,6 +1994,7 @@ sendemail..*::
sendemail.aliasesfile::
sendemail.aliasfiletype::
+sendemail.annotate::
sendemail.bcc::
sendemail.cc::
sendemail.cccmd::
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..2facc18 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -45,8 +45,9 @@ Composing
~
--annotate::
- Review and edit each patch you're about to send. See the
- CONFIGURATION section for 'sendemail.multiedit'.
+ Review and edit each patch you're about to send. Default is the value
+ of 'sendemail.annotate'. See the CONFIGURATION section for
+ 'sendemail.multiedit'.
--bcc=::
Specify a "Bcc:" value for each email. Default is the value of
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..e7fe9fb 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -54,7 +54,7 @@ git send-email [options]
--[no-]bcc* Email Bcc:
--subject * Email "Subject:"
--in-reply-to * Email "In-Reply-To:"
---annotate * Review each patch that will be sent in an
editor.
+--[no-]annotate* Review each patch that will be sent in an
editor.
--compose * Open an editor for introduction.
--compose-encoding* Encoding to assume for introduction.
--8bit-encoding * Encoding to assume 8bit mails if
undeclared
@@ -143,7 +143,7 @@ my $auth;
# Variables we fill in automatically, or via prompting:
my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,
$initial_reply_to,$initial_subject,@files,
- $author,$sender,$smtp_authpass,$annotate,$compose,$time);
+ $author,$sender,$smtp_authpass,$annotate,$no_annotate,$compose,$time);
my $envelope_sender;
@@ -212,7 +212,8 @@ my %config_bool_settings = (
"signedoffbycc" => [\$signed_off_by_cc, undef],
"signedoffcc" => [\$signed_off_by_cc, undef], # Deprecated
"validate" => [\$validate, 1],
-"multiedit" => [\$multiedit, undef]
+"multiedit" => [\$multiedit, undef],
+"annotate" => [\$annotate, undef]
);
my %config_settings = (
@@ -305,6 +306,7 @@ my $rc = GetOptions("h" => \$help,
"smtp-domain:s" => \$smtp_domain,
"identity=s" => \$identity,
"annotate" => \$annotate,
+ "no-annotate" => \$no_annotate,
"compose" => \$compose,
"quiet" => \$quiet,
"cc-cmd=s" => \$cc_cmd,
@@ -389,6 +391,10 @@ foreach my $setting (values %config_bool_settings) {
${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]}));
}
+if ($no_annotate) {
+ $annotate = 0;
+}
+
# 'default' encryption is none -- this only prevents a warning
$smtp_encryption = '' unless (defined $smtp_encryption);
--
1.8.2
--
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/2] send-email: couple of improvements
Hi, First patch was already sent, I just added the --no-annotate option. Second one is new, it adds a configuration for --cover-letter, so it can automatically determine when to generated: 1 patch, no cover, otherwise there is. Felipe Contreras (2): send-email: make annotate configurable format-patch: add format.cover-letter configuration Documentation/config.txt | 7 + Documentation/git-format-patch.txt | 5 ++-- Documentation/git-send-email.txt | 5 ++-- builtin/log.c | 55 +++--- git-send-email.perl| 12 ++--- 5 files changed, 55 insertions(+), 29 deletions(-) -- 1.8.2 -- 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: Collective wisdom about repos on NFS accessed by concurrent clients (== corruption!?)
Kenneth Ölwing writes:
> On 2013-04-05 15:42, Thomas Rast wrote:
>> Can you run the same tests under strace or similar, and gather the
>> relevant outputs? Otherwise it's probably very hard to say what is
>> going wrong. In particular we've had some reports on lustre that
>> boiled down to "impossible" returns from libc functions, not git
>> issues. It's hard to say without some evidence.
> Thomas, thanks for your reply.
>
> I'm assuming I should strace the git commands as they're issued? I'm
> already collecting regular stdout/err output in a log as I go. Is
> there any debugging things I can turn on to make the calls issue
> internal tracing of some sort?
I don't think there's any internal debugging that helps at this point.
Usually errors pointing to corruption are caused by a chain of syscalls
failing in some way, and the final error shows only the last one, so
strace() output is very interesting.
> The main issue I see is that I suspect it will generate so much data
> that it'll overflow my disk ;-).
Well, assuming you have some automated way of detecting when it fails,
you can just overwrite the same strace output file repeatedly; we're
only interested in the last one (or all the last ones if several gits
fail concurrently).
Fiddling with strace will unfortunately change the timings somewhat
(causing a bunch of extra context switches per syscall), but I hope that
you can still get it to reproduce.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] perl: redirect stderr to /dev/null instead of closing
Petr Baudis writes:
> On Fri, Apr 05, 2013 at 11:57:19AM -0700, Junio C Hamano wrote:
> The thing is, I was confused about dup2() all along as my old UNIX
> masters taught me that I must close() the original descriptor first
> and since that's what's commonly done anyway, I never thought to
> double-check. Now I did and I learned something new, thanks!
Indeed, that's the crucial point here. dup2() is defined to close the
original FD first if needed.
It's much saner this way for the case of stderr, as there is no time
when we have no stderr available to report errors: the FD is replace
atomically from the POV of the program.
The manpage for dup2 does, however, say
If newfd was open, any errors that would have been reported at
close(2) time are lost. A careful programmer will not use dup2() or
dup3() without closing newfd first.
which is probably what you were referring to.
--
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
Aw: Re: [PATCH 0/3] Some small fixes to glossary-content.txt
> > > While proof-reading the user-manual I noticed some issues with > glossary-content.txt: > > I found now mention of my patches in the latest "What's cooking". Did I miss an action item on my side or got this lost in the noise? --- Thomas -- 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: [RFD] annnotating a pair of commit objects?
On 04/05/2013 09:36 PM, Antoine Pelisse wrote: > On Thu, Jan 3, 2013 at 10:59 AM, Michael Haggerty > wrote: >> How could M be stored? Assuming that these type of premerge merges are >> sparse, then Jeff's analysis seems good. Concretely, one could simply >> store pointers to M from both X and Y; e.g., >> >> * Add a note to X with the information "when merging this commit with Y, >> use premerge M" >> >> * Add a note to Y with the information "when merging this commit with X, >> use premerge M" >> >> Then, when merging, create the set J..B, scan all of the commits on B..J >> for these "premerge" notes (O(|B..J|)), and for each one, look in the >> set J..B to see if it is present. The effort should scale like >> >> O( |J..B| + |B..J| * lg(|J..B|) ) >> >> where, of course J and B could be exchanged for either aesthetic or >> performance reasons. (One would also need a mechanism for preventing M >> from being garbage-collected.) > > I like the idea of using notes and a kind of "pre-merge". The proposal > seems decent to me. > > Michael, have you started implementing such a thing ? If you did, I > would love to help as much as I can. > If you didn't, I would like to start implementing this feature (I > think I now have some time to do so). > Maybe that would require some kind of mentoring though. It could be a > nice opportunity for the community to improve that too as a fake > "gsoc" (no google, no summer, no student) No, I haven't pursued this topic per se. We don't use such a workflow on my projects, so it isn't a particular itch of mine. I am currently more interested in approaches to merging branches that have diverged "too far" from each other [1]. There will be some overlap with this idea and my "git-mergemate" project [2], if I ever find the time to make progress on it. For that project, I will also need to store lots of intermediate merge commits, in fact also merges between parts of two branches. I had planned to autogenerate branch names by simply sticking the SHA1s of the parents together, like maybe refs/mergemate/NAME/SHA1-SHA1 or refs/mergemate/NAME/SHA1/SHA1 where NAME is the overall name of a merge that is in progress. These references would be cleaned up when the merge was complete but would prevent garbage collection of the intermediate results until then. I would be happy to collaborate with you but it might not turn out so well because my time for open-sourcing is so limited and unpredictable. Michael [1] http://softwareswirl.blogspot.de/2012/12/the-conflict-frontier-of-nightmare-merge.html [2] https://github.com/mhagger/git-mergemate (yes I know the name sucks) -- 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
