Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file
Johannes Sixt j.s...@viscovery.net writes: The numbers defined in {FILE,PRODUCT}VERSION statements are intended for machine consumption and are always 4 positions (if the source contains fewer, they are padded with zeros). They can be used by installers to decide whether a file that already exists in the system should be overwritten by a newer version. OK, that makes sense. If you package 1.9 (padded as 1.9.0.0) and then 1.9.1 (padded as 1.9.1.0), you can update from 1.9 to 1.9.1 just fine. Unfortunately, these numbers are visible when the user invokes Properties from the context menu of git.exe in the file manager and then switches to the Version tab. All 4 positions are always listed. Therefore, the user will see 1.9.0.0 for the first release of the 1.9 series, which is wrong, because you will call 1.9, not 1.9.0.0, I assume. With sufficient effort, we could achieve that version 1.9.1 is listed as 1.9.1.0. That is still wrong. I would not be worried about showing 1.9.1.0 for 1.9.1 and/or 1.9.0.0 for 1.9 at all. But if the (receiving) system expects these to be monotonically increasing, I suspect the script I posted would not work well under that expectation. When you package 1.9.2.g43218765.dirty, that would become 1.9.2.0, and become indistinguishable from the package taken from v1.9.2 tag, which is not good at all. So the script should strip [0-9]*\.g[0-9a-f]*\(\.dirty\)? from the end first. But even without complications from the N-commit after the tag it won't work well if you cut packages from anything that is not tagged anyway. The only thing we know about any package taken from the tip of 'master' past v1.9 is that it is newer than the package taken from v1.9 tag. Sometimes it should be considered newer than a package taken from v1.9.x tag (i.e. the contents of the maintenance relase is fully included in 'master'), but not always (i.e. the tip of 'master' when the package was made may contain up to v1.9.3 but v1.9.4 may be newer than that). If you truncate down to only two, like your patch does, anything past v1.9 and before v1.10 (or v2.0) would have 1.9.0.0 and that is no worse than giving 1.9.3.0 for v1.9.3 and giving 1.9.0.0 for anything based on 'master'. Your user may have installed a package made from v1.9.1 and would want to update to the one taken from 'master' when it contained everything up to v1.9.3---under my earlier take numbers approach, we would be updating from 1.9.1.0 to 1.9.0.0, which does not look like updating at all to the system. The installers can use this to decide a file that already exists in the system is newer, which is wrong, if I am reading your earlier explanation corretly. With your we just take the first two numbers always, you would be sidegrading between two 1.9.0.0, which may fare better. Since we can't get this display right, I suggest that we just punt (as per my patch). That should work out nicely because we can fairly safely assume that there are no installers around that look at these particular version numbers. BTW, that same Version tab will have another entry, called Product Version later in the list. This one lists the string that we pass in -DGIT_VERSION (see quoted context below). It is the truely correct version that *users* should be interested in. diff --git a/Makefile b/Makefile index b4af1e2..99b2b89 100644 --- a/Makefile +++ b/Makefile @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES git.res: git.rc GIT-VERSION-FILE $(QUIET_RC)$(RC) \ - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, ,$(subst ., ,$(GIT_VERSION) \ + $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., ,$(GIT_VERSION) \ -DGIT_VERSION=\\\$(GIT_VERSION)\\\ $ -o $@ ifndef NO_PERL diff --git a/git.rc b/git.rc index bce6db9..33aafb7 100644 --- a/git.rc +++ b/git.rc @@ -1,6 +1,6 @@ 1 VERSIONINFO -FILEVERSION MAJOR,MINOR,PATCH,0 -PRODUCTVERSION MAJOR,MINOR,PATCH,0 +FILEVERSION MAJOR,MINOR,0,0 +PRODUCTVERSION MAJOR,MINOR,0,0 BEGIN BLOCK StringFileInfo BEGIN -- To unsubscribe from this list: send the line 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: [ANNOUNCE] Git v1.9-rc0
Vicent Martí tan...@gmail.com writes: Do these consume CPU every time somebody asks for a tarball? That might be considered wrong depending on the view. No, our infrastructure caches frequently requested tarballs so they don't have to be regenerated on the fly. Thanks. That is certainly good enough for consumers, and better than having to manually create and upload for me ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] repack: accept larger window-memory and max-pack-size
These quantities can be larger than an int. Use ulong to express them like the underlying pack-objects does, and also parse them with the human-friendly unit suffixes. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/repack.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index a0ff5c7..8ce396b 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -130,9 +130,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix) int pack_everything = 0; int delete_redundant = 0; char *unpack_unreachable = NULL; - int window = 0, window_memory = 0; + int window = 0; int depth = 0; - int max_pack_size = 0; + unsigned long max_pack_size = 0, window_memory = 0; int no_reuse_delta = 0, no_reuse_object = 0; int no_update_server_info = 0; int quiet = 0; @@ -159,11 +159,11 @@ int cmd_repack(int argc, const char **argv, const char *prefix) N_(with -A, do not loosen objects older than this)), OPT_INTEGER(0, window, window, N_(size of the window used for delta compression)), - OPT_INTEGER(0, window-memory, window_memory, + OPT_HUM_ULONG(0, window-memory, window_memory, N_(same as the above, but limit memory size instead of entries count)), OPT_INTEGER(0, depth, depth, N_(limits the maximum delta depth)), - OPT_INTEGER(0, max-pack-size, max_pack_size, + OPT_HUM_ULONG(0, max-pack-size, max_pack_size, N_(maximum size of each packfile)), OPT_END() }; @@ -187,11 +187,11 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (window) argv_array_pushf(cmd_args, --window=%u, window); if (window_memory) - argv_array_pushf(cmd_args, --window-memory=%u, window_memory); + argv_array_pushf(cmd_args, --window-memory=%lu, window_memory); if (depth) argv_array_pushf(cmd_args, --depth=%u, depth); if (max_pack_size) - argv_array_pushf(cmd_args, --max_pack_size=%u, max_pack_size); + argv_array_pushf(cmd_args, --max_pack_size=%lu, max_pack_size); if (no_reuse_delta) argv_array_pushf(cmd_args, --no-reuse-delta); if (no_reuse_object) -- 1.9-rc0-151-ga5225c0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] parse-options: refactor human-friendly-integer parser out of pack-objects
We only had code to understand unit suffixes such as g/m/k (as in 2g/400m/8k) in the configuration parser but not in the command line option parser. git pack-objects worked it around by having a custom extension to the parse-options API; make it available to other callers. The new macro is not called OPT_ULONG() but OPT_HUM_ULONG(), as it would be bizzarre to have ULONG that understands human friendly units while INTEGER that does not. It is not named with a shorter OPT_HULONG, primarily to avoid having to name a future parallel for parsing human friendly integer OPT_HINT. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/pack-objects.c | 25 - parse-options.c| 17 + parse-options.h| 5 + 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index f069462..2fa8e1e 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -2417,23 +2417,6 @@ static int option_parse_unpack_unreachable(const struct option *opt, return 0; } -static int option_parse_ulong(const struct option *opt, - const char *arg, int unset) -{ - if (unset) - die(_(option %s does not accept negative form), - opt-long_name); - - if (!git_parse_ulong(arg, opt-value)) - die(_(unable to parse value '%s' for option %s), - arg, opt-long_name); - return 0; -} - -#define OPT_ULONG(s, l, v, h) \ - { OPTION_CALLBACK, (s), (l), (v), n, (h), \ - PARSE_OPT_NONEG, option_parse_ulong } - int cmd_pack_objects(int argc, const char **argv, const char *prefix) { int use_internal_rev_list = 0; @@ -2455,16 +2438,16 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) { OPTION_CALLBACK, 0, index-version, NULL, N_(version[,offset]), N_(write the pack index file in the specified idx format version), 0, option_parse_index_version }, - OPT_ULONG(0, max-pack-size, pack_size_limit, - N_(maximum size of each output pack file)), + OPT_HUM_ULONG(0, max-pack-size, pack_size_limit, + N_(maximum size of each output pack file)), OPT_BOOL(0, local, local, N_(ignore borrowed objects from alternate object store)), OPT_BOOL(0, incremental, incremental, N_(ignore packed objects)), OPT_INTEGER(0, window, window, N_(limit pack window by objects)), - OPT_ULONG(0, window-memory, window_memory_limit, - N_(limit pack window by memory in addition to object limit)), + OPT_HUM_ULONG(0, window-memory, window_memory_limit, + N_(limit pack window by memory in addition to object limit)), OPT_INTEGER(0, depth, depth, N_(maximum length of delta chain allowed in the resulting pack)), OPT_BOOL(0, reuse-delta, reuse_delta, diff --git a/parse-options.c b/parse-options.c index c2cbca2..948ade7 100644 --- a/parse-options.c +++ b/parse-options.c @@ -136,6 +136,23 @@ static int get_value(struct parse_opt_ctx_t *p, return opterror(opt, expects a numerical value, flags); return 0; + case OPTION_ULONG: + if (unset) { + *(unsigned long *)opt-value = 0; + } else if (opt-flags PARSE_OPT_OPTARG !p-opt) { + *(unsigned long *)opt-value = opt-defval; + } else if (get_arg(p, opt, flags, arg)) { + return -1; + } else if (opt-flags PARSE_OPT_HUMAN_NUMBER) { + if (!git_parse_ulong(arg, (unsigned long *)opt-value)) + return opterror(opt, expects a numerical value, flags); + } else { + *(unsigned long *)opt-value = strtoul(arg, (char **)s, 10); + if (*s) + return opterror(opt, expects a numerical value, flags); + } + return 0; + default: die(should not happen, someone must be hit on the forehead); } diff --git a/parse-options.h b/parse-options.h index 9b94596..d65ecdb 100644 --- a/parse-options.h +++ b/parse-options.h @@ -16,6 +16,7 @@ enum parse_opt_type { /* options with arguments (usually) */ OPTION_STRING, OPTION_INTEGER, + OPTION_ULONG, OPTION_CALLBACK, OPTION_LOWLEVEL_CALLBACK, OPTION_FILENAME @@ -40,6 +41,7 @@ enum parse_opt_option_flags { PARSE_OPT_LASTARG_DEFAULT = 16, PARSE_OPT_NODASH = 32, PARSE_OPT_LITERAL_ARGHELP = 64, + PARSE_OPT_HUMAN_NUMBER = 128
[PATCH v2 0/2] Fix repack --window-memory=4g regression in 1.8.4
The command line parser was broken when the command was reimplemented in C in two ways. It incorrectly limited the value range of window-memory and max-pack-size to int, and also stopped grokking the unit suffixes like 2g/400m/8k. These two patches apply on top of 35c14176 (Reword repack documentation to no longer state it's a script, 2013-10-19) and later can be merged down to maint-1.8.4 track and upwards. Junio C Hamano (2): parse-options: refactor human-friendly-integer parser out of pack-objects repack: accept larger window-memory and max-pack-size builtin/pack-objects.c | 25 - builtin/repack.c | 12 ++-- parse-options.c| 17 + parse-options.h| 5 + 4 files changed, 32 insertions(+), 27 deletions(-) -- 1.9-rc0-151-ga5225c0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file
Junio C Hamano gits...@pobox.com writes: Johannes Sixt j.s...@viscovery.net writes: ... ..., I suggest that we just punt (as per my patch). That should work out nicely because we can fairly safely assume that there are no installers around that look at these particular version numbers. OK. I do not think we care too deeply about how a forced to be four dewey-decimal numbers looks compared to 2 or 3 numbers in the $(GIT_VERSION), as I think we always had that (non-)issue, but not being able to compile is not very nice. So can you, Pat or Ramsay send a tested patch with a proposed log message? Preferrably by -rc1 but I think the change is low impact that it can be in -rc2, leaving -rc1 broken. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] gitk: Replace next and prev buttons with down and up arrows.
Paul Mackerras pau...@samba.org writes: On Tue, Jan 21, 2014 at 10:33:02AM -0500, Marc Branchaud wrote: On 13-12-18 11:04 AM, Marc Branchaud wrote: Users often find that next and prev do the opposite of what they expect. For example, next moves to the next match down the list, but that is almost always backwards in time. Replacing the text with arrows makes it clear where the buttons will take the user. Any opinions on this, either way? I've grown fond of the down/up arrows. I find them much clearer than the current next/prev buttons. My only niggle about this patch is that the buttons are much smaller, requiring a bit more precision clicking. But the smaller buttons allow more room for other widgets. I showed it to a few colleagues who use gitk a lot. One was indifferent, the others liked it, so I have applied it. Thanks, Paul. Is this a good time for me to pull from you? I see these on your 'master' branch. 8f86339 gitk: Comply with XDG base directory specification 786f15c gitk: Replace next and prev buttons with down and up arrows c61f3a9 gitk: chmod +x po2msg.sh 6c626a0 gitk: Update copyright dates 45f884c gitk: Add Bulgarian translation (304t) 1f3c872 gitk: Fix mistype Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v1.9-rc0
Ken Moffat zarniwh...@ntlworld.com writes: I note that all of these *are* still available at googlecode for the moment : https://code.google.com/p/git-core/downloads/list As I said, Cgc is not the ony download site. The end of http://git-blame.blogspot.com/p/git-public-repositories.html lists the two sites that currently have the material. I may replace Cgc with something else (and add it/them to the list), but in the meantime I do not think k.org will go out of business in anytime soon, 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: [RFC PATCH] Make 'git request-pull' more strict about matching local/remote branches
Linus Torvalds torva...@linux-foundation.org writes: This means that git request-pull will never rewrite the ref-name you gave it. If the local branch name is xyzzy, that is the only branch name that request-pull will ask the other side to fetch. If the remote has that branch under a different name, that's your problem and git request-pull will not try to fix it up (but git request-pull will warn about the fact that no exact matching branch is found, and you can edit the end result to then have the remote name you want if it doesn't match your local one). The new find local ref code will also complain loudly if you give an ambiguous refname (eg you have both a tag and a branch with that same name, and you don't specify heads/name or tags/name). I agree with the basic direction, especially the part we will never rewrite, is quite attractive. But this part might be a bit problematic. $3=master will almost always have refs/heads/master and refs/remotes/origin/master listed because the call to show-ref comes before rev-parse --verify, no? +head=$(git symbolic-ref -q ${3-HEAD}) +head=${head:-$(git show-ref ${3-HEAD} | cut -d' ' -f2)} +head=${head:-$(git rev-parse --quiet --verify $3)} + +# None of the above? Bad. +test -z $head die fatal: Not a valid revision: $3 + +# This also verifies that the resulting head is unique: +# git show-ref could have shown multiple matching refs.. headrev=$(git rev-parse --verify --quiet $head^0) -if test -z $headrev +test -z $headrev die fatal: Ambiguous revision: $3 ... and it would die here. $3=for-linus may be the most common case on the kernel list, and remotes/origin/for-linus may be unlikely to appear in the real life (hmph, really? perhaps people keep track of what they pushed out the last time with it, I dunno). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Make 'git request-pull' more strict about matching local/remote branches
Linus Torvalds torva...@linux-foundation.org writes: That may be very helpful if your local tree doesn't match the layout of the remote branches, but for the common case it's been a recurring disaster, when request-pull is done against a delayed remote update, and it rewrites the target branch randomly to some other branch name that happens to have the same expected SHA1 (or more commonly, leaves it blank). Thinking about this a bit more... Comments? It passes the tests I put it through locally, but I did *not* make it pass the test-suite, since it very much does change the rules. Some of the test suite code literally tests for the old completely broken case (at least t5150, subtests 4 and 5). I looked at 5150.4 and found that what it attempts to do is halfway sensible. The contributor works on the local 'master' branch, publishes the result to 'for-linus' in its 'origin' repository, and asks his state to be pulled, with: git push origin master:for-linus git request-pull initial origin The contributor could be more explicit in his request-pull and say git request-pull initial origin master but there is no 'master' on the publishing repository in this case (or even if there is, it does not match what is being pushed out), and there is no 'for-linus' branch locally, so there is no way for him to say git request-pull initial origin for-linus unless he creates it locally first. I am starting to wonder if it is a better fix to check potentially ambiguous cases (e.g. the publishing repository does have 'master' that does not point at the commit local 'master' points at, and 'for-linus' that points at the same commit, and the user asks for 'master' to be pulled) or clearly broken cases (e.g. the user gave something other than HEAD explicitly from the command line, but we ended up computing blank) and die loudly, without breaking cases this test tries to protect. On the other hand, I tend to think what 5150.5 wants is convoluted and expects a broken behaviour. Its publishing repository has 'master' and 'for-upstream', and also has 'tags/full' that is an annotated tag that points at the commit, runs request-pull with its local 'master', and expects the resulting message to ask 'tags/full' to be pulled. If the contributor wants such a non-commit to be pulled, I think it should be made more explicit, i.e., not with git request-pull initial $origin_url but with git request-pull initial $origin_url tags/full or something. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Make 'git request-pull' more strict about matching local/remote branches
Junio C Hamano gits...@pobox.com writes: ... there is no 'for-linus' branch locally, so there is no way for him to say git request-pull initial origin for-linus unless he creates it locally first. In real life on the kernel list, for-linus may have to be a signed tag, and pushed out 1-to-1 name mapping, so in that sense, unless he creates it locally first may not be a problem. I'd throw this into No, this is not the only way to do so and there are workarounds available if we suddenly tightened the rule and broke those who relied on this behaviour. But this is not a less good way compared to the alternative of creating the same-named ref first, so we _are_ breaking people deliberately---is that worth the safety for always-push-one-to-one people? category. I'd throw the other one (i.e. 5150.5) into that is crazy enough that a short apology in the Release Notes is sufficient before breaking those who relied on that behaviour category, on the other hand ;-). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Jan 2014, #04; Wed, 22)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * fp/submodule-checkout-mode (2014-01-07) 1 commit (merged to 'next' on 2014-01-10 at 647ba9b) + git-submodule.sh: 'checkout' is a valid update mode (this branch is used by wk/submodule-on-branch.) submodule.*.update=checkout, when propagated from .gitmodules to .git/config, turned into a submodule.*.update=none, which did not make much sense. * nd/shallow-clone (2014-01-09) 31 commits (merged to 'next' on 2014-01-09 at 6608634) + t5537: fix incorrect expectation in test case 10 (merged to 'next' on 2014-01-06 at 3dc7fab) + shallow: remove unused code + send-pack.c: mark a file-local function static (merged to 'next' on 2014-01-03 at a776065) + git-clone.txt: remove shallow clone limitations + prune: clean .git/shallow after pruning objects + clone: use git protocol for cloning shallow repo locally + send-pack: support pushing from a shallow clone via http + receive-pack: support pushing to a shallow clone via http + smart-http: support shallow fetch/clone + remote-curl: pass ref SHA-1 to fetch-pack as well + send-pack: support pushing to a shallow clone + receive-pack: allow pushes that update .git/shallow + connected.c: add new variant that runs with --shallow-file + add GIT_SHALLOW_FILE to propagate --shallow-file to subprocesses + receive/send-pack: support pushing from a shallow clone + receive-pack: reorder some code in unpack() + fetch: add --update-shallow to accept refs that update .git/shallow + upload-pack: make sure deepening preserves shallow roots + fetch: support fetching from a shallow repository + clone: support remote shallow repository + fetch-pack.h: one statement per bitfield declaration + fetch-pack.c: move shallow update code out of fetch_pack() + shallow.c: steps 6 and 7 to select new commits for .git/shallow + shallow.c: the 8 steps to select new commits for .git/shallow + shallow.c: extend setup_*_shallow() to accept extra shallow commits + connect.c: teach get_remote_heads to parse shallow lines + make the sender advertise shallow commits to the receiver + clone: prevent --reference to a shallow repository + send-pack: forbid pushing from a shallow repository + remote.h: replace struct extra_have_objects with struct sha1_array + transport.h: remove send_pack prototype, already defined in send-pack.h Fetching from a shallow-cloned repository used to be forbidden, primarily because the codepaths involved were not carefully vetted and we did not bother supporting such usage. This attempts to allow object transfer out of a shallow-cloned repository in a controlled way (i.e. the receiver become a shallow repository with truncated history). -- [New Topics] * jn/ignore-doc (2014-01-16) 1 commit (merged to 'next' on 2014-01-22 at a07ac63) + gitignore doc: add global gitignore to synopsis Explicitly list $HOME/.config/git/ignore as one of the places you can use to keep ignore patterns that depend on your personal choice of tools, e.g. *~ for Emacs users. Will merge to 'master'. * rk/send-email-ssl-cert (2014-01-16) 1 commit (merged to 'next' on 2014-01-22 at 2fb13f2) + send-email: /etc/ssl/certs/ directory may not be usable as ca_path The if /etc/ssl/certs/ directory exists, explicitly tell the library to use it as SSL_ca_path blind-defaulting in git send-email broke platforms where /etc/ssl/certs/ directory exists, but it cannot used as SSL_ca_path (e.g. Fedora rawhide). Fix it by not specifying any SSL_ca_path/SSL_ca_file but still asking for peer verification in such a case. Will merge to 'master'. * ef/mingw-write (2014-01-17) 2 commits (merged to 'next' on 2014-01-22 at b9ddab2) + mingw: remove mingw_write + prefer xwrite instead of write Will merge to 'master'. * jk/branch-at-publish-rebased (2014-01-17) 5 commits - t1507 (rev-parse-upstream): fix typo in test title - implement @{publish} shorthand - branch_get: provide per-branch pushremote pointers - branch_get: return early on error - sha1_name: refactor upstream_mark (this branch uses jk/interpret-branch-name-fix.) Give an easier access to the tracking branches from other side in a triangular workflow by introducing B@{publish} that works in a similar way to how B@{upstream} does. * jk/color-for-more-pagers (2014-01-17) 4 commits - pager: disable colors for some known-bad configurations - DONOTMERGE: needs matching change to git-sh-setup - setup_pager: set MORE=R - setup_pager: refactor LESS/LV environment setting * jk/diff-filespec-cleanup (2014-01-17) 5 commits (merged to 'next' on
Re: [PATCH v3] git-svn: memoize _rev_list and rebuild
Eric Wong normalper...@yhbt.net writes: Pushed for Junio. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] gitk: Replace next and prev buttons with down and up arrows.
Paul Mackerras pau...@samba.org writes: Yes, please pull. I have just pushed one more: 76d64ca gitk: Indent word-wrapped lines in commit display header Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] setup_pager: set MORE=R
Jeff King p...@peff.net writes: ..., but it feels awfully wrong to be so intimate with a subprogram that we do not control. Yeah, I think we are in agreement on that 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: Want to do some cleanups in this round of l10n
Jiang Xin worldhello@gmail.com writes: Maybe three weeks left. You can estimate it by checking the date for history tags, such as v1.8.5-rc0 and v1.8.5-rc3. v1.8.5-rc0: Wed Oct 30 12:17:56 2013 -0700 v1.8.5-rc3: Wed Nov 20 11:27:39 2013 -0800 v1.8.5: Wed Nov 27 12:14:52 2013 -0800 Or please consult https://tinyurl.com/gitcal Yes, fuzz messages are not included. I double checked the history of da.po and nl.po, and I'm sure there are no further updates since their maintainers sent me Email like Hi, add me as the maintainer for that language, and I will send translations latter... Remove them can make the Git package smaller and give opportunities to other contributors. Yeah, I think removing them is sane and 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: [PATCH v2] Makefile: Fix compilation of Windows resource file
Pat Thoyts pattho...@gmail.com writes: GIT_VERSION=1.9.rc0 all: echo $(join -DMAJOR= -DMINOR= -DPATCH=, \ $(wordlist 1,3,$(filter-out rc%,$(subst -, ,$(subst ., ,$(GIT_VERSION 0 0)) This removes any rc* parts and appends a couple of zeros so that all missing elements should appear as 0 in the final list. As Junio already pointed out, this records the wrong number in the 1.9 track before 1.9.1 is out because the third position is the commit count, not the patch level. -- Hannes OK - I cehcked and you are right in that the GIT_VERSION value is the one showing up the properties dialog at least under Windows 7. As this is the most likely to be examined I agree that just taking the first two digits is the simplest fix here. So, fine by me then. Acked-by: Pat Thoyts pattho...@users.sourceforge.net Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v1.9-rc0
Jeff King p...@peff.net writes: Junio, since you prepare such tarballs[1] anyway for kernel.org, it might be worth uploading them to the Releases page of git/git. I imagine there is a programmatic way to do so via GitHub's API, but I don't know offhand. I can look into it if you are interested. I already have a script that takes the three tarballs and uploads them to two places, so adding GitHub as the third destination should be a natural and welcome way to automate 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 v2 2/2] repack: accept larger window-memory and max-pack-size
Jeff King p...@peff.net writes: On Wed, Jan 22, 2014 at 08:06:42PM -0500, Jeff King wrote: But I think there is a subtle problem. Here (and elsewhere) we use the parsed value of 0 as a sentinel. I think that is OK for --max-pack-size, where 0 is not a reasonable value. But git-repack(1) says: --window-memory=0 makes memory usage unlimited, which is the default. What does: git config pack.windowMemory 256m git repack --window-memory=0 do? It should override the config, but I think it does not with your patch (nor with the current code). Using a string would fix that (though you could also fix it by using a different sentinel, like ULONG_MAX). Here is a series that does that (and fixes the other issue I found). It would probably be nice to test these things, but checking that they actually had an impact is tricky (how do you know that --window-memory did the right thing?). [1/3]: repack: fix typo in max-pack-size option [2/3]: repack: make parsed string options const-correct [3/3]: repack: propagate pack-objects options as strings -Peff Sounds sensible; will chuck the hum-ulong series and replace with these patches. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote
Linus Torvalds torva...@linux-foundation.org writes: So this relaxes the remote matching, and allows using the local:remote syntax to say that the local branch is differently named from the remote one. It is probably worth folding it into the previous patch if you think this whole approach is workable. Haven't thought enough to decide on that part, yet. # $3 must be a symbolic ref, a unique ref, or -# a SHA object expression +# a SHA object expression. It can also be of +# the format 'local-name:remote-name'. # -head=$(git symbolic-ref -q ${3-HEAD}) -head=${head:-$(git show-ref ${3-HEAD} | cut -d' ' -f2)} -head=${head:-$(git rev-parse --quiet --verify $3)} +local=${3%:*} +local=${local:-HEAD} +remote=${3#*:} +head=$(git symbolic-ref -q $local) +head=${head:-$(git show-ref --heads --tags $local | cut -d' ' -f2)} +head=${head:-$(git rev-parse --quiet --verify $local)} # None of the above? Bad. -test -z $head die fatal: Not a valid revision: $3 +test -z $head die fatal: Not a valid revision: $local # This also verifies that the resulting head is unique: I am not sure if it is a good idea to hand-craft resulting head is unique constraint here. We already have disambiguation rules (and warning mechanism) we use in other places---this part should use the same rule, I think. A short experiment tells me that we are almost there: $ git init git commit --allow-empty -m initial $ git branch other git tag master $ git -c core.warnambiguousrefs=true \ rev-parse --symbolic-full-name other refs/heads/other $ git -c core.warnambiguousrefs=true \ rev-parse --symbolic-full-name master; echo $? warning: refname 'master' is ambiguous. error: refname 'master' is ambiguous 0 We say error but we do not actually error out, but that shouldn't be too hard to fix. # git show-ref could have shown multiple matching refs.. headrev=$(git rev-parse --verify --quiet $head^0) -test -z $headrev die fatal: Ambiguous revision: $3 +test -z $headrev die fatal: Ambiguous revision: $local # Was it a branch with a description? branch_name=${head#refs/heads/} @@ -69,9 +73,6 @@ then branch_name= fi -prettyhead=${head#refs/} -prettyhead=${prettyhead#heads/} - merge_base=$(git merge-base $baserev $headrev) || die fatal: No commits in common between $base and $head @@ -81,30 +82,37 @@ die fatal: No commits in common between $base and $head # # Otherwise find a random ref that matches $headrev. find_matching_ref=' + my ($head,$headrev) = (@ARGV); + my ($found); + while (STDIN) { + chomp; my ($sha1, $ref, $deref) = /^(\S+)\s+([^^]+)(\S*)$/; + my ($pattern); + next unless ($sha1 eq $headrev); + + $pattern=/$head\$; I think $head is constant inside the loop, so lift it outside? + if ($ref eq $head) { + $found = $ref; + } + if ($ref =~ /$pattern/) { + $found = $ref; } + if ($sha1 eq $head) { I think this is $headrev ($head may be $remote or HEAD), but then anything that does not point at $headrev has already been rejected at the beginning of this loop, so...? $found = $sha1; } } + if ($found) { print $found\n; } ' I somehow feel that this is inadequate to catch the delayed propagation error in the opposite direction. The publish repository may have an unrelated ref pointing at the $headrev and we may guess that is the ref to be fetched by the integrator based on that, but by the time the integrator fetches from the repository, the ref may have been updated to its new value that does not match $headrev. But I do not think of a way to solve that one. In any case, shouldn't we be catching duplicate matches here, if the real objective is to make it less likely for the users to make mistakes? Otherwise, if there are two 'master' over there (e.g. refs/heads/master and refs/remotes/origin/master), it seems to me that $ref =~ /$pattern/ will trigger twice in your loop and ends up reporting the last match. In other words, my (@found) = (); my (@guessed) = (); while (STDIN) { next unless ($sha1 eq $headrev); ... if ($ref =~ /$pattern/) { push @found, $ref; } else { push @guessed, $ref; } } if (@found == 1) { print $found[0]\n; } else if (@guessed == 1) { print $guessed[0]\n; } or something like that? -ref=$(git ls-remote $url | @@PERL@@ -e $find_matching_ref $head $headrev) +ref=$(git ls-remote $url | @@PERL@@ -e $find_matching_ref ${remote:-HEAD} $headrev) There are three cases as far as ${remote:-HEAD} aka $head in the script is concerned: 1. The
Re: [PATCH 0/2] solaris test fixups
Jeff King p...@peff.net writes: and assume that it will fail. It doesn't. Solaris happily renames some-file to a regular file named no-such-dir. So we fail later during the index-update, complaining about adding the entry no-such-dir/, but still exit(0) at the end. I'm mostly willing to just call Solaris crazy for allowing the rename (Linux returns ENOTDIR), but I do wonder if the index codepath could be improved (and especially to return an error). I think j6t has a patch for that, a8933469 (mv: let 'git mv file no-such-dir/' error out on Windows, too, 2014-01-08). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: When you do git diff HEAD submodule/, submodule from the index is picked out and match_pathspec_depth() in charge of matching it with the pathspec submodule/. Is ... is called or something missing at the end of this sentence? Unlike tree_entry_interesting(), match_pathspec_depth() has no knowledge about entry mode to realize submodule is a directory and treat the trailing slash specially. And it does not have too, mostly, s/too/to/, I think. because the index only contains files, not directories (not until submodules come) I have no solutions for it (no, stripping '/' at pathspec preprocessing phase seems like a workaround than a solution). So let's mark it. Maybe I or somebody else could revisit it later. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- t/t4010-diff-pathspec.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh index 15a4912..b54251a 100755 --- a/t/t4010-diff-pathspec.sh +++ b/t/t4010-diff-pathspec.sh @@ -127,4 +127,10 @@ test_expect_success 'diff-tree ignores trailing slash on submodule path' ' test_cmp expect actual ' +test_expect_failure 'diff-cache ignores trailing slash on submodule path' ' + git diff --name-only HEAD^ submod expect + git diff --name-only HEAD^ submod/ actual I actually doubt that the second line is expecting the right behaviour in the first place. As far as the top-level project is concerned, submod is the name it wants, as there is nothing underneath it. Even if asked to recurse infinite levels, the caller shouldn't be feeding paths like submod/a/b/c to match_pathspec_depth() in the first place, no? + test_cmp expect actual +' + test_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 2/2] t7700: do not use touch -r
Jeff King p...@peff.net writes: ... The test does not care about the timestamp of the .keep file it creates at all, only that it exists. Please refrain from using touch for such use cases in the first place. It appears that pack-$packsha1.keep is what the test wants. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] t7700: do not use touch -r
Jeff King p...@peff.net writes: Agreed. I was making the minimal change, but I think there is no reason not to fix both while we are there. Do you want to just mark up the patch in transit? Let's just queue this instead. -- 8 -- From: Jeff King p...@peff.net Date: Thu, 23 Jan 2014 14:55:18 -0500 Subject: [PATCH] t7700: do not use touch unnecessarily Some versions of touch (such as /usr/ucb/touch on Solaris) do not know about the -r option. This would make sense as a feature of test-chmtime, but fortunately this fix is even easier. The test does not care about the timestamp of the .keep file it creates at all, only that it exists. For such a use case, with or without portability issues around -r, touch should not be used in the first place. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7700-repack.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index d954b84..b45bd1e 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -17,7 +17,7 @@ test_expect_success 'objects in packs marked .keep are not repacked' ' # The second pack will contain the excluded object packsha1=$(git rev-list --objects --all | grep file2 | git pack-objects pack) - touch -r pack-$packsha1.pack pack-$packsha1.keep + pack-$packsha1.keep objsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 | sed -e s/^\([0-9a-f]\{40\}\).*/\1/) mv pack-* .git/objects/pack/ -- 1.9-rc0-234-g8e6341d -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
Jens Lehmann jens.lehm...@web.de writes: ... But a single trailing '/' does mark submod as a directory, which I think is ok for a submodule. And it makes life easier for the user if we accept that, as shell completion will add it there automatically. OK, that would be annoying. Perhaps the completion is what is broken here, then? I dunno, and haven't thought things through. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote
Linus Torvalds torva...@linux-foundation.org writes: Yes, so you'll get a warning (or, if you get a partial match, maybe not even that), but the important part about all these changes is that it DOESN'T MATTER. Why? Because it no longer re-writes the target branch name based on that match or non-match. So the pull request will be fine. Will be fine, provided if they always use local:remote syntax, I'd agree. In other words, the really fundamental change here i that the oops, I couldn't find things on the remote no longer affects the output. It only affects the warning. And I think that's important. It used to be that the remote matching actually changed the output of the request-pull, and *THAT* was the fundamental problem. The fingers of users can be retrained to use the local:remote syntax after we apologize for this change in the Release Notes, I see only one issue (we seem to lose the message from the annotated/signed tag when asking to pull it) remaining, after looking at what updates are needed for t5150. Thanks. -- 8 -- Subject: [PATCH] pull-request: test updates This illustrates behaviour changes that result from the recent change by Linus. Most shows good changes, but there may be usability regressions: - The command continues to fail when the user forgot to push out before running the command, but the wording of the message has been slightly changed. - The command no longer guesses when asked to request the commit at the HEAD be pulled after pushing it to a branch 'for-upstream', even when that branch points at the correct commit. The user must ask the command with the new master:for-upstream syntax. - The command no longer favours a tag that peels to the requested commit over a branch that points at the same commit. This needs to be asked explicitly by specifying the tag object, not the commit. But somehow this does not see to work (yet); somewhere the tag-ness of the requested ref seems to be lost. The new behaviour needs to be documented in any case, but we need to agree what the new behaviour should be before doing so, so... Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t5150-request-pull.sh | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh index 1afa0d5..412ee4f 100755 --- a/t/t5150-request-pull.sh +++ b/t/t5150-request-pull.sh @@ -86,7 +86,7 @@ test_expect_success 'setup: two scripts for reading pull requests' ' s/[-0-9]\{10\} [:0-9]\{8\} [-+][0-9]\{4\}/DATE/g s/[^ ].*/SUBJECT/g s/ [^ ].* (DATE)/ SUBJECT (DATE)/g - s/for-upstream/BRANCH/g + s|tags/full|BRANCH|g s/mnemonic.txt/FILENAME/g s/^version [0-9]/VERSION/ /^ FILENAME | *[0-9]* [-+]*\$/ b diffstat @@ -127,7 +127,7 @@ test_expect_success 'pull request when forgot to push' ' test_must_fail git request-pull initial $downstream_url \ 2../err ) - grep No branch of.*is at:\$ err + grep No match for commit .* err grep Are you sure you pushed err ' @@ -141,7 +141,7 @@ test_expect_success 'pull request after push' ' git checkout initial git merge --ff-only master git push origin master:for-upstream - git request-pull initial origin ../request + git request-pull initial origin master:for-upstream ../request ) sed -nf read-request.sed request digest cat digest @@ -160,7 +160,7 @@ test_expect_success 'pull request after push' ' ' -test_expect_success 'request names an appropriate branch' ' +test_expect_success 'request asks HEAD to be pulled' ' rm -fr downstream.git git init --bare downstream.git @@ -179,11 +179,11 @@ test_expect_success 'request names an appropriate branch' ' read repository read branch } digest - test $branch = tags/full + test -z $branch ' -test_expect_success 'pull request format' ' +test_expect_failure 'pull request format' ' rm -fr downstream.git git init --bare downstream.git @@ -212,8 +212,8 @@ test_expect_success 'pull request format' ' cd local git checkout initial git merge --ff-only master - git push origin master:for-upstream - git request-pull initial $downstream_url ../request + git push origin tags/full + git request-pull initial $downstream_url tags/full ../request ) request sed -nf fuzz.sed request.fuzzy test_i18ncmp expect request.fuzzy @@ -229,7 +229,7 @@ test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' ' git checkout initial git merge --ff-only master git push origin master:for-upstream
Re: [PATCH 5/5] implement @{publish} shorthand
Jeff King p...@peff.net writes: In a triangular workflow, you may have a distinct @{upstream} that you pull changes from, but publish by default (if you typed git push) to a different remote (or a different branch on the remote). It may sometimes be useful to be able to quickly refer to that publishing point (e.g., to see which changes you have that have not yet been published). This patch introduces the branch@{publish} shorthand (or @{pu} to be even shorter). It refers to the tracking branch of the remote branch to which you would push if you were to push the named branch. That's a mouthful to explain, so here's an example: $ git checkout -b foo origin/master $ git config remote.pushdefault github $ git push Signed-off-by: Jeff King p...@peff.net --- As there is no @{pu} in publish_mark() as far as I can see, and also I found it is a bit unclear what the example in the last paragraph wants to illustrate, I'll reword the above as the following before merging it to 'next'. This patch introduces the branch@{publish} shorthand that refers to the tracking branch of the remote branch to which you would push if you were to push the named branch. That's a mouthful to explain, so here's an example: $ git checkout -b foo origin/master $ git config remote.pushdefault github $ git push With this, foo@{upstream} and foo@{publish} would be origin/master and github/foo, respectively (assuming that git fetch github is configured to use refs/remotes/github/* remote-tracking branches). The implementation feels weird, like the where do we push to code should be factored out from somewhere else. I think what we're doing here is not _wrong_, but I don't like repeating what git push is doing elsewhere. And I just punt on simple as a result. :) I think we can polish that in-tree. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/9] About the trailing slashes
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: While looking at this, I found a funny behavior of fill_directory. $ git init $ mkdir b $ b/c $ b/d $ git status b Untracked files: b/ $ git status b/ Untracked files: b/c b/d Notice how the trailing slash produces different untracked listing. This is because of common_prefix_len(). In the b case, common_prefix_len() returns empty prefix, so read_directory reads top directory, traverses through, reaches b and eventually calls treat_directory() on it, which results in b/ in the output. In the b/ case, common_prefix_len() found the prefix b, so read_directory() starts at b instead of b's parent. treat_directory() is never called on b itself, which results in b/c and b/d. Hmm, this feels like a borderline case. If the user asked git status ?, or even git status '?', it seems to me that the user wanted to get git status output but with a scope limited to top-level entries with a single letter. And the unlimited output asks to consolidate output for directories that have nothing interesting. git status b and git status '[b]' are asking similar thing, but not just limiting the scope to any one letter but to 'b'. So the output you showed above looks correct to me. On the other hand, the other request git status b/ seems to me that the user is very aware that 'b' is a directory and wants to look inside, so again the output you showed looks correct to me. How does git status '[b]/' behave? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 3/3] merge-recursive: Tolerate missing file when HEAD is up to date
Brad King brad.k...@kitware.com writes: Teach add_cacheinfo to optionally tolerate make_cache_entry failure when the reason is ENOENT from lstat. Tell it to do so in the call path when the entry from HEAD is known to be up to date. It somehow feels wrong to force callers of make_cache_entry() to be so intimate with the implementation details of refresh_cache_ent() by having them inspect the errno from lstat(2) so deep in the callchain, and to force callers of make_cache_entry() that says refresh=NoThanks to pass a useless NULL. Looking at refresh_cache_ent(), I notice that we already have cases where we do not bother to lstat and instead say Yeah, the cache entry you have is good, and have to wonder if this new feature should be modeled after them instead, namely, by introducing a new option bit CE_MATCH_MISSING_OK that asks it to treat a path that is missing from the working tree as if it is checked out unmodified. -- To unsubscribe from this list: send the line 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: Globbing for ignored branches?
Jeff King p...@peff.net writes: On Fri, Jan 24, 2014 at 07:32:22PM +0100, Markus Trippelsdorf wrote: However, you do have to specify each branch individually. You probably want to say all branches except X, and you cannot currently specify a negative refspec like that. Yes, that was the question I wanted to ask (, sorry for not formulating it more clearly). Is this negative refspec for branches a feature that is planned for the future? It is something that has been talked about before, but I do not think anybody is actively working on. It would probably not be too hard a feature if you are interested in getting your feet wet in git development. :) The end result might be not so hard in the mechanical sense, but designing the interface would be hard. I do not offhand think of a good way to do this. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/9] add --gpg-sign to rebase and pull
Nicolas Vigier bo...@mars-attacks.org writes: On Fri, 24 Jan 2014, brian m. carlson wrote: This series was posted to the list some time back, but it fell through the cracks. This is a re-send of Nicolas Vigier's work with an additional patch that adds --gpg-sign to pull as well. I added my sign-off to his patches because SubmittingPatches (section (c)) seems to imply that I should, although I can rebase it out if it's a problem. Thanks! An improvement I was thinking to do on this series but had not time to do yet is to make the '--no-gpg-sign' option disable gpg signing when the commit.gpgsign config option is set to true. By the way, a configuration variable that has no way of getting overriden per invocation should not exist without a very good reason (core.bare is an example of a exception with a good reason---the bareness of the repository does not change per command invocation). An escape hatch --no-gpg-sign is a must-have requirement, not a nice-to-have improvement. Thanks for not forgetting. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote
Linus Torvalds torva...@linux-foundation.org writes: So I don't actually think anybody should need to be retrained, or always use the local:remote syntax. The local:remote syntax exists only for that special insane case where you used (the same) local:remote syntax to push out a branch under a different name. [ And yeah, maybe that behavior is more common than I think, but even if it is, such behavior would always be among people who are *very* aware of the whole local branch vs remote branch name is different situation. ] As the new default for git push would push to the same name, I agree that people who are now forced to use local:remote syntax would be the ones who know what they are doing [*1*]. So there are two remaining items, I think. - After creating a tags/for-linus signed tag and pushing it to tags/for-linus, asking request-pull to request that tag to be pulled seems to lose the tag message from the output. - Docs. [Footnote] *1* Not that it is always acceptable to break the existing users as long as they are clueful ones and they are given an escape hatch. But this time I know I won't be in the middle of firestorm like the one we had immediately after 1.6.0, as long as I keep the URL of the message I am responding to in the list archive ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] read-cache.c: Optionally tolerate missing files in make_cache_entry
Brad King brad.k...@kitware.com writes: +extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh, int refresh_flags); Why a new parameter? If refresh_flags can be ANY when refresh=NoThanks, shouldn't they be a single variable that tells the callee how the entry should be refreshed (e.g. not at all, normally, missing is ok, etc.)? +static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really, +int flags) { - return refresh_cache_ent(the_index, ce, really, NULL, NULL); + int not_new = (flags REFRESH_IGNORE_MISSING) != 0; + int cache_errno = 0; + struct cache_entry *new; + + new = refresh_cache_ent(the_index, ce, really, cache_errno, NULL); + + if(!new not_new cache_errno == ENOENT) + return ce; I think this is still one level too high in the abstraction chain. int really might be of type signed int by historical accidents, but it is unsigned int options for the underlying refresh_cache_ent(). I'd suggest renaming this to unsigned int refresh_options or something, and then define a new constatnt similar to the existing CE_MATCH_IGNORE_*. -- To unsubscribe from this list: send the line 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: Globbing for ignored branches?
Markus Trippelsdorf mar...@trippelsdorf.de writes: On 2014.01.24 at 12:00 -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Fri, Jan 24, 2014 at 07:32:22PM +0100, Markus Trippelsdorf wrote: However, you do have to specify each branch individually. You probably want to say all branches except X, and you cannot currently specify a negative refspec like that. Yes, that was the question I wanted to ask (, sorry for not formulating it more clearly). Is this negative refspec for branches a feature that is planned for the future? It is something that has been talked about before, but I do not think anybody is actively working on. It would probably not be too hard a feature if you are interested in getting your feet wet in git development. :) The end result might be not so hard in the mechanical sense, but designing the interface would be hard. I do not offhand think of a good way to do this. I don't know if the in-tree regex engine supports negative lookaheads. If it does, then something like the following should work (to use my hjl example): ^(.(?!hjl))* refspec wildcards are *NOT* regular expressions. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/9] cherry-pick, revert: add the --gpg-sign option
brian m. carlson sand...@crustytoothpaste.net writes: +-S[keyid]:: +--gpg-sign[=keyid]:: + GPG-sign commits. + Does this accept --no-gpg-sign? If not, shouldn't it? diff --git a/sequencer.c b/sequencer.c index 90cac7b..bde5f04 100644 --- a/sequencer.c +++ b/sequencer.c @@ -392,11 +392,18 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, { struct argv_array array; int rc; + char *gpg_sign; argv_array_init(array); argv_array_push(array, commit); argv_array_push(array, -n); + if (opts-gpg_sign) { + gpg_sign = xmalloc(3 + strlen(opts-gpg_sign)); + sprintf(gpg_sign, -S%s, opts-gpg_sign); + argv_array_push(array, gpg_sign); + free(gpg_sign); + } Here you would need to invent a new way to propagate --no-gpg-sign to subsequent invocations, I think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Globbing for ignored branches?
Jeff King p...@peff.net writes: I had imagined a not token at the front of the refspec, like: git fetch origin +refs/heads/*:refs/remotes/origin/* ^refs/heads/foo In this case, a colon in the refspec would be an error. An alternative would be: git fetch origin +refs/heads/*:refs/remotes/origin/* refs/heads/foo: I.e., to say put foo to nowhere. But generally refspecs do not affect each other. Not really. You do not have to view it as 'not refs/heads/foo' is affecting the previous '+refs/heads/*:refs/remotes/origin/*'. You can think of two refspecs refs/heads/foo refs/heads/bar are both affecting the end result; so far we only had a single way for multiple refspecs to affect the end result and that was a union. Introducing subtract as another mode of combining is not too bad, I would think, at the conceptual level. ... Making the null destination work differently might be confusing. I tend to agree that refs/heads/foo: is being too cute and may be confusing, at least if it will be the only way to express this in the end-user-facing UI. Even some people were confused enough on a very sensible push nothing to ref means deletion to make us add another explicit way, push --delete, to ask for the same thing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/9] cherry-pick, revert: add the --gpg-sign option
brian m. carlson sand...@crustytoothpaste.net writes: On Fri, Jan 24, 2014 at 01:00:06PM -0800, Junio C Hamano wrote: brian m. carlson sand...@crustytoothpaste.net writes: +-S[keyid]:: +--gpg-sign[=keyid]:: + GPG-sign commits. + Does this accept --no-gpg-sign? If not, shouldn't it? It does not. I took Nicolas's patches from the list and applied them to master, so nothing from next is there, including the commit.gpgsign stuff. Would you prefer I rebased them on next instead? Not really. It is debatable if it should mean that the user wants to sign commits that are created by running other commands like am and stash when he sets commit.gpgsign to true, but even if the answer to that question were true, the configuration must be overridable with e.g. git stash --no-gpg-sign, explicitly from the command line. Until that happens, the series with 2af2ef3c (Add the commit.gpgsign option to sign all commits, 2013-11-05) cannot be merged to 'master'. A series that lets you specify positives from the command line without any sticky configuration variable, i.e. these patches, do not have to wait for that to happen. So this series should come first and then the commit.gpgsign ones can be rebased on top of this series, I would think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] implement @{publish} shorthand
Ramkumar Ramachandra artag...@gmail.com writes: On that note, can you hold off graduating jk/branch-at-publish-rebased, Junio? Hopefully, I'll come up with a replacement over the weekend. Sure. This close to the feature freeze, I'd rather see all contributors, not limited to you, not rush on new and shiny things, but instead spend time looking at bugs and fixes proposed for the upcoming release in the codepaths they were involved. The send-email SSL issue $gmane/240479 is one of the things I'd like to see your sanity-checking ;-) -- To unsubscribe from this list: send the line 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] repack.c: chmod +w before rename()
Torsten Bögershausen tbo...@web.de writes: Another solution could be to do the chmod +x in mingw_rename(). This may be done in another commit, because a) It improves git gc only when Git for Windows is used on the client machine b) Windows refuses to delete a file when the file is read-only. So setting a file to read-only under Windows is a way for a user to protect it from being deleted. Changing the behaviour of rename() globally may not be what we want. But doesn't this affect non Windows platforms in negative ways? We unconditionally run stat(2) and chmod(2) unnecessarily, and we leave the resulting file writable when it originally may have been marked read-only on purpose. Also, it feels to me that doing this in mingw_rename() the right approach in the first place. If the user wants git mv a b to rename a in the working tree and if that path is read-only, what happens, and what should happen? Does chmod -w on a file in the working tree mean I want to make sure nobody accidentally writes to it, and also I want to protect it from being renamed or deleted? So perhaps mingw_rename() can be taught to - First try to just do rename(), and if it succeeds, happily return without doing anything extra. - If it fails, then - check if the failure is due to read-only-ness (optional: if you cannot reliably tell this from the error code, do not bother), and if not, return failure. - otherwise, stat() to grab the current permission bits, do the chmod(), retry the rename, then restore the permission bits with another chmod(). Return failure if any of these steps fail. That way, it can cover any call to rename(), be it for packfiles or a path in the working tree, and you would need to pay the overhead of stat/chmod only when it matters, no? Reported-by: Jochen Haag zwanzi...@googlemail.com Signed-off-by: Torsten Bögershausen tbo...@web.de --- builtin/repack.c | 4 1 file changed, 4 insertions(+) diff --git a/builtin/repack.c b/builtin/repack.c index ba66c6e..033b4c2 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -324,6 +324,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix) statbuffer.st_mode = ~(S_IWUSR | S_IWGRP | S_IWOTH); chmod(fname_old, statbuffer.st_mode); } + if (!stat(fname, statbuffer)) { + statbuffer.st_mode |= (S_IWUSR | S_IWGRP | S_IWOTH); + chmod(fname, statbuffer.st_mode); + } if (rename(fname_old, fname)) die_errno(_(renaming '%s' failed), fname_old); free(fname); -- 1.9.rc0.143.g6fd479e -- -- To unsubscribe from this list: send the line 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] repack.c: chmod +w before rename()
Johannes Schindelin johannes.schinde...@gmx.de writes: In any case, I'd rather change the permissions only when the rename failed. *And* I feel uncomfortable ignoring the return value... Good judgement I'd agree with 100%. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to substructure rewrites?
David Kastrup d...@gnu.org writes: As it can easily be guessed, the add xxx function commits are basically adding not-yet-used code (and so will not disrupt compilation), but everything starting with Reorganize blame data structures up until the final commit will not work or compile since the code does not match the data structures. So there is little point in substructing all that, right? Even something seemingly isolated like commit f64b41c472442ae9971321fe8f62c3885ba4d8b7 Author: David Kastrup d...@gnu.org Date: Sun Jan 19 02:16:21 2014 +0100 blame.c: Let output determine MORE_THAN_ONE_PATH more efficiently is not really useful as a separate commit since while it does implement a particular task, this is done starting with non-working code relying on no-longer existent data structures. Small pieces that are incrementally added with their own documentation would certainly be a lot easier to read than one big ball of wax. I am wondering if it would make it easier for everybody to tentatively do git-blame vs git-blame2 dance here, just like we did git-blame vs git-annotate dance some years ago. That is, to add a completely new command and have them in parallel while cooking in 'next' (or we could even keep them in a few releases if we are not absolutely certain about the correctness of the result of the new code), aiming to eventually retire the current implementation and replace it with the new one. We have already have test infrastructure to allow us to run variants of blames, too, to help that kind of transition. In general, the rule is likely any commit should not create a non-working state right? Yes. -- To unsubscribe from this list: send the line 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] send-email: If the ca path is not specified, use the defaults
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: This change could introduce a regression for people on a platform whose certificate directory is /etc/ssl/certs but its IO::Socket:SSL somehow fails to use it as SSL_ca_path without being told. I can confirm that my git-send-email doesn't regress to the pre-35035bbf state; my certificate directory is /etc/ssl/certs. I'm somewhat surprised that IO::Socket::SSL picks the right file/ directory on every platform without being told explicitly. This change definitely looks like the right fix. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] doc: remote author/documentation sections from more pages
Michael Haggerty mhag...@alum.mit.edu writes: On 01/27/2014 01:15 AM, Eric Sunshine wrote: On Sun, Jan 26, 2014 at 6:43 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Subject: [PATCH] doc: remote author/documentation sections from more pages s/remote/remove/ Gaah! Git is a virus that invades your muscle memory and prevents you from typing words that are similar to git subcommands. Thanks for noticing. Michael Thanks; s/remote/remove/ will be done on this end; no need to resend. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] setup: Don't dereference in-tree symlinks for absolute paths
Martin Erik Werner martinerikwer...@gmail.com writes: In order to manipulate symliks in the work tree using absolute paths, symlinks should only be dereferenced outside the work tree. I agree 100% with this reasoning (modulo s/symliks/symlinks/). As to the implementation, it looks a bit overly complicated, though. I haven't tried writing the same myself, but * I suspect that strbuf would help simplifying the allocation and deallocation; * Also I suspect that use of string-list to split and then join is making the code unnecessarily complex. In Python/Perl, that would be a normal approach, but in C, it would be a lot simpler if you prepare a writable temporary in wtpart[], walk from left to right finding '/' and replacing it temporarily with NUL to terminate in order to check with real_path(), restore the NUL back to '/' to check deeper, and rinse and repeat. Having said that, I am not absolutely sure if the repeated calls to real_path() are doing the right thing, though ;-) diff --git a/setup.c b/setup.c index 5432a31..0789a96 100644 --- a/setup.c +++ b/setup.c @@ -22,11 +22,51 @@ char *prefix_path_gently(const char *prefix, int len, const char *orig = path; char *sanitized; if (is_absolute_path(orig)) { - const char *temp = real_path(path); - sanitized = xmalloc(len + strlen(temp) + 1); - strcpy(sanitized, temp); + int i, match; + size_t wtpartlen; + char *npath, *wtpart; + struct string_list list = STRING_LIST_INIT_DUP; + const char *work_tree = get_git_work_tree(); + if (!work_tree) + return NULL; + npath = xmalloc(strlen(path) + 1); if (remaining_prefix) *remaining_prefix = 0; + if (normalize_path_copy_len(npath, path, remaining_prefix)) { + free(npath); + return NULL; + } + + string_list_split(list, npath, '/', -1); + wtpart = xmalloc(strlen(npath) + 1); + i = 0; + match = 0; + strcpy(wtpart, list.items[i++].string); + strcat(wtpart, /); + if (strcmp(real_path(wtpart), work_tree) == 0) { + match = 1; + } else { + while (i list.nr) { + strcat(wtpart, list.items[i++].string); + if (strcmp(real_path(wtpart), work_tree) == 0) { + match = 1; + break; + } + strcat(wtpart, /); + } + } + string_list_clear(list, 0); + if (!match) { + free(npath); + free(wtpart); + return NULL; + } + + wtpartlen = strlen(wtpart); + sanitized = xmalloc(strlen(npath) - wtpartlen); + strcpy(sanitized, npath + wtpartlen + 1); + free(npath); + free(wtpart); } else { sanitized = xmalloc(len + strlen(path) + 1); if (len) @@ -34,26 +74,10 @@ char *prefix_path_gently(const char *prefix, int len, strcpy(sanitized + len, path); if (remaining_prefix) *remaining_prefix = len; - } - if (normalize_path_copy_len(sanitized, sanitized, remaining_prefix)) - goto error_out; - if (is_absolute_path(orig)) { - size_t root_len, len, total; - const char *work_tree = get_git_work_tree(); - if (!work_tree) - goto error_out; - len = strlen(work_tree); - root_len = offset_1st_component(work_tree); - total = strlen(sanitized) + 1; - if (strncmp(sanitized, work_tree, len) || - (len root_len sanitized[len] != '\0' sanitized[len] != '/')) { - error_out: + if (normalize_path_copy_len(sanitized, sanitized, remaining_prefix)) { free(sanitized); return NULL; } - if (sanitized[len] == '/') - len++; - memmove(sanitized, sanitized + len, total - len); } return sanitized; } diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 3a0677a..03a12ac 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -190,7 +190,7 @@ test_expect_success SYMLINKS 'real path works on symlinks' ' test $sym = $(test-path-utils real_path $dir2/syml) ' -test_expect_failure SYMLINKS 'prefix_path works with work tree symlinks' ' +test_expect_success SYMLINKS 'prefix_path works with work tree symlinks' '
Re: [PATCH 3/3] builtin/blame.c: large-scale rewrite
David Kastrup d...@gnu.org writes: The previous implementation uses a sorted linear list of struct blame_entry in a struct scoreboard for organizing all partial or completed work. Every task that is done requires going through the whole list where most entries are not relevant to the task at hand. This commit reorganizes the data structures in order to have each remaining subtask work with its own sorted linear list it can work off front to back. Subtasks are organized into struct origin chains hanging off particular commits. Commits are organized into a priority queue, processing them in commit date order in order to keep most of the work affecting a particular blob collated even in the presence of an extensive merge history. In that manner, linear searches can be basically avoided anywhere. They still are done for identifying one of multiple analyzed files in a given commit, but the degenerate case of a single large file being assembled from a multitude of smaller files in the past is not likely to occur often enough to warrant special treatment. --- Sign-off? Actually, I'd like to take my previous suggestion to add this as blame2 (to replace blame in the future) back. That was based on my fear/hope to see an implementation based on a completely different approach, but the basic premise of having one blame_entry record per the lines of the final image in the scoreboard, using diff between parents to the child to find common lines for passing the blame up, etc. have not changed at all and the change is all about organizing the pieces of data in a *much* *better* way to avoid needlessly finding what we already have computed. It does look big (and if you removed those #if 0/#endif, the final patch would be a lot bigger because we will see a lot of deleted lines), but I do not think the code conceptually is a different implementation to warrant create a totally new blame2 and let it sit next to the original. Looks nice overall. diff --git a/builtin/blame.c b/builtin/blame.c index ead6148..994a9e8 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -18,7 +18,9 @@ #include cache-tree.h #include string-list.h #include mailmap.h +#include mergesort.h #include parse-options.h +#include prio-queue.h #include utf8.h #include userdiff.h #include line-range.h @@ -83,11 +85,43 @@ static unsigned blame_copy_score; */ struct origin { int refcnt; + /* Record preceding blame record for this blob */ struct origin *previous; + /* origins are put in a list linked via `next' hanging off the + * corresponding commit's util field in order to make finding + * them fast. The presence in this chain does not count + * towards the origin's reference count. It is tempting to + * let it count as long as the commit is pending examination, + * but even under circumstances where the commit will be + * present multiple times in the priority queue of unexamined + * commits, processing the first instance will not leave any + * work requiring the origin data for the second instance. An + * interspersed commit changing that would have to be + * preexisting with a different ancestry and with the same + * commit date in order to wedge itself between two instances + * of the same commit in the priority queue _and_ produce + * blame entries relevant for it. While we don't want to let + * us get tripped up by this case, it certainly does not seem + * worth optimizing for. + */ Style; please make /* and */ sit on their own line without anything else in multi-line comments. + struct origin *next; struct commit *commit; + /* `suspects' contains blame entries that may be attributed to + * this origin's commit or to parent commits. When a commit + * is being processed, all suspects will be moved, either by + * assigning them to an origin in a different commit, or by + * shipping them to the scoreboard's ent list because they + * cannot be attributed to a different commit. + */ + struct blame_entry *suspects; mmfile_t file; unsigned char blob_sha1[20]; unsigned mode; + /* shipped gets set when shipping any suspects to the final + * blame list instead of other commits + */ + char shipped; Unused? + char guilty; char path[FLEX_ARRAY]; }; @@ -176,10 +210,23 @@ static inline struct origin *origin_incref(struct origin *o) static void origin_decref(struct origin *o) { if (o --o-refcnt = 0) { + struct origin *p, *l = NULL; if (o-previous) origin_decref(o-previous); free(o-file.ptr); - free(o); + /* Should be present exactly once in commit chain */ + for (p = o-commit-util; p; l = p, p = p-next) { + if (p == o) +
Re: [PATCH] tree_entry_interesting: match against all pathspecs
Duy Nguyen pclo...@gmail.com writes: Ack. Perhaps this on top to verify it -- 8 -- diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh index af5134b..d9f37c3 100755 --- a/t/t4010-diff-pathspec.sh +++ b/t/t4010-diff-pathspec.sh @@ -110,4 +110,17 @@ test_expect_success 'diff-tree -r with wildcard' ' test_cmp expected result ' +test_expect_success 'diff multiple wildcard pathspecs' ' + mkdir path2 + echo rezrov path2/file1 + git update-index --add path2/file1 + tree3=`git write-tree` + git diff --name-only $tree $tree3 -- path2*1 path1*1 actual + cat EOF expect +path1/file1 +path2/file1 +EOF + test_cmp expect actual +' + test_done -- 8 -- Thanks, both. Will queue. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/4] read-cache.c: Refactor --ignore-missing implementation
Brad King brad.k...@kitware.com writes: Move lstat ENOENT handling from refresh_index to refresh_cache_ent and activate it with a new CE_MATCH_IGNORE_MISSING option. This will allow other call paths into refresh_cache_ent to use the feature. Signed-off-by: Brad King brad.k...@kitware.com --- Good! I forgot that we had update-index --ignore-missing --refresh, and that is conceptually the thing you want to use in your perform merge-recursive in an empty tree while pretending that the working tree is fully populated and up-to-date scenario. cache.h | 2 ++ read-cache.c | 8 +--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/cache.h b/cache.h index c9efe88..c96ada7 100644 --- a/cache.h +++ b/cache.h @@ -498,6 +498,8 @@ extern void *read_blob_data_from_index(struct index_state *, const char *, unsig #define CE_MATCH_RACY_IS_DIRTY 02 /* do stat comparison even if CE_SKIP_WORKTREE is true */ #define CE_MATCH_IGNORE_SKIP_WORKTREE04 +/* ignore non-existent files during stat update */ +#define CE_MATCH_IGNORE_MISSING 0x08 extern int ie_match_stat(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); extern int ie_modified(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); diff --git a/read-cache.c b/read-cache.c index 33dd676..d61846c 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1031,6 +1031,7 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate, int changed, size; int ignore_valid = options CE_MATCH_IGNORE_VALID; int ignore_skip_worktree = options CE_MATCH_IGNORE_SKIP_WORKTREE; + int ignore_missing = options CE_MATCH_IGNORE_MISSING; if (ce_uptodate(ce)) return ce; @@ -1050,6 +1051,8 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate, } if (lstat(ce-name, st) 0) { + if (ignore_missing errno == ENOENT) + return ce; if (err) *err = errno; return NULL; @@ -1127,7 +1130,8 @@ int refresh_index(struct index_state *istate, unsigned int flags, int ignore_submodules = (flags REFRESH_IGNORE_SUBMODULES) != 0; int first = 1; int in_porcelain = (flags REFRESH_IN_PORCELAIN); - unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0; + unsigned int options = ((really ? CE_MATCH_IGNORE_VALID : 0) | + (not_new ? CE_MATCH_IGNORE_MISSING : 0)); const char *modified_fmt; const char *deleted_fmt; const char *typechange_fmt; @@ -1176,8 +1180,6 @@ int refresh_index(struct index_state *istate, unsigned int flags, if (!new) { const char *fmt; - if (not_new cache_errno == ENOENT) - continue; if (really cache_errno == EINVAL) { /* If we are doing --really-refresh that * means the index is not valid anymore. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] builtin/blame.c: large-scale rewrite
David Kastrup d...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: David Kastrup d...@gnu.org writes: The previous implementation uses a sorted linear list of struct blame_entry in a struct scoreboard for organizing all partial or completed work. Every task that is done requires going through the whole list where most entries are not relevant to the task at hand. This commit reorganizes the data structures in order to have each remaining subtask work with its own sorted linear list it can work off front to back. Subtasks are organized into struct origin chains hanging off particular commits. Commits are organized into a priority queue, processing them in commit date order in order to keep most of the work affecting a particular blob collated even in the presence of an extensive merge history. In that manner, linear searches can be basically avoided anywhere. They still are done for identifying one of multiple analyzed files in a given commit, but the degenerate case of a single large file being assembled from a multitude of smaller files in the past is not likely to occur often enough to warrant special treatment. --- Sign-off? Not while this is not fit for merging because of #if 0 etc and missing functionality. This is just for review. That is not what Signing off a patch means, though ;-) Actually, I'd like to take my previous suggestion to add this as blame2 (to replace blame in the future) back. That was based on my fear/hope to see an implementation based on a completely different approach, but the basic premise of having one blame_entry record per the lines of the final image in the scoreboard, using diff between parents to the child to find common lines for passing the blame up, etc. have not changed at all and the change is all about organizing the pieces of data in a *much* *better* way to avoid needlessly finding what we already have computed. Yes. Basically, the call graph outside of blame.c itself should be pretty much the same. ... Ok. I'm also certain to have a few space between function name and arguments left and will grep for those before submitting the next version. Next version will at least include -M option, possibly leaving -C for later. OK. As we do not want to break the build in the middle of the series, but we still want to keep the individual steps reviewable incrementally, I would think the best structure would be have the series that consists of multiple steps the basic infrastructure is there, but no rename following, and neither -M or -C works, now renames are followed, now -M works, etc., with tests that are not yet working (in the beginning, most of git blame test may no longer work until the series is finished) marked with s/test_expect_success/test_expect_failure/ and turn expect_failure into expect_success as the series advances. That way, we may get a better picture of what each step achieves. I dunno. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/17] Add interpret-trailers builtin
Is this from the same Christian? The series seems to have unusually high rate of style violations compared to the usual submission, like these: ERROR: open brace '{' following function declarations go on the next line #78: FILE: trailer.c:44: +static size_t alnum_len(const char *buf, size_t len) { ERROR: trailing statements should be on next line #79: FILE: trailer.c:45: + while (--len = 0 !isalnum(buf[len])); ERROR: space required before the open parenthesis '(' #70: FILE: trailer.c:90: + switch(arg_tok-conf-if_exist) { WARNING: braces {} are not necessary for any arm of this statement #66: FILE: trailer.c:270: + if (!strcasecmp(doNothing, value)) { [...] + } else if (!strcasecmp(add, value)) { [...] + } else [...] ERROR: that open brace { should be on the previous line #96: FILE: trailer.c:300: + for (previous = NULL, item = first_conf_item; +item; +previous = item, item = item-next) + { Just trying to see if there is an impersonation ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 17/17] Documentation: add documentation for 'git interpret-trailers'
Christian Couder chrisc...@tuxfamily.org writes: +'git interpret-trailers' [--trim-empty] [--infile=file] [token[=value]...] Would it be more consistent with existing documentation to format this as so? [--infile=file] [token[=value]]... No, it would be very inconsistent: $ grep '\.\.\.\]' *.txt | wc -l 103 $ grep '\]\.\.\.' *.txt | wc -l 0 I have a feeling that you are missing the point Eric is making. The value given to the --infile option can be anything, i.e. 'file' there is a placeholder, hence --infile=file not --infile=file as you wrote. Also I think [token[=value]]... is the correct way to spell that there can be 0 or more token[=value]. token[=value] in the original does not make any sense, as is meant to say this thing is a placeholder, and we do not try to say, with the string inside , what shape that placeholder takes. In fact '=' part is _not_ a placeholder but is required syntactically when you want to supply a value to the token, so the original doubly is incorrect. I find it a bad taste to allow unbound set of token on the LHS of '=' on the command line, but that is a separate issue in the design, not in the documentation of the design. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v1.9-rc1
the object. This led callers to weird inconsistencies. (merge 663a856 cc/replace-object-info later to maint). * git cat-file --batch=, an admittedly useless command, did not behave very well. (merge 6554dfa jk/cat-file-regression-fix later to maint). * git rev-parse revs -- paths did not implement the usual disambiguation rules the commands in the git log family used in the same way. (merge 62f162f jk/rev-parse-double-dashes later to maint). * git mv A B/, when B does not exist as a directory, should error out, but it didn't. (merge c57f628 mm/mv-file-to-no-such-dir-with-slash later to maint). * A workaround to an old bug in glibc prior to glibc 2.17 has been retired; this would remove a side effect of the workaround that corrupts system error messages in non-C locales. * SSL-related options were not passed correctly to underlying socket layer in git send-email. (merge 5508f3e tr/send-email-ssl later to maint). * git commit -v appends the patch to the log message before editing, and then removes the patch when the editor returned control. However, the patch was not stripped correctly when the first modified path was a submodule. (merge 1a72cfd jl/commit-v-strip-marker later to maint). * git fetch --depth=0 was a no-op, and was silently ignored. Diagnose it as an error. (merge 5594bca nd/transport-positive-depth-only later to maint). * Remote repository URL expressed in scp-style host:path notation are parsed more carefully (e.g. foo/bar:baz is local, [::1]:/~user asks to connect to user's home directory on host at address ::1. (merge a2036d7 tb/clone-ssh-with-colon-for-port later to maint). * git diff -- ':(icase)makefile' was unnecessarily rejected at the command line parser. (merge 887c6c1 nd/magic-pathspec later to maint). * git cat-file --batch-check=ok did not check the existence of the named object. (merge 4ef8d1d sb/sha1-loose-object-info-check-existence later to maint). * git am --abort sometimes complained about not being able to write a tree with an 0{40} object in it. (merge 77b43ca jk/two-way-merge-corner-case-fix later to maint). * Two processes creating loose objects at the same time could have failed unnecessarily when the name of their new objects started with the same byte value, due to a race condition. (merge b2476a6 jh/loose-object-dirs-creation-race later to maint). Changes since v1.9-rc0 are as follows: Alexander Shopov (4): git-gui i18n: Initial glossary in Bulgarian git-gui l10n: Add 29 more terms to glossary git-gui i18n: Added Bulgarian translation gitk: Add Bulgarian translation (304t) Andy Spencer (1): tree_entry_interesting: match against all pathspecs Anthony Baire (1): subtree: fix argument validation in add/pull/push Astril Hayato (1): gitk: Comply with XDG base directory specification Erik Faye-Lund (2): prefer xwrite instead of write mingw: remove mingw_write Jeff King (18): fetch-pack: do not filter out one-level refs interpret_branch_name: factor out upstream handling interpret_branch_name: rename cp variable to at interpret_branch_name: always respect namelen parameter interpret_branch_name: avoid @{upstream} past colon interpret_branch_name: find all possible @-marks diff_filespec: reorder dirty_submodule macro definitions diff_filespec: drop funcname_pattern_ident field diff_filespec: drop xfrm_flags field diff_filespec: reorder is_binary field diff_filespec: use only 2 bits for is_binary flag t/perf: time rev-list with UNINTERESTING commits list-objects: only look at cmdline trees with edge_hint repack: fix typo in max-pack-size option repack: make parsed string options const-correct repack: propagate pack-objects options as strings t7501: fix empty commit test with NO_PERL t7700: do not use touch unnecessarily Johannes Sixt (1): Makefile: Fix compilation of Windows resource file John Keeping (3): completion: complete merge-base options completion: handle --[no-]fork-point options to git-rebase Makefile: remove redundant object in git-http{fetch,push} Jonathan Nieder (3): gitignore doc: add global gitignore to synopsis git-gui: chmod +x po2msg, windows/git-gui.sh gitk: chmod +x po2msg.sh Junio C Hamano (6): Documentation: exclude irrelevant options from git pull Documentation: git pull does not have the -m option revision: mark contents of an uninteresting tree uninteresting revision: propagate flag bits from tags to pointees Documentation: make it easier to maintain enumerated documents Git 1.9-rc1 Marc Branchaud (1): gitk: Replace next and prev buttons with down and up arrows Max Kirillov (2): git-gui: fallback right pane
What's cooking in git.git (Jan 2014, #05; Mon, 27)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The tip of 'master' is at 1.9-rc1; as far as new features are concerned, this is pretty much it until the final. On the maintenance track, we may have another 1.8.5.x maintenance release before 1.9 final is tagged in 2-3 weeks. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * ab/subtree-doc (2014-01-13) 1 commit (merged to 'next' on 2014-01-22 at e352c75) + subtree: fix argument validation in add/pull/push * ef/mingw-write (2014-01-17) 2 commits (merged to 'next' on 2014-01-22 at b9ddab2) + mingw: remove mingw_write + prefer xwrite instead of write * jc/maint-pull-docfix (2014-01-14) 2 commits (merged to 'next' on 2014-01-22 at 2e62ef4) + Documentation: git pull does not have the -m option + Merge branch 'jc/maint-pull-docfix-for-409b8d82' into jc/maint-pull-docfix (this branch uses jc/maint-pull-docfix-for-409b8d82.) * jc/revision-range-unpeel (2014-01-15) 2 commits (merged to 'next' on 2014-01-22 at ab2a159) + revision: propagate flag bits from tags to pointees + revision: mark contents of an uninteresting tree uninteresting git log --left-right A...B lost the leftness of commits reachable from A when A is a tag as a side effect of a recent bugfix. This is a regression in 1.8.4.x series. * jk/allow-fetch-onelevel-refname (2014-01-15) 1 commit (merged to 'next' on 2014-01-22 at 15089b1) + fetch-pack: do not filter out one-level refs git clone would fail to clone from a repository that has a ref directly under refs/, e.g. refs/stash, because different validation paths do different things on such a refname. Loosen the client side's validation to allow such a ref. * jk/complete-merge-base (2014-01-13) 2 commits (merged to 'next' on 2014-01-22 at 91c428d) + completion: handle --[no-]fork-point options to git-rebase + completion: complete merge-base options * jk/diff-filespec-cleanup (2014-01-17) 5 commits (merged to 'next' on 2014-01-22 at fb26e43) + diff_filespec: use only 2 bits for is_binary flag + diff_filespec: reorder is_binary field + diff_filespec: drop xfrm_flags field + diff_filespec: drop funcname_pattern_ident field + diff_filespec: reorder dirty_submodule macro definitions * jk/interpret-branch-name-fix (2014-01-15) 5 commits (merged to 'next' on 2014-01-22 at f113c68) + interpret_branch_name: find all possible @-marks + interpret_branch_name: avoid @{upstream} past colon + interpret_branch_name: always respect namelen parameter + interpret_branch_name: rename cp variable to at + interpret_branch_name: factor out upstream handling (this branch is used by jk/branch-at-publish-rebased.) Fix a handful of bugs around interpreting $branch@{upstream} notation and its lookalike, when $branch part has interesting characters, e.g. @, and :. * jk/mark-edges-uninteresting (2014-01-21) 2 commits (merged to 'next' on 2014-01-22 at d8807c4) + list-objects: only look at cmdline trees with edge_hint + t/perf: time rev-list with UNINTERESTING commits Fix to regression in v1.8.4.x and later. * jk/test-fixes (2014-01-23) 2 commits (merged to 'next' on 2014-01-23 at 6515089) + t7700: do not use touch unnecessarily + t7501: fix empty commit test with NO_PERL * jn/ignore-doc (2014-01-16) 1 commit (merged to 'next' on 2014-01-22 at a07ac63) + gitignore doc: add global gitignore to synopsis Explicitly list $HOME/.config/git/ignore as one of the places you can use to keep ignore patterns that depend on your personal choice of tools, e.g. *~ for Emacs users. * mh/attr-macro-doc (2014-01-14) 1 commit (merged to 'next' on 2014-01-22 at e3ed767) + gitattributes: document more clearly where macros are allowed * mh/retire-ref-fetch-rules (2014-01-14) 1 commit (merged to 'next' on 2014-01-22 at 753b1f6) + refname_match(): always use the rules in ref_rev_parse_rules Code simplification. * mh/safe-create-leading-directories (2014-01-21) 17 commits (merged to 'next' on 2014-01-22 at ad38e73) + rename_tmp_log(): on SCLD_VANISHED, retry + rename_tmp_log(): limit the number of remote_empty_directories() attempts + rename_tmp_log(): handle a possible mkdir/rmdir race + rename_ref(): extract function rename_tmp_log() + remove_dir_recurse(): handle disappearing files and directories + remove_dir_recurse(): tighten condition for removing unreadable dir + lock_ref_sha1_basic(): if locking fails with ENOENT, retry + lock_ref_sha1_basic(): on SCLD_VANISHED, retry + safe_create_leading_directories(): add new error value SCLD_VANISHED + cmd_init_db(): when creating directories, handle errors conservatively + safe_create_leading_directories(): introduce enum for return values +
Re: [PATCH v2 0/9] About the trailing slashes
Thanks; will try to rebase on top of more recent codebase and then review. -- To unsubscribe from this list: send the line 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: [ANNOUNCE] Git v1.9-rc0
Jeff King p...@peff.net writes: On Thu, Jan 23, 2014 at 10:15:33AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: Junio, since you prepare such tarballs[1] anyway for kernel.org, it might be worth uploading them to the Releases page of git/git. I imagine there is a programmatic way to do so via GitHub's API, but I don't know offhand. I can look into it if you are interested. I already have a script that takes the three tarballs and uploads them to two places, so adding GitHub as the third destination should be a natural and welcome way to automate it. I came up with the script below, which you can use like: ./script v1.8.2.3 git-1.8.2.3.tar.gz It expects the tag to already be pushed up to GitHub. I'll leave sticking it on the todo branch and integrating it into RelUpload to you. This can also be used to backfill the old releases (though I looked on k.org and it seems to have only partial coverage). It sets the prerelease flag for -rc releases, but I did not otherwise fill in any fields, including the summary and description. GitHub seems to display reasonably if they are not set. Thanks. -- 8 -- #!/bin/sh # # usage: $0 tag tarball repo=git/git # replace this with however you store your oauth token # if you don't have one, make one here: # https://github.com/settings/tokens/new token() { pass -n github.web.oauth Hmph, what is this pass thing? } post() { curl -H Authorization: token $(token) $@ } # usage: create tag-name create() { case $1 in *-rc*) prerelease=true ;; *) prerelease=false ;; esac post -d ' { tag_name: '$1', prerelease: '$prerelease' }' https://api.github.com/repos/$repo/releases; } # use: upload release-id filename upload() { url=https://uploads.github.com/repos/$repo/releases/$1/assets; url=$url?name=$(basename $2) post -H Content-Type: $(file -b --mime-type $2) \ --data-binary @$2 \ $url } # This is a hack. If you don't mind a dependency on # perl's JSON (or another parser), we can do a lot better. extract_id() { perl -lne '/id:\s*(\d+)/ or next; print $1; exit 0' } create $1 release.json id=$(extract_id release.json) upload $id $2 /dev/null rm -f release.json -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/9] git-sh-setup.sh: add variable to use the stuck-long mode
brian m. carlson sand...@crustytoothpaste.net writes: From: Nicolas Vigier bo...@mars-attacks.org If the variable $OPTIONS_STUCKLONG is not empty, then rev-parse option parsing is done in --stuck-long mode. Signed-off-by: Nicolas Vigier bo...@mars-attacks.org Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- contrib/examples/git-checkout.sh | 1 + contrib/examples/git-clean.sh| 1 + contrib/examples/git-clone.sh| 1 + contrib/examples/git-merge.sh| 1 + contrib/examples/git-repack.sh | 1 + Hmmm, especially given that nobody is exercising the ``updated'' code even in our testsuite, is it a good idea to touch these? The STUCKLONG never existed back when they were scripted. It just adds noise when trying to view them for hints, no? diff --git a/git-sh-setup.sh b/git-sh-setup.sh index fffa3c7..5f28b32 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -72,6 +72,8 @@ if test -n $OPTIONS_SPEC; then parseopt_extra= [ -n $OPTIONS_KEEPDASHDASH ] parseopt_extra=--keep-dashdash + [ -n $OPTIONS_STUCKLONG ] + parseopt_extra=$parseopt_extra --stuck-long eval $( echo $OPTIONS_SPEC | -- To unsubscribe from this list: send the line 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 9/9] pull: add the --gpg-sign option.
brian m. carlson sand...@crustytoothpaste.net writes: git merge already allows us to sign commits, and git rebase has recently learned how to do so as well. Teach git pull to parse the -S/--gpg-sign option and pass this along to merge or rebase, as appropriate. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- git-pull.sh | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/git-pull.sh b/git-pull.sh index 0a5aa2c..4164dac 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -138,6 +138,15 @@ do --no-verify-signatures) verify_signatures=--no-verify-signatures ;; + --gpg-sign|-S) + gpg_sign_args=-S + ;; + --gpg-sign=*) + gpg_sign_args=-S${1#--gpg-sign=} + ;; Here, $1 is taken from the end-user without any extra quoting... + -S*) + gpg_sign_args=-S${1#-S} + ;; --d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run) dry_run=--dry-run ;; @@ -305,11 +314,13 @@ merge_name=$(git fmt-merge-msg $log_arg $GIT_DIR/FETCH_HEAD) || exit case $rebase in true) eval=git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity + eval=$eval $gpg_sign_args ... but here it is used as if it is properly quoted so that later eval $eval will take it as a single argument. git pull --gpg-sign='foo bar' will probably ask the command to use 'foo' as the signer key id, with 'bar' as an extra, unknown token on the command line of the underlying 'git merge', I suspect. A git rev-parse --sq-quote in the earlier hunk may be all it takes to fix it. Thanks. eval=$eval --onto $merge_head ${oldremoteref:-$merge_head} ;; *) eval=git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only - eval=$eval $log_arg $strategy_args $merge_args $verbosity $progress + eval=$eval $log_arg $strategy_args $merge_args $verbosity $progress + eval=$eval $gpg_sign_args eval=$eval \\$merge_name\ HEAD $merge_head ;; esac -- To unsubscribe from this list: send the line 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 8/9] rebase: add the --gpg-sign option
brian m. carlson sand...@crustytoothpaste.net writes: diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 43c19e0..73d32dd 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -181,7 +181,7 @@ exit_with_patch () { git rev-parse --verify HEAD $amend warn You can amend the commit now, with warn - warn git commit --amend + warn git commit --amend $gpg_sign_opt I think this is meant to be cut--pasted, so you may have to quote the RHS of --gpg-sign=key (or the key part of -Skey). The same comment as the one for 'git pull' patch applies around 'eval's in the remainder of the patch. @@ -248,7 +248,8 @@ pick_one () { test -d $rewritten pick_one_preserving_merges $@ return - output eval git cherry-pick $strategy_args $empty_args $ff $@ + output eval git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} \ + $strategy_args $empty_args $ff $@ } [rest snipped] Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] diff: turn skip_stat_unmatch on selectively
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: skip_stat_unmatch flag is added in fb13227 (git-diff: squelch empty diffs - 2007-08-03) to ignore empty diffs caused by stat-only dirtiness. In some diff case, stat is not involved at all. While the code is written in a way that no expensive I/O is done, we still need to move all file pairs from the old queue to the new queue in diffcore_skip_stat_unmatch(). Only enable it when worktree is involved: diff and diff rev. This should help track down how skip_stat_unmatch is actually used when bugs occur. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- This replaces 'diff: turn off skip_stat_unmatch on diff --cached' The previous patch obviously leaves skip_stat_unmatch on in diff rev rev and maybe other cases. Oops, I lost track. Sorry. -- To unsubscribe from this list: send the line 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] repack: add `repack.honorpackkeep` config var
Jeff King p...@peff.net writes: The git-repack command always passes `--honor-pack-keep` to pack-objects. This has traditionally been a good thing, as we do not want to duplicate those objects in a new pack, and we are not going to delete the old pack. ... Note that this option just disables the pack-objects behavior. We still leave packs with a .keep in place, as we do not necessarily know that we have duplicated all of their objects. Signed-off-by: Jeff King p...@peff.net --- Intended for the jk/pack-bitmap topic. Two comments. - It seems that this adds a configuration variable that cannot be countermanded from the command line. It has to come with either a very good justification in the documentation describing why it is a bad idea to even allow overriding from the command line in a repository that sets it, or a command line option to let the users override it. I personally prefer the latter, because that will be one less thing for users to remember (i.e. usually you can override the configured default from the command line, but this variable cannot be because of these very good reasons). - In the context of pack-objects, the name --honor-pack-keep makes sense; it is understood that pack-objects will _not_ remove kept packfile, so honoring can only mean do not attempt to pick objects out of kept packs to add to the pack being generated. and there is no room for --no-honor-pack-keep to be mistaken as you canremove the ones marked to be kept after saving the still-used objects in it away. But does the same name make sense in the context of repack? Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse: Check argc before using argv[i+1]
David Sharp dhsh...@google.com writes: @@ -738,9 +740,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, --resolve-git-dir)) { - const char *gitdir = resolve_gitdir(argv[i+1]); + if (++i = argc) + die(--resolve-git-dir requires an argument); + const char *gitdir = resolve_gitdir(argv[i]); if (!gitdir) - die(not a gitdir '%s', argv[i+1]); + die(not a gitdir '%s', argv[i]); This adds decl-after-statement. As referencing argv[argc] is not a crime (you will grab a NULL), how about doing it this way to see if the value is a NULL, instead of comparing the index and argc? const char *gitdir; if (!argv[++i]) die(--resolve-git-dir requires an argument); gitdir = resolve_gitdir(argv[i]); if (!gitdir) die(not a gitdir '%s', argv[i]); Same comment may apply to other hunks. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] rev-parse: Check argc before using argv[i+1]
David Sharp dhsh...@google.com writes: Without this patch, git-rev-parse --prefix, --default, or --resolve-git-dir, without a value argument, would result in a segfault. Instead, die() with a message. When I sent the review message, I actually was on the fence between checking i vs argc and checking the nullness myself. I realize, after seeing the actual patch below, that we are protecting against picking up a NULL and blindly passing it on in the codepaths that follow, so the updated code does look a lot better, at least to me. Thanks. Signed-off-by: David Sharp dhsh...@google.com --- builtin/rev-parse.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index aaeb611..45901df 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -547,15 +547,17 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, --default)) { - def = argv[i+1]; - i++; + def = argv[++i]; + if (!def) + die(--default requires an argument); continue; } if (!strcmp(arg, --prefix)) { - prefix = argv[i+1]; + prefix = argv[++i]; + if (!prefix) + die(--prefix requires an argument); startup_info-prefix = prefix; output_prefix = 1; - i++; continue; } if (!strcmp(arg, --revs-only)) { @@ -738,9 +740,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, --resolve-git-dir)) { - const char *gitdir = resolve_gitdir(argv[i+1]); + const char *gitdir = argv[++i]; if (!gitdir) - die(not a gitdir '%s', argv[i+1]); + die(--resolve-git-dir requires an argument); + gitdir = resolve_gitdir(gitdir); + if (!gitdir) + die(not a gitdir '%s', argv[i]); puts(gitdir); continue; } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] combine-diff: Optimize combine_diff_path sets intersection
Kirill Smelkov k...@mns.spb.ru writes: diff --git a/combine-diff.c b/combine-diff.c index 3b92c448..98c2562 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -15,8 +15,8 @@ ... + while (1) { ... + if (cmp 0) { + if (pprev) + pprev-next = p-next; + ptmp = p; + p = p-next; + free(ptmp); + if (curr == ptmp) + curr = p; continue; ... + if (cmp 0) { + i++; + continue; } ... + + pprev = p; + p = p-next; + i++; } return curr; } Thanks. I very much like the approach. I was staring at the above part of the code, but couldn't help recalling this gem (look for understand pointers in the article): http://meta.slashdot.org/story/12/10/11/0030249/linus-torvalds-answers-your-questions How about doing it this way (on top of your patch)? It reduces 7 lines even though it adds two comment lines ;-) combine-diff.c | 37 +++-- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index 2d79312..0809e79 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -15,11 +15,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent) { struct diff_queue_struct *q = diff_queued_diff; - struct combine_diff_path *p, *pprev, *ptmp; + struct combine_diff_path *p, **tail = curr; int i, cmp; if (!n) { - struct combine_diff_path *list = NULL, **tail = list; for (i = 0; i q-nr; i++) { int len; const char *path; @@ -43,35 +42,30 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, *tail = p; tail = p-next; } - return list; + return curr; } /* -* NOTE paths are coming sorted here (= in tree order) +* paths in curr (linked list) and q-queue[] (array) are +* both sorted in the tree order. */ - - pprev = NULL; - p = curr; i = 0; + while ((p = *tail) != NULL) { + cmp = ((i = q-nr) + ? -1 : strcmp(p-path, q-queue[i]-two-path)); - while (1) { - if (!p) - break; - - cmp = (i = q-nr) ? -1 - : strcmp(p-path, q-queue[i]-two-path); if (cmp 0) { - if (pprev) - pprev-next = p-next; - ptmp = p; - p = p-next; - free(ptmp); - if (curr == ptmp) - curr = p; + /* p-path not in q-queue[]; drop it */ + struct combine_diff_path *next = p-next; + + if ((*tail = next) != NULL) + tail = next-next; + free(p); continue; } if (cmp 0) { + /* q-queue[i] not in p-path; skip it */ i++; continue; } @@ -80,8 +74,7 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, p-parent[n].mode = q-queue[i]-one-mode; p-parent[n].status = q-queue[i]-status; - pprev = p; - p = p-next; + tail = p-next; i++; } return curr; -- To unsubscribe from this list: send the line 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 3/3] diff: turn skip_stat_unmatch on selectively
Junio C Hamano gits...@pobox.com writes: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: skip_stat_unmatch flag is added in fb13227 (git-diff: squelch empty diffs - 2007-08-03) to ignore empty diffs caused by stat-only dirtiness. In some diff case, stat is not involved at all. While the code is written in a way that no expensive I/O is done, we still need to move all file pairs from the old queue to the new queue in diffcore_skip_stat_unmatch(). Only enable it when worktree is involved: diff and diff rev. This should help track down how skip_stat_unmatch is actually used when bugs occur. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- This replaces 'diff: turn off skip_stat_unmatch on diff --cached' The previous patch obviously leaves skip_stat_unmatch on in diff rev rev and maybe other cases. Oops, I lost track. Sorry. Together with {1,2}/3 applied on maint-1.8.4, this sems to break t3417 (there may be others, but I didn't have time to check). -- To unsubscribe from this list: send the line 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] repack.c: Use move_temp_to_file() instead of rename()
Torsten Bögershausen tbo...@web.de writes: In a1bbc6c0 a shell command mv -f was replaced with the rename() function. Use move_temp_to_file() from sha1_file.c instead of rename(). This is in line with the handling of other Git internal tmp files, and calls adjust_shared_perm() Signed-off-by: Torsten Bögershausen tbo...@web.de --- Thanks for all comments. I haven't been able to reproduce the problem here. Tips and information how to reproduce it are wellcome. I think this patch makes sense to support core.sharedRepository(), but I haven't made a test case for the pack/idx files. Jochen, doess this patch fix your problem, or do we need another patch on top of this? Turning the call to rename(2) in the last hunk in your patch to move_temp_to_file() may be an improvement, but the other changes are wrong, I think. There are two big differences between the rename(2) used here and move_temp_to_file(). You need to understand what move_temp_to_file() is from its name. - It is meant to move a temporary file we create ourselves, and the most important characteristic of such a file is that we do not create it read-only. The function uses platform's rename(2) and returns a failure if the platform's rename(2) fails, but that is not a problem even on Windows because of this. - When returning a failure, it unlinks the temporary file. This unlinking is necessary to clean up after our half-done mess. The other two calls to rename(2) you changed in the patch are *not* about turning a newly created temporary file into the final one. They are about moving existing packfiles away just in case we fail in the later phases of the command so that they can be moved back to their original name to restore the state before we started, and actually moving them back to their original name. We do not want to remove these files. The platform's rename(2) must do the right thing in these two calls and there is no guarantee these existing packfiles are read-write. Earlier, I said that turning the call to rename(2) in the last hunk in your patch to move_temp_to_file() may be an improvement, because I thought that it is a good thing that the latter makes a call to adjust_shared_perm(). But after a closer inspection, I no longer think that hunk is an improvement. These new packfiles were created by pack-objects, which finishes each packfile it produces by calling finish_tmp_packfile(), and that is where adjust_shared_perm() happens already. As far as pack-objects that was called from repack is concerned, these new packfiles are not temporary; they are finished product. It may be OK to remove them as part of rewind back to the original state, as a later phase of repack failed if we saw a failure (but note that the original git-repack.sh didn't), but a plain vanilla rename(2) without any frills is what we want to happen to them. builtin/repack.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index ba66c6e..4b6d663 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -271,7 +271,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (unlink(fname_old)) failed = 1; - if (!failed rename(fname, fname_old)) { + if (!failed move_temp_to_file(fname, fname_old)) { free(fname); failed = 1; break; @@ -288,7 +288,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) char *fname, *fname_old; fname = mkpathdup(%s/%s, packdir, item-string); fname_old = mkpath(%s/old-%s, packdir, item-string); - if (rename(fname_old, fname)) + if (move_temp_to_file(fname_old, fname)) string_list_append(rollback_failure, fname); free(fname); } @@ -324,7 +324,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) statbuffer.st_mode = ~(S_IWUSR | S_IWGRP | S_IWOTH); chmod(fname_old, statbuffer.st_mode); } - if (rename(fname_old, fname)) + if (move_temp_to_file(fname_old, fname)) die_errno(_(renaming '%s' failed), fname_old); free(fname); free(fname_old); -- 1.8.5.2 -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Makefile: add cppcheck target
Elia Pinto gitter.spi...@gmail.com writes: Add cppcheck target to Makefile. Cppcheck is a static analysis tool for C/C++ code. Cppcheck primarily detects the types of bugs that the compilers normally do not detect. It is an useful target for doing QA analysis. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- Makefile |6 ++ config.mak.in |1 + 2 files changed, 7 insertions(+) diff --git a/Makefile b/Makefile index dddaf4f..1d25a70 100644 --- a/Makefile +++ b/Makefile @@ -2602,3 +2602,9 @@ cover_db: coverage-report cover_db_html: cover_db cover -report html -outputdir cover_db_html cover_db +### cppcheck static coverage analysis +# +.PHONY: cppcheck + +cppcheck: + cppcheck --enable=all -q $(top_srcdir) Why isn't this .? In other words, why is the change to config.mak.in even necessary? diff --git a/config.mak.in b/config.mak.in index e6a6d0f..86b95fb 100644 --- a/config.mak.in +++ b/config.mak.in @@ -22,3 +22,4 @@ docdir = @docdir@ mandir = @mandir@ htmldir = @htmldir@ +top_srcdir = @top_srcdir@ -- To unsubscribe from this list: send the line 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] archive.c: reduce scope of variables
Elia Pinto gitter.spi...@gmail.com writes: Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- Either the patch is whitespace damaged during the mail transport, or you are incorrectly indenting the lines with all spaces. archive.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/archive.c b/archive.c index 346f3b2..49b79f8 100644 --- a/archive.c +++ b/archive.c @@ -112,7 +112,6 @@ static int write_archive_entry(const unsigned char *sha1, const char *base, write_archive_entry_fn_t write_entry = c-write_entry; struct git_attr_check check[2]; const char *path_without_prefix; - int err; args-convert = 0; strbuf_reset(path); @@ -132,6 +131,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base, } if (S_ISDIR(mode) || S_ISGITLINK(mode)) { +int err; if (args-verbose) fprintf(stderr, %.*s\n, (int)path.len, path.buf); err = write_entry(args, sha1, path.buf, path.len, mode); @@ -319,7 +319,6 @@ static int parse_archive_args(int argc, const char **argv, const char *output = NULL; int compression_level = -1; int verbose = 0; - int i; int list = 0; int worktree_attributes = 0; struct option opts[] = { @@ -366,6 +365,7 @@ static int parse_archive_args(int argc, const char **argv, base = ; if (list) { +int i; for (i = 0; i nr_archivers; i++) if (!is_remote || archivers[i]-flags ARCHIVER_REMOTE) printf(%s\n, archivers[i]-name); -- To unsubscribe from this list: send the line 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 3/3] diff: turn skip_stat_unmatch on selectively
Duy Nguyen pclo...@gmail.com writes: On Tue, Jan 28, 2014 at 02:51:45PM -0800, Junio C Hamano wrote: This replaces 'diff: turn off skip_stat_unmatch on diff --cached' The previous patch obviously leaves skip_stat_unmatch on in diff rev rev and maybe other cases. Oops, I lost track. Sorry. Together with {1,2}/3 applied on maint-1.8.4, this sems to break t3417 (there may be others, but I didn't have time to check). My bad. I thought I covered all cases in my last patch (and didn't retest it!). It turns out I should have set skip_stat_unmatch in builtin_diff_b_f() too. This on top of 3/3 passes the tests Thanks, will squash it in. This however shows that the existing test *KNEW* that it was enough to check just a few cases (especially, there is no reason to make sure that blob vs file-in-working-tree case behaves sanely), because the auto-refresh would kick in for all codepaths. Now you are making that assumption invalid, shouldn't the patch also split the tests to cover individual cases? -- 8 -- diff --git a/builtin/diff.c b/builtin/diff.c index 88542d9..8ab5e3d 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -89,6 +89,7 @@ static int builtin_diff_b_f(struct rev_info *revs, if (blob[0].mode == S_IFINVALID) blob[0].mode = canon_mode(st.st_mode); + revs-diffopt.skip_stat_unmatch = !!diff_auto_refresh_index; stuff_change(revs-diffopt, blob[0].mode, canon_mode(st.st_mode), blob[0].sha1, null_sha1, -- 8 -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 17/17] Documentation: add documentation for 'git interpret-trailers'
Christian Couder chrisc...@tuxfamily.org writes: I find it a bad taste to allow unbound set of token on the LHS of '=' on the command line, but that is a separate issue in the design, not in the documentation of the design. I don't understand this sentence, sorry. It is a bad design taste to structure the command line argument in such a way that it takes git cmd xyzzy=frotz nitfol=rezrov some other args where these 'xyzzy', 'nitfol', etc. form an unbound set. It prevents future enhancements to the command from allowing anything that contain '=' in some other args part. Allowing not just '=' but also ':' makes it even worse. But these are issues in the design itself. Not the issue in the patch 17/17 that is trying to document the design. This patch under discussion documents the bad design correctly ;-) -- To unsubscribe from this list: send the line 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: C standard compliance?
David Kastrup d...@gnu.org writes: Hi, I am wondering if I may compare pointers with that have been created using different calls of malloc. The C standard does not allow this (inequalities are only allowed for pointers into the same structure) to allow for some cheapskate sort of comparison in segmented architectures. Hmm... if you were to implement a set of pointers in such a way that you can cheaply tell if an unknown pointer belongs to that set, you would use a hashtable, keyed with something that is derived from the value of the pointer casted to uintptr_t, I would think. Is such a use of ((uintptr_t)ptr) unallowed? If it is allowed, comparing two unrelated pointers after casting them to uintptr_t would equally be valid, I would have to think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Jan 2014, #06; Wed, 29)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The second release candidate is expected to happen this weekend. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * bc/gpg-sign-everywhere (2014-01-27) 9 commits - pull: add the --gpg-sign option. - rebase: add the --gpg-sign option - rebase: parse options in stuck-long mode - rebase: don't try to match -M option - rebase: remove useless arguments check - am: add the --gpg-sign option - am: parse options in stuck-long mode - git-sh-setup.sh: add variable to use the stuck-long mode - cherry-pick, revert: add the --gpg-sign option Teach --gpg-sign option to many commands that create commits. Changes to some scripted Porcelains use unsafe variable substitutions and need to be tightened. Waiting for a reroll. * ds/rev-parse-required-args (2014-01-28) 1 commit - rev-parse: check i before using argv[i] against argc git rev-parse --default without the required option argument did not diagnose it as an error. Will merge to 'next'. * jk/config-path-include-fix (2014-01-28) 2 commits - handle_path_include: don't look at NULL value - expand_user_path: do not look at NULL path include.path variable (or any variable that expects a path that can use ~username expansion) in the configuration file is not a boolean, but the code failed to check it. Will merge to 'next'. * jk/repack-honor-pack-keep (2014-01-28) 1 commit - repack: add `repack.honorpackkeep` config var (this branch uses jk/pack-bitmap.) Optionally allow git repack to include objects that exist in kept packs in newly created packfiles. Waiting for response to review comments. * nd/submodule-pathspec-ending-with-slash (2014-01-27) 8 commits - clean: use cache_name_is_other() - clean: replace match_pathspec() with dir_path_match() - Pass directory indicator to match_pathspec_item() - match_pathspec: match pathspec foo/ against directory foo - dir.c: prepare match_pathspec_item for taking more flags - Rename match_pathspec_depth() to match_pathspec() - Convert some match_pathspec_depth() to dir_path_match() - Convert some match_pathspec_depth() to ce_path_match() Allow git cmd path/, when the 'path' is where a submodule is bound to the top-level working tree, to match 'path', despite the extra and unnecessary trailing slash. Will merge to 'next'. -- [Stalled] * jk/color-for-more-pagers (2014-01-17) 4 commits - pager: disable colors for some known-bad configurations - DONOTMERGE: needs matching change to git-sh-setup - setup_pager: set MORE=R - setup_pager: refactor LESS/LV environment setting 'more' implementation of BSD wants to be told with MORE=R environment before it shows colored output, while 'more' on some other platforms will die when seeing MORE=R environment. It appears that we are coming to the consensus that trying to be too intimately knowledgeable about quirks of various pager implementations on different platforms is a losing proposition. Waiting for a reroll. * po/everyday-doc (2014-01-27) 1 commit - Make 'git help everyday' work This may make the said command to emit something, but the source is not meant to be formatted into a manual pages to begin with, and also its contents are a bit stale. It may be a good first step in the right direction, but needs more work to at least get the mark-up right before public consumption. Will hold. * jk/branch-at-publish-rebased (2014-01-17) 5 commits - t1507 (rev-parse-upstream): fix typo in test title - implement @{publish} shorthand - branch_get: provide per-branch pushremote pointers - branch_get: return early on error - sha1_name: refactor upstream_mark Give an easier access to the tracking branches from other side in a triangular workflow by introducing B@{publish} that works in a similar way to how B@{upstream} does. Will hold. * rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits - merge: drop unused arg from abort_commit method signature - merge: make prepare_to_commit responsible for write_merge_state - t7505: ensure cleanup after hook blocks merge - t7505: add missing Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that run during git merge. The log message stresses too much on one hook, prepare-commit-msg, but it would equally apply to other hooks like post-merge, I think. Waiting for a reroll. * jl/submodule-recursive-checkout (2013-12-26) 5 commits - Teach checkout to recursively checkout submodules - submodule: teach unpack_trees() to update submodules - submodule: teach unpack_trees() to repopulate submodules - submodule: teach unpack_trees() to remove submodule contents -
Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote
Junio C Hamano gits...@pobox.com writes: So there are two remaining items, I think. - After creating a tags/for-linus signed tag and pushing it to tags/for-linus, asking request-pull to request that tag to be pulled seems to lose the tag message from the output. - Docs. [Footnote] *1* Not that it is always acceptable to break the existing users as long as they are clueful ones and they are given an escape hatch. But this time I know I won't be in the middle of firestorm like the one we had immediately after 1.6.0, as long as I keep the URL of the message I am responding to in the list archive ;-) I am not yet doing the docs, but here is a minimal (and I think is the most sensible) fix to the If I asked a tag to be pulled, I used to get the message from the tag in the output---the updated code no longer does so problem. With this fix, the updates to t5150 I queued on top of the two patches can lose a test_expect_failure. I would not be surprised if there are other regressions, though [*1*]. I am worried about regressions when the user explicitly asks a ref to be pulled---e.g the command refuses to produce output and instead fails (perhaps because the ambiguity check is overly stricter than it should be), or the command produces output that is different from what we used to produce (this patch is a fix to the problem in that latter category, but there may be other differences the existing tests are not covering). [Footnote] *1* No, I do not count I used to be able to ask 'master' (or implicitly 'HEAD' that I happen to be sitting on) to be pulled and rely on that the command figures out that I have that commit on 'for-linus' in my publish repository, but that feature was removed as a regression. Removing that cleverness is the point of this series. -- 8 -- Subject: [PATCH] request-pull: pick up tag message as before The previous two steps were meant to stop promoting the explicit refname the user gave to the command to a different ref that points at it. Most notably, we no longer substitute a branch name the user used with a name of the tqag that points at the commit at the tip of the branch. However, they also lost the code that included the message in a tag when the user _did_ ask the tag to be pulled. Resurrect it. Signed-off-by: Junio C Hamano gits...@pobox.com --- git-request-pull.sh | 8 1 file changed, 8 insertions(+) diff --git a/git-request-pull.sh b/git-request-pull.sh index c8ab0e9..93b4135 100755 --- a/git-request-pull.sh +++ b/git-request-pull.sh @@ -132,6 +132,14 @@ for you to fetch changes up to %H: ' $headrev +if test $(git cat-file -t $head) = tag +then + git cat-file tag $head | + sed -n -e '1,/^$/d' -e '/^-BEGIN PGP /q' -e p + echo + echo +fi + if test -n $branch_name then echo (from the branch description for $branch_name local branch) -- 1.9-rc1-183-g614c158 -- To unsubscribe from this list: send the line 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 3/3] diff: turn skip_stat_unmatch on selectively
Duy Nguyen pclo...@gmail.com writes: On Thu, Jan 30, 2014 at 2:25 AM, Junio C Hamano gits...@pobox.com wrote: On Tue, Jan 28, 2014 at 02:51:45PM -0800, Junio C Hamano wrote: This however shows that the existing test *KNEW* that it was enough to check just a few cases (especially, there is no reason to make sure that blob vs file-in-working-tree case behaves sanely), because the auto-refresh would kick in for all codepaths. Now you are making that assumption invalid, shouldn't the patch also split the tests to cover individual cases? Drop the last patch, then. It's a while at there cleanup patch. If it's non trivial then it could be taken up later... I am leaning towards that because... ... not sure I'll go through diff.c to identify and write tests for all cases. ... the effort to ensure the correctness of the patch itself involves the same identification of the cases. We know the single place skip-stat-unmatch was assigned used to cover all cases, and the patch was to stop covering cases the unnecessary assignments are made while making sure the resulting code still covers cases that assignments are necessary. -- To unsubscribe from this list: send the line 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 jn/pager-lv-default-env] pager test: make fake pager consume all its input
Jonathan Nieder jrnie...@gmail.com writes: Otherwise there is a race: if 'git log' finishes writing before the pager terminates and closes the pipe, all is well, and if the pager finishes quickly enough then 'git log' terminates with SIGPIPE. died of signal 13 at /build/buildd/git-1.9~rc1/t/test-terminal.perl line 33. not ok 6 - LESS and LV envvars are set for pagination Noticed on Ubuntu PPA builders, where the race was lost about half the time. Compare v1.7.0.2~6^2 (tests: Fix race condition in t7006-pager, 2010-02-22). Reported-by: Anders Kaseorg ande...@mit.edu Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Thanks. The use of wc looks cute (I would have used cat). Anders Kaseorg wrote: On 01/06/2014 09:14 PM, Jonathan Nieder wrote: + PAGER=env pager-env.out + export PAGER + + test_terminal git log [...] On the Ubuntu PPA builders, I’m seeing this new test fail with SIGPIPE about half the time: died of signal 13 at /build/buildd/git-1.9~rc1/t/test-terminal.perl line 33. not ok 6 - LESS and LV envvars are set for pagination Good catch. Sorry for the trouble. t/t7006-pager.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index 7fe3367..b9365b4 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -40,7 +40,7 @@ test_expect_failure TTY 'pager runs from subdir' ' test_expect_success TTY 'LESS and LV envvars are set for pagination' ' ( sane_unset LESS LV - PAGER=env pager-env.out + PAGER=env pager-env.out; wc export PAGER test_terminal git log -- To unsubscribe from this list: send the line 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: Having Git follow symlinks
Peter Krefting pe...@softwolves.pp.se writes: ...if I have the time, maybe I can come up with a patch. There is already some hacks in the core.symlinks setting, so I guess it should be possible. That is totally unrelated. The variable only says on this platform and/or filesystem, you cannot symlink(2), so instead create a regular file with the symlink contents when checking out a symlink blob. Most importantly, the variable does not change the fact that symbolic links are explicitly tracked without dereferencing. If you have, under a directory foo/, a symbolic link foo/bar that points at ../baz, a directory foo/baz/, and you have a file at foo/baz/hello: 1. git add foo and git add foo/bar will add foo/bar as a symlink; and 2. git add foo/bar/hello is an error. The variable does not have any interactions with the logic to make sure 2. errors out correctly, so the presense of it does not imply anything. -- To unsubscribe from this list: send the line 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] Cleanly redefine (v)snprintf when needed.
Benoit Sigoure tsuna...@gmail.com writes: When we detect that vsnprintf / snprintf are broken, we #define them to an alternative implementation. On OS X, stdio.h already #define's them, which causes a warning to be issued at the point we re-define them in `git-compat-util.h'. --- Makes perfect sense. Please sign-off your patch (see Documentation/SubmittingPatches). Thanks. git-compat-util.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index cbd86c3..614a5e9 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -480,9 +480,15 @@ extern FILE *git_fopen(const char*, const char*); #endif #ifdef SNPRINTF_RETURNS_BOGUS +#ifdef snprintf +#undef snprintf +#endif #define snprintf git_snprintf extern int git_snprintf(char *str, size_t maxsize, const char *format, ...); +#ifdef vsnprintf +#undef vsnprintf +#endif #define vsnprintf git_vsnprintf extern int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap); -- To unsubscribe from this list: send the line 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] add: don't complain when adding empty project root
Torsten Bögershausen tbo...@web.de writes: But, look at https://www.kernel.org/pub/software/scm/git/docs/git-add.html This page seems to need an update too, and I wonder why: a) The makefile did'nt re-generate html even if it should have b) That page is not owned or updated by the git.git maintainer c) Any other reason? Sorry, but if I understand correctly, k.org these days requires each file to be GPG signed and uploaded individually (i.e. there is no way as far as I can tell to say here is a tarball, and I've even signed it with my key to convince you that it is from me. Please accept it and then unpack the contents there). There is no way I'd do that every time I update git-htmldocs repository for 500+ files. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v1.9-rc2
A release candidate Git v1.9-rc2 is now available for testing at the usual places. I've heard rumours that various third-party tools do not like the two-digit version numbers (e.g. Git 2.0) and started barfing left and right when the users install v1.9-rc1. While it is tempting to laugh at them for their sloppy assumption, I am also practical and do not mind calling the upcoming release v1.9.0 to help them. If we go that route (and I am inclined to go that route at this moment), the versioning scheme will be - The next release candidate will be v1.9.0-rc3, not v1.9-rc3; - The first maintenance release for v1.9.0 will be v1.9.1 (and Nth one be v1.9.N); and - The feature release after v1.9.0 will be either v1.10.0 or v2.0.0, depending on how big the feature jump we are looking at. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: c21bb5f8a138a5af9d330ce7289f5c533d49e2a2 git-1.9.rc2.tar.gz dec30e4b187c84c08c02d53aa5d5d4d3fae9bbda git-htmldocs-1.9.rc2.tar.gz 207e1e91357eed351b5b7cd4a7d2cc8985cd git-manpages-1.9.rc2.tar.gz The following public repositories all have a copy of the v1.9-rc2 tag and the master branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v1.9 Release Notes (draft) == Backward compatibility notes git submodule foreach $cmd $args used to treat $cmd $args the same way ssh did, concatenating them into a single string and letting the shell unquote. Careless users who forget to sufficiently quote $args gets their argument split at $IFS whitespaces by the shell, and got unexpected results due to this. Starting from this release, the command line is passed directly to the shell, if it has an argument. Read-only support for experimental loose-object format, in which users could optionally choose to write in their loose objects for a short while between v1.4.3 to v1.5.3 era, has been dropped. The meanings of --tags option to git fetch has changed; the command fetches tags _in addition to_ what are fetched by the same command line without the option. The way git push $there $what interprets $what part given on the command line, when it does not have a colon that explicitly tells us what ref at the $there repository is to be updated, has been enhanced. A handful of ancient commands that have long been deprecated are finally gone (repo-config, tar-tree, lost-found, and peek-remote). Backward compatibility notes (for Git 2.0) -- When git push [$there] does not say what to push, we have used the traditional matching semantics so far (all your branches were sent to the remote as long as there already are branches of the same name over there). In Git 2.0, the default will change to the simple semantics, which pushes: - only the current branch to the branch with the same name, and only when the current branch is set to integrate with that remote branch, if you are pushing to the same remote as you fetch from; or - only the current branch to the branch with the same name, if you are pushing to a remote that is not where you usually fetch from. Use the user preference configuration variable push.default to change this. If you are an old-timer who is used to the matching semantics, you can set the variable to matching to keep the traditional behaviour. If you want to live in the future early, you can set it to simple today without waiting for Git 2.0. When git add -u (and git add -A) is run inside a subdirectory and does not specify which paths to add on the command line, it will operate on the entire tree in Git 2.0 for consistency with git commit -a and other commands. There will be no mechanism to make plain git add -u behave like git add -u .. Current users of git add -u (without a pathspec) should start training their fingers to explicitly say git add -u . before Git 2.0 comes. A warning is issued when these commands are run without a pathspec and when you have local changes outside the current directory, because the behaviour in Git 2.0 will be different from today's version in such a situation. In Git 2.0, git add path will behave as git add -A path, so that git add dir/ will notice paths you removed from the directory and record the removal. Versions before Git 2.0, including this release, will keep ignoring removals, but the users who rely on this behaviour are encouraged to start using git add --ignore-removal path now before 2.0 is released. The default prefix for git svn will change in Git 2.0. For a long time, git svn created its remote-tracking branches directly under refs/remotes, but it will place
[RFC/PATCH] howto/maintain-git.txt: new version numbering scheme
We wanted to call the upcoming release Git 1.9, with its maintenance track being Git 1.9.1, Git 1.9.2, etc., but various third-party tools are reported to assume that there are at least three dewey-decimal components in our version number. Adjust the plan so that vX.Y.0 are feature releases while vX.Y.Z (Z 0) are maintenance releases. Signed-off-by: Junio C Hamano gits...@pobox.com --- * Haven't committed to this outline, but I am raising a weather-balloon to see reaction from the list. Comments? Documentation/howto/maintain-git.txt | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Documentation/howto/maintain-git.txt b/Documentation/howto/maintain-git.txt index 33ae69c..ca43787 100644 --- a/Documentation/howto/maintain-git.txt +++ b/Documentation/howto/maintain-git.txt @@ -39,26 +39,26 @@ The policy on Integration is informally mentioned in A Note from the maintainer message, which is periodically posted to this mailing list after each feature release is made. - - Feature releases are numbered as vX.Y.Z and are meant to + - Feature releases are numbered as vX.Y.0 and are meant to contain bugfixes and enhancements in any area, including functionality, performance and usability, without regression. - One release cycle for a feature release is expected to last for eight to ten weeks. - - Maintenance releases are numbered as vX.Y.Z.W and are meant - to contain only bugfixes for the corresponding vX.Y.Z feature - release and earlier maintenance releases vX.Y.Z.V (V W). + - Maintenance releases are numbered as vX.Y.Z and are meant + to contain only bugfixes for the corresponding vX.Y.0 feature + release and earlier maintenance releases vX.Y.W (W Z). - 'master' branch is used to prepare for the next feature release. In other words, at some point, the tip of 'master' - branch is tagged with vX.Y.Z. + branch is tagged with vX.Y.0. - 'maint' branch is used to prepare for the next maintenance - release. After the feature release vX.Y.Z is made, the tip + release. After the feature release vX.Y.0 is made, the tip of 'maint' branch is set to that release, and bugfixes will accumulate on the branch, and at some point, the tip of the - branch is tagged with vX.Y.Z.1, vX.Y.Z.2, and so on. + branch is tagged with vX.Y.1, vX.Y.2, and so on. - 'next' branch is used to publish changes (both enhancements and fixes) that (1) have worthwhile goal, (2) are in a fairly @@ -86,6 +86,10 @@ this mailing list after each feature release is made. users are encouraged to test it so that regressions and bugs are found before new topics are merged to 'master'. +Note that before v1.9.0 release, the version numbers used to be +structured slightly differently. vX.Y.Z were feature releases while +vX.Y.Z.W were maintenance releases for vX.Y.Z. + A Typical Git Day - -- To unsubscribe from this list: send the line 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] init-db.c: honor case on case preserving fs
Torsten Bögershausen tbo...@web.de writes: On 2014-02-01 10.14, Reuben Hawkins wrote: Most case-insensitive filesystems are case-preserving. In these filesystems (such as HFS+ on OS X) you can name a file Filename.txt, then rename the file to FileName.txt. That file will be accessible by both filenames, but the case is otherwise honored. We don't want to have git ignore case on these case-preserving filesystem implementations. Yes, we want. Because the file system will treat Filename.txt and FileName.txt the same. Another important thing to remember is that we cannot have these two files at the same time on such a filesystem. Somebody may have Filename.txt in the commit at the tip of the history, you clone/fetch and check it out, and you will have Filename.txt with the original contents. We do not try to corrupt the filename on core.ignorecase filesystem by any canonicalization. But then you may edit that file, and you either deliberately or without knowing (because some of your tools do this behind your back) may end up saving the result as FileName.txt. What happens? When we ask what is the contents of Filename.txt now? (using the original name still in the index) to the underlying system, we will be given what you placed in FileName.txt. We won't see You do not have Filename.txt, but you now have FileName.txt. And that is the behaviour the end users (of not Git, but of a platform with such a filesystem) do expect from their tools. They do not want to see You no longer have Filename.txt, and you have a new file FileName.txt. Now think what git add Filename.txt should do at that point? It should not say I was told to add Filename.txt, but there is no such file, so I'll add nothing. If you run git add -u Filename.txt, it should not say I was told to add Filename.txt, but there is no such file, so I'll remove existing Filename.txt from the index. It must pick up the updated contents from your new FileName.txt, update the index entry Filename.txt, and the next git commit must record it as an update to the same file. If you are on the other hand trying to correct an earlier mistake of having named the file Filename.txt but you now want to rename it FileName.txt, the above behaviour by core.ignorecase may make it a bit cumbersome to do. You can first remove it from the index and then re-add it, I would think, as a workaround. Having to do a workaround is unfortunate but it is an unavoidable consequence of having to choose between the two and having to pick one. Most of the time you do not want such a rename (or rather, the loss of the file Filename.txt and the creation of the unrelated FileName.txt) and a change from Filename.txt to FileName.txt is most likely to be a mistake in the platform tool that mucked with the files on your filesystem, so we choose to make it easy for the user not to be disturbed by such a change. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: splitting a commit that adds new files
Duy Nguyen pclo...@gmail.com writes: I usually start splitting a commit with reset @^ then add -p back. The problem is reset @^ does not keep track of new files added in HEAD, so I often end up forgetting to add new files back (with add -p). I'm thinking of making reset to do add -N automatically for me so I won't miss changes in add -p. But maybe people already know how to deal with this case without adding more code? Is reset -p what you are looking for? I do not use that myself, 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] fast-import.c: always honor the filename case
Jeff King p...@peff.net writes: [+cc Joshua Jensen, who wrote 50906e0] On Sun, Feb 02, 2014 at 07:13:04AM -0600, Reuben Hawkins wrote: fast-import should not use strncmp_icase. I am not sure of that. My gut feeling is that core.ignorecase is completely about the _filesystem_, and that git should generally be case-sensitive internally. I agree; if squashing mixed cases into a single canonical one is desired in one use case (like Joshua described in $gmane/200597), that should have been done as an optional feature, not by default (ideally, it should probably be done in a filter between exporters and the fast-import, I would think). [1] I am mostly trying to connect people on various sides of the discussion here. So take my gut feeling above with a grain of salt, as it does not come from experience nor thinking too hard about the issue. Thanks; that is exactly what is expected of project elders ;-) -- To unsubscribe from this list: send the line 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 9/9] pull: add the --gpg-sign option.
brian m. carlson sand...@crustytoothpaste.net writes: ... It also happens to fix the issue where the help text is improperly quoted. With your suggested fix, it is now quoted (ugly, but quoted): I do not see anything ugly about the output below. Of course you could do -S'brian ...', but both look sensible. Stopped at aba3d3ff83b59627adbdafe1b334a46ed5b7ec17... am: add the --gpg-sign option You can amend the commit now, with git commit --amend '-Sbrian m. carlson sand...@crustytoothpaste.net' Since I expect most users are going to use -S, either because they have a key specifically specified in .gitconfig, or because the default key is the right thing anyway, I don't see this as a huge problem. I think I'll probably end up fixing it anyway and then send out the reroll. -- To unsubscribe from this list: send the line 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: Running make rpm fails on a CentOS 6.3 machine
Todd Zullinger t...@pobox.com writes: I know the Fedora/EPEL spec file and what's in git.git have grown apart a good bit, unfortunately. That's the cost of having a spec file that is meant to work across a very wide array of RPM-based systems, I guess. The Fedora/EPEL spec file is fairly specific to the Fedora/EPEL build tools (mock is the primary build tool). Hmph. Once it gets to the point where our spec file is so out of sync with anybody's RPM based distribution in the real world, I suspect that removing and not shipping it in tree might be more helpful to the users. Or at least have a set of pointers, one for each major RPM based distribution, where to obtain spec file more tailored for their platform, at the beginning of it, or something. Have we already reached that point? Does anybody still use 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: splitting a commit that adds new files
Jeff King p...@peff.net writes: [1] I _do_ use reset -p when splitting commits, but I do not think it is useful here. I use it for oops, I staged this change, but it actually belongs in the next commit. Undo my staging, but leave the changes in the working tree for the next one. Sure. I thought that was exactly what Duy was attempting to do when he splitted a commit into two (or more). -- To unsubscribe from this list: send the line 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 v5 1/5] t0060: Add test for manipulating symlinks via absolute paths
Martin Erik Werner martinerikwer...@gmail.com writes: When symlinks in the working tree are manipulated using the absolute path, git dereferences them, and tries to manipulate the link target instead. The above may a very good description of the root cause, but can we have description of a symptom that is visible by end-users that is caused by the bug being fixed with the series here, by ending the above like so: ... link target instead. This causes git foo bar to misbehave in this and that way. Testing the low-level underlying machinery like this patch does is not wrong per-se, but I suspect that this series was triggered by somebody noticing breakage in a end-user command git foo $path with $path being a full pathname to an in-tree symbolic link. It wouldn't be like somebody who was bored and ran test-path-utils for fun noticed the root cause without realizing how the fix would affect the behaviour that would be visible to end-users, right? Can we have that git foo $path to the testsuite as well? That is the breakage we do not want to repeat in the future by regressing. I am guessing foo is add, but I wasn't closely following the progression of the series. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] fixup! combine_diff: simplify intersect_paths() further
Kirill Smelkov k...@mns.spb.ru writes: That cleanup patch is good, but I've found a bug in it. In the item removal code + /* p-path not in q-queue[]; drop it */ + struct combine_diff_path *next = p-next; + + if ((*tail = next) != NULL) + tail = next-next; + free(p); continue; *tail = next is right, but tail = next-next is wrong, Heh, surely. We just have skipped, and the fact that tail points at the pointer variable that points at the first of the remaining items does not change with this skipping of next by assigning it to *tail. An extra assignment to tail will skip one more, which is unnecessary. -- To unsubscribe from this list: send the line 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 v5 1/5] t0060: Add test for manipulating symlinks via absolute paths
Junio C Hamano gits...@pobox.com writes: Can we have that git foo $path to the testsuite as well? That is the breakage we do not want to repeat in the future by regressing. Something like this, perhaps? t/t3004-ls-files-basic.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t3004-ls-files-basic.sh b/t/t3004-ls-files-basic.sh index 8d9bc3c..d93089d 100755 --- a/t/t3004-ls-files-basic.sh +++ b/t/t3004-ls-files-basic.sh @@ -36,4 +36,21 @@ test_expect_success 'ls-files -h in corrupt repository' ' test_i18ngrep [Uu]sage: git ls-files broken/usage ' +test_expect_success SYMLINKS 'ls-files with symlinks' ' + mkdir subs + ln -s nosuch link + ln -s ../nosuch subs/link + git add link subs/link + git ls-files -s link subs/link expect + git ls-files -s $(pwd)/link $(pwd)/subs/link actual + test_cmp expect actual + + ( + cd subs + git ls-files -s link ../expect + git ls-files -s $(pwd)/link ../actual + ) + test_cmp expect actual +' + test_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 v2] userdiff: update Ada patterns
Adrian Johnson ajohn...@redneon.com writes: - Allow extra space in is new and is separate - Fix bug in word regex for numbers Signed-off-by: Adrian Johnson ajohn...@redneon.com --- t/t4034/ada/expect | 2 +- userdiff.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t4034/ada/expect b/t/t4034/ada/expect index be2376e..a682d28 100644 --- a/t/t4034/ada/expect +++ b/t/t4034/ada/expect @@ -4,7 +4,7 @@ BOLD+++ b/postRESET CYAN@@ -1,13 +1,13 @@RESET Ada.Text_IO.Put_Line(Hello WorldRED!RESETGREEN?RESET); -1 1eRED-RESET10 16#FE12#E2 3.141_592 'REDxRESETGREENyRESET' +1 RED1e-10RESETGREEN1e10RESET 16#FE12#E2 3.141_592 'REDxRESETGREENyRESET' REDaRESETGREENxRESET+REDb aRESETGREENy xRESET-REDbRESET REDaRESETGREENyRESET GREENxRESET*REDb aRESETGREENy xRESET/REDbRESET diff --git a/userdiff.c b/userdiff.c index ea43a03..10b61ec 100644 --- a/userdiff.c +++ b/userdiff.c @@ -15,13 +15,13 @@ static int drivers_alloc; word_regex |[^[:space:]]|[\xc0-\xff][\x80-\xbf]+ } static struct userdiff_driver builtin_drivers[] = { IPATTERN(ada, - !^(.*[ \t])?(is new|renames|is separate)([ \t].*)?$\n + !^(.*[ \t])?(is[ \t]+new|renames|is[ \t]+separate)([ \t].*)?$\n !^[ \t]*with[ \t].*$\n ^[ \t]*((procedure|function)[ \t]+.*)$\n ^[ \t]*((package|protected|task)[ \t]+.*)$, /* -- */ [a-zA-Z][a-zA-Z0-9_]* - |[0-9][-+0-9#_.eE] + |[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)? This would match a lot wider than what I read you said you wanted to match in your previous message. Does -04##4_3_2Ee-9 count as a number, for example, or can we just ignore such syntactically incorrect sequence? |=|\\.\\.|\\*\\*|:=|/=|=|=|||), IPATTERN(fortran, !^([C*]|[ \t]*!)\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
Re: [PATCH v2] userdiff: update Ada patterns
George Spelvin li...@horizon.com writes: Another point is that Ada doesn't actually include leading + or - signs in the syntax for number, but rather makes them unary operators. This means that spaces are allowed, and whether you want to include them in the number pattern is a judgement call. I tend to agree. What do the patterns for other languages do? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 9/9] pull: add the --gpg-sign option.
brian m. carlson sand...@crustytoothpaste.net writes: git merge already allows us to sign commits, and git rebase has recently learned how to do so as well. Teach git pull to parse the -S/--gpg-sign option and pass this along to merge or rebase, as appropriate. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- git-pull.sh | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/git-pull.sh b/git-pull.sh index 0a5aa2c..3b2ea9e 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -138,6 +138,15 @@ do --no-verify-signatures) verify_signatures=--no-verify-signatures ;; + --gpg-sign|-S) + gpg_sign_args=-S + ;; + --gpg-sign=*) + gpg_sign_args=$(git rev-parse --sq-quote -S${1#--gpg-sign=}) + ;; + -S*) + gpg_sign_args=-S${1#-S} + ;; Interesting. Remove -S from the beginning and then prefix that with -S? Also, --gpg-sign='b m c s@c.n' and -S'b m c s@c.n' from the command line of this program would result in $gpg_sign_args that are differently quoted. Both of them cannot be correct at the same time. --d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run) dry_run=--dry-run ;; @@ -305,11 +314,13 @@ merge_name=$(git fmt-merge-msg $log_arg $GIT_DIR/FETCH_HEAD) || exit case $rebase in true) eval=git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity + eval=$eval $gpg_sign_args eval=$eval --onto $merge_head ${oldremoteref:-$merge_head} ;; *) eval=git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only - eval=$eval $log_arg $strategy_args $merge_args $verbosity $progress + eval=$eval $log_arg $strategy_args $merge_args $verbosity $progress + eval=$eval $gpg_sign_args eval=$eval \\$merge_name\ HEAD $merge_head ;; esac -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/9] cherry-pick, revert: add the --gpg-sign option
brian m. carlson sand...@crustytoothpaste.net writes: diff --git a/sequencer.c b/sequencer.c index 90cac7b..bde5f04 100644 --- a/sequencer.c +++ b/sequencer.c @@ -392,11 +392,18 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, { struct argv_array array; int rc; + char *gpg_sign; argv_array_init(array); argv_array_push(array, commit); argv_array_push(array, -n); + if (opts-gpg_sign) { + gpg_sign = xmalloc(3 + strlen(opts-gpg_sign)); + sprintf(gpg_sign, -S%s, opts-gpg_sign); + argv_array_push(array, gpg_sign); + free(gpg_sign); Perhaps argv_array_pushf(array, -S%s, opts-gpg_sign); without any temporary? That would save 5 lines in total. + } if (opts-signoff) argv_array_push(array, -s); if (!opts-edit) { diff --git a/sequencer.c b/sequencer.c index bde5f04..b200dce 100644 --- a/sequencer.c +++ b/sequencer.c @@ -392,18 +392,13 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, { struct argv_array array; int rc; - char *gpg_sign; argv_array_init(array); argv_array_push(array, commit); argv_array_push(array, -n); - if (opts-gpg_sign) { - gpg_sign = xmalloc(3 + strlen(opts-gpg_sign)); - sprintf(gpg_sign, -S%s, opts-gpg_sign); - argv_array_push(array, gpg_sign); - free(gpg_sign); - } + if (opts-gpg_sign) + argv_array_pushf(array, -S%s, opts-gpg_sign); if (opts-signoff) argv_array_push(array, -s); if (!opts-edit) { -- To unsubscribe from this list: send the line 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 v5 4/5] setup: Add 'abspath_part_inside_repo' function
Martin Erik Werner martinerikwer...@gmail.com writes: The path being exactly equal to the work tree is handled separately, since then there is no directory separator between the work tree and in-repo part. What is an in-repo part? Whatever it is, I am not sure if I follow that logic. After the while (*path) loop checks each level, you check the whole path---wouldn't that code handle that special case already? Because it is probably the normal case not to have any symbolic link in the leading part (hence not having to dereference them), I think checking is work_tree[] a prefix of path[] early is justified from performance point of view, though. /* + * No checking if the path is in fact an absolute path is done, and it must + * already be normalized. This is not quite parsable to me. + * Find the part of an absolute path that lies inside the work tree by + * dereferencing symlinks outside the work tree, for example: + * /dir1/repo/dir2/file (work tree is /dir1/repo) - dir2/file + * /dir/file (work tree is /) - dir/file + * /dir/symlink1/symlink2 (symlink1 points to work tree) - symlink2 + * /dir/repolink/file (repolink points to /dir/repo) - file + * /dir/repo (exactly equal to work tree) - (empty string) + */ +static inline int abspath_part_inside_repo(char *path) It looks a bit too big to be marked inline; better leave it to the compiler to notice that there is only a single callsite and decide to (or not to) inline it. +{ + size_t len; + size_t wtlen; + char *path0; + int off; + + const char* work_tree = get_git_work_tree(); + if (!work_tree) + return -1; + wtlen = strlen(work_tree); + len = strlen(path); I expect that this is called from a callsite that _knows_ how long path[] is. Shouldn't len be a parameter to this function instead? + off = 0; + + /* check if work tree is already the prefix */ + if (strncmp(path, work_tree, wtlen) == 0) { I wonder if we want to be explicit and compare wtlen and len before even attempting strncmp(). If work_tree is longer than path, it cannot be a prefix of path, right? In other words, also with a style-fix to prefer !XXcmp() over XXcmp() == 0: if (wtlen = len !strncmp(path, work_tree, wtlen)) { perhaps? That would make it clear why referring to path[wtlen] on the next line is permissible (it is obvious that it won't access past the end of path[]): + if (path[wtlen] == '/') { + memmove(path, path + wtlen + 1, len - wtlen); + return 0; + } else if (path[wtlen - 1] == '/' || path[wtlen] == '\0') { + /* work tree is the root, or the whole path */ + memmove(path, path + wtlen, len - wtlen + 1); If work_tree[] == /, path[] == /a, then len == 2, wtlen == 1, path[wtlen - 1] == '/' and this shifts path[] to the left by one, leaving path[] = a, which is what we want. OK. If work_tree[] == path[] == /a, then len == wtlen == 2, path[wtlen] == '\0', and this becomes equivalent to path[0] = '\0'. Hmph How do our callers treat an empty path? In other words, should these three be equivalent? cd /a git ls-files /a cd /a git ls-files cd /a git ls-files . + return 0; + } + /* work tree might match beginning of a symlink to work tree */ + off = wtlen; + } + path0 = path; + path += offset_1st_component(path) + off; + + /* check each level */ + while (*path) { + path++; + if (*path == '/') { + *path = '\0'; + if (strcmp(real_path(path0), work_tree) == 0) { + memmove(path0, path + 1, len - (path - path0)); + return 0; + } + *path = '/'; + } + } + + /* check whole path */ + if (strcmp(real_path(path0), work_tree) == 0) { + *path0 = '\0'; + return 0; + } + + return -1; +} + +/* * Normalize path, prepending the prefix for relative paths. If * remaining_prefix is not NULL, return the actual prefix still * remains in the path. For example, prefix = sub1/sub2/ and path is -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 8/9] rebase: add the --gpg-sign option
brian m. carlson sand...@crustytoothpaste.net writes: diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 43c19e0..73d32dd 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -181,7 +181,7 @@ exit_with_patch () { git rev-parse --verify HEAD $amend warn You can amend the commit now, with warn - warn git commit --amend + warn git commit --amend $gpg_sign_opt warn warn Once you are satisfied with your changes, run warn @@ -248,7 +248,8 @@ pick_one () { test -d $rewritten pick_one_preserving_merges $@ return - output eval git cherry-pick $strategy_args $empty_args $ff $@ + output eval git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} \ + $strategy_args $empty_args $ff $@ This uses $gpg_sign_opt on eval, which means that the variable's contents must be properly shell quoted, e.g. gpg_sign_opt='-S'\''brian m. carson sand...@c.net'\' throughout this script, so that everything between the first double-quote and closing ket is passed as a single parameter without being broken up. @@ -359,7 +360,8 @@ pick_one_preserving_merges () { echo $sha1 $(git rev-parse HEAD^0) $rewritten_list ;; *) - output eval git cherry-pick $strategy_args $@ || + output eval git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} \ And this part has the same expectation. However... @@ -470,7 +472,8 @@ do_pick () { --no-post-rewrite -n -q -C $1 pick_one -n $1 git commit --allow-empty --allow-empty-message \ ---amend --no-post-rewrite -n -q -C $1 || +--amend --no-post-rewrite -n -q -C $1 \ +${gpg_sign_opt:+$gpg_sign_opt} || This does not want that extra level of quoting. It would want to have something like this instead: gpg_sign_opt='-Sbrian m. carson sand...@c.net' I am not sure how you are managing these two conflicting needs of the use sites. @@ -497,7 +500,7 @@ do_next () { mark_action_done do_pick $sha1 $rest - git commit --amend --no-post-rewrite || { + git commit --amend --no-post-rewrite ${gpg_sign_opt:+$gpg_sign_opt} || { Ditto. diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh index e7d96de..5381857 100644 --- a/git-rebase--merge.sh +++ b/git-rebase--merge.sh @@ -27,7 +27,7 @@ continue_merge () { cmt=`cat $state_dir/current` if ! git diff-index --quiet --ignore-submodules HEAD -- then - if ! git commit --no-verify -C $cmt + if ! git commit ${gpg_sign_opt:+$gpg_sign_opt} --no-verify -C $cmt Ditto. then echo Commit failed, please do not call \git commit\ echo directly, but instead do one of the following: diff --git a/git-rebase.sh b/git-rebase.sh index 842d7d4..055af1b 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -37,6 +37,7 @@ ignore-date! passed to 'git am' whitespace=! passed to 'git apply' ignore-whitespace! passed to 'git apply' C=!passed to 'git apply' +S,gpg-sign?GPG-sign commits Actions: continue! continue abort! abort and check out the original branch @@ -85,6 +86,7 @@ preserve_merges= autosquash= keep_empty= test $(git config --bool rebase.autosquash) = true autosquash=t +gpg_sign_opt= read_basic_state () { test -f $state_dir/head-name @@ -107,6 +109,8 @@ read_basic_state () { strategy_opts=$(cat $state_dir/strategy_opts) test -f $state_dir/allow_rerere_autoupdate allow_rerere_autoupdate=$(cat $state_dir/allow_rerere_autoupdate) + test -f $state_dir/gpg_sign_opt + gpg_sign_opt=$(cat $state_dir/gpg_sign_opt) } write_basic_state () { @@ -120,6 +124,7 @@ write_basic_state () { $state_dir/strategy_opts test -n $allow_rerere_autoupdate echo $allow_rerere_autoupdate \ $state_dir/allow_rerere_autoupdate + test -n $gpg_sign_opt echo $gpg_sign_opt $state_dir/gpg_sign_opt } output () { @@ -324,6 +329,15 @@ do --rerere-autoupdate|--no-rerere-autoupdate) allow_rerere_autoupdate=$1 ;; + --gpg-sign) + gpg_sign_opt=-S + ;; + --gpg-sign=*) + # Try to quote only the argument, as this will appear in human-readable + # output as well as being passed to commands. + gpg_sign_opt=-S$(git rev-parse --sq-quote ${1#--gpg-sign=} | + sed 's/^ //') Isn't an invocation of sed excessive? gpg_sign_opt=$(git rev-parse --sq-quote
Re: [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules
Jens Lehmann jens.lehm...@web.de writes: This commit adds the functions and files needed for configuration, Please just say Add the functions and files needed for +++ b/Documentation/recurse-submodules-update.txt @@ -0,0 +1,8 @@ +--[no-]recurse-submodules:: + Using --recurse-submodules will update the work tree of all + initialized submodules according to the commit recorded in the + superproject if their update configuration is set to checkout'. If That single quote does not seem to be closing any matching quote. The phrase according to feels a bit too fuzzy. Merging the commit to what is checked out is one possible implementation of according to. Applying the diff between the commit and what is checked out to work tree is another. Resetting the work tree files to exactly match the commit would be yet another. I think update the work trees to the commit (i.e. lose the according) would be the closest to what you are trying to say here. + local modifications in a submodule would be overwritten the checkout + will fail unless forced. Without this option or with + --no-recurse-submodules is, the work trees of submodules will not be + updated, only the hash recorded in the superproject will be updated. It is unclear what happens if their update configuration is set to something other than 'checkout'. diff --git a/submodule.c b/submodule.c index 613857e..b3eb28d 100644 --- a/submodule.c +++ b/submodule.c @@ -382,6 +384,48 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg) ... +int option_parse_update_submodules(const struct option *opt, +const char *arg, int unset) +{ + if (unset) { + *(int *)opt-value = RECURSE_SUBMODULES_OFF; + } else { + if (arg) + *(int *)opt-value = parse_update_recurse_submodules_arg(opt-long_name, arg); + else + *(int *)opt-value = RECURSE_SUBMODULES_ON; + } You can easily unnest to lose {} if (unset) value = off; else if (arg) value = parse...; else value = on; Also I suspect that git_config_maybe_bool() natively knows how to handle arg==NULL, so if (unset) value = off; else value = parse...; is sufficient? -- To unsubscribe from this list: send the line 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: [WIP/PATCH 2/9] Teach reset the --[no-]recurse-submodules option
Jens Lehmann jens.lehm...@web.de writes: This new option will allow the user to not only reset the work tree of the superproject but to also update the work tree of all initialized submodules (so they match the SHA-1 recorded in the superproject) when used together with --hard or --merge. But this commit only adds the I agree that --soft and --mixed should not do anything. I am not sure why --keep should not do anything to submodule working trees when asked to recurse, though. option without any functionality, that will be added to unpack_trees() in subsequent commits. Signed-off-by: Jens Lehmann jens.lehm...@web.de --- Documentation/git-reset.txt | 4 builtin/reset.c | 14 ++ 2 files changed, 18 insertions(+) diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt index f445cb3..8f833f4 100644 --- a/Documentation/git-reset.txt +++ b/Documentation/git-reset.txt @@ -94,6 +94,10 @@ OPTIONS --quiet:: Be quiet, only report errors. +include::recurse-submodules-update.txt[] ++ +This option only makes sense together with `--hard` and `--merge` and is +ignored when used without these options. EXAMPLES diff --git a/builtin/reset.c b/builtin/reset.c index 6004803..adf372e 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -20,6 +20,7 @@ #include parse-options.h #include unpack-trees.h #include cache-tree.h +#include submodule.h static const char * const git_reset_usage[] = { N_(git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [commit]), @@ -255,6 +256,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix) { int reset_type = NONE, update_ref_status = 0, quiet = 0; int patch_mode = 0, unborn; + const char *recurse_submodules_default = off; + int recurse_submodules = RECURSE_SUBMODULES_DEFAULT; const char *rev; unsigned char sha1[20]; struct pathspec pathspec; @@ -270,13 +273,24 @@ int cmd_reset(int argc, const char **argv, const char *prefix) OPT_SET_INT(0, keep, reset_type, N_(reset HEAD but keep local changes), KEEP), OPT_BOOL('p', patch, patch_mode, N_(select hunks interactively)), + { OPTION_CALLBACK, 0, recurse-submodules, recurse_submodules, + checkout, control recursive updating of submodules, + PARSE_OPT_OPTARG, option_parse_update_submodules }, + { OPTION_STRING, 0, recurse-submodules-default, + recurse_submodules_default, NULL, + default mode for recursion, PARSE_OPT_HIDDEN }, OPT_END() }; + gitmodules_config(); git_config(git_default_config, NULL); argc = parse_options(argc, argv, prefix, options, git_reset_usage, PARSE_OPT_KEEP_DASHDASH); + set_config_update_recurse_submodules( + parse_update_recurse_submodules_arg(--recurse-submodules-default, + recurse_submodules_default), + recurse_submodules); parse_args(pathspec, argv, prefix, patch_mode, rev); unborn = !strcmp(rev, HEAD) get_sha1(HEAD, 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: [WIP/PATCH 3/9] Teach checkout the --[no-]recurse-submodules option
Jens Lehmann jens.lehm...@web.de writes: + set_config_update_recurse_submodules( + parse_update_recurse_submodules_arg(--recurse-submodules-default, + recurse_submodules_default), + recurse_submodules); I think I saw these exact lines in another patch. Perhaps the whole thing can become a helper function that lets the caller avoid typing the whole long strings that needs a strange/unfortunate line break? diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh index 06b18f8..bc3e1ca 100755 --- a/t/t2013-checkout-submodule.sh +++ b/t/t2013-checkout-submodule.sh @@ -4,17 +4,57 @@ test_description='checkout can handle submodules' . ./test-lib.sh +submodule_creation_must_succeed() { Style: SP before (), i.e. submodule_creation_must_succeed () { + # checkout base ($1) + git checkout -f --recurse-submodules $1 + git diff-files --quiet + git diff-index --quiet --cached $1 Please make it a habit to quote a parameter that is intended not to be split at $IFS (e.g. write these as $1 not as $1). Otherwise the reader has to wonder if this can be called with a foo bar and the expects it to be split into two. + # checkout target ($2) + if test -d submodule; then Style: no semicolons in standard control structure, i.e. if test -d submodule then + echo changesubmodule/first.t Style: SP before but not after redirection operator, i.e. echo foo bar +submodule_removal_must_succeed() { Likewise. + # checkout base ($1) + git checkout -f --recurse-submodules $1 Likewise. + echo first file Likewise. +test_expect_success 'checkout --recurse-submodules replaces submodule with files' ' + git checkout -f base + git checkout -b replace_submodule_with_dir + git update-index --force-remove submodule + rm -rf submodule/.git .gitmodules + git add .gitmodules submodule/* + git commit -m submodule replaced + git checkout -f base + git submodule update -f + git checkout --recurse-submodules replace_submodule_with_dir + test -d submodule + ! test -e submodule/.git + test -f submodule/first.t + test -f submodule/second.t +' H. Is it sufficient for these files to just exist, or do we want to make sure they have expected contents? Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH 4/9] Teach merge the --[no-]recurse-submodules option
Jens Lehmann jens.lehm...@web.de writes: This new option will allow the user to not only update the work tree of the superproject according to the merge result but to also update the work tree of all initialized submodules (so they match the SHA-1 recorded in the superproject). But this commit only adds the option without any functionality, that will be added to unpack_trees() in subsequent commits. When the two branches of the superproject being merged wants to put a submodule project to commit A and B, that conflict needs to be resolved, but if they agree that the submodule project should be at C (which is different from what the current superproject HEAD has for the submodule in its gitlink), then we want a checkout of that commit to happen in that submodule. Makes sense. After resolving such a conflict between A and B, who is responsible to adjust the working tree state of the submodule involved, by the way? git merge --continue does not exist and its moral equivalent to conclude such an interrupted merge is git commit. Should it learn to do recurse-submodule, or should the user run a separate checkout --recurse-submodule? -- To unsubscribe from this list: send the line 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/8] tests: add checking that combine-diff emits only correct paths
Kirill Smelkov k...@mns.spb.ru writes: where correct paths stands for paths that are different to all parents. Up until now, we were testing combined diff only on one file, or on several files which were all different (t4038-diff-combined.sh). As recent thinko in simplify intersect_paths() further showed, and also, since we are going to rework code for finding paths different to all parents, lets write at least basic tests. Thanks. Some nitpicks. Signed-off-by: Kirill Smelkov k...@mns.spb.ru --- t/t4057-diff-combined-paths.sh | 106 + 1 file changed, 106 insertions(+) create mode 100755 t/t4057-diff-combined-paths.sh diff --git a/t/t4057-diff-combined-paths.sh b/t/t4057-diff-combined-paths.sh new file mode 100755 index 000..e6e457d --- /dev/null +++ b/t/t4057-diff-combined-paths.sh @@ -0,0 +1,106 @@ +#!/bin/sh + +test_description='combined diff show only paths that are different to all parents' + +. ./test-lib.sh + +# verify that diffc.expect matches output of +# `git diff -c --name-only HEAD HEAD^ HEAD^2` +diffc_verify() { Style: SP before (), i.e. diffc_verify () { + git diff -c --name-only HEAD HEAD^ HEAD^2 diffc.actual + test_cmp diffc.expect diffc.actual +} + +test_expect_success 'trivial merge - combine-diff empty' ' + for i in `test_seq 1 9` Style: prefer $() over `` + do + echo $i $i.txt + git add $i.txt Quiz. What happens when this git add fails with an earlier value of $i? + 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 3/8] tree-diff: no need to manually verify that there is no mode change for a path
Kirill Smelkov k...@mns.spb.ru writes: Because if there is, such two tree entries would never be compared as equal - the code in base_name_compare() explicitly compares modes, if there is a change for dir bit, even for equal paths, entries would compare as different. OK. -- To unsubscribe from this list: send the line 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/8] combine-diff: move show_log_first logic/action out of paths scanning
Kirill Smelkov k...@mns.spb.ru writes: Judging from sample outputs and tests nothing changes in diff -c output, Yuck. I do not think the processing done inside the loop for the first path (i.e. i==0) before we call show_log(rev) affects what that called show_log(rev) does, so it probably is a good readability change. Thanks. and this change will help later patches, when we'll be refactoring paths scanning into its own function with several variants - the show_log_first logic / code will stay common to all of them. NOTE: only now we have to take care to explicitly not show anything if parents array is empty, as in fact there are some clients in Git code, which calls diff_tree_combined() in such a way. Signed-off-by: Kirill Smelkov k...@mns.spb.ru --- combine-diff.c | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index a03147c..272931f 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -1313,6 +1313,20 @@ void diff_tree_combined(const unsigned char *sha1, struct combine_diff_path *p, *paths = NULL; int i, num_paths, needsep, show_log_first, num_parent = parents-nr; + /* nothing to do, if no parents */ + if (!num_parent) + return; + + show_log_first = !!rev-loginfo !rev-no_commit_id; + needsep = 0; + if (show_log_first) { + show_log(rev); + + if (rev-verbose_header opt-output_format) + printf(%s%c, diff_line_prefix(opt), +opt-line_termination); + } + diffopts = *opt; copy_pathspec(diffopts.pathspec, opt-pathspec); diffopts.output_format = DIFF_FORMAT_NO_OUTPUT; @@ -1321,8 +1335,6 @@ void diff_tree_combined(const unsigned char *sha1, /* tell diff_tree to emit paths in sorted (=tree) order */ diffopts.orderfile = NULL; - show_log_first = !!rev-loginfo !rev-no_commit_id; - needsep = 0; /* find set of paths that everybody touches */ for (i = 0; i num_parent; i++) { /* show stat against the first parent even @@ -1338,14 +1350,6 @@ void diff_tree_combined(const unsigned char *sha1, diffcore_std(diffopts); paths = intersect_paths(paths, i, num_parent); - if (show_log_first i == 0) { - show_log(rev); - - if (rev-verbose_header opt-output_format) - printf(%s%c, diff_line_prefix(opt), -opt-line_termination); - } - /* if showing diff, show it in requested order */ if (diffopts.output_format != DIFF_FORMAT_NO_OUTPUT opt-orderfile) { -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html