Re: [PATCH v4] Thunderbird: fix appp.sh format problems
2012/9/3 Junio C Hamano gits...@pobox.com: Marco Stornelli marco.storne...@gmail.com writes: I tried the Johannes's script, but it seems it doesn't work well with the pattern of format-patch (To: mail1,\n mail2,\n mailN). The multilines are not well managed. I am guessing that the reason why Jonahhes's copy our headers out with continuation lines intact approach does not work is because Thunderbird does not want to see its own header part (i.e. that which comes before that $SEP) contain RFC-2822 style continuation lines. Can you grab a typical input (i.e. the contents of what is passed as $1 to the appp.sh script) and show us how it looks like here so that we can take a look? It would be fine to paste the contents, but we might want to protect it from MUA by doing an attachment or a pastebin URL. I don't have thunderbird now but actually it's really simple: Subject: To: Cc: $SEP Each data must be in a signle line, for example Cc: mail1,.,mailN It appears that the original script tries very hard to keep the Subject: line at the beginning, but I am not sure if that is because Thunderbird wants to read its $1 that way, or it is just that original appp.sh script happened to be written that way without real reason. If I were updating this script, what I would do would be more like: Ok, good coding then. Regards, Marco -- To unsubscribe from this list: send the line 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: checkout extra files
Hi all, consider this example: $ mkdir gittest $ cd gittest $ git init Initialized empty Git repository in d:/gittest/.git/ $ touch f1 $ git add f1 $ git commit commit -m first commit [master (root-commit) e6f935e] first commit 0 files changed create mode 100644 f1 $ touch f2 $ git checkout e6f9 -- * error: pathspec 'f2' did not match any file(s) known to git. Note the error. It is clear that the set of file names that git checkout is taking is the union of the ones that match the specified path ('*') in the work directory (gittest) with the ones that match the path in the specified commit (e6f9). I am not questioning this behavior, but the documentation, which does not describe it: It updates the named paths in the working tree from the index file or from a named tree-ish ... There are two ways to read this sentence: 1. named paths referred to working tree, i.e. the files whose names match the paths among all the ones present in the working tree 2. named paths referred to the index or tree-ish, i.e. the files whose names match paths among oll the ones present in the index or tree-ish In both cases, nothing tells the user that the matching of the paths is done over the union of the set of file names of the working tree + ndex or tree-ish. Indeed, the first time I have seen the error above I got quite confused because I thought that the checkout would restore the working directory as it was at the time I made the commit, without bothering about extra files that the directory contained at the moment (and note that f2 is not even a tracked one). This behavior is a bit strange, but if it is a hundred percent clearly documented I can live with it. I think that knowing precisely what files are involved in this command is essential for the user. -Angelo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-submodule: respect -q for add/update
Signed-off-by: Orgad Shaneh org...@gmail.com --- git-submodule.sh | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index aac575e..dd57abb 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -266,6 +266,11 @@ cmd_add() repo=$1 sm_path=$2 + quiet= + if test -n $GIT_QUIET + then + quiet=-q + fi if test -z $sm_path; then sm_path=$(echo $repo | @@ -332,8 +337,8 @@ Use -f if you really want to add it. 2 cd $sm_path # ash fails to wordsplit ${branch:+-b $branch...} case $branch in - '') git checkout -f -q ;; - ?*) git checkout -f -q -B $branch origin/$branch ;; + '') git checkout -f $quiet ;; + ?*) git checkout -f $quiet -B $branch origin/$branch ;; esac ) || die $(eval_gettext Unable to checkout submodule '\$sm_path') fi @@ -527,6 +532,12 @@ cmd_update() shift done + quiet= + if test -n $GIT_QUIET + then + quiet=-q + fi + if test -n $init then cmd_init -- $@ || return @@ -619,7 +630,7 @@ Maybe you want to use 'update --init'?) must_die_on_failure=yes ;; *) - command=git checkout $subforce -q + command=git checkout $subforce $quiet die_msg=$(eval_gettext Unable to checkout '\$sha1' in submodule path '\$sm_path') say_msg=$(eval_gettext Submodule path '\$sm_path': checked out '\$sha1') ;; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] t0000: verify that absolute_path() fails if passed the empty string
From: Michael Haggerty mhag...@alum.mit.edu It doesn't, so mark the test as failing. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- t/t-basic.sh | 4 1 file changed, 4 insertions(+) diff --git a/t/t-basic.sh b/t/t-basic.sh index ccb5435..7347fb8 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -450,6 +450,10 @@ test_expect_success 'update-index D/F conflict' ' test $numpath0 = 1 ' +test_expect_failure 'absolute path rejects the empty string' ' + test_must_fail test-path-utils absolute_path +' + test_expect_success SYMLINKS 'real path works as expected' ' mkdir first ln -s ../.git first/.git -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] absolute_path(): reject the empty string
From: Michael Haggerty mhag...@alum.mit.edu Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- abspath.c| 4 +++- t/t-basic.sh | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/abspath.c b/abspath.c index f04ac18..5d62430 100644 --- a/abspath.c +++ b/abspath.c @@ -123,7 +123,9 @@ const char *absolute_path(const char *path) { static char buf[PATH_MAX + 1]; - if (is_absolute_path(path)) { + if (!*path) { + die(The empty string is not a valid path); + } else if (is_absolute_path(path)) { if (strlcpy(buf, path, PATH_MAX) = PATH_MAX) die(Too long path: %.*s, 60, path); } else { diff --git a/t/t-basic.sh b/t/t-basic.sh index 7347fb8..5963a6c 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -450,7 +450,7 @@ test_expect_success 'update-index D/F conflict' ' test $numpath0 = 1 ' -test_expect_failure 'absolute path rejects the empty string' ' +test_expect_success 'absolute path rejects the empty string' ' test_must_fail test-path-utils absolute_path ' -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] Fix some bugs in abspath.c
From: Michael Haggerty mhag...@alum.mit.edu I really just wanted to tidy up filter_refs(), but I've been sucked into a cascade of recursive yak shaving. This is my first attempt to pop the yak stack. I want to use real_path() for making the handling of GIT_CEILING_DIRECTORIES more robust, but I noticed that it is broken for some cases that will be important in this context. This patch series adds some new tests of that function and fixes the ones that are broken, including a similar breakage in absolute_path(). It applies to the current master. Please note that both absolute_path() and real_path() used to return the current directory followed by a slash. I believe that this was a bug, and that it is more appropriate for both functions to reject the empty string. The evidence is as follows: * If this were intended behavior, presumably the return value would *not* have a trailing slash. * I couldn't find any callers that appeared to depend on the old behavior. * The test suite runs without errors after the change. But I didn't do a thorough code review to ensure that no callers ever rely on the old behavior. The most likely scenario would be if one of these functions were used to parse something like $PATH, where the empty string denotes the current directory, but I didn't see any callers that seemed to be doing anything like this. Michael Haggerty (7): t: verify that absolute_path() fails if passed the empty string absolute_path(): reject the empty string t: verify that real_path() fails if passed the empty string real_path(): reject the empty string t: verify that real_path() works correctly with absolute paths real_path(): properly handle nonexistent top-level paths t: verify that real_path() removes extra slashes abspath.c| 12 ++-- t/t-basic.sh | 31 ++- 2 files changed, 40 insertions(+), 3 deletions(-) -- 1.7.11.3 -- To unsubscribe from this list: send the line 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 7/7] t0000: verify that real_path() removes extra slashes
From: Michael Haggerty mhag...@alum.mit.edu These tests already pass, but make sure they don't break in the future. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- It would be great if somebody would check whether these tests pass on Windows, and if not, give me a tip about how to fix them. t/t-basic.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t-basic.sh b/t/t-basic.sh index d929578..3c75e97 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -468,6 +468,17 @@ test_expect_success 'real path works on absolute paths' ' test $d/$nopath = $(test-path-utils real_path $d/$nopath) ' +test_expect_success 'real path removes extra slashes' ' + nopath=hopefully-absent-path + test / = $(test-path-utils real_path ///) + test /$nopath = $(test-path-utils real_path ///$nopath) + # We need an existing top-level directory to use in the + # remaining tests. Use the top-level ancestor of $(pwd): + d=$(pwd -P | sed -e s|^\(/[^/]*\)/.*|\1|) + test $d = $(test-path-utils real_path //$d///) + test $d/$nopath = $(test-path-utils real_path //$d///$nopath) +' + test_expect_success SYMLINKS 'real path works on symlinks' ' mkdir first ln -s ../.git first/.git -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] t0000: verify that real_path() works correctly with absolute paths
From: Michael Haggerty mhag...@alum.mit.edu There is currently a bug: if passed an absolute top-level path that doesn't exist (e.g., /foo) it incorrectly interprets the path as a relative path (e.g., returns $(pwd)/foo). So mark the test as failing. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- t/t-basic.sh | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/t/t-basic.sh b/t/t-basic.sh index 1a51634..ad002ee 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -458,7 +458,17 @@ test_expect_success 'real path rejects the empty string' ' test_must_fail test-path-utils real_path ' -test_expect_success SYMLINKS 'real path works as expected' ' +test_expect_failure 'real path works on absolute paths' ' + nopath=hopefully-absent-path + test / = $(test-path-utils real_path /) + test /$nopath = $(test-path-utils real_path /$nopath) + # Find an existing top-level directory for the remaining tests: + d=$(pwd -P | sed -e s|^\(/[^/]*\)/.*|\1|) + test $d = $(test-path-utils real_path $d) + test $d/$nopath = $(test-path-utils real_path $d/$nopath) +' + +test_expect_success SYMLINKS 'real path works on symlinks' ' mkdir first ln -s ../.git first/.git mkdir second -- 1.7.11.3 -- To unsubscribe from this list: send the line 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] test-generation: compute generation numbers and clock skews
It finds three commits that has older commit timestamp than the newest commit timestamp among its ancestor in our history; all of these are a direct child of a commit that is older (i.e. the clock skew lasts for only one hop). commit gen timestamp skew gen ancestor ed19f36 2870 2006-03-04 07:29:56 + (8) 2869 91a6bf4 7763987 6404 2007-09-02 06:53:47 + (373) 6403 86bab96 619a644 9982 2009-10-18 19:34:56 + (268948) 9981 46148dd On the other hand, the kernel history is littered with skewed chains. I counted 2239 commits that have an ancestor newer than themselves in total (they tend to cluster, but I haven't counted clusters), among 322345 commits (0.7%). For example, a 33-commit chain leading to b4e1b7e builds on top of 422e6c4 that was commited by Linus at Tue Mar 15 15:48:13 2011, but the tip commit claims to have been committed at Sun Feb 20 19:19:43 2011, which is clearly impossible. Signed-off-by: Junio C Hamano gits...@pobox.com --- * Take the usefulness and correctness of this patch with a large grain of salt, as it was done primarily because I couldn't sleep X-. The motivation behind this toy is to help analyzing the real world history and come up with a way to improve the robustness of history traversal, which depends on the SLOP heuristics, without having to give each and every commit object an extra generation number (worse yet, after the fact). We could instead mark only these 2200+ commits, and teach still_interesting() function not to rely on SLOP, but answer yes while one of these commits marked as unreliable/skewed are still on the list. When we no longer have these skewed commits (whose definition is its timestamp is older than one of its ancestor's timestamp), we know that the time-based priority queue has popped all the ancestors that possibly can matter, and stop the traversal with confidence. Makefile | 1 + test-generation.c | 105 ++ 2 files changed, 106 insertions(+) create mode 100644 test-generation.c diff --git a/Makefile b/Makefile index 66e8216..52f62b7 100644 --- a/Makefile +++ b/Makefile @@ -489,6 +489,7 @@ TEST_PROGRAMS_NEED_X += test-date TEST_PROGRAMS_NEED_X += test-delta TEST_PROGRAMS_NEED_X += test-dump-cache-tree TEST_PROGRAMS_NEED_X += test-genrandom +TEST_PROGRAMS_NEED_X += test-generation TEST_PROGRAMS_NEED_X += test-index-version TEST_PROGRAMS_NEED_X += test-line-buffer TEST_PROGRAMS_NEED_X += test-match-trees diff --git a/test-generation.c b/test-generation.c new file mode 100644 index 000..4df5a0d --- /dev/null +++ b/test-generation.c @@ -0,0 +1,105 @@ +#include cache.h +#include commit.h +#include diff.h +#include revision.h + + +struct gdata { + int generation; + struct commit *youngest_ancestor; +}; + +static struct gdata *util_gd(struct commit *commit) +{ + return commit-util; +} + +static void show_commit(struct commit *commit, struct gdata *gd) +{ + printf(%s %d, + find_unique_abbrev(commit-object.sha1, DEFAULT_ABBREV), + gd-generation); + if (gd-youngest_ancestor != commit) { + struct commit *ancestor = gd-youngest_ancestor; + const char *abbrev; + + abbrev = find_unique_abbrev(ancestor-object.sha1, + DEFAULT_ABBREV); + printf( %s , show_date(commit-date, 0, DATE_ISO8601)); + printf((%lu) , ancestor-date - commit-date); + printf(%d, util_gd(ancestor)-generation); + printf( %s, abbrev); + } + putchar('\n'); +} + +int main(int ac, const char **av) +{ + struct rev_info revs; + struct setup_revision_opt opt; + struct commit_list *list; + struct commit_list *stuck = NULL; + + memset(opt, 0, sizeof(opt)); + opt.def = HEAD; + init_revisions(revs, NULL); + setup_revisions(ac, av, revs, opt); + prepare_revision_walk(revs); + + list = revs.commits; + while (list || stuck) { + struct commit_list *parent, *next; + struct commit *commit; + struct gdata *gd; + int ready = 1; + int parent_generation; + struct commit *youngest_ancestor; + + if (!list) { + list = stuck; + stuck = NULL; + } + commit = list-item; + youngest_ancestor = commit; + parent_generation = 0; + parse_commit(commit); + if (!commit-util) + commit-util = xcalloc(1, sizeof(*gd)); + gd = commit-util; + if (gd-generation) { + /* we have handled this already */ + next = list-next; + free(list); + list = next; +
Re: checkout extra files
On Tue, Sep 4, 2012 at 9:55 AM, Junio C Hamano gits...@pobox.com wrote: Perhaps like this? Looks good. I was going to complain that this patch applied to git-checkout.txt only but I just saw git-add.txt also mentions about quoting wildcards. So I'm good. diff --git i/Documentation/git-checkout.txt w/Documentation/git-checkout.txt index 63a2516..e7272b6 100644 --- i/Documentation/git-checkout.txt +++ w/Documentation/git-checkout.txt @@ -360,20 +360,32 @@ mistake, and gets it back from the index. $ git checkout master 1 $ git checkout master~2 Makefile 2 $ rm -f hello.c $ git checkout hello.c3 + 1 switch branch 2 take a file out of another commit 3 restore hello.c from the index + +If you want to check out _all_ C source files out of the index, +you can say ++ + +$ git checkout -- '*.c' + ++ +Note the quotes around '*.c'. 'hello.c' will also be checked +out, even though it is no longer in the working tree, because +the pathspec is used to match entries in the index (not in the +working tree by your shell). ++ If you have an unfortunate branch that is named `hello.c`, this step would be confused as an instruction to switch to that branch. You should instead write: + $ git checkout -- hello.c . After working in the wrong branch, switching to the correct branch would be done using: -- 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 v2] Support for setitimer() on platforms lacking it
Poor man's getitimer(), simply based on alarm(). Currently needed on HP NonStop (#ifdef __TANDEM), which does provide struct itimerval, but no setitimer(). Alarm times are rounded up to the next full second. Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- Revert/remove my previous 2 patches for this first (from 'pu'). git-compat-util.h | 4 1 file changed, 4 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 18089f0..55b9421 100644 --- a/git-compat-util.h +++ b/git-compat-util.h@@ -163,6 +163,10 @@ #define probe_utf8_pathname_composition(a,b) #endif +#ifdef NO_SETITIMER /* poor man's setitimer() */ +#define setitimer(w,v,o) alarm((v)-it_value.tv_sec+((v)-it_value.tv_usec0)) +#endif + #ifdef MKDIR_WO_TRAILING_SLASH #define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b)) extern int compat_mkdir_wo_trailing_slash(const char*, mode_t); -- 1.7.12 -- To unsubscribe from this list: send the line 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] fetch: align new ref summary printout in UTF-8 locales
fetch does printf(%-*s, width, foo) where foo can be a utf-8 string, but width is in bytes, not columns. For ASCII it's fine as one byte takes one column. For utf-8, this may result in misaligned ref summary table. Introduce gettext_width() function that returns the string length in columns (currently only supports utf-8 locales). Make the code use TRANSPORT_SUMMARY(x) where the length is compensated properly in non-English locales. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- - rename gettext_length() to gettext_width() - use width instead of letters - leave other %-*s places unchanged if they always take ascii strings (i.e. no _() calls) - note to self, may need to i18n-ize print_ref_status() in transport.c, looks like it's used by git-push only builtin/fetch.c | 15 +++ gettext.c | 15 +-- gettext.h | 5 + transport.h | 1 + 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index bb9a074..85e291f 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -255,9 +255,8 @@ static int update_local_ref(struct ref *ref, if (!hashcmp(ref-old_sha1, ref-new_sha1)) { if (verbosity 0) strbuf_addf(display, = %-*s %-*s - %s, - TRANSPORT_SUMMARY_WIDTH, - _([up to date]), REFCOL_WIDTH, - remote, pretty_ref); + TRANSPORT_SUMMARY(_([up to date])), + REFCOL_WIDTH, remote, pretty_ref); return 0; } @@ -271,7 +270,7 @@ static int update_local_ref(struct ref *ref, */ strbuf_addf(display, _(! %-*s %-*s - %s (can't fetch in current branch)), - TRANSPORT_SUMMARY_WIDTH, _([rejected]), + TRANSPORT_SUMMARY(_([rejected])), REFCOL_WIDTH, remote, pretty_ref); return 1; } @@ -282,7 +281,7 @@ static int update_local_ref(struct ref *ref, r = s_update_ref(updating tag, ref, 0); strbuf_addf(display, %c %-*s %-*s - %s%s, r ? '!' : '-', - TRANSPORT_SUMMARY_WIDTH, _([tag update]), + TRANSPORT_SUMMARY(_([tag update])), REFCOL_WIDTH, remote, pretty_ref, r ? _( (unable to update local ref)) : ); return r; @@ -317,7 +316,7 @@ static int update_local_ref(struct ref *ref, r = s_update_ref(msg, ref, 0); strbuf_addf(display, %c %-*s %-*s - %s%s, r ? '!' : '*', - TRANSPORT_SUMMARY_WIDTH, what, + TRANSPORT_SUMMARY(what), REFCOL_WIDTH, remote, pretty_ref, r ? _( (unable to update local ref)) : ); return r; @@ -357,7 +356,7 @@ static int update_local_ref(struct ref *ref, return r; } else { strbuf_addf(display, ! %-*s %-*s - %s %s, - TRANSPORT_SUMMARY_WIDTH, _([rejected]), + TRANSPORT_SUMMARY(_([rejected])), REFCOL_WIDTH, remote, pretty_ref, _((non-fast-forward))); return 1; @@ -554,7 +553,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map) result |= delete_ref(ref-name, NULL, 0); if (verbosity = 0) { fprintf(stderr, x %-*s %-*s - %s\n, - TRANSPORT_SUMMARY_WIDTH, _([deleted]), + TRANSPORT_SUMMARY(_([deleted])), REFCOL_WIDTH, _((none)), prettify_refname(ref-name)); warn_dangling_symref(stderr, dangling_msg, ref-name); } diff --git a/gettext.c b/gettext.c index f75bca7..71e9545 100644 --- a/gettext.c +++ b/gettext.c @@ -4,6 +4,8 @@ #include git-compat-util.h #include gettext.h +#include strbuf.h +#include utf8.h #ifndef NO_GETTEXT # include locale.h @@ -27,10 +29,9 @@ int use_gettext_poison(void) #endif #ifndef NO_GETTEXT +static const char *charset; static void init_gettext_charset(const char *domain) { - const char *charset; - /* This trick arranges for messages to be emitted in the user's requested encoding, but avoids setting LC_CTYPE from the @@ -128,4 +129,14 @@ void git_setup_gettext(void) init_gettext_charset(git); textdomain(git); } + +/* return the number of columns of string 's' in current locale */ +int gettext_width(const char *s) +{ + static int is_utf8 = -1; + if (is_utf8 == -1) +
Feature request: short cuts for git add /diff etc
Hi, Git Change this: # git status # On branch yandex_mail_new_api # Your branch is ahead of 'origin/yandex_mail_new_api' by 2 commits. # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: a1.txt # modified: a2.txt # # Untracked files: # (use git add file... to include in what will be committed) # # new.txt to this: # git status # On branch yandex_mail_new_api # Your branch is ahead of 'origin/yandex_mail_new_api' by 2 commits. # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified(1): a1.txt # modified(2): a2.txt # # Untracked files: # (use git add file... to include in what will be committed) # # (3)new.txt sot this allow: git diff 1 same asgit diff a1.txt git diff 2 same asgit diff a2.txt git add 1same asgit add a1.txt in case there are may be files with such names I may appply option -s,for example git add [ -s | --stage ] 1 . This will force to add file from list of 'git status' and do not use '1' as file name. git add 3same as git add new.txt This very handy and will keep developer from useless typing =) -- С уважением, Коньков Евгений Программист Регистратор доменных имен REG.RU Телефон: +38 (097) 7654-676 www.reg.ru ___ Sincerely yours, Konkov Eugen Developer Accredited domain Registrar REG.RU, LLC. Phone: +38 (097) 7654-676 www.reg.ru/en/ mailto:k...@reg.ru -- To unsubscribe from this list: send the line 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] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Friday, August 24, 2012 9:47 PM To: Joachim Schmitz Cc: git@vger.kernel.org; 'Erik Faye-Lund' Subject: Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact Joachim Schmitz j...@schmitz-digital.de writes: Different, but related question: would poll.[ch] be allowed to #include git-compat-util.h? Seeing other existing generic wrappers directly under compat/, e.g. fopen.c, mkdtemp.c, doing so, I would say why not. Windows folks (I see Erik is already CC'ed, which is good ;-), please work with Joachim to make sure such a move won't break your builds. I believe that it should just be the matter of updating a couple of paths in the top-level Makefile. Haven't heard anything from the Windows folks yet. I'd prefer to move compat/win32/poll.[ch] into compat/poll. Then adjust a few paths in Makefile and that would be the 1st patch A 2nd patch would be my already proposed ones that make this usable for others (me in this case ;-)), namely wrapping 2 #inludes. diff --git a/compat/poll/poll.c b/compat/poll/poll.c index 403eaa7..49541f1 100644 --- a/compat/poll/poll.c +++ b/compat/poll/poll.c @@ -24,7 +24,9 @@ # pragma GCC diagnostic ignored -Wtype-limits #endif -#include malloc.h +#if defined(WIN32) +# include malloc.h +#endif #include sys/types.h @@ -48,7 +50,9 @@ #else # include sys/time.h # include sys/socket.h -# include sys/select.h +# ifndef NO_SYS_SELECT_H +# include sys/select.h +# endif # include unistd.h #endif -- 1.7.12 -- To unsubscribe from this list: send the line 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: Feature request: short cuts for git add /diff etc
On Tue, Sep 4, 2012 at 7:25 PM, Коньков Евгений k...@reg.ru wrote: sot this allow: git diff 1 same asgit diff a1.txt git diff 2 same asgit diff a2.txt git add 1same asgit add a1.txt in case there are may be files with such names I may appply option -s,for example git add [ -s | --stage ] 1 . This will force to add file from list of 'git status' and do not use '1' as file name. git add 3same as git add new.txt This very handy and will keep developer from useless typing =) Check out git-number[1] and scm_breeze[2]. Both provides similar functionality to the above. nazri shameless disclaimer: I wrote git-number. [1] https://github.com/holygeek/git-number [2] https://github.com/ndbroadbent/scm_breeze -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] i18n.pathencoding
On Sat, Sep 1, 2012 at 1:11 PM, Torsten Bögershausen tbo...@web.de wrote: diff --git a/parse-options.c b/parse-options.c index c1c66bd..5840c18 100644 --- a/parse-options.c +++ b/parse-options.c @@ -476,7 +476,7 @@ int parse_options(int argc, const char **argv, const char *prefix, usage_with_options(usagestr, options); } - precompose_argv(argc, argv); + reencode_argv(argc, argv); return parse_options_end(ctx); } If you have to re-encode command line arguments, what about paths coming --stdin or a file? I guess that paths that are argument to an option are not re-encoded too, which may put git in an inconsistent state where it cannot compare paths to paths because some may be re-encoded above, some not. -- 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 4/9] Refactor excluded_from_list
On Sun, Sep 2, 2012 at 7:12 AM, Adam Spiers g...@adamspiers.org wrote: diff --git a/dir.c b/dir.c index 57a5d11..3a532d5 100644 --- a/dir.c +++ b/dir.c @@ -509,22 +509,24 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen) dir-basebuf[baselen] = '\0'; } -/* Scan the list and let the last match determine the fate. - * Return 1 for exclude, 0 for include and -1 for undecided. +/* + * Scan the given exclude list in reverse to see whether pathname + * should be ignored. The first match (i.e. the last on the list), if + * any, determines the fate. Returns the exclude_list element which + * matched, or NULL for undecided. */ -int excluded_from_list(const char *pathname, - int pathlen, const char *basename, int *dtype, - struct exclude_list *el) +struct exclude *excluded_from_list_1(const char *pathname, int pathlen, +const char *basename, int *dtype, +struct exclude_list *el) { int i; You should keep the new function static. -- 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 5/9] Refactor excluded and path_excluded
On Sun, Sep 2, 2012 at 7:12 AM, Adam Spiers g...@adamspiers.org wrote: extern void path_exclude_check_init(struct path_exclude_check *, struct dir_struct *); extern void path_exclude_check_clear(struct path_exclude_check *); +extern struct exclude *path_excluded_1(struct path_exclude_check *, const char *, + int namelen, int *dtype); extern int path_excluded(struct path_exclude_check *, const char *, int namelen, int *dtype); Exported functions should have nicer names than *_1. No idea what are better names though, maybe exclude_path? -- 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] git-submodule: respect -q for add/update
Am 04.09.2012 09:31, schrieb Orgad Shaneh: Signed-off-by: Orgad Shaneh org...@gmail.com Before the Signed-off-by is the place where you should have explained why this would be a worthwhile change ;-) To me this looks like you make the default noisier and require an explicit -q to make it quiet again. There is a reason you don't normally get bothered with the output of the checkout command run under the hood of git submodule add/update, so I don't think this change makes things better. But you might want to think about adding a -v/--verbose flag to make the submodule add/update checkouts more verbose, in case you care about the output of the checkout command. That would be a sane thing to do, so what about changing your patch into this direction? --- git-submodule.sh | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index aac575e..dd57abb 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -266,6 +266,11 @@ cmd_add() repo=$1 sm_path=$2 + quiet= + if test -n $GIT_QUIET + then + quiet=-q + fi if test -z $sm_path; then sm_path=$(echo $repo | @@ -332,8 +337,8 @@ Use -f if you really want to add it. 2 cd $sm_path # ash fails to wordsplit ${branch:+-b $branch...} case $branch in - '') git checkout -f -q ;; - ?*) git checkout -f -q -B $branch origin/$branch ;; + '') git checkout -f $quiet ;; + ?*) git checkout -f $quiet -B $branch origin/$branch ;; esac ) || die $(eval_gettext Unable to checkout submodule '\$sm_path') fi @@ -527,6 +532,12 @@ cmd_update() shift done + quiet= + if test -n $GIT_QUIET + then + quiet=-q + fi + if test -n $init then cmd_init -- $@ || return @@ -619,7 +630,7 @@ Maybe you want to use 'update --init'?) must_die_on_failure=yes ;; *) - command=git checkout $subforce -q + command=git checkout $subforce $quiet die_msg=$(eval_gettext Unable to checkout '\$sha1' in submodule path '\$sm_path') say_msg=$(eval_gettext Submodule path '\$sm_path': checked out '\$sha1') ;; -- To unsubscribe from this list: send the line 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: checkout extra files
Junio C Hamano gits...@pobox.com writes: Now we have f1 and f2 in the working tree. $ git checkout e6f9 -- * That is the same as git checkout e6f935e -- f1 f2, as the shell expanded * into f1 and f2. error: pathspec 'f2' did not match any file(s) known to git. Note the error. Yes? It is clear that the set of file names that git checkout is taking is the union of the ones that match the specified path ('*') in the work directory (gittest) with the ones that match the path in the specified commit (e6f9). The command tells git to check out f1 and f2 out of the tree of e6f935e, and git found f1 but did not find f2 and reported an error. I do not see a room or need for union to come into the picture to explain what we see in the above transcript. Actually, I kind of sort of can see where that union is coming from, if I squint my eyes hard enough. Yes, it makes it look like the path affected has some relationship between two sets of paths: - set W, which consists of f1 and f2, that is the result of matching '*' against working tree files; and - set T, which consists of f1 (but not f2), that is the result of matching '*' against the tree contained in e6f935e and the intersection of W and T (i.e. f1) is what is checked out. But that is not what is happening at all. What goes on is far simpler than that. - the shell sees '*', matches it against working tree files, to obtain f1 and f2; - the shell tells git to checkout e6f935e -- f1 f2; - git looks into the tree of e6f935e to find paths that match f1 and f2. When git is run by the shell in the last step, it has _no_ clue that the end user typed * from the shell. It only sees f1 and f2 on the command line. There is no set T to be intersected with set W, so stop thinking in those terms, and you will be fine. Now the question is, _you_ will be fine, but can the documentation be updated in such a way so that it will help _others_ to also stop thinking about intersection between set W and set T? I do not have a good answer to that. -- To unsubscribe from this list: send the line 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: checkout extra files
Angelo Borsotti angelo.borso...@gmail.com writes: That of git checkout -- * is the problem when the directory is empty. Note that this happens with the shell that is shipped with git (in the windows distro). A note in the documentation could help the user to understand this. Passing unmatched glob intact to the program has always been the standard behaviour of shells for decades, and is not specific to the shell that is shipped with msysgit, no? I would imagine that msysgit could be shipped with man pages for bash, but I doubt anybody would consult it when hitting this I typed * in an empty directory situation. In any case, what to include in the package is something msysgit folks to decide. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] i18n.pathencoding
Nguyen Thai Ngoc Duy pclo...@gmail.com writes: On Sat, Sep 1, 2012 at 1:11 PM, Torsten Bögershausen tbo...@web.de wrote: diff --git a/parse-options.c b/parse-options.c index c1c66bd..5840c18 100644 --- a/parse-options.c +++ b/parse-options.c @@ -476,7 +476,7 @@ int parse_options(int argc, const char **argv, const char *prefix, usage_with_options(usagestr, options); } - precompose_argv(argc, argv); + reencode_argv(argc, argv); return parse_options_end(ctx); } If you have to re-encode command line arguments, what about paths coming --stdin or a file? That problem is inherited from the MacOS precompose topic this one builds on. Not that it is unimportant to fix, though. -- To unsubscribe from this list: send the line 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 5/9] Refactor excluded and path_excluded
Nguyen Thai Ngoc Duy pclo...@gmail.com writes: On Sun, Sep 2, 2012 at 7:12 AM, Adam Spiers g...@adamspiers.org wrote: extern void path_exclude_check_init(struct path_exclude_check *, struct dir_struct *); extern void path_exclude_check_clear(struct path_exclude_check *); +extern struct exclude *path_excluded_1(struct path_exclude_check *, const char *, + int namelen, int *dtype); extern int path_excluded(struct path_exclude_check *, const char *, int namelen, int *dtype); Exported functions should have nicer names than *_1. No idea what are better names though, maybe exclude_path? Which part is better? Just like between path_excluded_1() and path_excluded() nobody can tell how they differ (except perhaps the former smells more special purpose thanks to its funny name) and wouldn't be able to tell which one to call without looking at their sources, it is hard to tell path_excluded() and exclude_path() apart. In a sense, that pair is even worse as there is no hint to suggest which one is more exotic between them in their names. -- To unsubscribe from this list: send the line 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] cherry-pick: Append -x line on separate paragraph
Before this, git cherry-pick -x resulted in messages like this: Message of cherry-picked commit (cherry picked from commit 871e293c9acbeaacce59dcd98fab6028f552f5be) Which is not the recommended way to write commit messages. When the commit message ends with a Signed-off-by, it's less bad. But having it on a line apart from the others also doesn't hurt there. Now, care is taken that the line is appended as a separate paragraph: Message of cherry-picked commit (cherry picked from commit 871e293c9acbeaacce59dcd98fab6028f552f5be) Because some Git implementations (JGit) don't always terminate the commit message with a newline, this change also inserts a second newline if necessary. Signed-off-by: Robin Stocker ro...@nibor.org --- sequencer.c|4 t/t3511-cherry-pick-message.sh | 26 ++ 2 files changed, 30 insertions(+), 0 deletions(-) create mode 100755 t/t3511-cherry-pick-message.sh diff --git a/sequencer.c b/sequencer.c index bf078f2..41852e6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -484,6 +484,10 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } if (opts-record_origin) { + /* Some implementations don't terminate message with final \n, so add it */ + if (msg.message[strlen(msg.message)-1] != '\n') + strbuf_addch(msgbuf, '\n'); + strbuf_addch(msgbuf, '\n'); strbuf_addstr(msgbuf, (cherry picked from commit ); strbuf_addstr(msgbuf, sha1_to_hex(commit-object.sha1)); strbuf_addstr(msgbuf, )\n); diff --git a/t/t3511-cherry-pick-message.sh b/t/t3511-cherry-pick-message.sh new file mode 100755 index 000..6aba8b2 --- /dev/null +++ b/t/t3511-cherry-pick-message.sh @@ -0,0 +1,26 @@ +#!/bin/sh + +test_description='Test cherry-pick commit message with -x' + +. ./test-lib.sh + +test_expect_success setup ' + git config advice.detachedhead false + test_commit initial foo a + test_commit base foo b + git checkout initial +' + +test_expect_success 'cherry-pick -x appends message on separate line' ' + git cherry-pick -x base + cat expect -\EOF + base + + (cherry picked from commit 462a97d89aa55720c97984a3ce09665989193981) + + EOF + git log --format=%B -n 1 actual + test_cmp expect actual +' + +test_done -- 1.7.7.6 -- To unsubscribe from this list: send the line 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 9/9] Add git-check-ignores
Nguyen Thai Ngoc Duy pclo...@gmail.com writes: +static void output_exclude(const char *path, struct exclude *exclude) +{ + char *type = exclude-to_exclude ? excluded : included; + char *bang = exclude-to_exclude ? : !; + char *dir = (exclude-flags EXC_FLAG_MUSTBEDIR) ? / : ; + printf(_(%s: %s %s%s%s ), path, type, bang, exclude-pattern, dir); These English words excluded and included make the translator me want to translate them. But they could be the markers for scripts, so they may not be translated. How about using non alphanumeric letters instead? I agree they should not be translated, but it is a disease to think unintelligible mnemonic is a better input format for scripts than the spelled out words. excluded/included pair is just fine. -- To unsubscribe from this list: send the line 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: Feature request: short cuts for git add /diff etc
Коньков Евгений k...@reg.ru writes: # git status # On branch yandex_mail_new_api # Your branch is ahead of 'origin/yandex_mail_new_api' by 2 commits. # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified(1): a1.txt # modified(2): a2.txt # # Untracked files: # (use git add file... to include in what will be committed) # # (3)new.txt sot this allow: git diff 1 same asgit diff a1.txt git diff 2 same asgit diff a2.txt git add 1same asgit add a1.txt in case there are may be files with such names I may appply option -s,for example git add [ -s | --stage ] 1 . This will force to add file from list of 'git status' and do not use '1' as file name. git add 3same as git add new.txt This very handy and will keep developer from useless typing =) When I know I want to add a1.txt, I can say git add a1.txt, but with your version of Git, I now have to say git status to first find out what file number a1.txt corresponds to so that I can say git add 1? Worse yet, after saying git diff 1 to check the progress of a1.txt, which may be long and scroll the output from git status off the top of my screen, I need to run git status again to see what file number a2.txt corresponds to? That does not make any sense even as a typesaver. Besides, what if I have a file whose name is 1 that I would want to add? You may be looking for either git gui, or git add -i (if you do not like a windowed environment), both of which would let you choose pathnames with fewer keystrokes. -- To unsubscribe from this list: send the line 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 0/7] Fix some bugs in abspath.c
mhag...@alum.mit.edu writes: From: Michael Haggerty mhag...@alum.mit.edu I really just wanted to tidy up filter_refs(), but I've been sucked into a cascade of recursive yak shaving. This is my first attempt to pop the yak stack. Thanks. Please note that both absolute_path() and real_path() used to return the current directory followed by a slash. I believe that this was a bug, and that it is more appropriate for both functions to reject the empty string. The evidence is as follows: * If this were intended behavior, presumably the return value would *not* have a trailing slash. That is weak. The only thing you can infer from that observation is that the presense or absense of trailing '/' would not make any difference to the caller who wanted a path to the cwd (and is more convenient if the call is made so that a path relative to the cwd is tucked after it). * I couldn't find any callers that appeared to depend on the old behavior. That is a very good argument (especially if the audit were thorough). I would be tempted to say that we should die() on for now, cook the result outside master for a few weeks while auditing the callchains, and see if any of them complains. -- To unsubscribe from this list: send the line 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 7/7] t0000: verify that real_path() removes extra slashes
mhag...@alum.mit.edu writes: From: Michael Haggerty mhag...@alum.mit.edu These tests already pass, but make sure they don't break in the future. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- It would be great if somebody would check whether these tests pass on Windows, and if not, give me a tip about how to fix them. I think this (perhaps unwarranted) removal of the double leading slashes is the root cause of http://thread.gmane.org/gmane.comp.version-control.git/204469 (people involved in that thread Cc'ed). t/t-basic.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t-basic.sh b/t/t-basic.sh index d929578..3c75e97 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -468,6 +468,17 @@ test_expect_success 'real path works on absolute paths' ' test $d/$nopath = $(test-path-utils real_path $d/$nopath) ' +test_expect_success 'real path removes extra slashes' ' + nopath=hopefully-absent-path + test / = $(test-path-utils real_path ///) + test /$nopath = $(test-path-utils real_path ///$nopath) + # We need an existing top-level directory to use in the + # remaining tests. Use the top-level ancestor of $(pwd): + d=$(pwd -P | sed -e s|^\(/[^/]*\)/.*|\1|) + test $d = $(test-path-utils real_path //$d///) + test $d/$nopath = $(test-path-utils real_path //$d///$nopath) +' + test_expect_success SYMLINKS 'real path works on symlinks' ' mkdir first ln -s ../.git first/.git -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Support for setitimer() on platforms lacking it
Joachim Schmitz j...@schmitz-digital.de writes: Only with the observation of clone, I cannot tell if your timer is working. You can try repacking the test repository you created by your earlier git clone with git repack -a -d -f and see what happens. It does update the counter too. Yeah, that was not a very good way to diagnose it. You see the progress from pack-objects (which is the underlying machinery git repack uses) only because it knows how many objects it is going to pack, and it updates the progress meter for every per-cent progress it makes, without any help from the timer interrupt. -- To unsubscribe from this list: send the line 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] cherry-pick: Append -x line on separate paragraph
Robin Stocker ro...@nibor.org writes: if (opts-record_origin) { + /* Some implementations don't terminate message with final \n, so add it */ + if (msg.message[strlen(msg.message)-1] != '\n') + strbuf_addch(msgbuf, '\n'); I can agree that this is a good change. + strbuf_addch(msgbuf, '\n'); But this is somewhat dubious. Even if what we are adding is merely an extra LF, that changes the mechanically generated output format and can break existing hooks that read from these generated commit log template. Is there a reason better than having an empty line there look better to _me_ to justify this change? strbuf_addstr(msgbuf, (cherry picked from commit ); strbuf_addstr(msgbuf, sha1_to_hex(commit-object.sha1)); strbuf_addstr(msgbuf, )\n); Having said that, I've seen proposals to update this message to format more like the other trailers, so that we would see this: The title of the original commit The log message taken from the original commit comes here. Signed-off-by: First person who signed off the original Signed-off-by: Another person who signed off the original Cherry-picked-from: a9bbc121ea850e49d52ba3cb5a6b7f8077d195d2 in the editor, to allow you to add your own Sign-off at the end to make it look like this: The title of the original commit The log message taken from the original commit comes here. Signed-off-by: First person who signed off the original Signed-off-by: Another person who signed off the original Cherry-picked-from: a9bbc121ea850e49d52ba3cb5a6b7f8077d195d2 Signed-off-by: Me who did the cherry-pick I think that might be a worthwhile thing to do perhaps as an optional behaviour (e.g. perhaps triggered with a new option --trailer, or with the same -x but only when cherry-pick.origin = trailer configuration is set, or something). At that point, the output will look vastly different to existing hooks and those who care how this field looks like are forced to be updated, but as long as it is an opt-in feature, it may be worth 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 1/2] Support for setitimer() on platforms lacking it
Junio C Hamano gits...@pobox.com writes: Joachim Schmitz j...@schmitz-digital.de writes: Only with the observation of clone, I cannot tell if your timer is working. You can try repacking the test repository you created by your earlier git clone with git repack -a -d -f and see what happens. It does update the counter too. Yeah, that was not a very good way to diagnose it. You see the progress from pack-objects (which is the underlying machinery git repack uses) only because it knows how many objects it is going to pack, and it updates the progress meter for every per-cent progress it makes, without any help from the timer interrupt. I think the Counting objects: $number phase is purely driven by the timer, as there is no way to say we are done X per-cent so far. Doesn't your repack show Counting objects: with a number once, pause forever and then show Counting objects: $number, done.? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Support for setitimer() on platforms lacking it
Am 04.09.2012 19:23, schrieb Joachim Schmitz: From: Junio C Hamano [mailto:gits...@pobox.com] Only with the observation of clone, I cannot tell if your timer is working. You can try repacking the test repository you created by your earlier git clone with git repack -a -d -f and see what happens. It does update the counter too. Last time I looked at the progress code, it updated the progress text also every time the percent value changed. If you have a large count or large objects such that it takes more than a second to increase the percentage, than you don't get a smooth progress. Nor do you for phases that do not have a percentage, like the counting objects phase of pack-objects. -- Hannes -- To unsubscribe from this list: send the line 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 0/7] Fix some bugs in abspath.c
Junio C Hamano gits...@pobox.com writes: mhag...@alum.mit.edu writes: Please note that both absolute_path() and real_path() used to return the current directory followed by a slash. I believe that this was a bug, and that it is more appropriate for both functions to reject the empty string. The evidence is as follows: * If this were intended behavior, presumably the return value would *not* have a trailing slash. That is weak. The only thing you can infer from that observation is that the presense or absense of trailing '/' would not make any difference to the caller who wanted a path to the cwd (and is more convenient if the call is made so that a path relative to the cwd is tucked after it). I still wonder why you want to reject as an input, as it could be argued that it is a valid way to say the current directory. What does realpath(3) do? ... goes and looks ... The realpath(3) wants an existing path, and naturally gives NOENT, so we cannot really draw parallel from it. Ok, let's do this again. $ ./test-path-utils absolute_path /srv/git/git.git/ $ ./test-path-utils absolute_path foo /srv/git/git.git/foo so a caller has to be prepared to see the returned path sometimes terminated with slash and sometimes without, to form an absolute path to the file bar in the directory it gives to these functions (i.e. /srv/git/git.git/bar vs /srv/git/git.git/foo/bar). That is a reasonably good sign that either nobody calls these functions with or existing callers are buggy and would not handle that case well and need to be fixed anyway. So I am fine with die() in your patch. Thanks. * I couldn't find any callers that appeared to depend on the old behavior. That is a very good argument (especially if the audit were thorough). I would be tempted to say that we should die() on for now, cook the result outside master for a few weeks while auditing the callchains, and see if any of them complains. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Thunderbird: fix appp.sh format problems
Il 04/09/2012 17:49, Junio C Hamano ha scritto: Marco Stornelli marco.storne...@gmail.com writes: 2012/9/4 Junio C Hamano gits...@pobox.com: I would expect, at least when you are responding to an existing message, some of them are filled already (and if so, I think appp.sh wants to know exactly how, for example, has RFC2047 quoting already applied, or are we supposed to write in UTF-8 and let Thunderbird massage the contents when we give the file back to it?), and also there would appear In-Reply-To: field already filled (possibly there may be References: as well). Message reply is out of scope of my patch. The goal here is send a patch, so the execution flow is to open a new message, clik on external editor (configured properly), select patch file and send. It was the scope of the old script and it is the scope of my patch. I certainly can understand that you updated the script for that use case and that use case only, but given that the original tries very hard to preserve: - what was in $HEADERS (by only replacing Subject); - the recipients CC'ed in $HEADERS (by grabbing them into $CCS); and - the body of the message in $BODY (i.e. what came after $SEP), I find it hard to believe that it was meant to work on a freshly created empty message and nothing else. If people were depending on the recipients listed on Cc that are taken from $1 to be preserved, your patch will introduce a regression for them, no? I think all the process is different. Current script rely on the user to fill Cc: and To: in message composition window, it does a union of what found in commit message as signed-off-by (adding own address in cc?!) and Cc (usually not filled in the commit message and we should even count acked-by, tested-by and so on). My vision of things: the user can create a patch *already* with all information using --to and --cc. If you want to add optional addresses, you can of course add them in the composition window. In addition, with this approach is really easy to use external script as get_maintainer.pl of linux kernel, load the patch and send, really easy. So I don't think it's a regression, it's only a change in the work flow. Marco -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] misc cleanups to path functions
Hi Junio, These patches are from another unfinished branch. However, this time I can't remember why it's unfinished! ;-) ATB, Ramsay Jones Ramsay Jones (5): path.c: Remove the 'git_' prefix from a file scope function path.c: Don't discard the return value of vsnpath() path.c: Use vsnpath() in the implementation of git_path() Call git_pathdup() rather than xstrdup(git_path(...)) Call mkpathdup() rather than xstrdup(mkpath(...)) bisect.c | 2 +- builtin/add.c | 3 ++- builtin/branch.c | 2 +- builtin/clone.c | 4 ++-- builtin/prune.c | 2 +- merge-recursive.c | 13 +++-- path.c| 28 ++-- 7 files changed, 24 insertions(+), 30 deletions(-) -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] path.c: Remove the 'git_' prefix from a file scope function
In particular, the git_vsnpath() function, despite the 'git_' prefix suggesting otherwise, is (correctly) declared with file scope. Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- path.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/path.c b/path.c index 66acd24..9eb5333 100644 --- a/path.c +++ b/path.c @@ -48,7 +48,7 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...) return cleanup_path(buf); } -static char *git_vsnpath(char *buf, size_t n, const char *fmt, va_list args) +static char *vsnpath(char *buf, size_t n, const char *fmt, va_list args) { const char *git_dir = get_git_dir(); size_t len; @@ -72,7 +72,7 @@ char *git_snpath(char *buf, size_t n, const char *fmt, ...) { va_list args; va_start(args, fmt); - (void)git_vsnpath(buf, n, fmt, args); + (void)vsnpath(buf, n, fmt, args); va_end(args); return buf; } @@ -82,7 +82,7 @@ char *git_pathdup(const char *fmt, ...) char path[PATH_MAX]; va_list args; va_start(args, fmt); - (void)git_vsnpath(path, sizeof(path), fmt, args); + (void)vsnpath(path, sizeof(path), fmt, args); va_end(args); return xstrdup(path); } -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] path.c: Use vsnpath() in the implementation of git_path()
The current implementation of git_path() is essentially the same as that of vsnpath(), with two minor differences. First, git_path() currently insists that the git directory path is no longer than PATH_MAX-100 characters in length. However, vsnpath() does not attempt this arbitrary 100 character reservation for the remaining path components. Second, vsnpath() uses the is_dir_sep() macro, rather than comparing directly to '/', to determine if the git_dir path component ends with a path separator. In order to benefit from the above improvements, along with increased compatability with git_snpath() and git_pathdup(), we reimplement the git_path() function using vsnpath(). Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- path.c | 15 +++ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/path.c b/path.c index 741ae77..cbbdf7d 100644 --- a/path.c +++ b/path.c @@ -119,23 +119,14 @@ char *mkpath(const char *fmt, ...) char *git_path(const char *fmt, ...) { - const char *git_dir = get_git_dir(); char *pathname = get_pathname(); va_list args; - unsigned len; + char *ret; - len = strlen(git_dir); - if (len PATH_MAX-100) - return bad_path; - memcpy(pathname, git_dir, len); - if (len git_dir[len-1] != '/') - pathname[len++] = '/'; va_start(args, fmt); - len += vsnprintf(pathname + len, PATH_MAX - len, fmt, args); + ret = vsnpath(pathname, PATH_MAX, fmt, args); va_end(args); - if (len = PATH_MAX) - return bad_path; - return cleanup_path(pathname); + return ret; } void home_config_paths(char **global, char **xdg, char *file) -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] path.c: Don't discard the return value of vsnpath()
The git_snpath() and git_pathdup() functions both use the (static) function vsnpath() in their implementation. Also, they both discard the return value of vsnpath(), which has the effect of ignoring the side effect of calling cleanup_path() in the non-error return path. In order to ensure that the required cleanup happens, we use the pointer returned by vsnpath(), rather than the buffer passed into vsnpath(), to derive the return value from git_snpath() and git_pathdup(). Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- path.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/path.c b/path.c index 9eb5333..741ae77 100644 --- a/path.c +++ b/path.c @@ -70,21 +70,22 @@ bad: char *git_snpath(char *buf, size_t n, const char *fmt, ...) { + char *ret; va_list args; va_start(args, fmt); - (void)vsnpath(buf, n, fmt, args); + ret = vsnpath(buf, n, fmt, args); va_end(args); - return buf; + return ret; } char *git_pathdup(const char *fmt, ...) { - char path[PATH_MAX]; + char path[PATH_MAX], *ret; va_list args; va_start(args, fmt); - (void)vsnpath(path, sizeof(path), fmt, args); + ret = vsnpath(path, sizeof(path), fmt, args); va_end(args); - return xstrdup(path); + return xstrdup(ret); } char *mkpathdup(const char *fmt, ...) -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] Call mkpathdup() rather than xstrdup(mkpath(...))
In addition to updating the xstrdup(mkpath(...)) call sites with mkpathdup(), we also fix a memory leak (in merge_3way()) caused by neglecting to free the memory allocated to the 'base_name' variable. Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- builtin/branch.c | 2 +- builtin/clone.c | 4 ++-- builtin/prune.c | 2 +- merge-recursive.c | 13 +++-- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 0e060f2..bdf8495 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -196,7 +196,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, free(name); - name = xstrdup(mkpath(fmt, bname.buf)); + name = mkpathdup(fmt, bname.buf); if (read_ref(name, sha1)) { error(remote_branch ? _(remote branch '%s' not found.) diff --git a/builtin/clone.c b/builtin/clone.c index e314b0b..c819757 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -236,7 +236,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data) /* Beware: real_path() and mkpath() return static buffer */ ref_git = xstrdup(real_path(item-string)); if (is_directory(mkpath(%s/.git/objects, ref_git))) { - char *ref_git_git = xstrdup(mkpath(%s/.git, ref_git)); + char *ref_git_git = mkpathdup(%s/.git, ref_git); free(ref_git); ref_git = ref_git_git; } else if (!is_directory(mkpath(%s/objects, ref_git))) @@ -700,7 +700,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) git_dir = xstrdup(dir); else { work_tree = dir; - git_dir = xstrdup(mkpath(%s/.git, dir)); + git_dir = mkpathdup(%s/.git, dir); } if (!option_bare) { diff --git a/builtin/prune.c b/builtin/prune.c index 6cb9944..685f8a0 100644 --- a/builtin/prune.c +++ b/builtin/prune.c @@ -169,7 +169,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix) prune_packed_objects(show_only); remove_temporary_files(get_object_directory()); - s = xstrdup(mkpath(%s/pack, get_object_directory())); + s = mkpathdup(%s/pack, get_object_directory()); remove_temporary_files(s); free(s); return 0; diff --git a/merge-recursive.c b/merge-recursive.c index 7866ca1..d882060 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -844,14 +844,14 @@ static int merge_3way(struct merge_options *o, if (strcmp(a-path, b-path) || (o-ancestor != NULL strcmp(a-path, one-path) != 0)) { base_name = o-ancestor == NULL ? NULL : - xstrdup(mkpath(%s:%s, o-ancestor, one-path)); - name1 = xstrdup(mkpath(%s:%s, branch1, a-path)); - name2 = xstrdup(mkpath(%s:%s, branch2, b-path)); + mkpathdup(%s:%s, o-ancestor, one-path); + name1 = mkpathdup(%s:%s, branch1, a-path); + name2 = mkpathdup(%s:%s, branch2, b-path); } else { base_name = o-ancestor == NULL ? NULL : - xstrdup(mkpath(%s, o-ancestor)); - name1 = xstrdup(mkpath(%s, branch1)); - name2 = xstrdup(mkpath(%s, branch2)); + mkpathdup(%s, o-ancestor); + name1 = mkpathdup(%s, branch1); + name2 = mkpathdup(%s, branch2); } read_mmblob(orig, one-sha1); @@ -861,6 +861,7 @@ static int merge_3way(struct merge_options *o, merge_status = ll_merge(result_buf, a-path, orig, base_name, src1, name1, src2, name2, ll_opts); + free(base_name); free(name1); free(name2); free(orig.ptr); -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] v5 index branch breakage on cygwin
Hi Thomas, The current pu branch is *very* broken on cygwin; practically every test fails. A git-bisect session points to: $ git bisect good a4f6670277d5dc0b082139efe162100c6d7a91b8 is the first bad commit commit a4f6670277d5dc0b082139efe162100c6d7a91b8 Author: Thomas Gummerer t.gumme...@gmail.com Date: Thu Aug 16 11:58:38 2012 +0200 read-cache.c: Re-read index if index file changed Add the possibility of re-reading the index file, if it changed while reading. The index file might change during the read, causing outdated information to be displayed. We check if the index file changed by using its stat data as heuristic. Signed-off-by: Thomas Gummerer t.gumme...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com :100644 100644 6a8b4b1e6859b7a8df2624e80b9da47df6cd1648 cdd8480cc7c3ab373dab1ca9 4d77a3154f7d0fbd M read-cache.c $ Since this appears to be a cygwin specific problem, I wasn't too surprised to see that the code added in this commit involves the stat functions. (Yes, this is another example of problems caused by the schizophrenic stat() functions). A little debugging shows that the index_changed() function was always returning true, so that the loop was executed 50 times and git then die()s. We note that fstat() is a cygwin native function, whereas lstat() is executing the WIN32 optimised function. The problematic 'struct stat' fields are st_uid, st_gid and st_ino, which fstat() fills with appropriate values (st_uid=1005, st_gid=513, st_ino=non-zero value), whereas the the WIN32 version of lstat() always returns zero in these fields. I have no wish to spread the insanity, but nonetheless I implemented a WIN32 optimised version of fstat(). This was a great improvement, but there were still a few failing tests. git-stash, in particular, seemed to be problematic. A git-bisect session pointed at exactly the same commit as above! *ahem* A spot of debugging shows that index_changed() was always returning true ... Here, the new fstat() function was returning zero in those fields, whereas the lstat() function was returning appropriate values ... The difference here is caused by git-stash calling git with an GIT_INDEX_FILE set to an _absolute_ path, such as: /home/ramsay/git/t/trash directory.t3903-stash/.git/index.stash.2440 This has the effect of disabling the optimisation and calling the cygwin native lstat() function. *ho hum* So, the only sensible fix is to not include those fields in the index_changed predicate on cygwin; which is what the first patch does. The testsuite now runs just fine (well as fine as before, anyway!). The other two patches don't affect the correctness of the code, so please feel free to ignore them if you like. ATB, Ramsay Jones Ramsay Allan Jones (3): read-cache.c: Fix index reading breakage on cygwin read-cache.c: Pass 'struct stat' parameters by reference read-cache.c: Simplify do loop conditional expression read-cache.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] read-cache.c: Fix index reading breakage on cygwin
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- read-cache.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/read-cache.c b/read-cache.c index b69dd05..211b971 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1204,9 +1204,11 @@ static int index_changed(struct stat st_old, struct stat st_new) int changed = 0; if (st_old.st_mtime != st_new.st_mtime || +#if !defined(__CYGWIN__) st_old.st_uid != st_new.st_uid || st_old.st_gid != st_new.st_gid || st_old.st_ino != st_new.st_ino || +#endif st_old.st_size != st_new.st_size) changed = 1; #ifdef USE_NSEC -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] read-cache.c: Pass 'struct stat' parameters by reference
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- Hi Thomas, Passing the 'struct stat' parameters by value, while not *wrong*, looks somewhat odd to my eyes. As I said before, feel free to ignore this one. ATB, Ramsay Jones read-cache.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/read-cache.c b/read-cache.c index 211b971..36f0877 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1199,25 +1199,25 @@ int read_index(struct index_state *istate) return read_index_from(istate, get_index_file()); } -static int index_changed(struct stat st_old, struct stat st_new) +static int index_changed(struct stat *st_old, struct stat *st_new) { int changed = 0; - if (st_old.st_mtime != st_new.st_mtime || + if (st_old-st_mtime != st_new-st_mtime || #if !defined(__CYGWIN__) - st_old.st_uid != st_new.st_uid || - st_old.st_gid != st_new.st_gid || - st_old.st_ino != st_new.st_ino || + st_old-st_uid != st_new-st_uid || + st_old-st_gid != st_new-st_gid || + st_old-st_ino != st_new-st_ino || #endif - st_old.st_size != st_new.st_size) + st_old-st_size != st_new-st_size) changed = 1; #ifdef USE_NSEC - if (ST_MTIME_NSEC(st_old) != ST_MTIME_NSEC(st_new)) + if (ST_MTIME_NSEC(*st_old) != ST_MTIME_NSEC(*st_new)) changed = 1; #endif #ifdef USE_STDEV - if (st_old.st_dev != st_new.st_dev) + if (st_old-st_dev != st_new-st_dev) changed = 1; #endif @@ -1273,12 +1273,12 @@ int read_index_from(struct index_state *istate, const char *path) munmap(mmap, mmap_size); - if (!index_changed(st_old, st_new) !err) + if (!index_changed(st_old, st_new) !err) return istate-cache_nr; usleep(10*1000); i++; - } while ((err || index_changed(st_old, st_new)) i 50); + } while ((err || index_changed(st_old, st_new)) i 50); munmap(mmap, mmap_size); die(index file corrupt); -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] read-cache.c: Simplify do loop conditional expression
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- Hi Thomas, I note that index_changed(), which has no side effects, is called with unchanged parameters twice each time around the loop. I was about to suggest saving the result of a single call and using it in both places ... However, on first sight, it looks like the expression in the while condition will always be true (otherwise you would not reach that point in the code - you would already have returned), so we can simply remove that part of the expression (however, I didn't look too closely, so ...) :-D ATB, Ramsay Jones read-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/read-cache.c b/read-cache.c index 36f0877..5176d7a 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1278,7 +1278,7 @@ int read_index_from(struct index_state *istate, const char *path) usleep(10*1000); i++; - } while ((err || index_changed(st_old, st_new)) i 50); + } while (i 50); munmap(mmap, mmap_size); die(index file corrupt); -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Thunderbird: fix appp.sh format problems
Marco Stornelli marco.storne...@gmail.com writes: kernel, load the patch and send, really easy. So I don't think it's a regression, it's only a change in the work flow. Any change that forces the user to change an established work flow supporteed by the existing tool we gave them _is_ a regression, even if the person who forces that change believes the new way is better. -- To unsubscribe from this list: send the line 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 0/5] misc cleanups to path functions
Ramsay Jones ram...@ramsay1.demon.co.uk writes: These patches are from another unfinished branch. However, this time I can't remember why it's unfinished! ;-) That scares me. Perhaps you found some hard-to-debug problem but didn't have time to follow it through, or something? ;-) Thanks. ATB, Ramsay Jones Ramsay Jones (5): path.c: Remove the 'git_' prefix from a file scope function path.c: Don't discard the return value of vsnpath() path.c: Use vsnpath() in the implementation of git_path() Call git_pathdup() rather than xstrdup(git_path(...)) Call mkpathdup() rather than xstrdup(mkpath(...)) bisect.c | 2 +- builtin/add.c | 3 ++- builtin/branch.c | 2 +- builtin/clone.c | 4 ++-- builtin/prune.c | 2 +- merge-recursive.c | 13 +++-- path.c| 28 ++-- 7 files changed, 24 insertions(+), 30 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: checkout extra files
Hi, figuring out what the behavior is by guessing how a command is implemented and what are its interactions with the shell is a bit hard for the user: s/he should instead get it from the documentation. I tried to figure it out from the examples I have done, and as you see, I did not get it quite right. The issue here is that the paths must denote filenames that are present in the index or tree-ish, so, wildcards are misleading since they would instead be interpreted with respect to the working directory. A possible way to make this clear is to warn the user to quote paths that contain wildcards. Something like, e.g.: Note that paths that contain wildcards must be quoted in order to denote files that belong to the index or tree-ish. Otherwise, they are interpreted by the shell with respect to the current directory, with a result that may depend on the shell. -Angelo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] i18n.pathencoding
On 04.09.12 19:19, Junio C Hamano wrote: Nguyen Thai Ngoc Duy pclo...@gmail.com writes: On Sat, Sep 1, 2012 at 1:11 PM, Torsten Bögershausen tbo...@web.de wrote: diff --git a/parse-options.c b/parse-options.c index c1c66bd..5840c18 100644 --- a/parse-options.c +++ b/parse-options.c @@ -476,7 +476,7 @@ int parse_options(int argc, const char **argv, const char *prefix, usage_with_options(usagestr, options); } - precompose_argv(argc, argv); + reencode_argv(argc, argv); return parse_options_end(ctx); } If you have to re-encode command line arguments, what about paths coming --stdin or a file? That problem is inherited from the MacOS precompose topic this one builds on. Not that it is unimportant to fix, though. Thanks for the comments, (actually the precompose feature started as fully reencode, and was downsized to only do the readdir() and argv[] conversion) This leads to 2 questions: a) Do we want the reencode feature in git, so that I continue to work on it? From a performance point of view that would probably OK: The git status on a linux tree was slightly slower on my PC when measured with time. From the user experience there was not a difference. b) If yes, I have to admit that I don't use paths from --stdin or file so much, except git am or git format-patch Which commands are affected? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] i18n.pathencoding
Torsten Bögershausen tbo...@web.de writes: This leads to 2 questions: a) Do we want the reencode feature in git, so that I continue to work on it? From a performance point of view that would probably OK: The git status on a linux tree was slightly slower on my PC when measured with time. From the user experience there was not a difference. I am not fundamentally opposed to such a change, as long as the change does not affect performance at all when the feature is not used, and the resulting code does not become too ugly. Use of the reencode feature may have to make things slow, but you have to spend some cycle to do what the feature has to do anyway, so... b) If yes, I have to admit that I don't use paths from --stdin or file so much, except git am or git format-patch Which commands are affected? $ git grep -l -e '--stdin' Documentation/git* may be a good starting point. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git archive --format zip utf-8 issues
Am 31.08.2012 00:26, schrieb Jeff King: Ping on this stalled discussion. Sorry, I got distracted by other stuff again. I did some experiments, though, and here's a preliminary result. It seems like there are two separate issues here: 1. Knowing the encoding of pathnames in the repository. 2. Setting the right flags in zip output. A full solution would handle both parts, but let's ignore (1) for a moment, and assume we have utf-8 (or can massage into utf-8 from an encoding specified by the user). Yes, good thinking. Re-encoding may be beneficial for tar files as well, but we can ignore that point for the moment. It seems like just setting the magic utf-8 flag would be the only thing we need to do, according to the standard. But according to discussions referenced elsewhere in this thread, that flag was invented only in 2007, so we may be dealing with older implementations (I have no idea how common they would be; that may be the problem with Windows 7's zip you are seeing). We could re-encode to cp437, which the standard specifies, but apparently some implementations do not respect that (and use a local code page instead). And it cannot represent all utf-8 characters, anyway. Yes, we could do that, plus adding an extra field with a UTF-8 version of the path. That's the legacy method invented by Info-ZIP. They switched to using the new flag on Linux at least, though. It sounds like 7-zip has figured out a more portable solution. Can you show us a sample of 7-zip's output with utf-8 characters to compare to what git generates? I wonder if it is using a combination of methods. I'm not so sure they produce more portable files. I created an archive with files named jaя.txt, smørrebrød.txt, süd.txt and €uro.txt with 7-Zip on Windows 7 and while unzip on Ubuntu 12.04 managed to recreate the cyrillic character and the Euro symbol, it mangled the slashed o and the umlaut. With the following patch I could create archives with git on Linux and msysgit and extract them flawlessly on Windows with 7-Zip and with Info-ZIP unzip on Linux, but not with unzip on Windows, where it mangled all non-ASCII characters. This gets confusing; it would help to have a compatibility matrix for all intersting extractors and character classes -- for each proposed solution or archiver we'd like to imitate. But now for the patch, which is a bit confusing as well. I'm curious to hear about results for more platforms, extractors and character classes. Based on that we can see if we need to generate the extra fields instead of relying on the new flag. -- 8 -- Subject: [PATCH] archive-zip: support UTF-8 paths Set general purpose flag 11 if we encounter a path that contains non-ASCII characters. We assume that all paths are given as UTF-8; no conversion is done. The flag seems to be ignored by unzip unless we also mark the archive entry as coming from a Unix system. This is done by setting the field creator_version (version made by in the standard[1]) to 0x03NN. The NN part represents the version of the standard supported by us, and this patch sets it to 3f (for version 6.3) for Unix paths. We keep creator_version set to 0 (FAT filesystem, standard version 0) in the non-special cases, as before. But when we declare a file to have a Unix path, then we have to set the file mode as well, or unzip will extract the files with the permission set , i.e. inaccessible by all. [1] http://www.pkware.com/documents/casestudies/APPNOTE.TXT Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx --- archive-zip.c | 27 +-- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/archive-zip.c b/archive-zip.c index f5af81f..928da1d 100644 --- a/archive-zip.c +++ b/archive-zip.c @@ -4,6 +4,8 @@ #include cache.h #include archive.h #include streaming.h +#include commit.h +#include utf8.h static int zip_date; static int zip_time; @@ -16,7 +18,8 @@ static unsigned int zip_dir_offset; static unsigned int zip_dir_entries; #define ZIP_DIRECTORY_MIN_SIZE (1024 * 1024) -#define ZIP_STREAM (8) +#define ZIP_STREAM (1 3) +#define ZIP_UTF8 (1 11) struct zip_local_header { unsigned char magic[4]; @@ -173,7 +176,8 @@ static int write_zip_entry(struct archiver_args *args, { struct zip_local_header header; struct zip_dir_header dirent; - unsigned long attr2; + unsigned int creator_version = 0; + unsigned long attr2 = 0; unsigned long compressed_size; unsigned long crc; unsigned long direntsize; @@ -187,6 +191,13 @@ static int write_zip_entry(struct archiver_args *args, crc = crc32(0, NULL, 0); + if (has_non_ascii(path)) { + if (is_utf8(path)) + flags |= ZIP_UTF8; + else + warning(Path is not valid UTF-8: %s, path); + } + if (pathlen 0x) { return error(path too long (%d
Re: checkout extra files
Angelo Borsotti angelo.borso...@gmail.com writes: The issue here is that the paths must denote filenames that are present in the index or tree-ish, so, wildcards are misleading since they would instead be interpreted with respect to the working directory. When you are talking to a shell (and you almost never directly talk to Git), wildcards are always interpreted with respect to the working directory by the shell. And that is not specific to Git. A possible way to make this clear is to warn the user to quote paths that contain wildcards. Something like, e.g.: Note that paths that contain wildcards must be quoted in order to denote files that belong to the index or tree-ish. Otherwise, they are interpreted by the shell with respect to the current directory, with a result that may depend on the shell. Perhaps, if you drop , with a result... from that sentence. Even though that description is a bit too much on the side of shell primer than git documentation for my taste, I could see it may help some people, so I wouldn't reject such a phrasing out of hand. Let's see what others feel. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] path.c: Don't discard the return value of vsnpath()
Ramsay Jones ram...@ramsay1.demon.co.uk writes: The git_snpath() and git_pathdup() functions both use the (static) function vsnpath() in their implementation. Also, they both discard the return value of vsnpath(), which has the effect of ignoring the side effect of calling cleanup_path() in the non-error return path. In order to ensure that the required cleanup happens, we use the pointer returned by vsnpath(), rather than the buffer passed into vsnpath(), to derive the return value from git_snpath() and git_pathdup(). Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- path.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/path.c b/path.c index 9eb5333..741ae77 100644 --- a/path.c +++ b/path.c @@ -70,21 +70,22 @@ bad: char *git_snpath(char *buf, size_t n, const char *fmt, ...) { + char *ret; va_list args; va_start(args, fmt); - (void)vsnpath(buf, n, fmt, args); + ret = vsnpath(buf, n, fmt, args); va_end(args); - return buf; + return ret; } char *git_pathdup(const char *fmt, ...) { - char path[PATH_MAX]; + char path[PATH_MAX], *ret; va_list args; va_start(args, fmt); - (void)vsnpath(path, sizeof(path), fmt, args); + ret = vsnpath(path, sizeof(path), fmt, args); va_end(args); - return xstrdup(path); + return xstrdup(ret); } char *mkpathdup(const char *fmt, ...) cleanup_path() is a quick-and-dirty hack that only deals with leading ./// (e.g. foo//bar is not reduced to foo/bar), and the callers that allocate 4 bytes for buf to hold foo may not be able to fit it if for some reason there are some bytes that must be cleaned, e.g. .///foo. The worst part of its use is that buf and ret could be different, which means you cannot safely do: char *buf = xmalloc(...); buf = git_snpath(buf, n, %s, ...); ... use buf ... free(buf); but instead have to do something like: char *path, *buf = xmalloc(...); path = git_snpath(buf, n, %s, ...); ... use path ... free(buf); And this series goes in a direction of making more callers aware of the twisted calling convention, which smells really bad. As long as the callers are contained within this file, it is not making things _too_ bad, so... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git archive --format zip utf-8 issues
René Scharfe rene.scha...@lsrfire.ath.cx writes: But now for the patch, which is a bit confusing as well. I'm curious to hear about results for more platforms, extractors and character classes. Based on that we can see if we need to generate the extra fields instead of relying on the new flag. Thanks for keeping the ball rolling. Subject: [PATCH] archive-zip: support UTF-8 paths Set general purpose flag 11 if we encounter a path that contains non-ASCII characters. We assume that all paths are given as UTF-8; no conversion is done. The flag seems to be ignored by unzip unless we also mark the archive entry as coming from a Unix system. This is done by setting the field creator_version (version made by in the standard[1]) to 0x03NN. The NN part represents the version of the standard supported by us, and this patch sets it to 3f (for version 6.3) for Unix paths. We keep creator_version set to 0 (FAT filesystem, standard version 0) in the non-special cases, as before. But when we declare a file to have a Unix path, then we have to set the file mode as well, or unzip will extract the files with the permission set , i.e. inaccessible by all. [1] http://www.pkware.com/documents/casestudies/APPNOTE.TXT Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx --- archive-zip.c | 27 +-- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/archive-zip.c b/archive-zip.c index f5af81f..928da1d 100644 --- a/archive-zip.c +++ b/archive-zip.c @@ -4,6 +4,8 @@ #include cache.h #include archive.h #include streaming.h +#include commit.h +#include utf8.h static int zip_date; static int zip_time; @@ -16,7 +18,8 @@ static unsigned int zip_dir_offset; static unsigned int zip_dir_entries; #define ZIP_DIRECTORY_MIN_SIZE (1024 * 1024) -#define ZIP_STREAM (8) +#define ZIP_STREAM (1 3) +#define ZIP_UTF8 (1 11) struct zip_local_header { unsigned char magic[4]; @@ -173,7 +176,8 @@ static int write_zip_entry(struct archiver_args *args, { struct zip_local_header header; struct zip_dir_header dirent; - unsigned long attr2; + unsigned int creator_version = 0; + unsigned long attr2 = 0; unsigned long compressed_size; unsigned long crc; unsigned long direntsize; @@ -187,6 +191,13 @@ static int write_zip_entry(struct archiver_args *args, crc = crc32(0, NULL, 0); + if (has_non_ascii(path)) { Do we want to treat \033 as ascii in this codepath? The function primarily is used by the log formatter to see if we need 8-bit CTE when writing out in the e-mail format. + if (is_utf8(path)) + flags |= ZIP_UTF8; + else + warning(Path is not valid UTF-8: %s, path); + } + if (pathlen 0x) { return error(path too long (%d chars, SHA1: %s): %s, (int)pathlen, sha1_to_hex(sha1), path); @@ -204,10 +215,15 @@ static int write_zip_entry(struct archiver_args *args, enum object_type type = sha1_object_info(sha1, size); method = 0; - attr2 = S_ISLNK(mode) ? ((mode | 0777) 16) : - (mode 0111) ? ((mode) 16) : 0; if (S_ISREG(mode) args-compression_level != 0 size 0) method = 8; + if (S_ISLNK(mode) || (mode 0111) || (flags ZIP_UTF8)) { + creator_version = 0x033f; + attr2 = mode; + if (S_ISLNK(mode)) + attr2 |= 0777; + attr2 = 16; + } compressed_size = size; if (S_ISREG(mode) type == OBJ_BLOB !args-convert @@ -254,8 +270,7 @@ static int write_zip_entry(struct archiver_args *args, } copy_le32(dirent.magic, 0x02014b50); - copy_le16(dirent.creator_version, - S_ISLNK(mode) || (S_ISREG(mode) (mode 0111)) ? 0x0317 : 0); + copy_le16(dirent.creator_version, creator_version); copy_le16(dirent.version, 10); copy_le16(dirent.flags, flags); copy_le16(dirent.compression_method, method); -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] Support for setitimer() on platforms lacking it
From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Tuesday, September 04, 2012 8:47 PM To: Joachim Schmitz Cc: git@vger.kernel.org; 'Johannes Sixt' Subject: Re: [PATCH 1/2] Support for setitimer() on platforms lacking it Junio C Hamano gits...@pobox.com writes: Joachim Schmitz j...@schmitz-digital.de writes: Only with the observation of clone, I cannot tell if your timer is working. You can try repacking the test repository you created by your earlier git clone with git repack -a -d -f and see what happens. It does update the counter too. Yeah, that was not a very good way to diagnose it. You see the progress from pack-objects (which is the underlying machinery git repack uses) only because it knows how many objects it is going to pack, and it updates the progress meter for every per-cent progress it makes, without any help from the timer interrupt. I think the Counting objects: $number phase is purely driven by the timer, as there is no way to say we are done X per-cent so far. Doesn't your repack show Counting objects: with a number once, pause forever and then show Counting objects: $number, done.? Yes, only once, when it is done $ ./git repack -a -d -f warning: no threads support, ignoring --threads Counting objects: 140302, done. Compressing objects: 1% (1385/138407) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3.5/6] name-rev --weight: trivial optimization
Junio C Hamano gits...@pobox.com writes: Instead of naming a rev after a tip that is topologically closest, use the tip that is the oldest one among those which contain the rev. The semantics name-rev --weight would give us is closer to what people expect from describe --contains. Note that this is fairly expensive to compute; a later change in the series will cache the weight value using notes-cache. Signed-off-by: Junio C Hamano gits...@pobox.com Here is a trivial optimization on top of the one from the other day. -- 8 -- It often happens that immediately after traversing from v1.2.3 to find out its weight, we are asked to see if another commit v1.2.0 is heavier than that. Because the commits that are reachable from v1.2.3 are painted with the SHOWN flag until the next rev-list traversal, we can notice that the new commit v1.2.0 is reachable from v1.2.3 without doing any traversal. We cannot learn exactly how much it weighs, but we can tell it must weigh less than v1.2.3, and that is all that the caller wants to know. In the kernel history, git name-rev --tags 0136db586c which started this topic needs 26k calls to tip_weight_cmp(). 13k of them are comparisons between tips based on the same ref and answered without computing any weight. Among the remaining 13k, 1.8k comparisons are answered with this trivial fast-forward optimization. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/name-rev.c | 37 + 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/builtin/name-rev.c b/builtin/name-rev.c index 7cdb758..809553b 100644 --- a/builtin/name-rev.c +++ b/builtin/name-rev.c @@ -32,6 +32,9 @@ struct rev_name { */ static int use_weight; +/* To optimize revision traversal */ +static struct commit *painted_commit; + /* * NEEDSWORK: the result of this computation must be cached to * a dedicated notes tree, keyed by the commit object name. @@ -47,15 +50,15 @@ static int compute_ref_weight(struct commit *commit) prepare_revision_walk(revs); while (get_revision(revs)) weight++; + painted_commit = commit; return weight; } -static int ref_weight(const char *refname, size_t reflen) +static struct commit *ref_commit(const char *refname, size_t reflen) { struct strbuf buf = STRBUF_INIT; unsigned char sha1[20]; struct commit *commit; - struct rev_name *name; strbuf_add(buf, refname, reflen); if (get_sha1(buf.buf, sha1)) @@ -65,9 +68,16 @@ static int ref_weight(const char *refname, size_t reflen) commit = lookup_commit_reference_gently(sha1, 0); if (!commit) die(Internal error: cannot look up commit '%s', buf.buf); + return commit; +} + +static int ref_weight(struct commit *commit, const char *refname, size_t reflen) +{ + struct rev_name *name; + name = commit-util; if (!name) - die(Internal error: a tip without name '%s', buf.buf); + die(Internal error: a tip without name '%.*s', (int) reflen, refname); if (!name-weight) name-weight = compute_ref_weight(commit); return name-weight; @@ -76,6 +86,7 @@ static int ref_weight(const char *refname, size_t reflen) static int tip_weight_cmp(const char *a, const char *b) { size_t reflen_a, reflen_b; + struct commit *commit_a, *commit_b; static const char traversal[] = ^~; /* @@ -89,7 +100,25 @@ static int tip_weight_cmp(const char *a, const char *b) if (reflen_a == reflen_b !memcmp(a, b, reflen_a)) return 0; - return ref_weight(a, reflen_a) - ref_weight(b, reflen_b); + commit_a = ref_commit(a, reflen_a); + commit_b = ref_commit(b, reflen_b); + + /* Have we painted either one of these recently? */ + if (commit_a == painted_commit + (commit_b-object.flags SHOWN)) { + /* +* We know b can be reached from a, so b must be older +* (lighter, as it has fewer commits behind it) than +* a. +*/ + return 1; + } else if (commit_b == painted_commit + (commit_a-object.flags SHOWN)) { + /* Likewise */ + return -1; + } + + return ref_weight(commit_a, a, reflen_a) - ref_weight(commit_b, b, reflen_b); } static long cutoff = LONG_MAX; -- 1.7.12.321.g60f00e5 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Support for setitimer() on platforms lacking it
Joachim Schmitz j...@schmitz-digital.de writes: From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Tuesday, September 04, 2012 8:47 PM To: Joachim Schmitz Cc: git@vger.kernel.org; 'Johannes Sixt' Subject: Re: [PATCH 1/2] Support for setitimer() on platforms lacking it Junio C Hamano gits...@pobox.com writes: Joachim Schmitz j...@schmitz-digital.de writes: Only with the observation of clone, I cannot tell if your timer is working. You can try repacking the test repository you created by your earlier git clone with git repack -a -d -f and see what happens. It does update the counter too. Yeah, that was not a very good way to diagnose it. You see the progress from pack-objects (which is the underlying machinery git repack uses) only because it knows how many objects it is going to pack, and it updates the progress meter for every per-cent progress it makes, without any help from the timer interrupt. I think the Counting objects: $number phase is purely driven by the timer, as there is no way to say we are done X per-cent so far. Doesn't your repack show Counting objects: with a number once, pause forever and then show Counting objects: $number, done.? Yes, only once, when it is done $ ./git repack -a -d -f warning: no threads support, ignoring --threads Counting objects: 140302, done. Compressing objects: 1% (1385/138407) So this strongly suggests that (1) your poor-man's is not a real substitute for recurring itimer, and (2) users could live with the progress.c code without any itimer firing. Perhaps a no-op macro would work equally well? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
New git.pot generated with 2 new l10n messages
Dear l10n team members, New git.pot is generated from v1.7.12-146-g16d26 in the master branch. l10n: Update git.pot (2 new, 4 removed messages) Generate po/git.pot from v1.7.12-146-g16d26, and there are 2 new, 4 removed l10n messages. * 2 new messages are added at lines: 4151, 4172 * 4 old messages are deleted from the previous version at lines: 350, 354, 2069, 4166 Signed-off-by: Jiang Xin worldhello@gmail.com It's time for a new round of translation. * Fetch new commits from git://github.com/git-l10n/git-po * Update your XX.po according to the new git.pot file. * Start your translation and review your commits inside your l10n team. * Send a pull request to git-l10n/git-po on GitHub. -- Jiang Xin -- To unsubscribe from this list: send the line 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: checkout extra files
From: Junio C Hamano gits...@pobox.com Sent: Tuesday, September 04, 2012 9:44 PM Angelo Borsotti angelo.borso...@gmail.com writes: The issue here is that the paths must denote filenames that are present in the index or tree-ish, so, wildcards are misleading since they would instead be interpreted with respect to the working directory. When you are talking to a shell (and you almost never directly talk to Git), wildcards are always interpreted with respect to the working directory by the shell. And that is not specific to Git. A possible way to make this clear is to warn the user to quote paths that contain wildcards. Something like, e.g.: Note that paths that contain wildcards must be quoted in order to denote files that belong to the index or tree-ish. Otherwise, they are interpreted by the shell with respect to the current directory, with a result that may depend on the shell. Perhaps, if you drop , with a result... from that sentence. Even though that description is a bit too much on the side of shell primer than git documentation for my taste, I could see it may help some people, so I wouldn't reject such a phrasing out of hand. Let's see what others feel. A comment about the need to quote wild cards would certainly be of advantage to many Windows users who won't have used a shell in that way before. Plus I suspect that a large fraction of basic unix/linux users will have never really considered the difference between shell expansion and git expansion in this case where there are two diifferent 'file systems', as demonstrated by the initial query. -- To unsubscribe from this list: send the line 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: Grafting Alternate History
Andreas Schwab sch...@linux-m68k.org writes: You can set up (the git mirror of) the public repository as a remote in the private repository and git cherry-pick the commits you want to copy over. Now why didn't I think of that!? :) Thanks for helping an old bumbler along. :) -Dave -- To unsubscribe from this list: send the line 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] cvsimport: strip question marks from tags
The ? character can be present in a CVS tag name, but git's bad_ref_char does not allow question marks in git tags. If git-cvsimport encounters a CVS tag with a question mark, it will error and refuse to continue the import beyond that point. When importing CVS tags, strip ? characters from the tag names as we translate them to git tag names. Signed-off-by: Ken Dreyer ktdre...@ktdreyer.com --- git-cvsimport.perl | 1 + 1 file changed, 1 insertion(+) diff --git a/git-cvsimport.perl b/git-cvsimport.perl index 8d41610..36f59fe 100755 --- a/git-cvsimport.perl +++ b/git-cvsimport.perl @@ -890,6 +890,7 @@ sub commit { $xtag =~ tr/_/\./ if ( $opt_u ); $xtag =~ s/[\/]/$opt_s/g; $xtag =~ s/\[//g; + $xtag =~ s/\?//g; system('git' , 'tag', '-f', $xtag, $cid) == 0 or die Cannot create tag $xtag: $!\n; -- 1.7.11.4 -- To unsubscribe from this list: send the line 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] cvsimport: strip question marks from tags
Ken Dreyer ktdre...@ktdreyer.com writes: The ? character can be present in a CVS tag name, but git's bad_ref_char does not allow question marks in git tags. If git-cvsimport encounters a CVS tag with a question mark, it will error and refuse to continue the import beyond that point. When importing CVS tags, strip ? characters from the tag names as we translate them to git tag names. Signed-off-by: Ken Dreyer ktdre...@ktdreyer.com --- git-cvsimport.perl | 1 + 1 file changed, 1 insertion(+) diff --git a/git-cvsimport.perl b/git-cvsimport.perl index 8d41610..36f59fe 100755 --- a/git-cvsimport.perl +++ b/git-cvsimport.perl @@ -890,6 +890,7 @@ sub commit { $xtag =~ tr/_/\./ if ( $opt_u ); $xtag =~ s/[\/]/$opt_s/g; $xtag =~ s/\[//g; + $xtag =~ s/\?//g; I do not think this is a right and sustainable approach. The next patch would probably be to strip ~ and then another patch that strips ^, and yet another that squashes .. into one would surely follow. How about extending the s/\[//g we can see in the context to cover everything that are unacceptable (see refs.c:bad_ref_char()) once and for all? The result needs to be further massaged to avoid component that has two or more dots in a row, a dot at the beginning or at the end (see the comment at the beginning of refs.c, and also refs.c:check_refname_component()). system('git' , 'tag', '-f', $xtag, $cid) == 0 or die Cannot create tag $xtag: $!\n; -- To unsubscribe from this list: send the line 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] cvsimport: strip all inappropriate tag strings
Certain characters such as ? can be present in a CVS tag name, but git does not allow these characters in tags. If git-cvsimport encounters a CVS tag that git cannot handle, cvsimport will error and refuse to continue the import beyond that point. When importing CVS tags, strip all the inappropriate strings from the tag names as we translate them to git tag names. Signed-off-by: Ken Dreyer ktdre...@ktdreyer.com --- Thank you Junio for the review. I've taken your suggestion and amended my patch to eliminate all the bad strings in ref.c. git-cvsimport.perl | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/git-cvsimport.perl b/git-cvsimport.perl index 8d41610..0dc598d 100755 --- a/git-cvsimport.perl +++ b/git-cvsimport.perl @@ -889,7 +889,25 @@ sub commit { $xtag =~ s/\s+\*\*.*$//; # Remove stuff like ** INVALID ** and ** FUNKY ** $xtag =~ tr/_/\./ if ( $opt_u ); $xtag =~ s/[\/]/$opt_s/g; - $xtag =~ s/\[//g; + + # See ref.c for these rules. + # Tag cannot end with a '/' - this is already handled above. + # Tag cannot contain bad chars. See bad_ref_char in ref.c. + $xtag =~ s/[ ~\^:\\\*\?\[]//g; + # Tag cannot contain '..'. + $xtag =~ s/\.\.//g; + # Tag cannot contain '@{'. + $xtag =~ s/\@{//g; + # Tag cannot end with '.lock'. + $xtag =~ s/(?:\.lock)+$//; + # Tag cannot begin or end with '.'. + $xtag =~ s/^\.+//; + $xtag =~ s/\.+$//; + # Tag cannot consist of a single '.' - already handled above. + # Tag cannot be empty. + if ($xtag eq '') { + return; + } system('git' , 'tag', '-f', $xtag, $cid) == 0 or die Cannot create tag $xtag: $!\n; -- 1.7.11.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html