Re: [PATCH] contrib: remote-helpers: add move warnings (v2.0)
On 5/13/2014 5:47 PM, Felipe Contreras wrote: Junio C Hamano wrote: Felipe Contreras writes: Sigh, you just don't seem to understand that you are thinking about a different issue. I don't think there's any other way I can explain it to you. Perhaps pointing out which commit(s) to revert might be a good point to start. Oh, now you realize it might be nice to avoid this regression I warned you about. Why don't you continue schooling me about what constitutes a regression? I'm such a slow learner. I was going to do more than pointing to commits, I was going to provide the fixes with test cases and a detailed explanation. But then you made your decision. I believe the regression in question, mentioned at the bottom of this post http://thread.gmane.org/gmane.comp.version-control.git/248263/focus=248269 "Since you are not going to do so, I do not feel compelled to fix the synchronization crash regression that is present in v2.0.0-rc2 and I already warned you about." is referring to this patch http://thread.gmane.org/gmane.comp.version-control.git/247546/focus=247549 but I admit, I'm getting a bit fuzzy around these discussions. -- .marius -- 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 format-patch --signature
On Tue, May 13, 2014 at 09:07:12AM -0700, Jonathan Nieder wrote:
> Hi,
>
> Jeremiah Mahler wrote:
>
> > # from a string
> > $ git format-patch --signature "from a string" origin
> >
> > # or from a file
> > $ git format-patch --signature ~/.signature origin
>
> Interesting. But... what if I want my patch to end with
>
> --
> /home/jrnieder/.signature
>
> ? It seems safer to introduce a separate --signature-file option.
>
It is probably smarter to avoid that corner case entirely.
Good idea.
> [...]
> > builtin/log.c | 26 --
> > 1 file changed, 24 insertions(+), 2 deletions(-)
>
> Tests?
>
I added a test which checks that a valid patch is produced and that
the signature from the file appears in the output.
> Thanks and hope that helps,
> Jonathan
Attached is a revised patch.
--
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
>From e5cbeaf50d85236d6dd53e64f8f7cf466b1acecd Mon Sep 17 00:00:00 2001
From: Jeremiah Mahler
Date: Tue, 13 May 2014 18:10:53 -0700
Subject: [PATCH] format-patch --signature-file
Added feature that allows a signature file to be used with format-patch.
$ git format-patch --signature-file ~/.signature origin
Now signatures with newlines and other special characters can
be easily included.
Signed-off-by: Jeremiah Mahler
---
builtin/log.c | 24
t/t4014-format-patch.sh | 13 +
2 files changed, 37 insertions(+)
diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..1ec733b 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1147,6 +1147,27 @@ static int from_callback(const struct option *opt, const char *arg, int unset)
return 0;
}
+static int signature_file_callback(const struct option *opt, const char *arg,
+ int unset)
+{
+ const char **signature = opt->value;
+ static char buf[1024];
+ size_t sz;
+ FILE *fp;
+
+ fp = fopen(arg, "r");
+ if (fp) {
+ sz = sizeof(buf);
+ sz = fread(buf, 1, sz - 1, fp);
+ buf[sz] = '\0';
+ *signature = buf;
+ fclose(fp);
+ } else {
+ *signature = arg;
+ }
+ return 0;
+}
+
int cmd_format_patch(int argc, const char **argv, const char *prefix)
{
struct commit *commit;
@@ -1230,6 +1251,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
PARSE_OPT_OPTARG, thread_callback },
OPT_STRING(0, "signature", &signature, N_("signature"),
N_("add a signature")),
+ { OPTION_CALLBACK, 0, "signature-file", &signature, N_("signature-file"),
+N_("add a signature from contents of a file"),
+ PARSE_OPT_NONEG, signature_file_callback },
OPT__QUIET(&quiet, N_("don't print the patch filenames")),
OPT_END()
};
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 9c80633..19b67e3 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -762,6 +762,19 @@ test_expect_success 'format-patch --signature="" suppresses signatures' '
! grep "^-- \$" output
'
+cat > expect << EOF
+Test User
+http://git.kernel.org/cgit/git/git.git
+git.kernel.org/?p=git/git.git;a=summary
+EOF
+
+test_expect_success 'format-patch --signature-file file' '
+ git format-patch --stdout --signature-file expect -1 >output &&
+ check_patch output &&
+ fgrep -x -f output expect >output2 &&
+ diff expect output2
+'
+
test_expect_success TTY 'format-patch --stdout paginates' '
rm -f pager_used &&
test_terminal env GIT_PAGER="wc >pager_used" git format-patch --stdout --all &&
--
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
Re: [PATCH v6 04/42] refs.c: make ref_update_reject_duplicates take a strbuf argument for errors
Ronnie Sahlberg wrote: > Make ref_update_reject_duplicates return any error that occurs through a > new strbuf argument. Sensible. The caller-visible effect would be that now ref_transaction_commit() can pass back a helpful error message through its "err" argument when asked to make multiple updates for the same ref. Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib: remote-helpers: add move warnings (v2.0)
Ronnie Sahlberg wrote: > Could you please calm down and adjust your behavior. This constant > hostility and rudeness makes the mailing list very unpleasant. I explaind that to him multiple times. In the mail I replied to he is once again assuming I'm a *insert-your-favorite-non-smart-adjective*, and explaining to me what a regression is. How many times must one repeat something before one is entitled so thay something is not "getting through the skull" of another person? 5? 10? 20? But fine, let's assume I do have to adjust my behavior. Maybe I should have said "it doesn't register in your brain", or just "it fails to grab your attention". But if I have to adjust for saying that (which was true), what do you say to Junio for saying this? (which was not) > > Stop this idiocy. I presume nothing, because Junio is a riskier target. -- 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: Watchman support for git
On Sat, 2014-05-10 at 15:16 +0700, Duy Nguyen wrote: > On Sat, May 3, 2014 at 6:14 AM, wrote: > > The most sigificant patch uses Facebook's watchman daemon[1] to monitor > > the repository work tree for changes. This makes allows git status > > to avoid traversing the entire work tree to find changes. > > Some comments on this series. I still haven't been able to run it with > watchman so there are many guesses from my side. > > First of all, when I set USE_WATCHMAN=Yes in config.mak I expect it to > work out of the box, provided that all dependencies are installed. I > still need to set WATCHMAN_LIBS for it to build because you only set > it with configure script. Would be nice to have a default value for > non-configure people too. I'll fix that, thanks. > I'm not so happy that git now needs to link to libjansson.so and > libwatchman.so. I would load libwatchman.so at runtime only when > core.usewatchman is on, but this is more of personal taste. I assume you mean something with dlopen? I don't really like that because (a) nothing else in git works that way and (b) you would still need the libwatchman headers to build git (or the structs could be copied, but that is ugly). > I still prefer my old tracking model, where the majority of lstat() is > done by refresh operation and we only need to optimize those lstat > calls, not every single lstat statement in the code base. The lstat calls are only one of the problems. The others are opendir/readdir and open(.gitignore). > With that in > mind, I think you don't need to keep a fs cache on disk at all. All > you need to store (in the index) is the clock value from watchman. > After we parse the index, we perform a "since" query to get path names > (and perhaps "exists" and "mode" for later). Then we set CE_VALID bit > on entries that are _not_ in the query result. The remaining items > will be lstat'd by git (see [1] and read-cache.c changes on the next > few patches). Assuming the number of updated files is reasonably > small, we won't be punished by lookup time. I considered this, but rejected it for a few reasons: 1. We still need to know about the untracked files. I guess we could do an index extension for just that, like your untracked cache. 2. That doesn't eliminate opendir/readdir, I think. Those are a major cost. I did not thoroughly review your patches from January/February, but I didn't notice anything in there about this part of dir.c. Also the cost of open(nonexistent .gitignore) to do ignore processing. 3. Changing a file in the index (e.g. git add) requires clearing the CE_VALID bit; this means that third-party libraries (e.g. jgit) that change the git repo need to understand this extension for correct operation. Maybe that's just the nature of extensions, but it's not something that my present patch set requires. -- 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
What's cooking in git.git (May 2014, #03; Tue, 13)
What's cooking in git.git (May 2014, #03; Tue, 13)
--
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
The tip of the 'master' branch has past v2.0.0-rc3, which is in sync
with v1.9.3 with respect to bugfixes that was also tagged recently.
Hopefully we can tag v2.0.0 final late this week.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[New Topics]
* ab/add-interactive-show-diff-func-name (2014-05-12) 2 commits
- SQUASH??? git-add--interactive: Preserve diff heading when splitting hunks
- git-add--interactive: Preserve diff heading when splitting hunks
Waiting for a reroll.
* jk/do-not-run-httpd-tests-as-root (2014-05-12) 1 commit
- t/lib-httpd: require SANITY prereq
Will merge to 'next' and keep it there for the rest of this cycle.
* jk/index-pack-report-missing (2014-05-12) 1 commit
- index-pack: distinguish missing objects from type errors
Will merge to 'next' and keep it there for the rest of this cycle.
* tb/unicode-6.3-zero-width (2014-05-12) 2 commits
- utf8: make it easier to auto-update git_wcwidth()
- utf8.c: use a table for double_width
Update the logic to compute the display width needed for utf8
strings and allow us to more easily maintain the tables used in
that logic.
We may want to let the users choose if codepoints with ambiguous
widths are treated as a double or single width in a follow-up patch.
Will merge to 'next' and keep it there for the rest of this cycle.
* mk/show-s-no-extra-blank-line-for-merges (2014-05-13) 3 commits
- fixup! git-show: fix 'git show -s' to not add extra terminator after merge
commit
- t: git-show: adapt tests to fixed 'git show'
- git-show: fix 'git show -s' to not add extra terminator after merge commit
Waiting for a reroll.
* wk/doc-clarify-upstream (2014-05-13) 1 commit
- Documentation: mention config sources for @{upstream}
Will merge to 'next' and keep it there for the rest of this cycle.
--
[Stalled]
* tr/merge-recursive-index-only (2014-02-05) 3 commits
- merge-recursive: -Xindex-only to leave worktree unchanged
- merge-recursive: internal flag to avoid touching the worktree
- merge-recursive: remove dead conditional in update_stages()
(this branch is used by tr/remerge-diff.)
Will hold.
* tr/remerge-diff (2014-02-26) 5 commits
. log --remerge-diff: show what the conflict resolution changed
. name-hash: allow dir hashing even when !ignore_case
. merge-recursive: allow storing conflict hunks in index
. revision: fold all merge diff variants into an enum merge_diff_mode
. combine-diff: do not pass revs->dense_combined_merges redundantly
(this branch uses tr/merge-recursive-index-only.)
"log -p" output learns a new way to let users inspect a merge
commit by showing the differences between the automerged result
with conflicts the person who recorded the merge would have seen
and the final conflict resolution that was recorded in the merge.
Needs to be rebased, now kb/fast-hashmap topic is in.
* jk/makefile (2014-02-05) 16 commits
- FIXUP
- move LESS/LV pager environment to Makefile
- Makefile: teach scripts to include make variables
- FIXUP
- Makefile: auto-build C strings from make variables
- Makefile: drop *_SQ variables
- FIXUP
- Makefile: add c-quote helper function
- Makefile: introduce sq function for shell-quoting
- Makefile: always create files via make-var
- Makefile: store GIT-* sentinel files in MAKE/
- Makefile: prefer printf to echo for GIT-*
- Makefile: use tempfile/mv strategy for GIT-*
- Makefile: introduce make-var helper function
- Makefile: fix git-instaweb dependency on gitweb
- Makefile: drop USE_GETTEXT_SCHEME from GIT-CFLAGS
Simplify the Makefile rules and macros that exist primarily for
quoting purposes, and make it easier to robustly express the
dependency rules.
Expecting a reroll.
* po/everyday-doc (2014-01-27) 1 commit
- Make 'git help everyday' work
This may make the said command to emit something, but the source is
not meant to be formatted into a manual pages to begin with, and
also its contents are a bit stale. It may be a good first step in
the right direction, but needs more work to at least get the
mark-up right before public consumption.
Will hold.
* jk/branch-at-publish-rebased (2014-01-17) 5 commits
. t1507 (rev-parse-upstream): fix typo in test title
. implement @{publish} shorthand
. branch_get: provide per-branch pushremote pointers
. branch_get: return early on error
. sha1_name: refactor upstream_mark
Give an easier access to the tracking branches from "other" side in
a triangular workflow by introducing B@{publish} that works in a
similar wa
Re: [PATCH v2] contrib/subtree bugfix: Can't `add` annotated tag
James Denholm writes: > I'm not sure that can actually happen - peel_committish is essentially > implemented as `rev-parse $arg^0` (though with a bit of bling, of > course), and to my understanding FETCH_HEAD will always parse to a > committish - I could have missed something, of course. $ git fetch git://repo.or.cz/alt-git junio-gpg-pub $ git rev-parse FETCH_HEAD^0 -- 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: Watchman support for git
On Wed, 2014-05-14 at 05:54 +0700, Duy Nguyen wrote: > On Wed, May 14, 2014 at 5:38 AM, David Turner > wrote: > > On Mon, 2014-05-12 at 17:45 +0700, Duy Nguyen wrote: > >> This is your quote from above, moved down a bit: > >> > >> > update_fs_cache should only have to update based on what it has learned > >> > from watchman. So if no .gitignore has been changed, it should not have > >> > to do very much work. > >> > > >> > I could take the fe_excluded check and move it above the > >> > last_exclude_matching check in fs_cache_is_excluded; it causes t7300 to > >> > fail when run under watchman but presumably that's fixable > >> > >> So you exclude files early and make the real read_directory() pass do > >> pretty much nothing. This is probably not a good idea. Assume that I > >> touch $TOP/.gitignore then do something other than "git status" (or > >> "git add") then I have to pay read_directory() cost. > > > > I'm not sure I understand this. read_directory does something: it checks > > the fs_cache (instead of the filesystem) for untracked files. > > A lot of commands do read_cache() that that eventually calls > update_fs_cache, which does part of read_directory's work (the > fe_excluded thing). But not many of those commands actually call > read_directory(). It'd better if there's a way to mark that "this > .gitignore is changed", but delay the actual exclude processsing until > we are sure read_directory() will be used. OK, that would be straighforward, I think. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib: remote-helpers: add move warnings (v2.0)
On Tue, May 13, 2014 at 3:22 PM, Felipe Contreras wrote: > Junio C Hamano wrote: >> Felipe Contreras writes: >> >> > The tools are now maintained out-of-tree, and they have a regression in >> > v2.0. >> >> You seem not to understand at all what a regression is. >> >> My understanding is that versions of remote-hg shipped with all >> versions of Git did not work with Hg 3.0, so not working with Hg 3.0 >> is a regression in v2.0 at all. > > I explained to you multiple times already that is a different issue, but > it somehow doesn't get through your skull. Could you please calm down and adjust your behavior. This constant hostility and rudeness makes the mailing list very unpleasant. > > Let me try a different approach. > > git-remote-bzr has a regression in Git v2.0. > > Did you get the BAZAAR part? That's right, this is unrelated to > Mercurial v3.0 because it doesn't have anything to do with Mercurial. > > *BOTH* git-remote-hg and git-remote-bzr have a regression in Git v2.0. > >> A recent report was about Hg 3.0 not working with 1.9.3, but I think >> you earlier said all versions of Git does not work with Hg 3.0, and I >> can believe it. That is hardly a regression. >> >> You could argue that Hg has a new regression to its external users >> of its API when it went to 3.0. We actually had a similar breakage >> in 1.5.4, where it was reported late in the cycle after -rc0 [*1*] >> that cgit that linked with our internal API libgit.a was broken by a >> change on our side, which resulted in us fixing the breakage (even >> though technically you may be able to say that it was cgit's fault >> to link with libgit.a in the first place) with 18125644 (Move >> sha1_file_to_archive into libgit, 2008-01-14) very late in the >> cycle. Calling that a regression in cgit would have been insane, >> even if we did not patch our side up to accomodate it. >> >> Stop this idiocy. > > Sigh, you just don't seem to understand that you are thinking about a > different issue. I don't think there's any other way I can explain it to > you. > > -- > 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 -- 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 v6 03/42] refs.c: add a strbuf argument to ref_transaction_commit for error logging
Ronnie Sahlberg wrote:
> Add a strbuf argument to _commit so that we can pass an error string back to
> the caller. So that we can do error logging from the caller instead of from
> _commit.
>
> Longer term plan is to first convert all callers to use onerr==QUIET_ON_ERR
> and craft any log messages from the callers themselves and finally remove the
> onerr argument completely.
Very nice.
[...]
> +++ b/refs.c
[...]
> @@ -3443,6 +3444,9 @@ int ref_transaction_commit(struct ref_transaction
> *transaction,
> update->flags,
> &update->type, onerr);
> if (!update->lock) {
> + if (err)
> + strbuf_addf(err ,"Cannot lock the ref '%s'.",
> + update->refname);
Tiny nit: whitespace.
[...]
> --- a/refs.h
> +++ b/refs.h
> @@ -268,9 +268,12 @@ void ref_transaction_delete(struct ref_transaction
> *transaction,
> * Commit all of the changes that have been queued in transaction, as
> * atomically as possible. Return a nonzero value if there is a
> * problem. The ref_transaction is freed by this function.
> + * If err is non-NULL we will add an error string to it to explain why
> + * the transaction failed.
Probably worth mentioning the error string doesn't end with a newline
so the caller knows how to use it.
With the whitespace fix and with or without the comment tweak,
Reviewed-by: Jonathan Nieder
diff --git i/refs.c w/refs.c
index 64e8feb..2ca3169 100644
--- i/refs.c
+++ w/refs.c
@@ -3445,7 +3445,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
&update->type, onerr);
if (!update->lock) {
if (err)
- strbuf_addf(err ,"Cannot lock the ref '%s'.",
+ strbuf_addf(err, "Cannot lock the ref '%s'.",
update->refname);
ret = 1;
goto cleanup;
diff --git i/refs.h w/refs.h
index ff87e14..d45212f 100644
--- i/refs.h
+++ w/refs.h
@@ -268,8 +268,8 @@ void ref_transaction_delete(struct ref_transaction
*transaction,
* Commit all of the changes that have been queued in transaction, as
* atomically as possible. Return a nonzero value if there is a
* problem. The ref_transaction is freed by this function.
- * If err is non-NULL we will add an error string to it to explain why
- * the transaction failed.
+ * If err is non-NULL we will append an error string (with no trailing
+ * newline) to it to explain why the transaction failed.
*/
int ref_transaction_commit(struct ref_transaction *transaction,
const char *msg, struct strbuf *err,
--
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 01/17] contrib: remove outdated README
Junio C Hamano wrote: > Felipe Contreras writes: > > > Junio, do you honestly think I am a troll? > > You certainly are acting like one, aren't you? I'm deeply offended by the fact that would think that I'm purposely intent on provoking people, or disrupting more important discussions. I understand how my style of communication can upset people, mainly because people are not used to frankness. But I thought you of all people would see how much effort I've put into so many areas of Git, and therefore that my primary objective is to improve Git, not offend people. That you would understand that me offending people is a side-effect of me trying to improve Git, not that I improve Git just so I can offend people. I understand why you would choose not to reply to some mails that might be too flammable, or unimportant, or difficult. But in this case, the culmination of countless hours of work, what I had in mind since the beginning; that the tools graduate into the core, was finally there, and you took it away. And then you didn't give an explanation, and then you ignored me. I thought you would understand that most of the code that arrived to the mailing list had different versions behind, experiments, discussion in different channels, tests, and that was the reason why most of the code I submitted to remote-hg and remote-bzr simply worked, and it was simple. And why when other people did the same, the results were not so satisfactory. But no, apparently you didn't value my work at all. Maybe you thought each line I sent took me the time it takes to write a tweet, maybe you thought because it's in Python even kids in primary school could write it. In fact it's worth so little to you that it's not even worth the time to respond *one* question, not even in consideration of all these years of effort. And then you have the nerve to call me a troll on top of that? I'm done with you. Consider the bridge burned. Good bye. -- 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] contrib/subtree bugfix: Can't `add` annotated tag
On Tue, May 13, 2014 at 12:34:13PM -0700, Junio C Hamano wrote: > James Denholm writes: > > I felt that defining revp would be a little more self-documenting than > > using $rev^0. > > That is a good decision, but as long as we are attempting to peel, > don't we want to stop the damage when it does not peel to a commit? I'm not sure that can actually happen - peel_committish is essentially implemented as `rev-parse $arg^0` (though with a bit of bling, of course), and to my understanding FETCH_HEAD will always parse to a committish - I could have missed something, of course. subtree Will need error-catching at some point, of course, triggering resets or at least suggesting instructions to the user, but I think that's a touch out of the scope of a bugfix at this point (and, to be honest, I personally can't allocate the time to that for about a month due to the dark shadow of academic exams). Indeed, what to do in those cases is probably worth (re-)discussing the overall design and aims of subtree for, and so I'm not confident that I currently know the best way to do that. > I'll tentatively queue this. Thanks. Awesome, thanks again for this and the feedback. --- Regards, James Denholm. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib: remote-helpers: add move warnings (v2.0)
Junio C Hamano wrote: > Felipe Contreras writes: > > > Sigh, you just don't seem to understand that you are thinking about a > > different issue. I don't think there's any other way I can explain it to > > you. > > Perhaps pointing out which commit(s) to revert might be a good point > to start. Oh, now you realize it might be nice to avoid this regression I warned you about. Why don't you continue schooling me about what constitutes a regression? I'm such a slow learner. I was going to do more than pointing to commits, I was going to provide the fixes with test cases and a detailed explanation. But then you made your decision. This patch is what I'm suggesting you to do now. And I'll repeat what I already told you. Good luck with your tree. -- 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: Watchman support for git
On Wed, May 14, 2014 at 5:38 AM, David Turner wrote: > On Mon, 2014-05-12 at 17:45 +0700, Duy Nguyen wrote: >> This is your quote from above, moved down a bit: >> >> > update_fs_cache should only have to update based on what it has learned >> > from watchman. So if no .gitignore has been changed, it should not have >> > to do very much work. >> > >> > I could take the fe_excluded check and move it above the >> > last_exclude_matching check in fs_cache_is_excluded; it causes t7300 to >> > fail when run under watchman but presumably that's fixable >> >> So you exclude files early and make the real read_directory() pass do >> pretty much nothing. This is probably not a good idea. Assume that I >> touch $TOP/.gitignore then do something other than "git status" (or >> "git add") then I have to pay read_directory() cost. > > I'm not sure I understand this. read_directory does something: it checks > the fs_cache (instead of the filesystem) for untracked files. A lot of commands do read_cache() that that eventually calls update_fs_cache, which does part of read_directory's work (the fe_excluded thing). But not many of those commands actually call read_directory(). It'd better if there's a way to mark that "this .gitignore is changed", but delay the actual exclude processsing until we are sure read_directory() will be used. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 02/42] refs.c: allow passing NULL to ref_transaction_free
On Tue, May 13, 2014 at 3:44 PM, Jonathan Nieder wrote:
> Ronnie Sahlberg wrote:
>
>> Allow ref_transaction_free to be called with NULL and in extension allow
>> ref_transaction_rollback to be called for a NULL transaction.
>
> In extension = as a result?
>
> Makes sense. It lets someone do the usual
>
> struct ref_transaction *transaction;
> int ret = 0;
>
> if (something_fails()) {
> ret = -1;
> goto cleanup;
> }
> ...
>
> cleanup:
> ref_transaction_free(transaction);
> return ret;
>
> just like you can already do with free().
>
>> This allows us to write code that will
>>
>> if ( (!transaction ||
>> ref_transaction_update(...)) ||
>> (ref_transaction_commit(...) && !(transaction = NULL)) {
>> ref_transaction_rollback(transaction);
>> ...
>> }
>
> Somewhere in the whitespace and parentheses I'm lost.
>
> Is the idea that when ref_transaction_commit fails it will have
> freed the transaction so we need not to roll back to prevent a
> double free?
Yes. But also, this horribleness is also to illustrate a weak point in the API
in that ref_transaction_commit actually frees the transaction if
successful, so the
&& !(transaction = NULL) kludge is to avoid a double free in the
ref_transaction_rollback.
This is horrible, but all this is going away later in the patch
series when _commit is fixed so that
it does not free the transaction anymore.
When that patch comes in later in this series, this horribleness will go away.
I think it would be simpler for the caller to
> unconditionally set transaction to NULL after calling
> ref_transaction_commit in such a case to avoid use-after-free.
Yes. Later patches does that by having ref_transaction_commit not free
the transaction
and instead requiring the caller to explicitely free it by calling
ref_transaction_free.
Maybe see this as this is how ugly rollback is by the current _commit
semantics. Then see how beautiful it
all gets once _commit is repaired and the && !(transaction = NULL)
kludge is removed. :-)
>
> Even better if it is the caller's responsibility to free
> the transaction. At any rate, it doesn't seem related to this
> patch.
>
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3303,6 +3303,9 @@ static void ref_transaction_free(struct
>> ref_transaction *transaction)
>> {
>> int i;
>>
>> + if (!transaction)
>> + return;
>
> Except for the unclear commit message,
> Reviewed-by: Jonathan Nieder
--
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 v6 02/42] refs.c: allow passing NULL to ref_transaction_free
Ronnie Sahlberg wrote:
> Allow ref_transaction_free to be called with NULL and in extension allow
> ref_transaction_rollback to be called for a NULL transaction.
In extension = as a result?
Makes sense. It lets someone do the usual
struct ref_transaction *transaction;
int ret = 0;
if (something_fails()) {
ret = -1;
goto cleanup;
}
...
cleanup:
ref_transaction_free(transaction);
return ret;
just like you can already do with free().
> This allows us to write code that will
>
> if ( (!transaction ||
> ref_transaction_update(...)) ||
> (ref_transaction_commit(...) && !(transaction = NULL)) {
> ref_transaction_rollback(transaction);
> ...
> }
Somewhere in the whitespace and parentheses I'm lost.
Is the idea that when ref_transaction_commit fails it will have
freed the transaction so we need not to roll back to prevent a
double free? I think it would be simpler for the caller to
unconditionally set transaction to NULL after calling
ref_transaction_commit in such a case to avoid use-after-free.
Even better if it is the caller's responsibility to free
the transaction. At any rate, it doesn't seem related to this
patch.
> --- a/refs.c
> +++ b/refs.c
> @@ -3303,6 +3303,9 @@ static void ref_transaction_free(struct ref_transaction
> *transaction)
> {
> int i;
>
> + if (!transaction)
> + return;
Except for the unclear commit message,
Reviewed-by: Jonathan Nieder
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib: remote-helpers: add move warnings (v2.0)
Felipe Contreras writes: > Sigh, you just don't seem to understand that you are thinking about a > different issue. I don't think there's any other way I can explain it to > you. Perhaps pointing out which commit(s) to revert might be a good point to start. -- 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 01/17] contrib: remove outdated README
Martin Langhoff wrote: > I have had patches and contributions rejected in the past, sometimes > rudely. Same has happened to many others, if you contribute long > enough, it is pretty much guaranteed that it will happen to you. > Maintainer is wrong, or you are wrong, or someone is just having a bad > day. This is not about a couple of patches I worked in a weekend being rejected. This is about the work I've been doing since the past two years almost like a full-time job dropped to the floor with no explanation at all. I started with the expectation that they were going to move to the core, because Junio said so, then he changed his mind and didn't want to explain his reasoning. It's not just a bad day. -- 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: Watchman support for git
On Mon, 2014-05-12 at 17:45 +0700, Duy Nguyen wrote: > This is your quote from above, moved down a bit: > > > update_fs_cache should only have to update based on what it has learned > > from watchman. So if no .gitignore has been changed, it should not have > > to do very much work. > > > > I could take the fe_excluded check and move it above the > > last_exclude_matching check in fs_cache_is_excluded; it causes t7300 to > > fail when run under watchman but presumably that's fixable > > So you exclude files early and make the real read_directory() pass do > pretty much nothing. This is probably not a good idea. Assume that I > touch $TOP/.gitignore then do something other than "git status" (or > "git add") then I have to pay read_directory() cost. I'm not sure I understand this. read_directory does something: it checks the fs_cache (instead of the filesystem) for untracked files. > Back to the open vs fs_cache_open and the number of .gitignore files > above. I touch $TOP/.gitignore then do "git status" to make it read > all .gitignore files (6k of them) and change between open and > fs_cache_open. I think the numbers still do not make any visible > difference (~1620-1630ms). Yes, I would expect no win in that case. fs_cache_open will only save time in the common case where there is no .gitignore file, because it saves an open() call. If every possible .gitignore file exists, of course it makes no difference. But also, your processor may be sufficiently slow that the context-switch penalty for open() is less than the hash table lookup. For me, the win from fs_cache_open is about 7%. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib: remote-helpers: add move warnings (v2.0)
Junio C Hamano wrote: > Felipe Contreras writes: > > > The tools are now maintained out-of-tree, and they have a regression in > > v2.0. > > You seem not to understand at all what a regression is. > > My understanding is that versions of remote-hg shipped with all > versions of Git did not work with Hg 3.0, so not working with Hg 3.0 > is a regression in v2.0 at all. I explained to you multiple times already that is a different issue, but it somehow doesn't get through your skull. Let me try a different approach. git-remote-bzr has a regression in Git v2.0. Did you get the BAZAAR part? That's right, this is unrelated to Mercurial v3.0 because it doesn't have anything to do with Mercurial. *BOTH* git-remote-hg and git-remote-bzr have a regression in Git v2.0. > A recent report was about Hg 3.0 not working with 1.9.3, but I think > you earlier said all versions of Git does not work with Hg 3.0, and I > can believe it. That is hardly a regression. > > You could argue that Hg has a new regression to its external users > of its API when it went to 3.0. We actually had a similar breakage > in 1.5.4, where it was reported late in the cycle after -rc0 [*1*] > that cgit that linked with our internal API libgit.a was broken by a > change on our side, which resulted in us fixing the breakage (even > though technically you may be able to say that it was cgit's fault > to link with libgit.a in the first place) with 18125644 (Move > sha1_file_to_archive into libgit, 2008-01-14) very late in the > cycle. Calling that a regression in cgit would have been insane, > even if we did not patch our side up to accomodate it. > > Stop this idiocy. Sigh, you just don't seem to understand that you are thinking about a different issue. I don't think there's any other way I can explain it to you. -- 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: Error using git-remote-hg
William Giokas wrote:
> On Tue, May 13, 2014 at 03:24:51PM -0500, Felipe Contreras wrote:
> > William Giokas wrote:
> > > On Tue, May 13, 2014 at 02:09:55PM -0500, Felipe Contreras wrote:
> > > > As you say, it's perfectly OK.
> > >
> > > But wrong. Yes, it works, but it's not how it should be done when we
> > > have a code review such as this. It should simply not be done and makes
> > > no sense to do with an 'if ; else' kind of thing later in the
> > > application.
> >
> > That's exactly how it should be done. Maybe this visualization helps
> >
> > from mercurial import hg, ui, bookmarks, ...# for hg >= 3.0
> > from mercurial import node, error, extensions, ... # for hg >= 3.0
> > from mercurial import changegroup # for hg >= 3.0
> >
> > if check_version(3, 0):
> > cg = changegroup.getbundle(repo, 'push', ...# for hg >= 3.0
> > else:
> > cg = repo.getbundle('push', heads=list(p_revs) # for hg < 3.0
> >
> > Eventually the code would become:
> >
> > from mercurial import hg, ui, bookmarks, ...# for hg >= 3.0
> > from mercurial import node, error, extensions, ... # for hg >= 3.0
> > from mercurial import changegroup # for hg >= 3.0
> >
> > cg = changegroup.getbundle(repo, 'push', ...# for hg >= 3.0
>
> No, the way that it's going to change is that the import statements will
> change, not the 'if:else' things. It would work exactly the same with
> this, however the things that are used only in a specific version for
> this are stated up front. Maybe this visualization helps for what I have
> set up::
>
> from mercurial import ...# for all hg
>
> try:
> from mercurial.changegroup import getbundle # for hg with
># changegroup.getbundle,
># regardless of version
This would make sense if in our eyse all versions of Mercurial were
the same, and we would want the code to work optimally for all of them
forever.
But that's not the case. The current version of Mercurial is more
important, the fact that we have one unnecessary import in older
versions is not of consequence because eventually the won't be
supported.
> When we eventually remove support for mercurial that uses
> repo.getbundle:
>
> from mercurial import changegroup, ... # for all hg
> ...
> cg = changegroup.getbundle(...)
And the diff from my version to the final version is smaller.
> > The fact that at some point 'import changegroup' was not needed is
> > irrelevant.
> >
> > Primarily we want to support the current version of Mercurial.
> > Secondarily we want to support older versions. That's why we add the
> > repo.getbundle() code (as an addendum to the core part).
>
> So I use arch myself, and I am very used to the 'rolling release' model
> that it employs. I do agree that we should concentrate support for the
> latest release, but for a project like git the other versions of hg
> cannot be ignored, as this project is used on so many different systems.
Older versions are not ignored, they are supported. It's just not worth
tainting the code to avoid an 'import'.
> > > > We could try that, but that would assume we want to maintain getbundle()
> > > > for the long run, and I personally don't want to do that. I would rather
> > > > contact the Mercurial developers about ways in which the push() method
> > > > can be improved so we don't need to have our own version. Hopefully
> > > > after it's improved we wouldn't have to call getbundle().
> > >
> > > Assuming that mercurial <3.0 will not change, then this should never
> > > need to change.
> >
> > But it should. Otherwise the code will keep having more and more hacks
> > and it will become less and less maintainable.
> >
> > That's why we don't have code for hg 1.0; because it would require too
> > many hacks, and nobody cared anyway.
> >
> > Just like nobody cares about hg 1.0, eventually nobody will care about
> > hg 2.0.
>
> Yes, it can change. Eventually hg 2.0 will be defunct and no one will
> use it, but what happens if they go back to the old 2.0 style for
> getbundle in hg 4.0?
Then you can tell me I was wrong and we go back to your version. But
that's not going to happen.
And even if it does, we still would need to fix the code.
> We're already good. What if they switch it back in
> 4.1? This works for all of those conditions, and doesn't do anything if
> we're able to get mercurial.changegroup.getbundle.
Every method can change, we can't have wrappers for all of them.
In reality few of them do. And we deal with them on a case by case
basis.
There's no need to worry about the unlikely, especially since there
isn't much difference between the likely and unlikely; we are still
going to need to fix the code.
> > > Changes in 'getbundle' upstream would require changes either way.
> >
> > I doubt we will see any
Re: [GUILT v2 26/29] "guilt pop" now fails when there are no more patches to pop.
Signed-off-by: Josef 'Jeff' Sipek On Tue, May 13, 2014 at 10:31:02PM +0200, Per Cederqvist wrote: > This is analogous to how "guilt push" now fails when there are no more > patches to push. Like push, the "--all" argument still succeeds even > if there was no need to pop anything. > > Updated the test suite. > > Signed-off-by: Per Cederqvist > --- > guilt-pop| 17 +++-- > regression/t-021.out | 2 ++ > regression/t-021.sh | 6 ++ > regression/t-061.sh | 6 +- > 4 files changed, 24 insertions(+), 7 deletions(-) > > diff --git a/guilt-pop b/guilt-pop > index f0e647f..191313e 100755 > --- a/guilt-pop > +++ b/guilt-pop > @@ -49,9 +49,19 @@ fi > patch="$1" > [ ! -z "$all" ] && patch="-a" > > +# Treat "guilt pop" as "guilt pop -n 1". > +if [ -z "$patch" ]; then > + patch=1 > + num=t > +fi > + > if [ ! -s "$applied" ]; then > disp "No patches applied." > - exit 0 > + if [ "$patch" = "-a" ]; then > + exit 0 > + else > + exit 1 > + fi > elif [ "$patch" = "-a" ]; then > # we are supposed to pop all patches > > @@ -68,11 +78,6 @@ elif [ ! -z "$num" ]; then > # catch underflow > [ $eidx -lt 0 ] && eidx=0 > [ $eidx -eq $sidx ] && die "No patches requested to be removed." > -elif [ -z "$patch" ]; then > - # we are supposed to pop only the current patch on the stack > - > - sidx=`wc -l < "$applied"` > - eidx=`expr $sidx - 1` > else > # we're supposed to pop only up to a patch, make sure the patch is > # in the series > diff --git a/regression/t-021.out b/regression/t-021.out > index 9b42d9c..58be12f 100644 > --- a/regression/t-021.out > +++ b/regression/t-021.out > @@ -287,6 +287,8 @@ index 000..8baef1b > +++ b/def > @@ -0,0 +1 @@ > +abc > +% guilt pop > +No patches applied. > % guilt push --all > Applying patch..modify > Patch applied. > diff --git a/regression/t-021.sh b/regression/t-021.sh > index 614e870..e0d2dc1 100755 > --- a/regression/t-021.sh > +++ b/regression/t-021.sh > @@ -23,6 +23,12 @@ guilt series | _tac | while read n ; do > done > > # > +# pop when there is nothing to pop > +# > + > +shouldfail guilt pop > + > +# > # push all > # > cmd guilt push --all > diff --git a/regression/t-061.sh b/regression/t-061.sh > index 1411baa..6192f1b 100755 > --- a/regression/t-061.sh > +++ b/regression/t-061.sh > @@ -48,7 +48,11 @@ cmd list_files > > for i in `seq 5` > do > - cmd guilt pop > + if [ $i -ge 5 ]; then > + shouldfail guilt pop > + else > + cmd guilt pop > + fi > cmd git for-each-ref > cmd guilt push > cmd git for-each-ref > -- > 1.8.3.1 > -- Linux, n.: Generous programmers from around the world all join forces to help you shoot yourself in the foot for free. -- 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: [GUILT v2 16/29] Fix backslash handling when creating names of imported patches.
On Tue, May 13, 2014 at 10:30:52PM +0200, Per Cederqvist wrote:
> The 'echo %s' construct sometimes processes escape sequences. (This
%s? Should this be $s?
Otherwise, looks good.
> happens, for instance, under Ubuntu 14.04 when /bin/sh is actually
> dash.) We don't want that to happen when we are importing commits, so
> use 'printf %s "$s"' instead.
>
> (The -E option of bash that explicitly disables backslash expansion is
> not portable; it is not supported by dash.)
>
> Signed-off-by: Per Cederqvist
> ---
> guilt-import-commit | 2 +-
> regression/t-034.out | 14 +++---
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/guilt-import-commit b/guilt-import-commit
> index 6260c56..45f2404 100755
> --- a/guilt-import-commit
> +++ b/guilt-import-commit
> @@ -30,7 +30,7 @@ for rev in `git rev-list $rhash`; do
>
> # Try to convert the first line of the commit message to a
> # valid patch name.
> - fname=`echo $s | sed -e "s/&/and/g" -e "s/[ :]/_/g" -e "s,[/\\],-,g" \
> + fname=`printf %s "$s" | sed -e "s/&/and/g" -e "s/[ :]/_/g" -e
> "s,[/\\],-,g" \
> -e "s/['\\[{}]//g" -e 's/]//g' -e 's/\*/-/g' \
> -e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
> -e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
> diff --git a/regression/t-034.out b/regression/t-034.out
> index 7bc9459..bda4399 100644
> --- a/regression/t-034.out
> +++ b/regression/t-034.out
> @@ -236,7 +236,7 @@ Date: Mon Jan 1 00:00:00 2007 +
> About to begin conversion...
> Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541
> Converting 2a8b1889 as can-have-embedded-single-slashes
> -Converting 0a46f8fa as backslash-isorbidden
> +Converting 0a46f8fa as backslash-is-forbidden
> Converting aedb74fd as x
> Converting 30187ed0 as cannot@have@the@sequence@at-brace
> Converting 106e8e5a as cannot_end_in_
> @@ -300,7 +300,7 @@ Applying patch..cannot@have@the@sequence@at-brace.patch
> Patch applied.
> Applying patch..x.patch
> Patch applied.
> -Applying patch..backslash-isorbidden.patch
> +Applying patch..backslash-is-forbidden.patch
> Patch applied.
> Applying patch..can-have-embedded-single-slashes.patch
> Patch applied.
> @@ -311,7 +311,7 @@ Date: Mon Jan 1 00:00:00 2007 +
>
> Can/have/embedded/single/slashes
>
> -commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141
> (refs/patches/master/backslash-isorbidden.patch)
> +commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141
> (refs/patches/master/backslash-is-forbidden.patch)
> Author: Author Name
> Date: Mon Jan 1 00:00:00 2007 +
>
> @@ -518,8 +518,6 @@ d .git/patches/master
> d .git/refs/patches
> d .git/refs/patches/master
> f 06beca7069b9e576bd431f65d13862ed5d3e2a0f
> .git/patches/master/ctrlisforbidden.patch
> -f 08267ec6783ea9d1adae55b275198f7594764ed0 .git/patches/master/series
> -f 08267ec6783ea9d1adae55b275198f7594764ed0 .git/patches/master/status
> f 09b7e9be44ae5ec3a4bb30f5ee9d4ebc2c042f64
> .git/patches/master/two_consecutive_dots_(.)_is_forbidden.patch
> f 0b971c9a17aeca2319c93d700ffd98acc2a93451
> .git/patches/master/question-mark-is-forbidden.patch
> f 2b8392f63d61efc12add554555adae30883993cc
> .git/patches/master/cannot-end-in-slash-.patch
> @@ -529,7 +527,7 @@ f 34e07c584032df137f19bdb66d93f316f00a5ac8
> .git/patches/master/tildeisforbidden
> f 49bab499826b63deb2bd704629d60c7268c57aee
> .git/patches/master/the_sequence_-._is_forbidden.patch
> f 5bcddb8ccb6e6e5e8a61e9e56cb2e0f70cbab2f5
> .git/patches/master/cannot@have@the@sequence@at-brace.patch
> f 637b982fe14a240de181ae63226b27e0c406b3dc
> .git/patches/master/asterisk-is-forbidden.patch
> -f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3
> .git/patches/master/backslash-isorbidden.patch
> +f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3
> .git/patches/master/backslash-is-forbidden.patch
> f 7b103c3c7ae298cd2334f6f49da48bae1424f77b
> .git/patches/master/crisalsoforbidden.patch
> f 9b810b8c63779c51d2e7f61ab59cd49835041563 .git/patches/master/x.patch
> f a22958d9ae9976fd7b2b5a9d0bcd44bf7ad9b08a
> .git/patches/master/caretisforbidden.patch
> @@ -537,6 +535,8 @@ f ab325bf5a432937fc6f231d3e8a773a62d53952b
> .git/patches/master/multiple-slashes
> f cb9cffbd4465bddee266c20ccebd14eb687eaa89
> .git/patches/master/delisforbidden.patch
> f d0885a1a1fdee0fd1e4fedce3f7acd3100540bc4
> .git/patches/master/openbracketisforbidden.patch
> f d2903523fb66a346596eabbdd1bda4e52b266440
> .git/patches/master/check-multiple-.-dots-.-foo.patch
> +f da90de1c84138194524994e0bc3bc4ca8189c999 .git/patches/master/series
> +f da90de1c84138194524994e0bc3bc4ca8189c999 .git/patches/master/status
> f dfc11f76394059909671af036598c5fbe33001ba
> .git/patches/master/space_is_forbidden.patch
> f e47474c52d6c893f36d0457f885a6dd1267742bb
> .git/patches/master/colon_is_forbidden.patch
> f e7a5f8912592d9891e6159f5827c8b4f372cc406
> .git/patches/master/the_sequence_.lock-_is_forbidde
Re: [PATCH] contrib: remote-helpers: add move warnings (v2.0)
Felipe Contreras writes: > The tools are now maintained out-of-tree, and they have a regression in > v2.0. You seem not to understand at all what a regression is. My understanding is that versions of remote-hg shipped with all versions of Git did not work with Hg 3.0, so not working with Hg 3.0 is a regression in v2.0 at all. A recent report was about Hg 3.0 not working with 1.9.3, but I think you earlier said all versions of Git does not work with Hg 3.0, and I can believe it. That is hardly a regression. You could argue that Hg has a new regression to its external users of its API when it went to 3.0. We actually had a similar breakage in 1.5.4, where it was reported late in the cycle after -rc0 [*1*] that cgit that linked with our internal API libgit.a was broken by a change on our side, which resulted in us fixing the breakage (even though technically you may be able to say that it was cgit's fault to link with libgit.a in the first place) with 18125644 (Move sha1_file_to_archive into libgit, 2008-01-14) very late in the cycle. Calling that a regression in cgit would have been insane, even if we did not patch our side up to accomodate it. Stop this idiocy. [References] *1* http://thread.gmane.org/gmane.comp.version-control.git/70117/focus=71064 -- 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: [GUILT v2 10/29] Run test_failed if the exit status of a test script is bad.
Signed-off-by: Josef 'Jeff' Sipek On Tue, May 13, 2014 at 10:30:46PM +0200, Per Cederqvist wrote: > There were two problems with the old code: > > - Since "set -e" is in effect (that is set in scaffold) the run-test >script exited immediately if a t-*.sh script failed. This is not >nice, as we want the error report that test_failed prints. > > - The code ran "cd -" between running the t-*.sh script and checking >the exit status, so the exit status was lost. (Actually, the exit >status was saved in $ERR, but nothing ever looked at $ERR.) > > Signed-off-by: Per Cederqvist > --- > regression/run-tests | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/regression/run-tests b/regression/run-tests > index a10e796..8e0af9f 100755 > --- a/regression/run-tests > +++ b/regression/run-tests > @@ -55,11 +55,15 @@ function run_test > > # run the test > cd "$REPODIR" > /dev/null > - "$REG_DIR/t-$1.sh" 2>&1 > "$LOGFILE" > - ERR=$? > + if "$REG_DIR/t-$1.sh" 2>&1 > "$LOGFILE"; then > + ERR=false > + else > + ERR=true > + fi > + > cd - > /dev/null > > - [ $? -ne 0 ] && test_failed > + $ERR && test_failed > diff -u "t-$1.out" "$LOGFILE" || test_failed > > echo "done." > -- > 1.8.3.1 > -- Reality is merely an illusion, albeit a very persistent one. - Albert Einstein -- 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: [GUILT v2 17/29] "guilt graph" no longer loops when no patches are applied.
Signed-off-by: Josef 'Jeff' Sipek
On Tue, May 13, 2014 at 10:30:53PM +0200, Per Cederqvist wrote:
> Give an error message if no patches are applied. Added a test case
> that never terminates unless this fix is applied.
>
> Signed-off-by: Per Cederqvist
> ---
> guilt-graph | 9 +++--
> regression/t-033.out | 3 +++
> regression/t-033.sh | 13 +
> 3 files changed, 23 insertions(+), 2 deletions(-)
> create mode 100644 regression/t-033.out
> create mode 100755 regression/t-033.sh
>
> diff --git a/guilt-graph b/guilt-graph
> index b3469dc..56d0e77 100755
> --- a/guilt-graph
> +++ b/guilt-graph
> @@ -17,8 +17,13 @@ fi
>
> patchname="$1"
>
> -bottom=`git rev-parse refs/patches/$branch/$(head_n 1 < "$applied")`
> -base=`git rev-parse $bottom^`
> +bottompatch=$(head_n 1 < "$applied")
> +if [ -z "$bottompatch" ]; then
> + echo "No patch applied." >&2
> + exit 1
> +fi
> +
> +base=`git rev-parse "refs/patches/${branch}/${bottompatch}^"`
>
> if [ -z "$patchname" ]; then
> top=`git rev-parse HEAD`
> diff --git a/regression/t-033.out b/regression/t-033.out
> new file mode 100644
> index 000..76613f9
> --- /dev/null
> +++ b/regression/t-033.out
> @@ -0,0 +1,3 @@
> +% setup_repo
> +% guilt graph
> +No patch applied.
> diff --git a/regression/t-033.sh b/regression/t-033.sh
> new file mode 100755
> index 000..a3a8981
> --- /dev/null
> +++ b/regression/t-033.sh
> @@ -0,0 +1,13 @@
> +#!/bin/bash
> +#
> +# Test the graph code
> +#
> +
> +source "$REG_DIR/scaffold"
> +
> +cmd setup_repo
> +
> +# Check that "guilt graph" gives a proper "No patch applied" error
> +# message when no patches are applied. (An older version of guilt
> +# used to enter an endless loop in this situation.)
> +shouldfail guilt graph
> --
> 1.8.3.1
>
--
Reality is merely an illusion, albeit a very persistent one.
- Albert Einstein
--
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: [GUILT v2 20/29] "guilt graph": Handle patch names containing quotes.
On Tue, May 13, 2014 at 10:30:56PM +0200, Per Cederqvist wrote:
> Quote quotes with a backslash in the "guilt graph" output. Otherwise,
> the "dot" file could contain syntax errors.
>
> Added a test case.
>
> Signed-off-by: Per Cederqvist
> ---
> guilt-graph | 2 ++
> regression/t-033.out | 22 ++
> regression/t-033.sh | 9 +
> 3 files changed, 33 insertions(+)
>
> diff --git a/guilt-graph b/guilt-graph
> index 924a63e..0857e0d 100755
> --- a/guilt-graph
> +++ b/guilt-graph
> @@ -57,6 +57,8 @@ while [ "$current" != "$base" ]; do
> }"`
> [ -z "$pname" ] && pname="?"
>
> + pname="`printf \"%s\" \"$pname\" | sed 's/\"/\"/g'`"
Signed-off-by: Josef 'Jeff' Sipek
> +
> disp "# checking rev $current"
> disp " \"$current\" [label=\"$pname\"]"
>
> diff --git a/regression/t-033.out b/regression/t-033.out
> index 3d1c61f..c120d4f 100644
> --- a/regression/t-033.out
> +++ b/regression/t-033.out
> @@ -66,3 +66,25 @@ digraph G {
> "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> "891bc14b5603474c9743fd04f3da888644413dc5" ->
> "ff2775f8d1dc753f635830adcc3a067e0b681e2d"; // ?
> }
> +% guilt new a-"better&quicker'-patch.patch
> +% git add file.txt
> +% guilt refresh
> +Patch a-"better&quicker'-patch.patch refreshed
> +% guilt pop
> +Now at c.patch.
> +% guilt push
> +Applying patch..a-"better&quicker'-patch.patch
> +Patch applied.
> +% guilt graph
> +digraph G {
> +# checking rev bc7df666a646739eaf559af23cab72f2bfd01f0e
> + "bc7df666a646739eaf559af23cab72f2bfd01f0e"
> [label="a-\"better&quicker'-patch.patch"]
> +# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
> + "891bc14b5603474c9743fd04f3da888644413dc5" [label="c.patch"]
> + "bc7df666a646739eaf559af23cab72f2bfd01f0e" ->
> "891bc14b5603474c9743fd04f3da888644413dc5"; // ?
> +# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
> + "c7014443c33d2b0237293687ceb9cbd38313df65" [label="b.patch"]
> +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
> + "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> + "891bc14b5603474c9743fd04f3da888644413dc5" ->
> "ff2775f8d1dc753f635830adcc3a067e0b681e2d"; // ?
> +}
> diff --git a/regression/t-033.sh b/regression/t-033.sh
> index fac081e..9fe1827 100755
> --- a/regression/t-033.sh
> +++ b/regression/t-033.sh
> @@ -50,3 +50,12 @@ cmd git add file.txt
> cmd guilt refresh
> fixup_time_info c.patch
> cmd guilt graph
> +
> +# A patch name that contains funky characters, including unbalanced
> +# quotes.
> +cmd guilt new "a-\"better&quicker'-patch.patch"
> +cmd echo d >> file.txt
> +cmd git add file.txt
> +cmd guilt refresh
> +fixup_time_info "a-\"better&quicker'-patch.patch"
> +cmd guilt graph
> --
> 1.8.3.1
>
--
C is quirky, flawed, and an enormous success.
- Dennis M. Ritchie.
--
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: [GUILT v2 24/29] disp no longer processes backslashes.
On Tue, May 13, 2014 at 10:31:00PM +0200, Per Cederqvist wrote:
> Only one invocation of "disp" or "_disp" actually needed backslash
> processing. In quite a few instances, it was wrong to do backslash
> processing, as the message contained data derived from the user.
>
> Created the new function "disp_e" that should be used when backslash
> processing is required, and changed "disp" and "_disp" to use printf
> code %s instead of "%b".
Minor nit: "%s" (to match "%b")
Signed-off-by: Josef 'Jeff' Sipek
> Signed-off-by: Per Cederqvist
> ---
> guilt | 17 +
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/guilt b/guilt
> index 23cc2da..9947acc 100755
> --- a/guilt
> +++ b/guilt
> @@ -36,15 +36,24 @@ usage()
> exit 1
> }
>
> -# echo -n is a bashism, use printf instead
> +# Print arguments, but no trailing newline.
> +# (echo -n is a bashism, use printf instead)
> _disp()
> {
> - printf "%b" "$*"
> + printf "%s" "$*"
> }
>
> -# echo -e is a bashism, use printf instead
> +# Print arguments.
> +# (echo -E is a bashism, use printf instead)
> disp()
> {
> + printf "%s\n" "$*"
> +}
> +
> +# Print arguments, processing backslash sequences.
> +# (echo -e is a bashism, use printf instead)
> +disp_e()
> +{
> printf "%b\n" "$*"
> }
>
> @@ -117,7 +126,7 @@ else
>
> disp ""
> disp "Example:"
> - disp "\tguilt push"
> + disp_e "\tguilt push"
>
> # now, let's exit
> exit 1
> --
> 1.8.3.1
>
--
Reality is merely an illusion, albeit a very persistent one.
- Albert Einstein
--
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: [GUILT v2 25/29] "guilt push" now fails when there are no more patches to push.
On Tue, May 13, 2014 at 10:31:01PM +0200, Per Cederqvist wrote: > This makes it easier to script operations on the entire queue, for > example run the test suite on each patch in the queue: > > guilt pop -a;while guilt push; do make test||break; done > > This brings "guilt push" in line with the push operation in Mercurial > Queues (hg qpush), which fails when there are no patches to apply. > > Updated the test suite. > > "guilt push -a" still does not fail. (It successfully manages to > ensure that all patches are pushed, even if it did not have to do > anything to make it so.) > > Signed-off-by: Per Cederqvist > --- > guilt-push | 19 ++- > regression/t-020.out | 89 > > regression/t-020.sh | 13 +++- > 3 files changed, 113 insertions(+), 8 deletions(-) ... > diff --git a/regression/t-020.sh b/regression/t-020.sh > index 906aec6..0f9f85d 100755 > --- a/regression/t-020.sh > +++ b/regression/t-020.sh > @@ -26,6 +26,17 @@ guilt series | while read n ; do > done > > # > +# pushing when there is nothing to push > +# > + > +shouldfail guilt push > +cmd guilt push -a > + > +cmd list_files > + > +cmd git log -p I don't particularly care for the git-log. Otherwise it looks good. Signed-off-by: Josef 'Jeff' Sipek > + > +# > # pop all > # > cmd guilt pop --all > @@ -61,7 +72,7 @@ cmd guilt pop --all > > npatches=`guilt series | wc -l` > for n in `_seq -2 $npatches`; do > - if [ $n -ge 0 ]; then > + if [ $n -gt 0 ]; then > cmd guilt push -n $n > else > shouldfail guilt push -n $n > -- > 1.8.3.1 > -- Evolution, n.: A hypothetical process whereby infinitely improbable events occur with alarming frequency, order arises from chaos, and no one is given credit. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] contrib: remote-helpers: add move warnings (v2.0)
The tools are now maintained out-of-tree, and they have a regression in
v2.0. It's better to start warning the users as soon as possible.
Can't possibly introduce regressions.
Signed-off-by: Felipe Contreras
---
contrib/remote-helpers/git-remote-bzr | 3 +++
contrib/remote-helpers/git-remote-hg | 3 +++
2 files changed, 6 insertions(+)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index 9abb58e..be4b9a3 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -43,6 +43,9 @@ import re
import StringIO
import atexit, shutil, hashlib, urlparse, subprocess
+sys.stderr.write('WARNING: git-remote-bzr is now maintained independently.\n')
+sys.stderr.write('WARNING: For more information visit
https://github.com/felipec/git-remote-bzr\n')
+
NAME_RE = re.compile('^([^<>]+)')
AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
EMAIL_RE = re.compile(r'([^ \t<>]+@[^ \t<>]+)')
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 34cda02..989df66 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -25,6 +25,9 @@ import atexit
import urlparse, hashlib
import time as ptime
+sys.stderr.write('WARNING: git-remote-hg is now maintained independently.\n')
+sys.stderr.write('WARNING: For more information visit
https://github.com/felipec/git-remote-hg\n')
+
#
# If you want to see Mercurial revisions as Git commit notes:
# git config core.notesRef refs/notes/hg
--
1.9.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: [GUILT v2 19/29] Check that "guilt graph" works when working on a branch with a comma.
Signed-off-by: Josef 'Jeff' Sipek
On Tue, May 13, 2014 at 10:30:55PM +0200, Per Cederqvist wrote:
> git branch names can contain commas. Check that "guilt graph" works
> even in that case.
>
> Signed-off-by: Per Cederqvist
> ---
> regression/t-033.out | 65
>
> regression/t-033.sh | 39 +++
> 2 files changed, 104 insertions(+)
>
> diff --git a/regression/t-033.out b/regression/t-033.out
> index 76613f9..3d1c61f 100644
> --- a/regression/t-033.out
> +++ b/regression/t-033.out
> @@ -1,3 +1,68 @@
> % setup_repo
> % guilt graph
> No patch applied.
> +%% Testing branch a,graph
> +% git checkout -b a,graph master
> +Switched to a new branch 'a,graph'
> +% guilt init
> +% guilt new a.patch
> +% guilt pop
> +All patches popped.
> +% guilt push
> +Applying patch..a.patch
> +Patch applied.
> +% guilt graph
> +digraph G {
> +# checking rev 95275d7c05c6a6176d3941374115b91272877f6c
> + "95275d7c05c6a6176d3941374115b91272877f6c" [label="a.patch"]
> +}
> +% git add file.txt
> +% guilt refresh
> +Patch a.patch refreshed
> +% guilt pop
> +All patches popped.
> +% guilt push
> +Applying patch..a.patch
> +Patch applied.
> +% guilt graph
> +digraph G {
> +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
> + "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> +}
> +%% Adding an unrelated file in a new patch. No deps.
> +% guilt new b.patch
> +% git add file2.txt
> +% guilt refresh
> +Patch b.patch refreshed
> +% guilt pop
> +Now at a.patch.
> +% guilt push
> +Applying patch..b.patch
> +Patch applied.
> +% guilt graph
> +digraph G {
> +# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
> + "c7014443c33d2b0237293687ceb9cbd38313df65" [label="b.patch"]
> +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
> + "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> +}
> +%% Changing a file already changed in the first patch adds a dependency.
> +% guilt new c.patch
> +% git add file.txt
> +% guilt refresh
> +Patch c.patch refreshed
> +% guilt pop
> +Now at b.patch.
> +% guilt push
> +Applying patch..c.patch
> +Patch applied.
> +% guilt graph
> +digraph G {
> +# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
> + "891bc14b5603474c9743fd04f3da888644413dc5" [label="c.patch"]
> +# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
> + "c7014443c33d2b0237293687ceb9cbd38313df65" [label="b.patch"]
> +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
> + "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> + "891bc14b5603474c9743fd04f3da888644413dc5" ->
> "ff2775f8d1dc753f635830adcc3a067e0b681e2d"; // ?
> +}
> diff --git a/regression/t-033.sh b/regression/t-033.sh
> index a3a8981..fac081e 100755
> --- a/regression/t-033.sh
> +++ b/regression/t-033.sh
> @@ -3,6 +3,13 @@
> # Test the graph code
> #
>
> +function fixup_time_info
> +{
> + cmd guilt pop
> + touch -a -m -t "$TOUCH_DATE" ".git/patches/a,graph/$1"
> + cmd guilt push
> +}
> +
> source "$REG_DIR/scaffold"
>
> cmd setup_repo
> @@ -11,3 +18,35 @@ cmd setup_repo
> # message when no patches are applied. (An older version of guilt
> # used to enter an endless loop in this situation.)
> shouldfail guilt graph
> +
> +echo "%% Testing branch a,graph"
> +cmd git checkout -b a,graph master
> +
> +cmd guilt init
> +
> +cmd guilt new a.patch
> +
> +fixup_time_info a.patch
> +cmd guilt graph
> +
> +cmd echo a >> file.txt
> +cmd git add file.txt
> +cmd guilt refresh
> +fixup_time_info a.patch
> +cmd guilt graph
> +
> +echo "%% Adding an unrelated file in a new patch. No deps."
> +cmd guilt new b.patch
> +cmd echo b >> file2.txt
> +cmd git add file2.txt
> +cmd guilt refresh
> +fixup_time_info b.patch
> +cmd guilt graph
> +
> +echo "%% Changing a file already changed in the first patch adds a
> dependency."
> +cmd guilt new c.patch
> +cmd echo c >> file.txt
> +cmd git add file.txt
> +cmd guilt refresh
> +fixup_time_info c.patch
> +cmd guilt graph
> --
> 1.8.3.1
>
--
Research, n.:
Consider Columbus:
He didn't know where he was going.
When he got there he didn't know where he was.
When he got back he didn't know where he had been.
And he did it all on someone else's money.
--
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 01/17] contrib: remove outdated README
Felipe, someone can contribute positively, and also be very destructive. Your positive contributions, nobody will deny. However, you have to tame the other part to be good company. I have had patches and contributions rejected in the past, sometimes rudely. Same has happened to many others, if you contribute long enough, it is pretty much guaranteed that it will happen to you. Maintainer is wrong, or you are wrong, or someone is just having a bad day. But these are NOT good reasons to make a big scene. It is reasonable to be upset, it is reasonable to think that Junio is wrong or unfair; walk away, take some time off, cool down your own mind, give others time to cool down as well. cheers, m On Tue, May 13, 2014 at 5:05 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> Junio, do you honestly think I am a troll? > > You certainly are acting like one, aren't you? > -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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: [GUILT 19/28] Check that "guilt graph" works when working on a branch with a comma.
Sorry, I accidentally replied to the v1 of this patch...
On Tue, May 13, 2014 at 05:33:21PM -0400, Jeff Sipek wrote:
> On Fri, Mar 21, 2014 at 08:31:57AM +0100, Per Cederqvist wrote:
> > git branch names can contain commas. Check that "guilt graph" works
> > even in that case.
> >
> > Signed-off-by: Per Cederqvist
> > ---
> > regression/t-033.out | 62
> >
> > regression/t-033.sh | 37 +++
> > 2 files changed, 99 insertions(+)
> >
> > diff --git a/regression/t-033.out b/regression/t-033.out
> > index 76613f9..e638d7b 100644
> > --- a/regression/t-033.out
> > +++ b/regression/t-033.out
> > @@ -1,3 +1,65 @@
> > % setup_repo
> > % guilt graph
> > No patch applied.
> > +% git checkout -b a,graph master
> > +Switched to a new branch 'a,graph'
> > +% guilt init
> > +% guilt new a.patch
> > +% guilt pop
> > +All patches popped.
> > +% guilt push
> > +Applying patch..a.patch
> > +Patch applied.
> > +% guilt graph
> > +digraph G {
> > +# checking rev 95275d7c05c6a6176d3941374115b91272877f6c
> > + "95275d7c05c6a6176d3941374115b91272877f6c" [label="a.patch"]
> > +}
> > +% git add file.txt
> > +% guilt refresh
> > +Patch a.patch refreshed
> > +% guilt pop
> > +All patches popped.
> > +% guilt push
> > +Applying patch..a.patch
> > +Patch applied.
> > +% guilt graph
> > +digraph G {
> > +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
> > + "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> > +}
> > +% guilt new b.patch
> > +% git add file2.txt
> > +% guilt refresh
> > +Patch b.patch refreshed
> > +% guilt pop
> > +Now at a.patch.
> > +% guilt push
> > +Applying patch..b.patch
> > +Patch applied.
> > +% guilt graph
> > +digraph G {
> > +# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
> > + "c7014443c33d2b0237293687ceb9cbd38313df65" [label="b.patch"]
> > +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
> > + "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> > +}
> > +% guilt new c.patch
> > +% git add file.txt
> > +% guilt refresh
> > +Patch c.patch refreshed
> > +% guilt pop
> > +Now at b.patch.
> > +% guilt push
> > +Applying patch..c.patch
> > +Patch applied.
> > +% guilt graph
> > +digraph G {
> > +# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
> > + "891bc14b5603474c9743fd04f3da888644413dc5" [label="c.patch"]
> > +# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
> > + "c7014443c33d2b0237293687ceb9cbd38313df65" [label="b.patch"]
> > +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
> > + "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> > + "891bc14b5603474c9743fd04f3da888644413dc5" ->
> > "ff2775f8d1dc753f635830adcc3a067e0b681e2d"; // ?
> > +}
> > diff --git a/regression/t-033.sh b/regression/t-033.sh
> > index ae40577..57dce78 100755
> > --- a/regression/t-033.sh
> > +++ b/regression/t-033.sh
> > @@ -3,9 +3,46 @@
> > # Test the graph code
> > #
> >
> > +function fixup_time_info
> > +{
> > + cmd guilt pop
> > + touch -a -m -t "$TOUCH_DATE" ".git/patches/a,graph/$1"
> > + cmd guilt push
> > +}
> > +
> > source "$REG_DIR/scaffold"
> >
> > cmd setup_repo
> >
>
> A comment here to justify this seemingly useless guilt-graph call? Maybe
> adding one of the '%%' lines between each section. Otherwise, this looks
> good.
>
> > shouldfail guilt graph
> >
> > +cmd git checkout -b a,graph master
> > +
> > +cmd guilt init
> > +
> > +cmd guilt new a.patch
> > +
> > +fixup_time_info a.patch
> > +cmd guilt graph
> > +
> > +cmd echo a >> file.txt
> > +cmd git add file.txt
> > +cmd guilt refresh
> > +fixup_time_info a.patch
> > +cmd guilt graph
> > +
> > +# An unrelated file. No deps.
> > +cmd guilt new b.patch
> > +cmd echo b >> file2.txt
> > +cmd git add file2.txt
> > +cmd guilt refresh
> > +fixup_time_info b.patch
> > +cmd guilt graph
> > +
> > +# An change to an old file. Should add a dependency.
> > +cmd guilt new c.patch
> > +cmd echo c >> file.txt
> > +cmd git add file.txt
> > +cmd guilt refresh
> > +fixup_time_info c.patch
> > +cmd guilt graph
> > --
> > 1.8.3.1
> >
>
> --
> Ready; T=0.01/0.01 17:32:39
--
C is quirky, flawed, and an enormous success.
- Dennis M. Ritchie.
--
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: [GUILT 19/28] Check that "guilt graph" works when working on a branch with a comma.
On Fri, Mar 21, 2014 at 08:31:57AM +0100, Per Cederqvist wrote:
> git branch names can contain commas. Check that "guilt graph" works
> even in that case.
>
> Signed-off-by: Per Cederqvist
> ---
> regression/t-033.out | 62
>
> regression/t-033.sh | 37 +++
> 2 files changed, 99 insertions(+)
>
> diff --git a/regression/t-033.out b/regression/t-033.out
> index 76613f9..e638d7b 100644
> --- a/regression/t-033.out
> +++ b/regression/t-033.out
> @@ -1,3 +1,65 @@
> % setup_repo
> % guilt graph
> No patch applied.
> +% git checkout -b a,graph master
> +Switched to a new branch 'a,graph'
> +% guilt init
> +% guilt new a.patch
> +% guilt pop
> +All patches popped.
> +% guilt push
> +Applying patch..a.patch
> +Patch applied.
> +% guilt graph
> +digraph G {
> +# checking rev 95275d7c05c6a6176d3941374115b91272877f6c
> + "95275d7c05c6a6176d3941374115b91272877f6c" [label="a.patch"]
> +}
> +% git add file.txt
> +% guilt refresh
> +Patch a.patch refreshed
> +% guilt pop
> +All patches popped.
> +% guilt push
> +Applying patch..a.patch
> +Patch applied.
> +% guilt graph
> +digraph G {
> +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
> + "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> +}
> +% guilt new b.patch
> +% git add file2.txt
> +% guilt refresh
> +Patch b.patch refreshed
> +% guilt pop
> +Now at a.patch.
> +% guilt push
> +Applying patch..b.patch
> +Patch applied.
> +% guilt graph
> +digraph G {
> +# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
> + "c7014443c33d2b0237293687ceb9cbd38313df65" [label="b.patch"]
> +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
> + "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> +}
> +% guilt new c.patch
> +% git add file.txt
> +% guilt refresh
> +Patch c.patch refreshed
> +% guilt pop
> +Now at b.patch.
> +% guilt push
> +Applying patch..c.patch
> +Patch applied.
> +% guilt graph
> +digraph G {
> +# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
> + "891bc14b5603474c9743fd04f3da888644413dc5" [label="c.patch"]
> +# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
> + "c7014443c33d2b0237293687ceb9cbd38313df65" [label="b.patch"]
> +# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
> + "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
> + "891bc14b5603474c9743fd04f3da888644413dc5" ->
> "ff2775f8d1dc753f635830adcc3a067e0b681e2d"; // ?
> +}
> diff --git a/regression/t-033.sh b/regression/t-033.sh
> index ae40577..57dce78 100755
> --- a/regression/t-033.sh
> +++ b/regression/t-033.sh
> @@ -3,9 +3,46 @@
> # Test the graph code
> #
>
> +function fixup_time_info
> +{
> + cmd guilt pop
> + touch -a -m -t "$TOUCH_DATE" ".git/patches/a,graph/$1"
> + cmd guilt push
> +}
> +
> source "$REG_DIR/scaffold"
>
> cmd setup_repo
>
A comment here to justify this seemingly useless guilt-graph call? Maybe
adding one of the '%%' lines between each section. Otherwise, this looks
good.
> shouldfail guilt graph
>
> +cmd git checkout -b a,graph master
> +
> +cmd guilt init
> +
> +cmd guilt new a.patch
> +
> +fixup_time_info a.patch
> +cmd guilt graph
> +
> +cmd echo a >> file.txt
> +cmd git add file.txt
> +cmd guilt refresh
> +fixup_time_info a.patch
> +cmd guilt graph
> +
> +# An unrelated file. No deps.
> +cmd guilt new b.patch
> +cmd echo b >> file2.txt
> +cmd git add file2.txt
> +cmd guilt refresh
> +fixup_time_info b.patch
> +cmd guilt graph
> +
> +# An change to an old file. Should add a dependency.
> +cmd guilt new c.patch
> +cmd echo c >> file.txt
> +cmd git add file.txt
> +cmd guilt refresh
> +fixup_time_info c.patch
> +cmd guilt graph
> --
> 1.8.3.1
>
--
Ready; T=0.01/0.01 17:32:39
--
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: [GUILT v2 07/29] Added test cases for "guilt fold".
On Tue, May 13, 2014 at 10:30:43PM +0200, Per Cederqvist wrote:
> Test that we can combine any combination of patches with empty and
> non-empty messages, both with and without guilt.diffstat. (All
> patches are empty.)
>
> Signed-off-by: Per Cederqvist
> ---
> regression/t-035.out | 467
> +++
> regression/t-035.sh | 62 +++
> 2 files changed, 529 insertions(+)
> create mode 100644 regression/t-035.out
> create mode 100755 regression/t-035.sh
>
...
> diff --git a/regression/t-035.sh b/regression/t-035.sh
> new file mode 100755
> index 000..e914b32
> --- /dev/null
> +++ b/regression/t-035.sh
> @@ -0,0 +1,62 @@
> +#!/bin/bash
> +#
> +# Test the fold code
> +#
> +
> +source "$REG_DIR/scaffold"
> +
> +cmd setup_repo
> +
> +function fixup_time_info
> +{
> + cmd guilt pop
> + touch -a -m -t "$TOUCH_DATE" ".git/patches/master/$1"
> + cmd guilt push
> +}
> +
> +function empty_patch
> +{
> + cmd guilt new "empty$1"
> + fixup_time_info "empty$1"
> +}
> +
> +function nonempty_patch
> +{
> + if [ "$1" = -2 ]; then
> + msg="Another commit message."
> + else
> + msg="A commit message."
> + fi
> +
> + cmd guilt new -f -s -m "$msg" "nonempty$1"
> + fixup_time_info "nonempty$1"
> +}
> +
> +for using_diffstat in true false; do
> + cmd git config guilt.diffstat $using_diffstat
> + for patcha in empty nonempty; do
> + for patchb in empty nonempty; do
> +
> + if [ $patcha = $patchb ]
> + then
I know that this is before patch 29, but ... style? ;)
Otherwise, looks good. I like this way better than the unrolled loop in v1
of this patch.
Signed-off-by: Josef 'Jeff' Sipek
> + suffixa=-1
> + suffixb=-2
> + else
> + suffixa=
> + suffixb=
> + fi
> +
> + echo "%% $patcha + $patchb (diffstat=$using_diffstat)"
> + ${patcha}_patch $suffixa
> + ${patchb}_patch $suffixb
> + cmd guilt pop
> + cmd guilt fold $patchb$suffixb
> + fixup_time_info $patcha$suffixa
> + cmd list_files
> + cmd guilt pop
> + cmd guilt delete -f $patcha$suffixa
> + cmd list_files
> +
> + done
> + done
> +done
> --
> 1.8.3.1
>
--
*NOTE: This message is ROT-13 encrypted twice for extra protection*
--
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: [GUILT v2 00/29] Teach guilt import-commit how to create legal patch names, and more
On Tue, May 13, 2014 at 10:54 PM, Jeff Sipek wrote: > On Tue, May 13, 2014 at 04:45:47PM -0400, Theodore Ts'o wrote: >> On Tue, May 13, 2014 at 10:30:36PM +0200, Per Cederqvist wrote: > ... >> > - Changed behavior: by default, guilt no longer changes branch when >> >you push a patch. You need to do "git config guilt.reusebranch >> >false" to re-enable that. This patch sets the default value of >> >guilt.reusebranch to true; it should in my opinion change to false >> >a year or two after the next release. >> >> We've been living with the "origin" -> "guilt/origin" branch change >> for a year already, and in fact, these days I've gotten used to the >> new behavior. Is it really worth it to change the default? > > So, at first I was skeptical about the branch name prefix change. I've used > it for about a year now, and I love it. When I first read Per's idea to > change the default to the old-style, I was a bit sad but I understand the > motivation. > > I'm open to either mode being the default since it's easy enough for me to > change it for me (thanks, ~/.gitconfig) but I think more people should > benefit from the added safety against accidental git-push. (I also like > being able to use guilt/master..master to get only the commits I care > about.) Thoughts? I don't have a strong opinion on which the default value should be. The scenario where it matters, when you run multiple versions of guilt against the same directory, is probably very rare in practice. If it is mentioned in the release note that it can be changed if needed, that is probably good enough. /ceder -- 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 GUI PATCH] git-gui: use vcompare when comparing the git version
Since git 2.0.0 starting git gui in a submodule using a gitfile fails with
the following error:
No working directory ../../../
couldn't change working directory
to "../../../": no such file or
directory
This is because "git rev-parse --show-toplevel" is only run when git gui
sees a git version of at least 1.7.0 (which is the version in which the
--show-toplevel option was introduced). But it uses vsatisfies to check
that, which is documented to return false when the major version changes,
which is not what we want here.
Change vsatisfies to vcompare when testing for a git version to avoid the
problem when the major version changes (but still use vsatisfies in those
places where the Tk version is checked). This is done for both the place
that caused the reported bug and another spot where the git version is
tested for another feature.
Reported-by: Chris Packham
Reported-by: Yann Dirson
Signed-off-by: Jens Lehmann
---
Am 07.05.2014 09:49, schrieb Chris Packham:
> On 07/05/14 19:28, Chris Packham wrote:
>> On 07/05/14 00:10, Pat Thoyts wrote:
>>> Chris Packham writes:
>>>
On Tue, Apr 29, 2014 at 2:56 PM, Chris Packham
wrote:
> Hi Pat,
>
> I'm running git 2.0.0-rc0 (haven't got round to pulling down rc1 yet)
> which includes gitgui-0.19.0 and I'm getting a new error when I run
> 'git gui' in a repository with a .git file (created by git submodule).
>
> I can send you a screencap of the error message off list if you want
> but the text is
>
> "No working directory ../../../
>
> couldn't change working directory to ../../../: no such file or
> directory"
My tcl is a little rusty but I think the problem might be this snippet.
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
if {[package vsatisfies $_git_version 1.7.0]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git rev-parse
--show-toplevel]]}
} else {
set _gitworktree [git rev-parse --show-toplevel]
}
} else {
# try to set work tree from environment, core.worktree or use
# cdup to obtain a relative path to the top of the worktree. If
# run from the top, the ./ prefix ensures normalize expands pwd.
if {[catch { set _gitworktree $env(GIT_WORK_TREE) }]} {
set _gitworktree [get_config core.worktree]
if {$_gitworktree eq ""} {
set _gitworktree [file normalize ./[git rev-parse --show-cdup]]
}
}
}
The vsatisfies call probably doesn't handle '2.0.0.rc0' and the
fallback behaviour probably needs to normalise core.worktree
>>>
>>> The _git_version variable has already been trimmed to remove such
>>> suffixes so the version comparison here will be ok.
>>
>> I don't think that's true 'git rev-parse --show-toplevel' does the right
>> thing - if it's run.
>
> We'll the trimming works but vstatisfies doesn't
>
> puts $_git_version
> puts [package vsatisfies $_git_version 1.7.0]
>
> 2.0.0
> 0
Yup, looks like vsatisfies is doing just what it is supposed to (according
to http://www.astro.princeton.edu/~rhl/Tcl-Tk_docs/tcl/package.n.html):
package vsatisfies version1 version2
Returns 1 if scripts written for version2 will work unchanged
with version1 (i.e. version1 is equal to or greater than version2
and they both have the same major version number), 0 otherwise.
The bump in the major number from 1 to 2 makes vsatisfies assume that the
version is not compatible anymore, I believe we should have used vcompare
here and in another place.
git-gui.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index cf2209b..ed2418b 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1283,7 +1283,7 @@ load_config 0
apply_config
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
-if {[package vsatisfies $_git_version 1.7.0]} {
+if {[package vcompare $_git_version 1.7.0]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git rev-parse
--show-toplevel]]}
} else {
@@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {
close $fd
}
- if {[package vsatisfies $::_git_version 1.6.3]} {
+ if {[package vcompare $::_git_version 1.6.3]} {
set ls_others [list --exclude-standard]
} else {
set ls_others [list --exclude-per-directory=.gitignore]
--
2.0.0.rc3.2.g998f840
--
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] git-gui regression in 2.0rcX within submodule
Thanks for the reminder, I'm currently looking into that. Am 13.05.2014 10:30, schrieb Chris Packham: > Hi, > > On 13/05/14 11:45, Yann Dirson wrote: >> In 2.0rc2, git-gui is unable to work inside submodules, where 1.9.2 >> did not show such a problem: >> >> >> yann@home:~$ cd /tmp/ >> yann@home:tmp$ mkdir foo >> yann@home:tmp$ cd foo/ >> yann@home:foo$ git init >> Initialized empty Git repository in /tmp/foo/.git/ >> yann@home:foo (master)$ git submodule add >> git://git.debian.org/git/collab-maint/tulip.git debian >> Cloning into 'debian'... >> remote: Counting objects: 317, done. >> remote: Compressing objects: 100% (199/199), done. >> remote: Total 317 (delta 184), reused 166 (delta 95) >> Receiving objects: 100% (317/317), 73.81 KiB | 0 bytes/s, done. >> Resolving deltas: 100% (184/184), done. >> Checking connectivity... done. >> yann@home:foo (master)$ git status >> On branch master >> >> Initial commit >> >> Changes to be committed: >> (use "git rm --cached ..." to unstage) >> >> new file: .gitmodules >> new file: debian >> >> yann@home:foo (master)$ (cd debian/ && git gui) >> [errors out after showing the following error dialog] >> >> | No working directory ../../../debian: >> | >> | couldn't change working directory >> | to "../../../debian": no such file or >> | directory >> > > I've already reported the same issue[1] and have posted a possible > solution[2] although I haven't seen any feedback from Pat or anyone else. > >> >> strace shows the failing chdir call is from git-gui itself, after >> getcwd() told him that it is in the dir that is indeed the workdir >> already. >> > > -- > > [1] - http://article.gmane.org/gmane.comp.version-control.git/247511 > [2] - http://article.gmane.org/gmane.comp.version-control.git/247564 > > -- > 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: [GUILT v2 06/29] Fix the do_get_patch function.
On Tue, May 13, 2014 at 10:30:42PM +0200, Per Cederqvist wrote:
> A patch file consists of:
>
> (1) the description
> (2) optional diffstat
> (3) the patches
>
> When extracting the patch, we only want part 3. The do_get_patch used
> to give us part 2 and part 3. That made for instance this series of
> operations fail if guilt.diffstat is true:
>
> guilt push empty-1
> guilt push empty-2
> guilt pop
> guilt fold empty-2
> guilt pop
> guilt push
I would probably include the actual error here. I got the following (using
patch names a & b):
$ guilt pop
Now at a.
$ guilt fold b
error: No changes
$ guilt pop
All patches popped.
$ guilt pu
Applying patch..a
error: No changes
To force apply this patch, use 'guilt push -f'
The diff itself is good.
Signed-off-by: Josef 'Jeff' Sipek
>
> Signed-off-by: Per Cederqvist
> ---
> guilt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/guilt b/guilt
> index 8701481..3fc524e 100755
> --- a/guilt
> +++ b/guilt
> @@ -334,7 +334,7 @@ do_get_patch()
> {
> awk '
> BEGIN{}
> -/^(diff |---$|--- )/ {patch = 1}
> +/^(diff |--- )/ {patch = 1}
> patch == 1 {print $0}
> END{}
> ' < "$1"
> --
> 1.8.3.1
>
--
I already backed up the [server] once, I can do it again.
- a sysadmin threatening to do more frequent backups
--
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: getbundle changed in mercurial 3.0
Junio C Hamano wrote: > The way I envision the longer term shape of git-remote-hg.py in the > contrib/ is either one of these two: > > (1) manage it just like contrib/hooks/multimail/, keeping a > reasonably fresh and known-to-be-good snapshot, while making it > clear that my tree is not the authoritative copy people should > work off of; > > (2) treat it just like contrib/emacs/vc-git.el, which found a > better home and left my tree at 78513869 (emacs: Remove the no > longer maintained vc-git package., 2009-02-07); or > > The first one may be more preferrable between the two, if only because > distros would need time to adjust where they pick up the source > material to package up, but it still needs cooperation with the > "authoritative upstream" and this project to allow us to at least > learn when the good time to import such good snapshots. I will not do that. If you want my help to improve *your* tree, you have to start by answering the *one* question I've repeatedly asked you to clarify. In fact if you go for this I would consider it an act of sabotage against these new projects. If you hadn't made me waste all this time chasing a non-attainable goal, these projects would already be packaged by distributions, instead of hidden in some corner of /usr/share. Distributions wouldn't even be aware of the move, and it might take bug reports to make them realize that. There will be already enough damage to the reputation of these tools with Git v2.0 shipping them broken. Not aligned at all with your previous statement that you wanted these to "flourisn". In fact, I think you should remove them already from v2.0. Because this doesn't need release candidates. Unless you think user feedback will make you change your mind about not graduating them, moving them in 2.0, or 2.1 will be the same, since you are going to ignore the feedback anyway. This also has the advantage that you won't be shipping them broken in v2.0. At the very least there should be a big fat message warning each time the tools are run, warning the users that they are unmaintained, and the right location for them. And yes, I also think this should be done for v2.0. -- 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: [GUILT v2 04/29] Allow "guilt import-commit" to run from a dir which contains spaces.
Signed-off-by: Josef 'Jeff' Sipek On Tue, May 13, 2014 at 10:30:40PM +0200, Per Cederqvist wrote: > Signed-off-by: Per Cederqvist > --- > guilt-import-commit | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/guilt-import-commit b/guilt-import-commit > index 20dcee2..f14647c 100755 > --- a/guilt-import-commit > +++ b/guilt-import-commit > @@ -23,7 +23,7 @@ if ! must_commit_first; then > fi > > disp "About to begin conversion..." >&2 > -disp "Current head: `cat $GIT_DIR/refs/heads/\`git_branch\``" >&2 > +disp "Current head: `git rev-parse \`git_branch\``" >&2 > > for rev in `git rev-list $rhash`; do > s=`git log --pretty=oneline -1 $rev | cut -c 42-` > @@ -46,7 +46,7 @@ for rev in `git rev-list $rhash`; do > do_make_header $rev > echo "" > git diff --binary $rev^..$rev > - ) > $GUILT_DIR/$branch/$fname > + ) > "$GUILT_DIR/$branch/$fname" > > # FIXME: grab the GIT_AUTHOR_DATE from the commit object and set the > # timestamp on the patch > @@ -68,6 +68,6 @@ for rev in `git rev-list $rhash`; do > done > > disp "Done." >&2 > -disp "Current head: `cat $GIT_DIR/refs/heads/\`git_branch\``" >&2 > +disp "Current head: `git rev-parse \`git_branch\``" >&2 > > } > -- > 1.8.3.1 > -- Once you have their hardware. Never give it back. (The First Rule of Hardware Acquisition) -- 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 01/17] contrib: remove outdated README
Felipe Contreras writes: > Junio, do you honestly think I am a troll? You certainly are acting like one, aren't you? -- 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: Error using git-remote-hg
On Tue, May 13, 2014 at 03:24:51PM -0500, Felipe Contreras wrote:
> William Giokas wrote:
> > On Tue, May 13, 2014 at 02:09:55PM -0500, Felipe Contreras wrote:
> > > As you say, it's perfectly OK.
> >
> > But wrong. Yes, it works, but it's not how it should be done when we
> > have a code review such as this. It should simply not be done and makes
> > no sense to do with an 'if ; else' kind of thing later in the
> > application.
>
> That's exactly how it should be done. Maybe this visualization helps
>
> from mercurial import hg, ui, bookmarks, ...# for hg >= 3.0
> from mercurial import node, error, extensions, ... # for hg >= 3.0
> from mercurial import changegroup # for hg >= 3.0
>
> if check_version(3, 0):
> cg = changegroup.getbundle(repo, 'push', ...# for hg >= 3.0
> else:
> cg = repo.getbundle('push', heads=list(p_revs) # for hg < 3.0
>
> Eventually the code would become:
>
> from mercurial import hg, ui, bookmarks, ...# for hg >= 3.0
> from mercurial import node, error, extensions, ... # for hg >= 3.0
> from mercurial import changegroup # for hg >= 3.0
>
> cg = changegroup.getbundle(repo, 'push', ...# for hg >= 3.0
No, the way that it's going to change is that the import statements will
change, not the 'if:else' things. It would work exactly the same with
this, however the things that are used only in a specific version for
this are stated up front. Maybe this visualization helps for what I have
set up::
from mercurial import ...# for all hg
try:
from mercurial.changegroup import getbundle # for hg with
# changegroup.getbundle,
# regardless of version
except ImportError: # for hg from before the
def getbundle(__empty__, **kwargs): # move to changegroup
return repo.getbundle(**kwargs)
...
cg = getbundle(...)
When we eventually remove support for mercurial that uses
repo.getbundle:
from mercurial import changegroup, ... # for all hg
...
cg = changegroup.getbundle(...)
That should make sense. You could even just remove the try: and except:
bits and just to 'from mercurial.changegroup import getbundle' and not
mess with the meat of the code at all.
> The fact that at some point 'import changegroup' was not needed is
> irrelevant.
>
> Primarily we want to support the current version of Mercurial.
> Secondarily we want to support older versions. That's why we add the
> repo.getbundle() code (as an addendum to the core part).
So I use arch myself, and I am very used to the 'rolling release' model
that it employs. I do agree that we should concentrate support for the
latest release, but for a project like git the other versions of hg
cannot be ignored, as this project is used on so many different systems.
> > > We could try that, but that would assume we want to maintain getbundle()
> > > for the long run, and I personally don't want to do that. I would rather
> > > contact the Mercurial developers about ways in which the push() method
> > > can be improved so we don't need to have our own version. Hopefully
> > > after it's improved we wouldn't have to call getbundle().
> >
> > Assuming that mercurial <3.0 will not change, then this should never
> > need to change.
>
> But it should. Otherwise the code will keep having more and more hacks
> and it will become less and less maintainable.
>
> That's why we don't have code for hg 1.0; because it would require too
> many hacks, and nobody cared anyway.
>
> Just like nobody cares about hg 1.0, eventually nobody will care about
> hg 2.0.
Yes, it can change. Eventually hg 2.0 will be defunct and no one will
use it, but what happens if they go back to the old 2.0 style for
getbundle in hg 4.0? We're already good. What if they switch it back in
4.1? This works for all of those conditions, and doesn't do anything if
we're able to get mercurial.changegroup.getbundle.
> > Changes in 'getbundle' upstream would require changes either way.
>
> I doubt we will see any more changes in getbundle, at least not until
> 4.0, and hopefully by then we wouldn't be using it anyway. I am willing
> to bet we won't see those changes.
>
> > > Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at
> > > some point we would want to remove the hacks for older versions. When we
> > > do so we would want the import to remain unconditionally, and remove the
> > > 'check_version(3, 0)' which is already helping to explain what the code
> > > is for without the need of comments.
> >
> > The same exact thing can be done with this. In fact, it would probably
> > allow us to have better future-proofing with regards to new versions of
> > mercurial, there would just be different try:except statements at the
> > beginning.
>
> No, your code doesn'
Re: [PATCH v2] remote-hg: getbundle changed in mercurial 3.0
William Giokas <1007...@gmail.com> writes:
> +try:
> +from mercurial.changegroup import getbundle
> +
> +except ImportError:
> +def getbundle(repo, **kwargs):
> +return repo.getbundle(**kwargs)
> +
> import re
> import sys
> import os
> @@ -985,7 +992,8 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
> if not checkheads(repo, remote, p_revs):
> return None
>
> -cg = repo.getbundle('push', heads=list(p_revs), common=common)
> +cg = getbundle(repo=repo, source='push', heads=list(p_revs),
> + common=common)
Yikes, this is starting to look bad, but the thing being in Python,
perhaps that is the best we could do if we want a solution that is
viable in the longer term.
As a short-term band-aid to be merged/cherry-picked to maintenance
tracks post 2.0 final, I actually would prefer 58aee086 (remote-hg:
add support for hg v3.0, 2014-05-03) for its simplicity.
I dunno. Thankfully I do not have to decide right now ;-)
--
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: [GUILT v2 29/29] Added a short style guide, and Emacs settings.
On Tue, May 13, 2014 at 10:31:05PM +0200, Per Cederqvist wrote: > Signed-off-by: Per Cederqvist > --- > .dir-locals.el | 3 +++ > Documentation/Contributing | 15 +++ > 2 files changed, 18 insertions(+) > create mode 100644 .dir-locals.el > > diff --git a/.dir-locals.el b/.dir-locals.el > new file mode 100644 > index 000..50ef2b7 > --- /dev/null > +++ b/.dir-locals.el > @@ -0,0 +1,3 @@ > +((nil . ((indent-tabs-mode . t) > + (tab-width . 8))) > + (sh-mode . ((sh-basic-offset . 8 I'll have to trust you on this one. All I see is a bunch of cons cells :) > diff --git a/Documentation/Contributing b/Documentation/Contributing > index abf3480..0da49d6 100644 > --- a/Documentation/Contributing > +++ b/Documentation/Contributing > @@ -4,6 +4,21 @@ Documentation/SubmittingPatches file. :) > > 1) Hack on the code a bit > > +Please follow this style guide: > + > + - Use tabs for indentation. > + > + - Put "then" on the same line as "if". > + > + - Follow the style of the existing code, except if it breaks the > + above guidlines. > + > + - If you change the code to conform to the style guide, please do so > + in a separate commit that does not change anything else. > + > +Please check that you change does not break "make test". Please add > +new testcases for any new functionality, and if you fix a bug. > + Adding this blurb here is a good idea. Thanks! Signed-off-by: Josef 'Jeff' Sipek > 2) Make a patch: > > Use "diff -up" or "diff -uprN" to create patches. Or simply use git to > -- > 1.8.3.1 > -- Evolution, n.: A hypothetical process whereby infinitely improbable events occur with alarming frequency, order arises from chaos, and no one is given credit. -- 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: [GUILT v2 00/29] Teach guilt import-commit how to create legal patch names, and more
On Tue, May 13, 2014 at 04:45:47PM -0400, Theodore Ts'o wrote: > On Tue, May 13, 2014 at 10:30:36PM +0200, Per Cederqvist wrote: ... > > - Changed behavior: by default, guilt no longer changes branch when > >you push a patch. You need to do "git config guilt.reusebranch > >false" to re-enable that. This patch sets the default value of > >guilt.reusebranch to true; it should in my opinion change to false > >a year or two after the next release. > > We've been living with the "origin" -> "guilt/origin" branch change > for a year already, and in fact, these days I've gotten used to the > new behavior. Is it really worth it to change the default? So, at first I was skeptical about the branch name prefix change. I've used it for about a year now, and I love it. When I first read Per's idea to change the default to the old-style, I was a bit sad but I understand the motivation. I'm open to either mode being the default since it's easy enough for me to change it for me (thanks, ~/.gitconfig) but I think more people should benefit from the added safety against accidental git-push. (I also like being able to use guilt/master..master to get only the commits I care about.) Thoughts? Jeff. -- Intellectuals solve problems; geniuses prevent them - Albert Einstein -- 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: [GUILT v2 00/29] Teach guilt import-commit how to create legal patch names, and more
On Tue, May 13, 2014 at 10:30:36PM +0200, Per Cederqvist wrote: > I recently found myself sitting on a train with a computer in front of > me. I tried to use "guilt import-commit", which seemed to work, but > when I tried to "guilt push" the commits I had just imported I got > some errors. It turned out that "guilt import-commit" had generated > invalid patch names. Thanks, I ran into this just last night (although I had manually created the patch file from an e-mail I received instead of using "guilt import-commit"). > - Changed behavior: by default, guilt no longer changes branch when >you push a patch. You need to do "git config guilt.reusebranch >false" to re-enable that. This patch sets the default value of >guilt.reusebranch to true; it should in my opinion change to false >a year or two after the next release. We've been living with the "origin" -> "guilt/origin" branch change for a year already, and in fact, these days I've gotten used to the new behavior. Is it really worth it to change the default? - Ted -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib: completion: fix 'eread()' namespace
A reasonable regression fix. Will queue for 2.0 final. 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
[GUILT v2 29/29] Added a short style guide, and Emacs settings.
Signed-off-by: Per Cederqvist --- .dir-locals.el | 3 +++ Documentation/Contributing | 15 +++ 2 files changed, 18 insertions(+) create mode 100644 .dir-locals.el diff --git a/.dir-locals.el b/.dir-locals.el new file mode 100644 index 000..50ef2b7 --- /dev/null +++ b/.dir-locals.el @@ -0,0 +1,3 @@ +((nil . ((indent-tabs-mode . t) +(tab-width . 8))) + (sh-mode . ((sh-basic-offset . 8 diff --git a/Documentation/Contributing b/Documentation/Contributing index abf3480..0da49d6 100644 --- a/Documentation/Contributing +++ b/Documentation/Contributing @@ -4,6 +4,21 @@ Documentation/SubmittingPatches file. :) 1) Hack on the code a bit +Please follow this style guide: + + - Use tabs for indentation. + + - Put "then" on the same line as "if". + + - Follow the style of the existing code, except if it breaks the + above guidlines. + + - If you change the code to conform to the style guide, please do so + in a separate commit that does not change anything else. + +Please check that you change does not break "make test". Please add +new testcases for any new functionality, and if you fix a bug. + 2) Make a patch: Use "diff -up" or "diff -uprN" to create patches. Or simply use git to -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 22/29] The log.decorate setting should not influence patchbomb.
Signed-off-by: Per Cederqvist Signed-off-by: Josef 'Jeff' Sipek --- guilt-patchbomb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guilt-patchbomb b/guilt-patchbomb index 1231418..164b10c 100755 --- a/guilt-patchbomb +++ b/guilt-patchbomb @@ -47,7 +47,7 @@ if [ $? -ne 0 ]; then fi # display the list of commits to be sent as patches -git log --pretty=oneline "$r" | cut -c 1-8,41- | $pager +git log --no-decorate --pretty=oneline "$r" | cut -c 1-8,41- | $pager _disp "Are these what you want to send? [Y/n] " read n -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 23/29] The log.decorate setting should not influence guilt rebase.
Signed-off-by: Per Cederqvist Signed-off-by: Josef 'Jeff' Sipek --- guilt-rebase | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guilt-rebase b/guilt-rebase index fd28e48..a1714a0 100755 --- a/guilt-rebase +++ b/guilt-rebase @@ -66,7 +66,7 @@ pop_all_patches git merge --no-commit $upstream > /dev/null 2> /dev/null disp "" -log=`git log -1 --pretty=oneline` +log=`git log -1 --no-decorate --pretty=oneline` disp "HEAD is now at `echo $log | cut -c 1-7`... `echo "$log" | cut -c 41-`" # -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 21/29] The log.decorate setting should not influence import-commit.
Use --no-decorate in the call to git log that tries to read the commit message to produce patch names. Otherwise, if the user has set log.decorate to short or full, the patch name will be less useful. Modify the t-034.sh test case to demonstrate that this is needed. Signed-off-by: Per Cederqvist Signed-off-by: Josef 'Jeff' Sipek --- guilt-import-commit | 2 +- regression/t-034.out | 2 ++ regression/t-034.sh | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/guilt-import-commit b/guilt-import-commit index 45f2404..1da7c8e 100755 --- a/guilt-import-commit +++ b/guilt-import-commit @@ -26,7 +26,7 @@ disp "About to begin conversion..." >&2 disp "Current head: `git rev-parse \`git_branch\``" >&2 for rev in `git rev-list $rhash`; do - s=`git log --pretty=oneline -1 $rev | cut -c 42-` + s=`git log --no-decorate --pretty=oneline -1 $rev | cut -c 42-` # Try to convert the first line of the commit message to a # valid patch name. diff --git a/regression/t-034.out b/regression/t-034.out index bda4399..5d81bd4 100644 --- a/regression/t-034.out +++ b/regression/t-034.out @@ -232,6 +232,7 @@ Date: Mon Jan 1 00:00:00 2007 + Signed-off-by: Commiter Name % guilt init +% git config log.decorate short % guilt import-commit base..HEAD About to begin conversion... Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541 @@ -259,6 +260,7 @@ Converting 45e81b51 as the_sequence_.lock-_is_forbidden Converting eebb76e9 as the_sequence_-._is_forbidden Done. Current head: d4850419ccc1146c7169f500725ce504b9774ed0 +% git config log.decorate no % guilt push -a Applying patch..the_sequence_-._is_forbidden.patch Patch applied. diff --git a/regression/t-034.sh b/regression/t-034.sh index f41f958..648d009 100755 --- a/regression/t-034.sh +++ b/regression/t-034.sh @@ -57,7 +57,9 @@ cmd git log # Import all the commits to guilt. cmd guilt init +cmd git config log.decorate short cmd guilt import-commit base..HEAD +cmd git config log.decorate no for patch in .git/patches/master/*.patch; do touch -a -m -t "$TOUCH_DATE" "$patch" -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 24/29] disp no longer processes backslashes.
Only one invocation of "disp" or "_disp" actually needed backslash
processing. In quite a few instances, it was wrong to do backslash
processing, as the message contained data derived from the user.
Created the new function "disp_e" that should be used when backslash
processing is required, and changed "disp" and "_disp" to use printf
code %s instead of "%b".
Signed-off-by: Per Cederqvist
---
guilt | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/guilt b/guilt
index 23cc2da..9947acc 100755
--- a/guilt
+++ b/guilt
@@ -36,15 +36,24 @@ usage()
exit 1
}
-# echo -n is a bashism, use printf instead
+# Print arguments, but no trailing newline.
+# (echo -n is a bashism, use printf instead)
_disp()
{
- printf "%b" "$*"
+ printf "%s" "$*"
}
-# echo -e is a bashism, use printf instead
+# Print arguments.
+# (echo -E is a bashism, use printf instead)
disp()
{
+ printf "%s\n" "$*"
+}
+
+# Print arguments, processing backslash sequences.
+# (echo -e is a bashism, use printf instead)
+disp_e()
+{
printf "%b\n" "$*"
}
@@ -117,7 +126,7 @@ else
disp ""
disp "Example:"
- disp "\tguilt push"
+ disp_e "\tguilt push"
# now, let's exit
exit 1
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 28/29] Added guilt.reusebranch configuration option.
When the option is true (the default), Guilt does not create a new Git branch when patches are applied. This way, you can switch between Guilt 0.35 and the current version of Guilt with no issues. At a future time, maybe a year after Guilt with guilt.reusebranch support is released, the default should be changed to "false" to take advantage of the ability to use a separate Git branch when patches are applied. Signed-off-by: Per Cederqvist --- guilt| 28 +++- regression/scaffold | 1 + regression/t-062.out | 441 +++ regression/t-062.sh | 137 4 files changed, 601 insertions(+), 6 deletions(-) create mode 100644 regression/t-062.out create mode 100755 regression/t-062.sh diff --git a/guilt b/guilt index 9947acc..7c830eb 100755 --- a/guilt +++ b/guilt @@ -853,6 +853,9 @@ guilt_push_diff_context=1 # default diffstat value: true or false DIFFSTAT_DEFAULT="false" +# default old_style_prefix value: true or false +REUSE_BRANCH_DEFAULT="true" + # Prefix for guilt branches. GUILT_PREFIX=guilt/ @@ -864,6 +867,10 @@ GUILT_PREFIX=guilt/ diffstat=`git config --bool guilt.diffstat` [ -z "$diffstat" ] && diffstat=$DIFFSTAT_DEFAULT +# reuse Git branch? +reuse_branch=`git config --bool guilt.reusebranch` +[ -z "$reuse_branch" ] && reuse_branch=$REUSE_BRANCH_DEFAULT + # # The following gets run every time this file is source'd # @@ -928,13 +935,22 @@ else die "Unsupported operating system: $UNAME_S" fi -if [ "$branch" = "$raw_git_branch" ] && [ -n "`get_top 2>/dev/null`" ] -then -# This is for compat with old repositories that still have a -# pushed patch without the new-style branch prefix. -old_style_prefix=true +if [ -n "`get_top 2>/dev/null`" ]; then + # If there is at least one pushed patch, we set + # old_style_prefix according to how it was pushed. It is only + # possible to change the prefix style while no patches are + # applied. + if [ "$branch" = "$raw_git_branch" ]; then + old_style_prefix=true + else + old_style_prefix=false + fi else -old_style_prefix=false + if $reuse_branch; then + old_style_prefix=true + else + old_style_prefix=false + fi fi _main "$@" diff --git a/regression/scaffold b/regression/scaffold index e4d7487..e4d2f35 100644 --- a/regression/scaffold +++ b/regression/scaffold @@ -93,6 +93,7 @@ function setup_git_repo git config log.date default git config log.decorate no git config guilt.diffstat false + git config guilt.reusebranch false } function setup_guilt_repo diff --git a/regression/t-062.out b/regression/t-062.out new file mode 100644 index 000..727b436 --- /dev/null +++ b/regression/t-062.out @@ -0,0 +1,441 @@ +% setup_repo +% git config guilt.reusebranch true +% guilt push -a +Applying patch..modify +Patch applied. +Applying patch..add +Patch applied. +Applying patch..remove +Patch applied. +Applying patch..mode +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 71596bf71b72c2717e1aee378aabefbfa19ab7c8 .git/patches/master/status +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +r 33633e7a1aa31972f125878baf7807be57b1672d .git/refs/patches/master/modify +r 37d588cc39848368810e88332bd03b083f2ce3ac .git/refs/patches/master/add +r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba .git/refs/patches/master/mode +r ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 .git/refs/patches/master/remove +% git for-each-ref +ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/heads/master +37d588cc39848368810e88332bd03b083f2ce3ac commitrefs/patches/master/add +ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/patches/master/mode +33633e7a1aa31972f125878baf7807be57b1672d commit refs/patches/master/modify +ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit refs/patches/master/remove +% git for-each-ref +ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/heads/master +37d588cc39848368810e88332bd03b083f2ce3ac commitrefs/patches/master/add +ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/patches/master/mode +33633e7a1aa31972f125878baf7807be57b1672d commit refs/patches/master/modify +ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit refs/patches/master/remove +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patch
[GUILT v2 25/29] "guilt push" now fails when there are no more patches to push.
This makes it easier to script operations on the entire queue, for
example run the test suite on each patch in the queue:
guilt pop -a;while guilt push; do make test||break; done
This brings "guilt push" in line with the push operation in Mercurial
Queues (hg qpush), which fails when there are no patches to apply.
Updated the test suite.
"guilt push -a" still does not fail. (It successfully manages to
ensure that all patches are pushed, even if it did not have to do
anything to make it so.)
Signed-off-by: Per Cederqvist
---
guilt-push | 19 ++-
regression/t-020.out | 89
regression/t-020.sh | 13 +++-
3 files changed, 113 insertions(+), 8 deletions(-)
diff --git a/guilt-push b/guilt-push
index 67687e7..39c125e 100755
--- a/guilt-push
+++ b/guilt-push
@@ -56,6 +56,12 @@ fi
patch="$1"
[ ! -z "$all" ] && patch="-a"
+# Treat "guilt push" as "guilt push -n 1".
+if [ -z "$patch" ]; then
+ patch=1
+ num=t
+fi
+
if [ "$patch" = "-a" ]; then
# we are supposed to push all patches, get the last one out of
# series
@@ -65,7 +71,7 @@ if [ "$patch" = "-a" ]; then
die "There are no patches to push."
fi
elif [ ! -z "$num" ]; then
- # we are supposed to pop a set number of patches
+ # we are supposed to push a set number of patches
[ "$patch" -lt 0 ] && die "Invalid number of patches to push."
@@ -78,11 +84,6 @@ elif [ ! -z "$num" ]; then
# clamp to minimum
[ $tidx -lt $eidx ] && eidx=$tidx
-elif [ -z "$patch" ]; then
- # we are supposed to push only the next patch onto the stack
-
- eidx=`wc -l < "$applied"`
- eidx=`expr $eidx + 1`
else
# we're supposed to push only up to a patch, make sure the patch is
# in the series
@@ -109,7 +110,11 @@ if [ "$sidx" -gt "$eidx" ]; then
else
disp "File series fully applied, ends at patch `get_series |
tail -n 1`"
fi
- exit 0
+ if [ -n "$all" ]; then
+ exit 0
+ else
+ exit 1
+ fi
fi
get_series | sed -n -e "${sidx},${eidx}p" | while read p
diff --git a/regression/t-020.out b/regression/t-020.out
index 7e07efa..23cb9db 100644
--- a/regression/t-020.out
+++ b/regression/t-020.out
@@ -270,6 +270,95 @@ index 000..8baef1b
+++ b/def
@@ -0,0 +1 @@
+abc
+% guilt push
+File series fully applied, ends at patch mode
+% guilt push -a
+File series fully applied, ends at patch mode
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove
+f 71596bf71b72c2717e1aee378aabefbfa19ab7c8 .git/patches/master/status
+f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add
+f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify
+r 33633e7a1aa31972f125878baf7807be57b1672d .git/refs/patches/master/modify
+r 37d588cc39848368810e88332bd03b083f2ce3ac .git/refs/patches/master/add
+r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba .git/refs/patches/master/mode
+r ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 .git/refs/patches/master/remove
+% git log -p
+commit ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba
+Author: Author Name
+Date: Mon Jan 1 00:00:00 2007 +
+
+patch mode
+
+diff --git a/def b/def
+old mode 100644
+new mode 100755
+
+commit ffb7faa126a6d91bcdd44a494f76b96dd860b8b9
+Author: Author Name
+Date: Mon Jan 1 00:00:00 2007 +
+
+patch remove
+
+diff --git a/abd b/abd
+deleted file mode 100644
+index fd3896d..000
+--- a/abd
/dev/null
+@@ -1 +0,0 @@
+-öuؽáZâñeÏÈE£ WÀV¼/U?Ú<|¢@6¤8'H¸1G_ͧ*·ðRÒ¤
ªÂ~·
+\ No newline at end of file
+
+commit 37d588cc39848368810e88332bd03b083f2ce3ac
+Author: Author Name
+Date: Mon Jan 1 00:00:00 2007 +
+
+patch add
+
+diff --git a/abd b/abd
+new file mode 100644
+index 000..fd3896d
+--- /dev/null
b/abd
+@@ -0,0 +1 @@
++öuؽáZâñeÏÈE£ WÀV¼/U?Ú<|¢@6¤8'H¸1G_ͧ*·ðRÒ¤
ªÂ~·
+\ No newline at end of file
+
+commit 33633e7a1aa31972f125878baf7807be57b1672d
+Author: Author Name
+Date: Mon Jan 1 00:00:00 2007 +
+
+patch modify
+
+diff --git a/def b/def
+index 8baef1b..7d69c2f 100644
+--- a/def
b/def
+@@ -1 +1,2 @@
+ abc
++asjhfksad
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name
+Date: Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
b/def
+@@ -0,0 +1 @@
++abc
% guilt pop --all
All patches popped.
% guilt push
diff --git a/regression/t-020.sh b/regression/t-020.sh
index 906aec6..0f9f85d 100755
--- a/regression/t-020.sh
+++ b/regression/t-020.sh
@@ -26,6 +26,17
[GUILT v2 27/29] Minor testsuite fix.
Fix remove_topic() in t-061.sh so that it doesn't print a git hash.
Signed-off-by: Per Cederqvist
Signed-off-by: Josef 'Jeff' Sipek
---
regression/t-061.out | 1 -
regression/t-061.sh | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/regression/t-061.out b/regression/t-061.out
index ef0f335..60ad56d 100644
--- a/regression/t-061.out
+++ b/regression/t-061.out
@@ -381,7 +381,6 @@ ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commit
refs/patches/master/mode
ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit
refs/patches/master/remove
% guilt pop -a
No patches applied.
-ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba
% git checkout guilt/master
Switched to branch "guilt/master"
% guilt pop -a
diff --git a/regression/t-061.sh b/regression/t-061.sh
index 6192f1b..db26e12 100755
--- a/regression/t-061.sh
+++ b/regression/t-061.sh
@@ -15,7 +15,7 @@ old_style_branch() {
remove_topic() {
cmd guilt pop -a
- if git rev-parse --verify --quiet guilt/master
+ if git rev-parse --verify --quiet guilt/master >/dev/null
then
cmd git checkout guilt/master
else
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 26/29] "guilt pop" now fails when there are no more patches to pop.
This is analogous to how "guilt push" now fails when there are no more patches to push. Like push, the "--all" argument still succeeds even if there was no need to pop anything. Updated the test suite. Signed-off-by: Per Cederqvist --- guilt-pop| 17 +++-- regression/t-021.out | 2 ++ regression/t-021.sh | 6 ++ regression/t-061.sh | 6 +- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/guilt-pop b/guilt-pop index f0e647f..191313e 100755 --- a/guilt-pop +++ b/guilt-pop @@ -49,9 +49,19 @@ fi patch="$1" [ ! -z "$all" ] && patch="-a" +# Treat "guilt pop" as "guilt pop -n 1". +if [ -z "$patch" ]; then + patch=1 + num=t +fi + if [ ! -s "$applied" ]; then disp "No patches applied." - exit 0 + if [ "$patch" = "-a" ]; then + exit 0 + else + exit 1 + fi elif [ "$patch" = "-a" ]; then # we are supposed to pop all patches @@ -68,11 +78,6 @@ elif [ ! -z "$num" ]; then # catch underflow [ $eidx -lt 0 ] && eidx=0 [ $eidx -eq $sidx ] && die "No patches requested to be removed." -elif [ -z "$patch" ]; then - # we are supposed to pop only the current patch on the stack - - sidx=`wc -l < "$applied"` - eidx=`expr $sidx - 1` else # we're supposed to pop only up to a patch, make sure the patch is # in the series diff --git a/regression/t-021.out b/regression/t-021.out index 9b42d9c..58be12f 100644 --- a/regression/t-021.out +++ b/regression/t-021.out @@ -287,6 +287,8 @@ index 000..8baef1b +++ b/def @@ -0,0 +1 @@ +abc +% guilt pop +No patches applied. % guilt push --all Applying patch..modify Patch applied. diff --git a/regression/t-021.sh b/regression/t-021.sh index 614e870..e0d2dc1 100755 --- a/regression/t-021.sh +++ b/regression/t-021.sh @@ -23,6 +23,12 @@ guilt series | _tac | while read n ; do done # +# pop when there is nothing to pop +# + +shouldfail guilt pop + +# # push all # cmd guilt push --all diff --git a/regression/t-061.sh b/regression/t-061.sh index 1411baa..6192f1b 100755 --- a/regression/t-061.sh +++ b/regression/t-061.sh @@ -48,7 +48,11 @@ cmd list_files for i in `seq 5` do - cmd guilt pop + if [ $i -ge 5 ]; then + shouldfail guilt pop + else + cmd guilt pop + fi cmd git for-each-ref cmd guilt push cmd git for-each-ref -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 20/29] "guilt graph": Handle patch names containing quotes.
Quote quotes with a backslash in the "guilt graph" output. Otherwise,
the "dot" file could contain syntax errors.
Added a test case.
Signed-off-by: Per Cederqvist
---
guilt-graph | 2 ++
regression/t-033.out | 22 ++
regression/t-033.sh | 9 +
3 files changed, 33 insertions(+)
diff --git a/guilt-graph b/guilt-graph
index 924a63e..0857e0d 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -57,6 +57,8 @@ while [ "$current" != "$base" ]; do
}"`
[ -z "$pname" ] && pname="?"
+ pname="`printf \"%s\" \"$pname\" | sed 's/\"/\"/g'`"
+
disp "# checking rev $current"
disp " \"$current\" [label=\"$pname\"]"
diff --git a/regression/t-033.out b/regression/t-033.out
index 3d1c61f..c120d4f 100644
--- a/regression/t-033.out
+++ b/regression/t-033.out
@@ -66,3 +66,25 @@ digraph G {
"ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
"891bc14b5603474c9743fd04f3da888644413dc5" ->
"ff2775f8d1dc753f635830adcc3a067e0b681e2d"; // ?
}
+% guilt new a-"better&quicker'-patch.patch
+% git add file.txt
+% guilt refresh
+Patch a-"better&quicker'-patch.patch refreshed
+% guilt pop
+Now at c.patch.
+% guilt push
+Applying patch..a-"better&quicker'-patch.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev bc7df666a646739eaf559af23cab72f2bfd01f0e
+ "bc7df666a646739eaf559af23cab72f2bfd01f0e"
[label="a-\"better&quicker'-patch.patch"]
+# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
+ "891bc14b5603474c9743fd04f3da888644413dc5" [label="c.patch"]
+ "bc7df666a646739eaf559af23cab72f2bfd01f0e" ->
"891bc14b5603474c9743fd04f3da888644413dc5"; // ?
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+ "c7014443c33d2b0237293687ceb9cbd38313df65" [label="b.patch"]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+ "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
+ "891bc14b5603474c9743fd04f3da888644413dc5" ->
"ff2775f8d1dc753f635830adcc3a067e0b681e2d"; // ?
+}
diff --git a/regression/t-033.sh b/regression/t-033.sh
index fac081e..9fe1827 100755
--- a/regression/t-033.sh
+++ b/regression/t-033.sh
@@ -50,3 +50,12 @@ cmd git add file.txt
cmd guilt refresh
fixup_time_info c.patch
cmd guilt graph
+
+# A patch name that contains funky characters, including unbalanced
+# quotes.
+cmd guilt new "a-\"better&quicker'-patch.patch"
+cmd echo d >> file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info "a-\"better&quicker'-patch.patch"
+cmd guilt graph
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 19/29] Check that "guilt graph" works when working on a branch with a comma.
git branch names can contain commas. Check that "guilt graph" works
even in that case.
Signed-off-by: Per Cederqvist
---
regression/t-033.out | 65
regression/t-033.sh | 39 +++
2 files changed, 104 insertions(+)
diff --git a/regression/t-033.out b/regression/t-033.out
index 76613f9..3d1c61f 100644
--- a/regression/t-033.out
+++ b/regression/t-033.out
@@ -1,3 +1,68 @@
% setup_repo
% guilt graph
No patch applied.
+%% Testing branch a,graph
+% git checkout -b a,graph master
+Switched to a new branch 'a,graph'
+% guilt init
+% guilt new a.patch
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..a.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev 95275d7c05c6a6176d3941374115b91272877f6c
+ "95275d7c05c6a6176d3941374115b91272877f6c" [label="a.patch"]
+}
+% git add file.txt
+% guilt refresh
+Patch a.patch refreshed
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..a.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+ "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
+}
+%% Adding an unrelated file in a new patch. No deps.
+% guilt new b.patch
+% git add file2.txt
+% guilt refresh
+Patch b.patch refreshed
+% guilt pop
+Now at a.patch.
+% guilt push
+Applying patch..b.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+ "c7014443c33d2b0237293687ceb9cbd38313df65" [label="b.patch"]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+ "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
+}
+%% Changing a file already changed in the first patch adds a dependency.
+% guilt new c.patch
+% git add file.txt
+% guilt refresh
+Patch c.patch refreshed
+% guilt pop
+Now at b.patch.
+% guilt push
+Applying patch..c.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
+ "891bc14b5603474c9743fd04f3da888644413dc5" [label="c.patch"]
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+ "c7014443c33d2b0237293687ceb9cbd38313df65" [label="b.patch"]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+ "ff2775f8d1dc753f635830adcc3a067e0b681e2d" [label="a.patch"]
+ "891bc14b5603474c9743fd04f3da888644413dc5" ->
"ff2775f8d1dc753f635830adcc3a067e0b681e2d"; // ?
+}
diff --git a/regression/t-033.sh b/regression/t-033.sh
index a3a8981..fac081e 100755
--- a/regression/t-033.sh
+++ b/regression/t-033.sh
@@ -3,6 +3,13 @@
# Test the graph code
#
+function fixup_time_info
+{
+ cmd guilt pop
+ touch -a -m -t "$TOUCH_DATE" ".git/patches/a,graph/$1"
+ cmd guilt push
+}
+
source "$REG_DIR/scaffold"
cmd setup_repo
@@ -11,3 +18,35 @@ cmd setup_repo
# message when no patches are applied. (An older version of guilt
# used to enter an endless loop in this situation.)
shouldfail guilt graph
+
+echo "%% Testing branch a,graph"
+cmd git checkout -b a,graph master
+
+cmd guilt init
+
+cmd guilt new a.patch
+
+fixup_time_info a.patch
+cmd guilt graph
+
+cmd echo a >> file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info a.patch
+cmd guilt graph
+
+echo "%% Adding an unrelated file in a new patch. No deps."
+cmd guilt new b.patch
+cmd echo b >> file2.txt
+cmd git add file2.txt
+cmd guilt refresh
+fixup_time_info b.patch
+cmd guilt graph
+
+echo "%% Changing a file already changed in the first patch adds a dependency."
+cmd guilt new c.patch
+cmd echo c >> file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info c.patch
+cmd guilt graph
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 18/29] guilt-graph: Handle commas in branch names.
This fix relies on the fact that git branch names can not contain ":".
Signed-off-by: Per Cederqvist
Signed-off-by: Josef 'Jeff' Sipek
---
guilt-graph | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guilt-graph b/guilt-graph
index 56d0e77..924a63e 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -51,7 +51,7 @@ safebranch=`echo "$branch"|sed 's%/%/%g'`
while [ "$current" != "$base" ]; do
pname=`git show-ref | sed -n -e "
/^$current refs\/patches\/$safebranch/ {
- s,^$current refs/patches/$branch/,,
+ s:^$current refs/patches/$branch/::
p
q
}"`
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 16/29] Fix backslash handling when creating names of imported patches.
The 'echo %s' construct sometimes processes escape sequences. (This
happens, for instance, under Ubuntu 14.04 when /bin/sh is actually
dash.) We don't want that to happen when we are importing commits, so
use 'printf %s "$s"' instead.
(The -E option of bash that explicitly disables backslash expansion is
not portable; it is not supported by dash.)
Signed-off-by: Per Cederqvist
---
guilt-import-commit | 2 +-
regression/t-034.out | 14 +++---
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/guilt-import-commit b/guilt-import-commit
index 6260c56..45f2404 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -30,7 +30,7 @@ for rev in `git rev-list $rhash`; do
# Try to convert the first line of the commit message to a
# valid patch name.
- fname=`echo $s | sed -e "s/&/and/g" -e "s/[ :]/_/g" -e "s,[/\\],-,g" \
+ fname=`printf %s "$s" | sed -e "s/&/and/g" -e "s/[ :]/_/g" -e
"s,[/\\],-,g" \
-e "s/['\\[{}]//g" -e 's/]//g' -e 's/\*/-/g' \
-e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
-e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
diff --git a/regression/t-034.out b/regression/t-034.out
index 7bc9459..bda4399 100644
--- a/regression/t-034.out
+++ b/regression/t-034.out
@@ -236,7 +236,7 @@ Date: Mon Jan 1 00:00:00 2007 +
About to begin conversion...
Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541
Converting 2a8b1889 as can-have-embedded-single-slashes
-Converting 0a46f8fa as backslash-isorbidden
+Converting 0a46f8fa as backslash-is-forbidden
Converting aedb74fd as x
Converting 30187ed0 as cannot@have@the@sequence@at-brace
Converting 106e8e5a as cannot_end_in_
@@ -300,7 +300,7 @@ Applying patch..cannot@have@the@sequence@at-brace.patch
Patch applied.
Applying patch..x.patch
Patch applied.
-Applying patch..backslash-isorbidden.patch
+Applying patch..backslash-is-forbidden.patch
Patch applied.
Applying patch..can-have-embedded-single-slashes.patch
Patch applied.
@@ -311,7 +311,7 @@ Date: Mon Jan 1 00:00:00 2007 +
Can/have/embedded/single/slashes
-commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141
(refs/patches/master/backslash-isorbidden.patch)
+commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141
(refs/patches/master/backslash-is-forbidden.patch)
Author: Author Name
Date: Mon Jan 1 00:00:00 2007 +
@@ -518,8 +518,6 @@ d .git/patches/master
d .git/refs/patches
d .git/refs/patches/master
f 06beca7069b9e576bd431f65d13862ed5d3e2a0f
.git/patches/master/ctrlisforbidden.patch
-f 08267ec6783ea9d1adae55b275198f7594764ed0 .git/patches/master/series
-f 08267ec6783ea9d1adae55b275198f7594764ed0 .git/patches/master/status
f 09b7e9be44ae5ec3a4bb30f5ee9d4ebc2c042f64
.git/patches/master/two_consecutive_dots_(.)_is_forbidden.patch
f 0b971c9a17aeca2319c93d700ffd98acc2a93451
.git/patches/master/question-mark-is-forbidden.patch
f 2b8392f63d61efc12add554555adae30883993cc
.git/patches/master/cannot-end-in-slash-.patch
@@ -529,7 +527,7 @@ f 34e07c584032df137f19bdb66d93f316f00a5ac8
.git/patches/master/tildeisforbidden
f 49bab499826b63deb2bd704629d60c7268c57aee
.git/patches/master/the_sequence_-._is_forbidden.patch
f 5bcddb8ccb6e6e5e8a61e9e56cb2e0f70cbab2f5
.git/patches/master/cannot@have@the@sequence@at-brace.patch
f 637b982fe14a240de181ae63226b27e0c406b3dc
.git/patches/master/asterisk-is-forbidden.patch
-f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3
.git/patches/master/backslash-isorbidden.patch
+f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3
.git/patches/master/backslash-is-forbidden.patch
f 7b103c3c7ae298cd2334f6f49da48bae1424f77b
.git/patches/master/crisalsoforbidden.patch
f 9b810b8c63779c51d2e7f61ab59cd49835041563 .git/patches/master/x.patch
f a22958d9ae9976fd7b2b5a9d0bcd44bf7ad9b08a
.git/patches/master/caretisforbidden.patch
@@ -537,6 +535,8 @@ f ab325bf5a432937fc6f231d3e8a773a62d53952b
.git/patches/master/multiple-slashes
f cb9cffbd4465bddee266c20ccebd14eb687eaa89
.git/patches/master/delisforbidden.patch
f d0885a1a1fdee0fd1e4fedce3f7acd3100540bc4
.git/patches/master/openbracketisforbidden.patch
f d2903523fb66a346596eabbdd1bda4e52b266440
.git/patches/master/check-multiple-.-dots-.-foo.patch
+f da90de1c84138194524994e0bc3bc4ca8189c999 .git/patches/master/series
+f da90de1c84138194524994e0bc3bc4ca8189c999 .git/patches/master/status
f dfc11f76394059909671af036598c5fbe33001ba
.git/patches/master/space_is_forbidden.patch
f e47474c52d6c893f36d0457f885a6dd1267742bb
.git/patches/master/colon_is_forbidden.patch
f e7a5f8912592d9891e6159f5827c8b4f372cc406
.git/patches/master/the_sequence_.lock-_is_forbidden.patch
@@ -548,7 +548,7 @@ r 1626a11d979a1e9e775c766484172212277153df
.git/refs/patches/master/asterisk-is
r 3a0d5ccef0359004fcaa9cee98fbd6a2c4432e74
.git/refs/patches/master/tildeisforbidden.patch
r 434e07cacdd8e7eb4723e67cb2d100b3a4121a3a
.git/refs/patches/master/can-have-embedded-single-slashes.patch
[GUILT v2 17/29] "guilt graph" no longer loops when no patches are applied.
Give an error message if no patches are applied. Added a test case
that never terminates unless this fix is applied.
Signed-off-by: Per Cederqvist
---
guilt-graph | 9 +++--
regression/t-033.out | 3 +++
regression/t-033.sh | 13 +
3 files changed, 23 insertions(+), 2 deletions(-)
create mode 100644 regression/t-033.out
create mode 100755 regression/t-033.sh
diff --git a/guilt-graph b/guilt-graph
index b3469dc..56d0e77 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -17,8 +17,13 @@ fi
patchname="$1"
-bottom=`git rev-parse refs/patches/$branch/$(head_n 1 < "$applied")`
-base=`git rev-parse $bottom^`
+bottompatch=$(head_n 1 < "$applied")
+if [ -z "$bottompatch" ]; then
+ echo "No patch applied." >&2
+ exit 1
+fi
+
+base=`git rev-parse "refs/patches/${branch}/${bottompatch}^"`
if [ -z "$patchname" ]; then
top=`git rev-parse HEAD`
diff --git a/regression/t-033.out b/regression/t-033.out
new file mode 100644
index 000..76613f9
--- /dev/null
+++ b/regression/t-033.out
@@ -0,0 +1,3 @@
+% setup_repo
+% guilt graph
+No patch applied.
diff --git a/regression/t-033.sh b/regression/t-033.sh
new file mode 100755
index 000..a3a8981
--- /dev/null
+++ b/regression/t-033.sh
@@ -0,0 +1,13 @@
+#!/bin/bash
+#
+# Test the graph code
+#
+
+source "$REG_DIR/scaffold"
+
+cmd setup_repo
+
+# Check that "guilt graph" gives a proper "No patch applied" error
+# message when no patches are applied. (An older version of guilt
+# used to enter an endless loop in this situation.)
+shouldfail guilt graph
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 14/29] Use "git check-ref-format" to validate patch names.
The valid_patchname now lets "git check-ref-format" do its job instead
of trying (and failing) to implement the same rules. See
git-check-ref-format(1) for a list of the rules.
Refer to the git-check-ref-format(1) man page in the error messages
produced when valid_patchname indicates that the name is bad.
Added testcases that breaks most of the rules in that man-page.
Git version 1.8.5 no longer allows the single character "@" as a
branch name. Guilt always rejects that name, for increased
compatibility.
Signed-off-by: Per Cederqvist
---
guilt| 21 ++-
guilt-fork | 2 +-
guilt-import | 2 +-
guilt-new| 2 +-
regression/t-025.out | 426 +--
regression/t-025.sh | 12 +-
regression/t-032.out | 4 +-
7 files changed, 446 insertions(+), 23 deletions(-)
diff --git a/guilt b/guilt
index 3fc524e..23cc2da 100755
--- a/guilt
+++ b/guilt
@@ -132,14 +132,19 @@ fi
# usage: valid_patchname
valid_patchname()
{
- case "$1" in
- /*|./*|../*|*/./*|*/../*|*/.|*/..|*/|*\ *|*\*)
- return 1;;
- *:*)
- return 1;;
- *)
- return 0;;
- esac
+ if git check-ref-format --allow-onelevel "$1"; then
+ # Starting with Git version 1.8.5, a branch cannot be
+ # the single character "@". Make sure guilt rejects
+ # that name even if we are currently using an older
+ # version of Git. This ensures that the test suite
+ # runs fine using any version of Git.
+ if [ "$1" = "@" ]; then
+ return 1
+ fi
+ return 0
+ else
+ return 1
+ fi
}
get_branch()
diff --git a/guilt-fork b/guilt-fork
index a85d391..6447e55 100755
--- a/guilt-fork
+++ b/guilt-fork
@@ -37,7 +37,7 @@ else
fi
if ! valid_patchname "$newpatch"; then
- die "The specified patch name contains invalid characters (:)."
+ die "The specified patch name is invalid according to
git-check-ref-format(1)."
fi
if [ -e "$GUILT_DIR/$branch/$newpatch" ]; then
diff --git a/guilt-import b/guilt-import
index 3e9b3bb..928e325 100755
--- a/guilt-import
+++ b/guilt-import
@@ -40,7 +40,7 @@ if [ -e "$GUILT_DIR/$branch/$newname" ]; then
fi
if ! valid_patchname "$newname"; then
- die "The specified patch name contains invalid characters (:)."
+ die "The specified patch name is invalid according to
git-check-ref-format(1)."
fi
# create any directories as needed
diff --git a/guilt-new b/guilt-new
index 9528438..9f7fa44 100755
--- a/guilt-new
+++ b/guilt-new
@@ -64,7 +64,7 @@ fi
if ! valid_patchname "$patch"; then
disp "Patchname is invalid." >&2
- die "it cannot begin with '/', './' or '../', or contain /./, /../, or
whitespace"
+ die "It must follow the rules in git-check-ref-format(1)."
fi
# create any directories as needed
diff --git a/regression/t-025.out b/regression/t-025.out
index 7811ab1..01bc406 100644
--- a/regression/t-025.out
+++ b/regression/t-025.out
@@ -141,7 +141,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new white space
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -211,7 +211,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new /abc
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -235,7 +235,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new ./blah
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -259,7 +259,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new ../blah
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -283,7 +283,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new abc/./blah
Patchname
[GUILT v2 15/29] Produce legal patch names in guilt-import-commit.
Try harder to create patch names that adhere to the rules in
git-check-ref-format(1) when deriving a patch name from the commit
message. Verify that the derived name using "git check-ref-format",
and as a final fallback simply use the patch name "x" (to ensure that
the code is future-proof in case new rules are added in the future).
Always append a ".patch" suffix to the patch name.
Added test cases.
Signed-off-by: Per Cederqvist
---
guilt-import-commit | 20 +-
regression/t-034.out | 567 +++
regression/t-034.sh | 71 +++
3 files changed, 656 insertions(+), 2 deletions(-)
create mode 100644 regression/t-034.out
create mode 100755 regression/t-034.sh
diff --git a/guilt-import-commit b/guilt-import-commit
index f14647c..6260c56 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -28,19 +28,35 @@ disp "Current head: `git rev-parse \`git_branch\``" >&2
for rev in `git rev-list $rhash`; do
s=`git log --pretty=oneline -1 $rev | cut -c 42-`
+ # Try to convert the first line of the commit message to a
+ # valid patch name.
fname=`echo $s | sed -e "s/&/and/g" -e "s/[ :]/_/g" -e "s,[/\\],-,g" \
-e "s/['\\[{}]//g" -e 's/]//g' -e 's/\*/-/g' \
- -e 's/\?/-/g' | tr A-Z a-z`
+ -e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
+ -e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
+
+ if ! valid_patchname "$fname"; then
+ # Try harder to make it a legal commit name by
+ # removing all but a few safe characters.
+ fname=`echo $fname|tr -d -c _a-zA-Z0-9---/\\n`
+ fi
+ if ! valid_patchname "$fname"; then
+ # If we failed to derive a legal patch name, use the
+ # name "x". (If this happens, we likely have to
+ # append a suffix to make the name unique.)
+ fname=x
+ fi
disp "Converting `echo $rev | cut -c 1-8` as $fname"
mangle_prefix=1
fname_base=$fname
- while [ -f "$GUILT_DIR/$branch/$fname" ]; do
+ while [ -f "$GUILT_DIR/$branch/$fname.patch" ]; do
fname="$fname_base-$mangle_prefix"
mangle_prefix=`expr $mangle_prefix + 1`
disp "Patch under that name exists...trying '$fname'"
done
+ fname="$fname".patch
(
do_make_header $rev
diff --git a/regression/t-034.out b/regression/t-034.out
new file mode 100644
index 000..7bc9459
--- /dev/null
+++ b/regression/t-034.out
@@ -0,0 +1,567 @@
+% setup_git_repo
+% git tag base
+% create_commit a The sequence /. is forbidden.
+[master eebb76e] The sequence /. is forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+ create mode 100644 a
+% create_commit a The sequence .lock/ is forbidden.
+[master 45e81b5] The sequence .lock/ is forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a A/component/may/not/end/in/foo.lock
+[master bbf3f59] A/component/may/not/end/in/foo.lock
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Two consecutive dots (..) is forbidden.
+[master 1535e67] Two consecutive dots (..) is forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Check/multiple/../dots/./foo..patch
+[master 48eb60c] Check/multiple/../dots/./foo..patch
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Space is forbidden.
+[master 10dea83] Space is forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Tilde~is~forbidden.
+[master 70a83b7] Tilde~is~forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Caret^is^forbidden.
+[master ee6ef2c] Caret^is^forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Colon:is:forbidden.
+[master c077fe2] Colon:is:forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Delisforbidden.
+[master 589ee30] Delisforbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% git branch some-branch
+% git tag some-tag
+% create_commit a Ctrlisforbidden.
+[master e63cdde] Ctrlisforbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a CR
is
also
forbidden.
+[master 21ad093] CR
is
also
forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Question-mark?is?forbidden.
+[master be2fa9b] Question-mark?is?forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Asterisk*is*forbidden.
+[master af7b50f] Asterisk*is*forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Open[bracket[is[forbidden.
+[master 689f618] Open[bracket[is[forbidden.
+ Author: Author Name
+ 1 file changed, 1 insertion(+)
+% create_commit a Multiple/slashes//are//forbidden.
+[master 6e7d
Re: Error using git-remote-hg
William Giokas wrote:
> On Tue, May 13, 2014 at 02:09:55PM -0500, Felipe Contreras wrote:
> > William Giokas wrote:
> > > On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote:
> >
> > > > Why do we "import changegroup" unconditionally, even though it
> > > > is only used in the new codepath meant only for version 3.0 or
> > > > higher, not inside the "if" block that decides if we need that
> > > > module?
> >
> > > changegroup is prefectly /okay/ to import unconditionally, though as you
> > > say it will never be used.
> >
> > As you say, it's perfectly OK.
>
> But wrong. Yes, it works, but it's not how it should be done when we
> have a code review such as this. It should simply not be done and makes
> no sense to do with an 'if ; else' kind of thing later in the
> application.
That's exactly how it should be done. Maybe this visualization helps
from mercurial import hg, ui, bookmarks, ...# for hg >= 3.0
from mercurial import node, error, extensions, ... # for hg >= 3.0
from mercurial import changegroup # for hg >= 3.0
if check_version(3, 0):
cg = changegroup.getbundle(repo, 'push', ...# for hg >= 3.0
else:
cg = repo.getbundle('push', heads=list(p_revs) # for hg < 3.0
Eventually the code would become:
from mercurial import hg, ui, bookmarks, ...# for hg >= 3.0
from mercurial import node, error, extensions, ... # for hg >= 3.0
from mercurial import changegroup # for hg >= 3.0
cg = changegroup.getbundle(repo, 'push', ...# for hg >= 3.0
The fact that at some point 'import changegroup' was not needed is
irrelevant.
Primarily we want to support the current version of Mercurial.
Secondarily we want to support older versions. That's why we add the
repo.getbundle() code (ass an addendum to the core part).
> > > We can also be even more explicit with what we import by doing something
> > > like::
> > >
> > > try:
> > > from mercurial.changegroup import getbundle
> > >
> > > except ImportError:
> > > def getbundle(__empty__, **kwargs):
> > > return repo.getbundle(**kwargs)
> >
> > We could try that, but that would assume we want to maintain getbundle()
> > for the long run, and I personally don't want to do that. I would rather
> > contact the Mercurial developers about ways in which the push() method
> > can be improved so we don't need to have our own version. Hopefully
> > after it's improved we wouldn't have to call getbundle().
>
> Assuming that mercurial <3.0 will not change, then this should never
> need to change.
But it should. Otherwise the code will keep having more and more hacks
and it will become less and less maintainable.
That's why we don't have code for hg 1.0; because it would require too
many hacks, and nobody cared anyway.
Just like nobody cares about hg 1.0, eventually nobody will care about
hg 2.0.
> Changes in 'getbundle' upstream would require changes either way.
I doubt we will see any more changes in getbundle, at least not until
4.0, and hopefully by then we wouldn't be using it anyway. I am willing
to bet we won't see those changes.
> > Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at
> > some point we would want to remove the hacks for older versions. When we
> > do so we would want the import to remain unconditionally, and remove the
> > 'check_version(3, 0)' which is already helping to explain what the code
> > is for without the need of comments.
>
> The same exact thing can be done with this. In fact, it would probably
> allow us to have better future-proofing with regards to new versions of
> mercurial, there would just be different try:except statements at the
> beginning.
No, your code doesn't show that this is for versiosn <= 3.0,
'check_version(3, 0)' does.
Plus, when we remove this code my version makes it straight forward:
-if check_version(3, 0):
-cg = changegroup.getbundle(repo, 'push', ...
-else:
-cg = repo.getbundle('push', heads=list(p_revs), ...
+cg = changegroup.getbundle(repo, 'push', ...
Not so with your code:
-
-try:
-from mercurial.changegroup import getbundle
-
-except ImportError:
-def getbundle(__empty__, **kwargs):
-return repo.getbundle(**kwargs)
+from mercurial import getbundle
import re
import sys
@@ -1036,7 +1030,7 @@ def push_unsafe(repo, remote, ...
if not checkheads(repo, remote, p_revs):
return None
-cg = getbundle(repo, 'push', heads=list(p_revs), ...
+cg = changegroup.getbundle(repo, 'push', ...
> > > I was really sad to see that, and didn't have time to really look at it
> > > because of work and other projects, but I hope this presents a better
> > > solution than the current patch.
> >
> > Either way Junio doesn't maintain this code, I do. And it's not
> > maintained in git.git, git's maintained out-of-tree (thanks to Junio's
> > decisions).
>
> I still see it in git.git, and
[GUILT v2 10/29] Run test_failed if the exit status of a test script is bad.
There were two problems with the old code: - Since "set -e" is in effect (that is set in scaffold) the run-test script exited immediately if a t-*.sh script failed. This is not nice, as we want the error report that test_failed prints. - The code ran "cd -" between running the t-*.sh script and checking the exit status, so the exit status was lost. (Actually, the exit status was saved in $ERR, but nothing ever looked at $ERR.) Signed-off-by: Per Cederqvist --- regression/run-tests | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/regression/run-tests b/regression/run-tests index a10e796..8e0af9f 100755 --- a/regression/run-tests +++ b/regression/run-tests @@ -55,11 +55,15 @@ function run_test # run the test cd "$REPODIR" > /dev/null - "$REG_DIR/t-$1.sh" 2>&1 > "$LOGFILE" - ERR=$? + if "$REG_DIR/t-$1.sh" 2>&1 > "$LOGFILE"; then + ERR=false + else + ERR=true + fi + cd - > /dev/null - [ $? -ne 0 ] && test_failed + $ERR && test_failed diff -u "t-$1.out" "$LOGFILE" || test_failed echo "done." -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 12/29] "guilt header": more robust header selection.
If you run something like "guilt header '.*'" the command would crash, because the grep comand that tries to ensure that the patch exist would detect a match, but the later code expected the match to be exact. Fixed by comparing exact strings. And as a creeping feature "guilt header" will now try to use the supplied patch name as an unachored regexp if no exact match was found. If the regexp yields a unique match, it is used; if more than one patch matches, the names of all patches are listed and the command fails. (Exercise left to the reader: generalized this so that "guilt push" also accepts a unique regular expression.) Signed-off-by: Per Cederqvist --- guilt-header | 28 +--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/guilt-header b/guilt-header index 41e00cc..4701b31 100755 --- a/guilt-header +++ b/guilt-header @@ -45,10 +45,32 @@ esac [ -z "$patch" ] && die "No patches applied." # check that patch exists in the series -ret=`get_full_series | grep -e "^$patch\$" | wc -l` -if [ $ret -eq 0 ]; then - die "Patch $patch is not in the series" +full_series=`get_tmp_file series` +get_full_series > "$full_series" +found_patch= +while read x; do + if [ "$x" = "$patch" ]; then + found_patch="$patch" + break + fi +done < "$full_series" +if [ -z "$found_patch" ]; then + TMP_MATCHES=`get_tmp_file series` + grep "$patch" < "$full_series" > "$TMP_MATCHES" + nr=`wc -l < $TMP_MATCHES` + if [ $nr -gt 1 ]; then + echo "$patch does not uniquely identify a patch. Did you mean any of these?" >&2 + sed 's/^/ /' "$TMP_MATCHES" >&2 + rm -f "$TMP_MATCHES" + exit 1 + elif [ $nr -eq 0 ]; then + rm -f "$TMP_MATCHES" + die "Patch $patch is not in the series" + fi + found_patch=`cat $TMP_MATCHES` + rm -f "$TMP_MATCHES" fi +patch="$found_patch" # FIXME: warn if we're editing an applied patch -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 13/29] Check that "guilt header '.*'" fails.
Signed-off-by: Per Cederqvist --- regression/t-028.out | 7 +++ regression/t-028.sh | 4 2 files changed, 11 insertions(+) diff --git a/regression/t-028.out b/regression/t-028.out index 1564c09..ea72a3a 100644 --- a/regression/t-028.out +++ b/regression/t-028.out @@ -49,3 +49,10 @@ Signed-off-by: Commiter Name % guilt header non-existant Patch non-existant is not in the series +% guilt header .* +.* does not uniquely identify a patch. Did you mean any of these? + modify + add + remove + mode + patch-with-some-desc diff --git a/regression/t-028.sh b/regression/t-028.sh index 88e9adb..2ce0378 100755 --- a/regression/t-028.sh +++ b/regression/t-028.sh @@ -31,4 +31,8 @@ done shouldfail guilt header non-existant +# This is an evil variant of a non-existant patch. However, this +# patch name is a regexp that just happens to match an existing patch. +shouldfail guilt header '.*' + # FIXME: how do we check that -e works? -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 11/29] test suite: remove pointless redirection.
The shouldfail function already redirects stderr to stdout, so there is no need to do the same in t-028.sh and t-021.sh. Signed-off-by: Per Cederqvist Signed-off-by: Josef 'Jeff' Sipek --- regression/t-021.sh | 2 +- regression/t-025.sh | 2 +- regression/t-028.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/regression/t-021.sh b/regression/t-021.sh index 6337d7b..614e870 100755 --- a/regression/t-021.sh +++ b/regression/t-021.sh @@ -61,7 +61,7 @@ for n in `_seq -2 $npatches`; do if [ $n -gt 0 ]; then cmd guilt pop -n $n else - shouldfail guilt pop -n $n 2>&1 + shouldfail guilt pop -n $n fi cmd list_files diff --git a/regression/t-025.sh b/regression/t-025.sh index 3824608..985fed4 100755 --- a/regression/t-025.sh +++ b/regression/t-025.sh @@ -44,7 +44,7 @@ shouldfail guilt new "white space" cmd list_files for pname in prepend mode /abc ./blah ../blah abc/./blah abc/../blah abc/. abc/.. abc/ ; do - shouldfail guilt new "$pname" 2>&1 + shouldfail guilt new "$pname" cmd list_files done diff --git a/regression/t-028.sh b/regression/t-028.sh index 8480100..88e9adb 100755 --- a/regression/t-028.sh +++ b/regression/t-028.sh @@ -29,6 +29,6 @@ guilt series | while read n; do cmd guilt header $n done -shouldfail guilt header non-existant 2>&1 +shouldfail guilt header non-existant # FIXME: how do we check that -e works? -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 09/29] Test suite: properly check the exit status of commands.
The "cmd" and "shouldfail" functions checked the exit status of the
replace_path function instead of the actual command that was running.
(The $? construct checks the exit status of the last command in a
pipeline, not the first command.)
Updated t-032.sh, which used "shouldfail" instead of "cmd" in one
place. (The comment in the script makes it clear that the command is
expected to succeed.)
Signed-off-by: Per Cederqvist
---
regression/scaffold | 17 +++--
regression/t-032.sh | 2 +-
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/regression/scaffold b/regression/scaffold
index 5c8b73e..e4d7487 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -51,18 +51,23 @@ function filter_dd
function cmd
{
echo "% $@"
- "$@" 2>&1 | replace_path && return 0
- return 1
+ (
+ exec 3>&1
+ rv=`(("$@" 2>&1; echo $? >&4) | replace_path >&3 ) 4>&1`
+ exit $rv
+ )
+ return $?
}
# usage: shouldfail ..
function shouldfail
{
echo "% $@"
- (
- "$@" 2>&1 || return 0
- return 1
- ) | replace_path
+ ! (
+ exec 3>&1
+ rv=`(("$@" 2>&1; echo $? >&4) | replace_path >&3 ) 4>&1`
+ exit $rv
+ )
return $?
}
diff --git a/regression/t-032.sh b/regression/t-032.sh
index b1d5f19..bba401e 100755
--- a/regression/t-032.sh
+++ b/regression/t-032.sh
@@ -28,7 +28,7 @@ shouldfail guilt import -P foo3 foo
cmd guilt import -P foo2 foo
# ok
-shouldfail guilt import foo
+cmd guilt import foo
# duplicate patch name (implicit)
shouldfail guilt import foo
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: getbundle changed in mercurial 3.0
William Giokas <1007...@gmail.com> writes:
> In mercurial 3.0, getbundle was moved to the changegroup module, and
> gained a new argument. Due to this we cannot simply start using
> getbundle(...) imported from either one unconditionally, as that would
> cause errors in mercurial 3.0 without changing the syntax, and errors in
> mercurial <3.0 if we do change it.
>
> The try:except block at the beginning of git-remote-hg.py tries first to
> import mercurial.changegroup.getbundle, and if that fails we set the
> function 'getbundle' to work correctly with mercurial.repo.getbundle by
> removing the first argument.
>
> Signed-off-by: William Giokas <1007...@gmail.com>
> ---
>
> I have tested this briefly with mercurial 3.0, but have not yet really run
> through its paces. The tests that are included in next do pass with
> mercurial 3.0.
>
> git-remote-hg.py | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/git-remote-hg.py b/git-remote-hg.py
> index 34cda02..3dc9e11 100755
> --- a/git-remote-hg.py
> +++ b/git-remote-hg.py
> @@ -14,6 +14,13 @@
>
> from mercurial import hg, ui, bookmarks, context, encoding, node, error,
> extensions, discovery, util
>
> +try:
> +from mercurial.changegroup import getbundle
> +
> +except ImportError:
> +def getbundle(__empty__, **kwargs):
> +return repo.getbundle(**kwargs)
> +
> import re
> import sys
> import os
> @@ -985,7 +992,7 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
> if not checkheads(repo, remote, p_revs):
> return None
>
> -cg = repo.getbundle('push', heads=list(p_revs), common=common)
> +cg = getbundle(repo, 'push', heads=list(p_revs), common=common)
>
> unbundle = remote.capable('unbundle')
> if unbundle:
Thanks. I agree with you that this would probably be a better
way to future-proof the code without unconditionally including what
may not be used at all, as you said in the other thread.
I can be pursuaded to queue this particular patch for maintenance
tracks after 2.0 final to my tree, but I do not think I would want
to keep taking patches to this area myself in the longer run.
The way I envision the longer term shape of git-remote-hg.py in the
contrib/ is either one of these two:
(1) manage it just like contrib/hooks/multimail/, keeping a
reasonably fresh and known-to-be-good snapshot, while making it
clear that my tree is not the authoritative copy people should
work off of;
(2) treat it just like contrib/emacs/vc-git.el, which found a
better home and left my tree at 78513869 (emacs: Remove the no
longer maintained vc-git package., 2009-02-07); or
The first one may be more preferrable between the two, if only
because distros would need time to adjust where they pick up the
source material to package up, but it still needs cooperation with
the "authoritative upstream" and this project to allow us to at
least learn when the good time to import such good snapshots. In
the case of vc-git, the separation was not about lack of will to
coordinate, but because it was not necessary to keep it in my tree,
as the "better home" was where the users expect to find the latest
and greatest anyway.
If Felipe turns out to be a maintainer users do not trust for
remote-hg, it is possible that you and other people can maintain it
and work with git.git better by forking off of his tree. It is far
early to know if that will be the case at this point, and I
personally do not think that will happen (no, I do not have a
concrete reason I can cite to explain why I think that way).
But even in such an extreme hypothetical case, the difference it
makes to my tree would only be to take (1) or (2) and point to that
forked remote-hg repository, instead of Felipe's, as the source of
the "authoritative copy". And I wouldn't be taking individual
patches directly to my tree in either case.
--
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
[GUILT v2 08/29] Added more test cases for "guilt new": empty patches.
Test that empty patches are handled correctly, both with and without the guilt.diffstat configuration option. Signed-off-by: Per Cederqvist --- regression/t-020.out | 250 +++ regression/t-020.sh | 60 + 2 files changed, 310 insertions(+) diff --git a/regression/t-020.out b/regression/t-020.out index af45734..7e07efa 100644 --- a/regression/t-020.out +++ b/regression/t-020.out @@ -1128,3 +1128,253 @@ f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status +% guilt new empty.patch +% guilt pop +All patches popped. +% guilt push +Applying patch..empty.patch +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d15a1d2d34493f790c78ddacb8815b0b9536ee2b .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty.patch +f e90b964f01cbef60bbe00c38c55d9ea86618a66a .git/patches/master/status +r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 .git/refs/patches/master/empty.patch +% git log -p +commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 +Author: Author Name +Date: Mon Jan 1 00:00:00 2007 + + +patch empty.patch + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author: Author Name +Date: Mon Jan 1 00:00:00 2007 + + +initial + +Signed-off-by: Commiter Name + +diff --git a/def b/def +new file mode 100644 +index 000..8baef1b +--- /dev/null b/def +@@ -0,0 +1 @@ ++abc +% git config guilt.diffstat true +% guilt refresh +Patch empty.patch refreshed +% guilt pop +All patches popped. +% guilt push +Applying patch..empty.patch +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 7d261b8caad0f161c21daf5de65eeb521ff8c067 .git/patches/master/empty.patch +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d15a1d2d34493f790c78ddacb8815b0b9536ee2b .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty.patch~ +f e90b964f01cbef60bbe00c38c55d9ea86618a66a .git/patches/master/status +r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 .git/refs/patches/master/empty.patch +% git log -p +commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 +Author: Author Name +Date: Mon Jan 1 00:00:00 2007 + + +patch empty.patch + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author: Author Name +Date: Mon Jan 1 00:00:00 2007 + + +initial + +Signed-off-by: Commiter Name + +diff --git a/def b/def +new file mode 100644 +index 000..8baef1b +--- /dev/null b/def +@@ -0,0 +1 @@ ++abc +% git config guilt.diffstat false +--- + +% guilt pop +All patches popped. +% guilt push +Applying patch..empty.patch +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 7d261b8caad0f161c21daf5de65eeb521ff8c067 .git/patches/master/empty.patch +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d15a1d2d34493f790c78ddacb8815b0b9536ee2b .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty.patch~ +f e90b964f01cbef60bbe00c38c55d9ea86618a66a .git/patches/master/status +r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 .git/refs/patches/master/empty.patch +% git log -p +commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 +Author: Author Name +Date: Mon Jan 1 00:00:00 2007 + + +patch empty.patch + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author: Author Name +Date: Mon Jan 1 00:00:00 2007 + + +initial + +Signed-off-by: Commiter Name + +diff --git a/def b/def +new file mode 100644 +index 000..8baef1b +--- /dev/null b/def +@@ -0,0 +1 @@ ++abc +% git config guilt.diffstat true +% guilt refresh +Patch empty.patch refreshed +% guilt pop +All patches popped. +% guilt push +Applying patch..empty.patch +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1
[GUILT v2 07/29] Added test cases for "guilt fold".
Test that we can combine any combination of patches with empty and non-empty messages, both with and without guilt.diffstat. (All patches are empty.) Signed-off-by: Per Cederqvist --- regression/t-035.out | 467 +++ regression/t-035.sh | 62 +++ 2 files changed, 529 insertions(+) create mode 100644 regression/t-035.out create mode 100755 regression/t-035.sh diff --git a/regression/t-035.out b/regression/t-035.out new file mode 100644 index 000..cc16fb4 --- /dev/null +++ b/regression/t-035.out @@ -0,0 +1,467 @@ +% setup_repo +% git config guilt.diffstat true +%% empty + empty (diffstat=true) +% guilt new empty-1 +% guilt pop +All patches popped. +% guilt push +Applying patch..empty-1 +Patch applied. +% guilt new empty-2 +% guilt pop +Now at empty-1. +% guilt push +Applying patch..empty-2 +Patch applied. +% guilt pop +Now at empty-1. +% guilt fold empty-2 +% guilt pop +All patches popped. +% guilt push +Applying patch..empty-1 +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 4ea806e306f0228a8ef41f186035e7b04097f1f2 .git/patches/master/status +f 7d261b8caad0f161c21daf5de65eeb521ff8c067 .git/patches/master/empty-1 +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d28d87b88c1e24d637e390dc3603cfa7c1715711 .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +r bde3d337af70f36836ad606c800d194006f883b3 .git/refs/patches/master/empty-1 +% guilt pop +All patches popped. +% guilt delete -f empty-1 +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status +%% empty + nonempty (diffstat=true) +% guilt new empty +% guilt pop +All patches popped. +% guilt push +Applying patch..empty +Patch applied. +% guilt new -f -s -m A commit message. nonempty +% guilt pop +Now at empty. +% guilt push +Applying patch..nonempty +Patch applied. +% guilt pop +Now at empty. +% guilt fold nonempty +% guilt pop +All patches popped. +% guilt push +Applying patch..empty +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 15aab0fd8b937eb3bb01841693f35dcb75da2faf .git/patches/master/status +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd .git/patches/master/empty~ +f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd .git/patches/master/nonempty~ +f 683678040eef9334d6329e00d5b9babda3e65b57 .git/patches/master/empty +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f a26a22287b500a2a372e42c2bab03599bbe37cdf .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +r 4eedaa32894fc07af3298d8c1178052942a3ca6a .git/refs/patches/master/empty +% guilt pop +All patches popped. +% guilt delete -f empty +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd .git/patches/master/empty~ +f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd .git/patches/master/nonempty~ +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status +%% nonempty + empty (diffstat=true) +% guilt new -f -s -m A commit message. nonempty +% guilt pop +All patches popped. +% guilt push +Applying p
[GUILT v2 05/29] "guilt new": Accept more than 4 arguments.
The argument parser arbitrarily refused to accept more than 4
arguments. That made it impossible to run "guilt new -f -s -m msg
patch". Removed the checks for the number of arguments from the
"guilt new" parser -- the other checks that are already there are
enough to catch all errors.
Give a better error message if "-m" isn't followed by a message
argument.
Signed-off-by: Per Cederqvist
Signed-off-by: Josef 'Jeff' Sipek
---
guilt-new | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/guilt-new b/guilt-new
index bb68924..9528438 100755
--- a/guilt-new
+++ b/guilt-new
@@ -11,10 +11,6 @@ fi
_main() {
-if [ $# -lt 1 ] || [ $# -gt 4 ]; then
- usage
-fi
-
while [ $# -gt 0 ] ; do
case "$1" in
-f)
@@ -31,6 +27,9 @@ while [ $# -gt 0 ] ; do
fi
;;
-m)
+ if [ $# -eq 1 ]; then
+ usage
+ fi
msg="$2"
shift
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 06/29] Fix the do_get_patch function.
A patch file consists of:
(1) the description
(2) optional diffstat
(3) the patches
When extracting the patch, we only want part 3. The do_get_patch used
to give us part 2 and part 3. That made for instance this series of
operations fail if guilt.diffstat is true:
guilt push empty-1
guilt push empty-2
guilt pop
guilt fold empty-2
guilt pop
guilt push
Signed-off-by: Per Cederqvist
---
guilt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guilt b/guilt
index 8701481..3fc524e 100755
--- a/guilt
+++ b/guilt
@@ -334,7 +334,7 @@ do_get_patch()
{
awk '
BEGIN{}
-/^(diff |---$|--- )/ {patch = 1}
+/^(diff |--- )/ {patch = 1}
patch == 1 {print $0}
END{}
' < "$1"
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 03/29] Added test case for "guilt delete -f".
Ensure that the file really is deleted. Signed-off-by: Per Cederqvist Signed-off-by: Josef 'Jeff' Sipek --- regression/t-026.out | 15 +++ regression/t-026.sh | 5 - 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/regression/t-026.out b/regression/t-026.out index 3b9fb14..be50b48 100644 --- a/regression/t-026.out +++ b/regression/t-026.out @@ -29,3 +29,18 @@ f 7848194fd2e9ee0eb6589482900687d799d60a12 .git/patches/master/series f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status +% guilt new delete-me +% guilt pop +All patches popped. +% guilt delete -f delete-me +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 7848194fd2e9ee0eb6589482900687d799d60a12 .git/patches/master/series +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status diff --git a/regression/t-026.sh b/regression/t-026.sh index 0ccdf85..e25d0df 100755 --- a/regression/t-026.sh +++ b/regression/t-026.sh @@ -20,4 +20,7 @@ cmd guilt delete add cmd list_files -# FIXME: test delete -f +cmd guilt new delete-me +cmd guilt pop +cmd guilt delete -f delete-me +cmd list_files -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 04/29] Allow "guilt import-commit" to run from a dir which contains spaces.
Signed-off-by: Per Cederqvist --- guilt-import-commit | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/guilt-import-commit b/guilt-import-commit index 20dcee2..f14647c 100755 --- a/guilt-import-commit +++ b/guilt-import-commit @@ -23,7 +23,7 @@ if ! must_commit_first; then fi disp "About to begin conversion..." >&2 -disp "Current head: `cat $GIT_DIR/refs/heads/\`git_branch\``" >&2 +disp "Current head: `git rev-parse \`git_branch\``" >&2 for rev in `git rev-list $rhash`; do s=`git log --pretty=oneline -1 $rev | cut -c 42-` @@ -46,7 +46,7 @@ for rev in `git rev-list $rhash`; do do_make_header $rev echo "" git diff --binary $rev^..$rev - ) > $GUILT_DIR/$branch/$fname + ) > "$GUILT_DIR/$branch/$fname" # FIXME: grab the GIT_AUTHOR_DATE from the commit object and set the # timestamp on the patch @@ -68,6 +68,6 @@ for rev in `git rev-list $rhash`; do done disp "Done." >&2 -disp "Current head: `cat $GIT_DIR/refs/heads/\`git_branch\``" >&2 +disp "Current head: `git rev-parse \`git_branch\``" >&2 } -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 02/29] Allow "guilt delete -f" to run from a dir which contains spaces.
Signed-off-by: Per Cederqvist Signed-off-by: Josef 'Jeff' Sipek --- guilt-delete | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guilt-delete b/guilt-delete index 3e394f8..967ac10 100755 --- a/guilt-delete +++ b/guilt-delete @@ -49,7 +49,7 @@ series_remove_patch "$patch" guilt_hook "delete" "$patch" -[ ! -z "$force" ] && rm -f $GUILT_DIR/$branch/$patch +[ ! -z "$force" ] && rm -f "$GUILT_DIR/$branch/$patch" exit 0 -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 01/29] The tests should not fail if guilt.diffstat is set.
Explicitly set guilt.diffstat to its default value. Without this, the 027 test (and possibly others) fail if guilt.diffstat is set to true in ~/.gitconfig. Signed-off-by: Per Cederqvist Signed-off-by: Josef 'Jeff' Sipek --- regression/scaffold | 1 + 1 file changed, 1 insertion(+) diff --git a/regression/scaffold b/regression/scaffold index 546d8c6..5c8b73e 100644 --- a/regression/scaffold +++ b/regression/scaffold @@ -87,6 +87,7 @@ function setup_git_repo # Explicitly set config that the tests rely on. git config log.date default git config log.decorate no + git config guilt.diffstat false } function setup_guilt_repo -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v2 00/29] Teach guilt import-commit how to create legal patch names, and more
This is version two of the patch series I sent back on 21 Mar 2014. I have addressed all feedback to date, updated the coding style, and added signed-off-by lines from Jeff Sipek for those commits that I have received an explicit approval from him (but only if I have not made any other change to that particular commit). I have also added one more patch: a very limited coding style guide, and accompanying settings for Emacs. See the last commit in the series. All other commits have retained their numbering. To see how the patches have evolved, you might find http://www.lysator.liu.se/~ceder/guilt-oslo-2014-v2/ useful. It displays diffs of all the patches, in pdiffdiff output format. Here is the original blurb for the series, slightly edited: I recently found myself sitting on a train with a computer in front of me. I tried to use "guilt import-commit", which seemed to work, but when I tried to "guilt push" the commits I had just imported I got some errors. It turned out that "guilt import-commit" had generated invalid patch names. I decided to fix the issue, and write a test case that demonstrated the problem. One thing led to another, and here I am, a few late nights at a hotel and a return trip on the train later, submitting a patch series in 28 parts. Sorry about the number of patches, but this is what happens when you uncover a bug while writing a test case for the bug you uncovered while writing a test case for your original problem. The patch series consists of: - A number of fixes to the test suite. - A number of bug fixes which I hope are non-controversial. Most of the fixes have test cases. - Changed behavior: "guilt push" when there is nothing more to push now uses exit status 1. This makes it possible to write shell loops such as "while guilt push; do make test||break; done". Also, "guilt pop" when no patches are applied also uses exit status 1. (This aligns "guilt push" and "guilt pop" with how "hg qpush" and "hg qpop" has worked for several years.) - Changed behavior: by default, guilt no longer changes branch when you push a patch. You need to do "git config guilt.reusebranch false" to re-enable that. This patch sets the default value of guilt.reusebranch to true; it should in my opinion change to false a year or two after the next release. - The humble beginnings of a coding style guide. A more detailed overview of the patches: 1. Some tests fail if "git config guilt.diffstat true" is in effect. 2-4. Some commands fail if run from a directory with spaces in its name. 5. "guilt new" had an overly restrictive argument parser. 6-8. guilt.diffstat could break "guilt fold" and "guilt new". 9-10. The test suite did not test exit status properly. 11. Remove pointless redirections in the test suite. 12-13. "guilt header" tried to check if a patch existed, but the check was broken. 14-16. Various parts of guilt tried to ensure that patch names were legal git branch names, but failed. 17-20. "guilt graph" failed when no patch was applied, and when a branch name contained a comma or a quote. 21-23. "git config log.decorate short" caused "guilt import-commit", "guilt patchbomb" and "guilt rebase" to fail in various ways. 24. Patches may contain backslashes, but various informative messages from guilt did backslash processing. 25-26. "guilt push" and "guilt pop" should fail when there is nothing to do. 27-28. These two commits were things I intended to contribute a while back, when contributing the "Change git branch when patches are applied" change (commit 67d3af63f422). These commits makes that behavior optional, and it defaults to being disabled, so that you can use both Guilt v0.35 (and earlier) and the current Guilt code against the same repo. These commits add some code complexity, and you might want to skip them if you think the current behavior is better. 29. A coding style guide, with Emacs support. This patch series is also available on http://repo.or.cz/w/guilt/ceder.git in the "oslo-2014-v2" branch. If you already have a copy of guilt, you should be able to fetch that branch with something like: git remote add ceder http://repo.or.cz/r/guilt/ceder.git git fetch ceder refs/heads/oslo-2014-v2:refs/remotes/ceder/oslo-2014-v2 A few of the regression/t-*.out files contain non-ASCII characters. I hope they survive the mail transfer; if not, please use the repo above to fetch the commits. Per Cederqvist (29): The tests should not fail if guilt.diffstat is set. Allow "guilt delete -f" to run from a dir which contains spaces. Added test case for "guilt delete -f". Allow "guilt import-commit" to run from a dir which contains spaces. "guilt new": Accept more than 4 arguments. Fix the do_get_patch function. Added test cases for "guilt fold". Added more test cases for "guilt new": empty patches. Test suite:
Re: [PATCH v6 00/42] Use ref transactions for all ref updates
Hi, Ronnie Sahlberg wrote: > This patch series is based on next and expands on the transaction API. Sorry to take so long to get to this. For the future, it's easier to review patches based on some particular branch that got merged into next, since next is a moving target (series come and go from there depending on what seems to need testing at a given moment). Is mh/ref-transaction the relevant branch to build on? Trying to apply the series to mh/ref-transaction, I get conflicts in patch 13 due to absence of rs/ref-update-check-errors-early. Trying to apply the series to a merge of mh/ref-transaction and rs/ref-update-check-errors-early, I get a minor conflict in patch 15 but it is easy to resolve and the rest goes smoothly. Looking forward to reading the rest. Thanks. 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
[PATCH v2] remote-hg: getbundle changed in mercurial 3.0
In mercurial 3.0, getbundle was moved to the changegroup module, and
gained a new argument. Due to this we cannot simply start using
getbundle(...) imported from either one unconditionally, as that would
cause errors in mercurial 3.0 without changing the syntax, and errors in
mercurial <3.0 if we do change it.
The try:except block at the beginning of git-remote-hg.py tries first to
import mercurial.changegroup.getbundle, and if that fails we set the
function 'getbundle' to work correctly with mercurial.repo.getbundle by
removing the first argument.
Signed-off-by: William Giokas <1007...@gmail.com>
---
So, what I had in there before would not work at all on repo.getbundle
because **kwargs only unpacks keyword args, not normal args. Sometimes my
brain works.
git-remote-hg.py | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/git-remote-hg.py b/git-remote-hg.py
index 34cda02..32eeffb 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -14,6 +14,13 @@
from mercurial import hg, ui, bookmarks, context, encoding, node, error,
extensions, discovery, util
+try:
+from mercurial.changegroup import getbundle
+
+except ImportError:
+def getbundle(repo, **kwargs):
+return repo.getbundle(**kwargs)
+
import re
import sys
import os
@@ -985,7 +992,8 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
if not checkheads(repo, remote, p_revs):
return None
-cg = repo.getbundle('push', heads=list(p_revs), common=common)
+cg = getbundle(repo=repo, source='push', heads=list(p_revs),
+ common=common)
unbundle = remote.capable('unbundle')
if unbundle:
--
2.0.0.rc3
--
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] git-show: fix 'git show -s' to not add extra terminator after merge commit
On Tue, May 13, 2014 at 07:57:08AM +0200, Johannes Sixt wrote: > Am 5/13/2014 1:10, schrieb Max Kirillov: >> --- a/t/t7007-show.sh >> +++ b/t/t7007-show.sh >> @@ -25,6 +25,7 @@ test_expect_success 'set up a bit of history' ' >> git checkout -b side HEAD^^ && >> test_commit side2 && >> test_commit side3 >> +test_merge merge main3 >> ' > > Broken &&-chain. Thank you, will fix it. -- Max -- 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] t: git-show: adapt tests to fixed 'git show'
On Tue, May 13, 2014 at 12:26:42PM -0700, Junio C Hamano wrote:
> Hmph. Having these as two separate commits would mean that 1/2
> alone will break the test, hurting bisectability a little bit. The
> necessary adjustments in this patch is small enough that we may be
> better off squashing them into one.
Ok, will squash them.
>> t/t1507-rev-parse-upstream.sh | 2 +-
>> t/t7600-merge.sh | 11 +--
>> 2 files changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
>> index 2a19e79..672280b 100755
>> --- a/t/t1507-rev-parse-upstream.sh
>> +++ b/t/t1507-rev-parse-upstream.sh
>> @@ -100,7 +100,7 @@ test_expect_success 'merge my-side@{u} records the
>> correct name' '
>> git branch -D new ;# can fail but is ok
>> git branch -t new my-side@{u} &&
>> git merge -s ours new@{u} &&
>> -git show -s --pretty=format:%s >actual &&
>> +git show -s --pretty=tformat:%s >actual &&
>> echo "Merge remote-tracking branch ${sq}origin/side${sq}" >expect &&
>> test_cmp expect actual
>> )
>
> This seems to me that it is updating how the output is produced, not
> updating the expected outputs from commands with arguments we have
> been testing. I have been expecting to see changes to the pieces of
> the code that prepare the expected output, so that we can compare
> old expectations, which would have been expecting something strange,
> with new expectations from the updated code, which would show that
> the new behaviour is a welcome change because it would produce more
> sensible output.
>
> Having said all that, for this particular test piece, I agree with
> the patch that using --pretty=tformat:%s is a lot more sensible and
> using --pretty=format:%s and expecting its output to be terminated
> with the newline was an unrealistic test. After all, "tformat" is
> the version with "line terminator" semantics, as opposed to "item
> separator" semantics offered by "--pretty=format:", and the test
> merely was depending on the bug to expect a complete line output
> (i.e. "echo" without "-n"), which you fixed. The new test makes a
> lot more sense and is relevant to the real world use, and I would
> have preferred to see it explained as such in the log message.
In analogous cases with non-merge commits I have found the
form "--format=...", in t3505-cherry-pick-empty.sh for
example, so I have decided that merges should also use it. The
form "--pretty=tformat:..." seems more explicit to me, so I
have chosen this one.
Will add the message as you have suggested.
>> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
>> index 10aa028..b164621 100755
>> --- a/t/t7600-merge.sh
>> +++ b/t/t7600-merge.sh
>> @@ -57,11 +57,10 @@ create_merge_msgs () {
>> git log --no-merges ^HEAD c2 c3
>> } >squash.1-5-9 &&
>> : >msg.nologff &&
>> -echo >msg.nolognoff &&
>> +: >msg.nolognoff &&
>> {
>> echo "* tag 'c3':" &&
>> -echo " commit 3" &&
>> -echo
>> +echo " commit 3"
>> } >msg.log
>> }
>
> These are updating the expectation. We used to expect an
> unnecessary trailing blank line, and now we do not, which clearly
> shows that the previous fix is a welcome change.
>
>> @@ -71,7 +70,7 @@ verify_merge () {
>> git diff --exit-code &&
>> if test -n "$3"
>> then
>> -git show -s --pretty=format:%s HEAD >msg.act &&
>> +git show -s --pretty=tformat:%s HEAD >msg.act &&
>> test_cmp "$3" msg.act
>> fi
>> }
>
> It is hard to judge what is fed as "$3" here without context, but
> this is in line with the "--pretty=tformat aka --format is the
> normal thing to use" we saw in 1507. The same for the other hunk.
>
>> @@ -620,10 +619,10 @@ test_expect_success 'merge early part of c2' '
>> git tag c6 &&
>> git branch -f c5-branch c5 &&
>> git merge c5-branch~1 &&
>> -git show -s --pretty=format:%s HEAD >actual.branch &&
>> +git show -s --pretty=tformat:%s HEAD >actual.branch &&
>> git reset --keep HEAD^ &&
>> git merge c5~1 &&
>> -git show -s --pretty=format:%s HEAD >actual.tag &&
>> +git show -s --pretty=tformat:%s HEAD >actual.tag &&
>> test_cmp expected.branch actual.branch &&
>> test_cmp expected.tag actual.tag
>> '
>
> How about squashing these two into a single patch, and at the end of
> the log message for 1/2, add this to explain the changes to the
> test:
>
> Also existing tests are updated to demonstrate the new
> behaviour. Earlier, the tests that used "git show -s
> --pretty=format:%s", even though "--pretty=format:%s" calls for
> item separator semantics and does not ask for the terminating
> newline after the last item, expected the output to end with
> such a newline. They were relying on the buggy behaviour. Use
> of "--format=%s", which is equivalent to "--pretty=tformat:%s"
> that asks for
Re: Error using git-remote-hg
On Tue, May 13, 2014 at 02:09:55PM -0500, Felipe Contreras wrote: > William Giokas wrote: > > On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote: > > > > Why do we "import changegroup" unconditionally, even though it > > > is only used in the new codepath meant only for version 3.0 or > > > higher, not inside the "if" block that decides if we need that > > > module? > > > changegroup is prefectly /okay/ to import unconditionally, though as you > > say it will never be used. > > As you say, it's perfectly OK. But wrong. Yes, it works, but it's not how it should be done when we have a code review such as this. It should simply not be done and makes no sense to do with an 'if ; else' kind of thing later in the application. > > > We can also be even more explicit with what we import by doing something > > like:: > > > > try: > > from mercurial.changegroup import getbundle > > > > except ImportError: > > def getbundle(__empty__, **kwargs): > > return repo.getbundle(**kwargs) > > We could try that, but that would assume we want to maintain getbundle() > for the long run, and I personally don't want to do that. I would rather > contact the Mercurial developers about ways in which the push() method > can be improved so we don't need to have our own version. Hopefully > after it's improved we wouldn't have to call getbundle(). Assuming that mercurial <3.0 will not change, then this should never need to change. Changes in 'getbundle' upstream would require changes either way. > Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at > some point we would want to remove the hacks for older versions. When we > do so we would want the import to remain unconditionally, and remove the > 'check_version(3, 0)' which is already helping to explain what the code > is for without the need of comments. The same exact thing can be done with this. In fact, it would probably allow us to have better future-proofing with regards to new versions of mercurial, there would just be different try:except statements at the beginning. > > > I was really sad to see that, and didn't have time to really look at it > > because of work and other projects, but I hope this presents a better > > solution than the current patch. > > Either way Junio doesn't maintain this code, I do. And it's not > maintained in git.git, git's maintained out-of-tree (thanks to Junio's > decisions). I still see it in git.git, and I will contribute this upstream for as long as it is in the tree. If you want to use the patch that I sent to this list, feel free. > So please post your suggestions and patches to git...@googlegroups.com, > and use the latest code at https://github.com/felipec/git-remote-hg. Thanks, -- William Giokas | KaiSforza | http://kaictl.net/ GnuPG Key: 0x73CD09CF Fingerprint: F73F 50EF BBE2 9846 8306 E6B8 6902 06D8 73CD 09CF pgp_mLEPyhosF.pgp Description: PGP signature
Re: Error using git-remote-hg
William Giokas wrote: > On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote: > > Why do we "import changegroup" unconditionally, even though it > > is only used in the new codepath meant only for version 3.0 or > > higher, not inside the "if" block that decides if we need that > > module? > changegroup is prefectly /okay/ to import unconditionally, though as you > say it will never be used. As you say, it's perfectly OK. > We can also be even more explicit with what we import by doing something > like:: > > try: > from mercurial.changegroup import getbundle > > except ImportError: > def getbundle(__empty__, **kwargs): > return repo.getbundle(**kwargs) We could try that, but that would assume we want to maintain getbundle() for the long run, and I personally don't want to do that. I would rather contact the Mercurial developers about ways in which the push() method can be improved so we don't need to have our own version. Hopefully after it's improved we wouldn't have to call getbundle(). Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at some point we would want to remove the hacks for older versions. When we do so we would want the import to remain unconditionally, and remove the 'check_version(3, 0)' which is already helping to explain what the code is for without the need of comments. > I was really sad to see that, and didn't have time to really look at it > because of work and other projects, but I hope this presents a better > solution than the current patch. Either way Junio doesn't maintain this code, I do. And it's not maintained in git.git, git's maintained out-of-tree (thanks to Junio's decisions). So please post your suggestions and patches to git...@googlegroups.com, and use the latest code at https://github.com/felipec/git-remote-hg. 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] contrib/subtree bugfix: Can't `add` annotated tag
James Denholm writes: > cmd_add_commit() is passed FETCH_HEAD by cmd_add_repository, which is > then rev-parsed into an object ID. However, if the user is fetching a > tag rather than a branch HEAD, such as by executing: > > $ git subtree add -P oldGit https://github.com/git/git.git tags/v1.8.0 > > The object ID is a tag and is never peeled, and the git commit-tree call > (line 561) slaps us in the face because it doesn't handle tag IDs. > > Because peeling a committish ID doesn't do anything if it's already a > commit, fix by peeling[1] the object ID before assigning it to $rev, as > per the patch. > > [*1*]: Via peel_committish(), from git:git-sh-setup.sh, pre-existing > dependency of git-subtree. > > Reported-by: Kevin Cagle > Helped-by: Junio C Hamano > Signed-off-by: James Denholm > --- > I felt that defining revp would be a little more self-documenting than > using $rev^0. That is a good decision, but as long as we are attempting to peel, don't we want to stop the damage when it does not peel to a commit? I'll tentatively queue this. Thanks. -- >8 -- From: James Denholm Date: Tue, 13 May 2014 14:08:58 +1000 Subject: [PATCH] contrib/subtree: allow adding an annotated tag cmd_add_commit() is passed FETCH_HEAD by cmd_add_repository, which is then rev-parsed into an object name. However, if the user is fetching a tag rather than a branch HEAD, such as by executing: $ git subtree add -P oldGit https://github.com/git/git.git tags/v1.8.0 the object name refers to a tag and is never peeled, and the git commit-tree call (line 561) slaps us in the face because it doesn't peel tags to commits. Because peeling a committish doesn't do anything if it's already a commit, fix by peeling the object name before assigning it to $rev, using peel_committish() from git:git-sh-setup.sh, a pre-existing dependency of git-subtree. Reported-by: Kevin Cagle Helped-by: Junio C Hamano Signed-off-by: James Denholm Signed-off-by: Junio C Hamano --- contrib/subtree/git-subtree.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index db925ca..fa1a583 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -558,8 +558,9 @@ cmd_add_commit() commit=$(add_squashed_msg "$rev" "$dir" | git commit-tree $tree $headp -p "$rev") || exit $? else + revp=$(peel_committish "$rev") && commit=$(add_msg "$dir" "$headrev" "$rev" | -git commit-tree $tree $headp -p "$rev") || exit $? +git commit-tree $tree $headp -p "$revp") || exit $? fi git reset "$commit" || exit $? -- 2.0.0-rc3-404-gb0be553 -- 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] t: git-show: adapt tests to fixed 'git show'
Max Kirillov writes:
> 'git show' used to print extra newline after merge commit, and it was
> recorded so into the test reference data. Now when the behavior is
> fixed, the tests should be updated.
>
> Signed-off-by: Max Kirillov
> ---
Hmph. Having these as two separate commits would mean that 1/2
alone will break the test, hurting bisectability a little bit. The
necessary adjustments in this patch is small enough that we may be
better off squashing them into one.
> t/t1507-rev-parse-upstream.sh | 2 +-
> t/t7600-merge.sh | 11 +--
> 2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
> index 2a19e79..672280b 100755
> --- a/t/t1507-rev-parse-upstream.sh
> +++ b/t/t1507-rev-parse-upstream.sh
> @@ -100,7 +100,7 @@ test_expect_success 'merge my-side@{u} records the
> correct name' '
> git branch -D new ;# can fail but is ok
> git branch -t new my-side@{u} &&
> git merge -s ours new@{u} &&
> - git show -s --pretty=format:%s >actual &&
> + git show -s --pretty=tformat:%s >actual &&
> echo "Merge remote-tracking branch ${sq}origin/side${sq}" >expect &&
> test_cmp expect actual
> )
This seems to me that it is updating how the output is produced, not
updating the expected outputs from commands with arguments we have
been testing. I have been expecting to see changes to the pieces of
the code that prepare the expected output, so that we can compare
old expectations, which would have been expecting something strange,
with new expectations from the updated code, which would show that
the new behaviour is a welcome change because it would produce more
sensible output.
Having said all that, for this particular test piece, I agree with
the patch that using --pretty=tformat:%s is a lot more sensible and
using --pretty=format:%s and expecting its output to be terminated
with the newline was an unrealistic test. After all, "tformat" is
the version with "line terminator" semantics, as opposed to "item
separator" semantics offered by "--pretty=format:", and the test
merely was depending on the bug to expect a complete line output
(i.e. "echo" without "-n"), which you fixed. The new test makes a
lot more sense and is relevant to the real world use, and I would
have preferred to see it explained as such in the log message.
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 10aa028..b164621 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -57,11 +57,10 @@ create_merge_msgs () {
> git log --no-merges ^HEAD c2 c3
> } >squash.1-5-9 &&
> : >msg.nologff &&
> - echo >msg.nolognoff &&
> + : >msg.nolognoff &&
> {
> echo "* tag 'c3':" &&
> - echo " commit 3" &&
> - echo
> + echo " commit 3"
> } >msg.log
> }
These are updating the expectation. We used to expect an
unnecessary trailing blank line, and now we do not, which clearly
shows that the previous fix is a welcome change.
> @@ -71,7 +70,7 @@ verify_merge () {
> git diff --exit-code &&
> if test -n "$3"
> then
> - git show -s --pretty=format:%s HEAD >msg.act &&
> + git show -s --pretty=tformat:%s HEAD >msg.act &&
> test_cmp "$3" msg.act
> fi
> }
It is hard to judge what is fed as "$3" here without context, but
this is in line with the "--pretty=tformat aka --format is the
normal thing to use" we saw in 1507. The same for the other hunk.
> @@ -620,10 +619,10 @@ test_expect_success 'merge early part of c2' '
> git tag c6 &&
> git branch -f c5-branch c5 &&
> git merge c5-branch~1 &&
> - git show -s --pretty=format:%s HEAD >actual.branch &&
> + git show -s --pretty=tformat:%s HEAD >actual.branch &&
> git reset --keep HEAD^ &&
> git merge c5~1 &&
> - git show -s --pretty=format:%s HEAD >actual.tag &&
> + git show -s --pretty=tformat:%s HEAD >actual.tag &&
> test_cmp expected.branch actual.branch &&
> test_cmp expected.tag actual.tag
> '
How about squashing these two into a single patch, and at the end of
the log message for 1/2, add this to explain the changes to the
test:
Also existing tests are updated to demonstrate the new
behaviour. Earlier, the tests that used "git show -s
--pretty=format:%s", even though "--pretty=format:%s" calls for
item separator semantics and does not ask for the terminating
newline after the last item, expected the output to end with
such a newline. They were relying on the buggy behaviour. Use
of "--format=%s", which is equivalent to "--pretty=tformat:%s"
that asks for a terminating newline after each item, is a more
realistic way to use the command.
The update to verify_merge in t7600 adjusts the the merge
message that used to expect an extra blank line in the output,
which has been eliminated with this fix.
o
[PATCH] remote-hg: getbundle changed in mercurial 3.0
In mercurial 3.0, getbundle was moved to the changegroup module, and
gained a new argument. Due to this we cannot simply start using
getbundle(...) imported from either one unconditionally, as that would
cause errors in mercurial 3.0 without changing the syntax, and errors in
mercurial <3.0 if we do change it.
The try:except block at the beginning of git-remote-hg.py tries first to
import mercurial.changegroup.getbundle, and if that fails we set the
function 'getbundle' to work correctly with mercurial.repo.getbundle by
removing the first argument.
Signed-off-by: William Giokas <1007...@gmail.com>
---
I have tested this briefly with mercurial 3.0, but have not yet really run
through its paces. The tests that are included in next do pass with
mercurial 3.0.
git-remote-hg.py | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/git-remote-hg.py b/git-remote-hg.py
index 34cda02..3dc9e11 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -14,6 +14,13 @@
from mercurial import hg, ui, bookmarks, context, encoding, node, error,
extensions, discovery, util
+try:
+from mercurial.changegroup import getbundle
+
+except ImportError:
+def getbundle(__empty__, **kwargs):
+return repo.getbundle(**kwargs)
+
import re
import sys
import os
@@ -985,7 +992,7 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
if not checkheads(repo, remote, p_revs):
return None
-cg = repo.getbundle('push', heads=list(p_revs), common=common)
+cg = getbundle(repo, 'push', heads=list(p_revs), common=common)
unbundle = remote.capable('unbundle')
if unbundle:
--
2.0.0.rc3
--
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 01/17] contrib: remove outdated README
Junio C Hamano wrote: > Martin Langhoff writes: > > > On Fri, May 9, 2014 at 5:54 PM, Felipe Contreras > > wrote: > >> You are once more twisting the sequence of events. > > > > Found this gem looking for background to the proposed removal to code of > > mine. > > > > Felipe, if you are wanting to have a war of words with Junio, go have > > it, with him. > > Please don't feed the troll. I was happy to be done with it. I was going to let this comment go (as I let the endless stream of ad hominem attacks go), but this just one ridiculous. I've contributed 400 patches, changed 12700 lines, sent 4200 mails on the list. Then I'm not happy with a decision you made, and I ask you *one* question to clarify your rationale, and I'm still waiting for an answer. I think after this insane amount of work I'm entitled to an answer for this *one* question. Instead you passive aggressively label me as a troll? This is really disquieting. Junio, do you honestly think I am a troll? Have at least the decency of telling it to me. -- 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: Bash prompt "namespace" issue
sze...@ira.uka.de writes: > Commit 59d3924fb (prompt: fix missing file errors in zsh) added the > helper function eread() to git-prompt.sh. That function should be in > the git "namespace", i.e. prefixed with __git_, otherwise it might > collide with a function of the same name in the user's environment. > > It's already in master and I don't have the means to send a patch > fixing this ATM, sorry. Thanks. I think the patch Felipe posted to rename it to __git_eread is a reasonable regression fix, so I'll queue it on top of the problematic commit 59d3924f (prompt: fix missing file errors in zsh, 2014-04-11) and merge the result to 'master'. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Error using git-remote-hg
On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote: > Torsten Bögershausen writes: > > > I did some testing with Git 2.0-rc3 + 58aee0864adeeb5363f. > > The remote-helper tests for hg-git worked OK > > with both hg version 2.9 and 3.0 under both Mac OS and Linux. > > > > Should we consider 58aee086 to be included in Git 2.0 ? > > So the answer to your question is an unambiguous "no". > > It is a different matter if we want a change to allow us to operate > with both older and newer version of mercurial in a release of Git > in near future. The answer is a resounding "yes", even if the > solution may not be the exact 58aee0864 commit [*2*]. I think an > update should eventurally go to the maintenance tracks of even older > releases, perhaps maint-1.8.5 and upwards, after we merge it to > 'master' in preparation for the feature release that comes after Git > 2.0, which probably will be called 2.1 but do not quote me on that. > > Regarding the commit in question, I asked Felipe a question and > never got a straight answer. > > Why do we "import changegroup" unconditionally, even though it > is only used in the new codepath meant only for version 3.0 or > higher, not inside the "if" block that decides if we need that > module? It should not be, as it is not used outside of hg 3.0. In fact, what would be even better would be to test whether changegroup.getbundle is available, and then set a function like `getbundl()` to run either `changegroup.getbundl()` with the correct args, or `repo.getbundle()` as a fallback. changegroup is prefectly /okay/ to import unconditionally, though as you say it will never be used. We can also be even more explicit with what we import by doing something like:: try: from mercurial.changegroup import getbundle except ImportError: def getbundle(__empty__, **kwargs): return repo.getbundle(**kwargs) and then just calling:: cg = getbundle(repo, 'push', heads=list(p_revs), common=common) The `__empty__` arg is there because repo.getbundle uses a different number of arguments than the changegroup.getbundle function. It's a much cleaner solution than assuming that that will stay in changegroup, and not get moved back to repo in the future. Seems to be what you described in the second bit, though. If you would like I can submit a patch once I've finished running the tests on it, and possibly after having Felipe run it through his tests to be sure thta the 2.[7-9] series works as well, and maybe you would want to test it on other versions of mercurial that you are running alongside git. Just my 2 cents. > > I had a few more questions in mind that I didn't have a chance to > ask, and the commit log message did not explain. > > Is the reason why this fix is needed is because "repo" stopped > being a way to ask ".getbundle()" and in the new world order > "changegroup" is the new way to ask ".getbundle()"? > > If the answer is "yes", then asking "are we with 3.0 or > later---if so ask changegroup for ".getbundle()?", which is this > patch does, may not be the right condition to trigger the new > codepath. "Does repo know how to do .getbundle()? If not, > import changegroup and ask that module to perform .getbundle() > instead" may be a way to base the decision on a more directly > relevant condition. > > Answers to these questions, if they came, were meant to convince > myself that the patch 58aee0864 is the best solution to the problem, > but because I only got "Of course it is not a mistake" [*1*], seeing > it did not lead to a productive discussion, I gave up. So I haven't > even managed to convince myself that that commit is the best > solution to the problem. I was really sad to see that, and didn't have time to really look at it because of work and other projects, but I hope this presents a better solution than the current patch. Thanks, -- William Giokas | KaiSforza | http://kaictl.net/ GnuPG Key: 0x73CD09CF Fingerprint: F73F 50EF BBE2 9846 8306 E6B8 6902 06D8 73CD 09CF pgpC7ZLGG4hd9.pgp Description: PGP signature
[PATCH] Documentation: mention config sources for @{upstream}
The earlier documentation made vague references to "is set to build
on". Flesh that out with references to the config settings, so folks
can use git-config(1) to get more detail on what @{upstream} means.
For example, @{upstream} does not care about remote.pushdefault or
branch..pushremote.
Signed-off-by: W. Trevor King
---
Documentation/revisions.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 5a286d0..0796118 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -94,7 +94,9 @@ some output processing may assume ref names in UTF-8.
'@\{upstream\}', e.g. 'master@\{upstream\}', '@\{u\}'::
The suffix '@\{upstream\}' to a branchname (short form '@\{u\}')
refers to the branch that the branch specified by branchname is set to build
on
- top of. A missing branchname defaults to the current one.
+ top of (configured with `branch..remote` and
+ `branch..merge`). A missing branchname defaults to the
+ current one.
'{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
A suffix '{caret}' to a revision parameter means the first parent of
--
1.9.1.353.gc66d89d
--
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 v1 1/2] Remove 'git archimport'
Martin Langhoff wrote: > On Tue, May 13, 2014 at 2:05 PM, Felipe Contreras > wrote: > > This tool doesn't even work anyway. > > It doesn't? Bug report / more info please? Show me that it does. The documentation is lacking, and there's no tests at all. So it's hard to say is anybody supposed to use this and verify that it works, but I ran this from Jeff: tla register-archive http://www.atai.org/archarchives/a...@atai.org--public/ tla my-default-archive a...@atai.org--public mkdir repo cd repo git archimport a...@atai.org--public And the command threw hundreds of errors and the result seemed to miss tons of commits. Does it work? -- 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: Error using git-remote-hg
Junio C Hamano wrote: > It is time to catch regressions by asking wider audiences who do not > normally follow Git development (i.e. those who are not the ones that > follow 'master' and rebuild/install it once or twice a week for their > daily use). And you have one of those regressions in Git v2.0. > My understanding is that versions of remote-hg shipped with all > versions of Git did not work with Hg 3.0, so 58aee08 is not a > regression fix at all. Is working with Hg 3.0 at such a high priority > that it makes it worth to slip the whole release schedule by a few > weeks? I do not think so. It is in the contrib/ area, you don't need to slip the schedule for something that is not part of the core. Moreover, it *CANNOT POSSIBLY INTRODUCE REGRESSIONS*. I bet my reputation on that. Some time ago I asked you to trust my judgement and introduce changes late into a release, and I told you there wouldn't be any problems, and to trust me. If any problems arise I wouldn't be asking for something like that again. Well, I was right, there were no problems. And I'm right this time too, there would be no issues. But you don't care about reality and facts. You don't care about the intent behind the release-candidates rule, you would rather follow the rule blindly. > Regarding the commit in question, I asked Felipe a question and > never got a straight answer. Nor will you get one, because thanks to your stupid decision for which you still haven't provided a rationale, the git-remote-hg and git-remote-hg are no longer maintained in git.git. If you hadn't made such a move, you would have your answer, the fix would be properly explained, the regression fixed, and all your users would be happy that git-remote-hg and git-remote-bzr get distributed by default. -- 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 01/17] contrib: remove outdated README
Martin Langhoff writes: > On Fri, May 9, 2014 at 5:54 PM, Felipe Contreras > wrote: >> You are once more twisting the sequence of events. > > Found this gem looking for background to the proposed removal to code of mine. > > Felipe, if you are wanting to have a war of words with Junio, go have > it, with him. Please don't feed the troll. I was happy to be done with it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/2] Remove 'git archimport'
Martin Langhoff writes: > On Thu, May 8, 2014 at 9:33 PM, Felipe Contreras > wrote: >> No updates since 2010, and no tests. > > NAK. > > IMHO, this is quite unfriendly. > > Is this removal based on your opinion, or Junio's position (or > consensus from maintainers from the list)? If there is a clear > consensus or direction for old code such as this, please let me know > (but copy martin.langh...@gmail.com, not just my very old address!). > >> Plus, foreign SCM tools should live out-of-tree anyway. > > Says who? Is there consensus on this? > > It's generally the privilege of the maintainer -- in this case Junio > or perhaps Linus -- to take harsh stances like this. > > Junio, what's your position? We may think longer when somebody proposes to add a new thing that may better live outside our tree (including the contrib/ area) than we used to, simply because Git is more mature these days and the ecosystem is there to support successful third-party tools, but removal of existing subcommands needs to weigh the impact of such a removal to existing users. "No recent updates" does not say anything with respect to that---we cannot tell between "The tool is perfect to fill needs of the users" and "Even though the users are reporting issues, the area maintainer is not being responsive" by non activity alone, and we know there weren't many unresponded issues in the recent past. "There is no longer any project that still hosts anything worth salvaging in tla", if such a claim can be substantiated, might be a valid reason to propose a removal, but I do not think this is such a proposal. -- 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 v1 1/2] Remove 'git archimport'
On Tue, May 13, 2014 at 2:05 PM, Felipe Contreras wrote: > This tool doesn't even work anyway. It doesn't? Bug report / more info please? cheers, m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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 v1 1/2] Remove 'git archimport'
On Fri, May 9, 2014 at 1:44 PM, Junio C Hamano wrote: > Eric Wong writes: > >> Felipe Contreras wrote: >>> No updates since 2010, and no tests. >> >> Who benefits from this removal? Is this causing a maintenance >> burden for Junio? > > No. See http://thread.gmane.org/gmane.comp.version-control.git/248587 Thanks for this link. Took me a while to find -- git ML is quite busy :-) -- to be honest it might be good if you make it a separate post, rather than having to find buried in the removal threads that "everything's ok, safe to ignore this very thread you're reading"; specially for the casual readers. Can we ban Felipe from the ML? If he's been a positive contributor in the past, perhaps it can be a temporary ban. Right now he is far from a positive member of the community. About code I wrote... I'm still around, and care if folks find significant bugs. Don't read the list very actively. If maintenance standards change, I'll make an effort to meet them. m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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 v1 1/2] Remove 'git archimport'
Martin Langhoff wrote: > On Thu, May 8, 2014 at 9:33 PM, Felipe Contreras > wrote: > > No updates since 2010, and no tests. > > NAK. > > IMHO, this is quite unfriendly. > > Is this removal based on your opinion, or Junio's position (or > consensus from maintainers from the list)? If there is a clear > consensus or direction for old code such as this, please let me know > (but copy martin.langh...@gmail.com, not just my very old address!). This tool doesn't even work anyway. Why do we want to distribute code that doesn't work? -- 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
