Re: [PATCH 0/2] format-patch: introduce option to suppress commit hashes
"brian m. carlson" writes: > The hash of the source file isn't generally as much of a problem, > because the patch tends to change, even incidentally (line numbers and > such), when the hash of the file changes. It's also something that we > have in our history, whereas the temporary branch we rebased in is not. That is exactly the kind of workflow specific reasoning that tells you "object name of the commit that the patch was taken from is the only thing that is undesired" that makes me wonder if the feature is too workflow specific. You do something on a temporary branch without worrying about producing unnecessary object name churn, and end up wanting not to see object names. But I can buy that a step in the workflow to rebuild the history on a temporary branch before going to the next step is a common thing to have, so let's decide to accept the goal as a good thing to have, and see how well the patched code implements, documents and tests the advertised new feature. 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 1/8] update-index: add untracked cache notifications
On Wed, Dec 2, 2015 at 8:16 PM, Duy Nguyen wrote: > On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder > wrote: >> Doing: >> >> cd /tmp >> git --git-dir=/git/somewhere/else/.git update-index --untracked-cache >> >> doesn't work how one would expect. It hardcodes "/tmp" as the directory >> that "works" into the index, so if you use the working tree, you'll >> never use the untracked cache. >> >> With this patch "git update-index --untracked-cache" tells the user in >> which directory tests are performed and in which working directory the >> untracked cache is allowed. >> >> Signed-off-by: Christian Couder >> --- >> builtin/update-index.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/builtin/update-index.c b/builtin/update-index.c >> index 7431938..e568acc 100644 >> --- a/builtin/update-index.c >> +++ b/builtin/update-index.c >> @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void) >> if (!mkdtemp(mtime_dir.buf)) >> die_errno("Could not make temporary directory"); >> >> - fprintf(stderr, _("Testing ")); >> + fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd()); > > We probably should respect --verbose. I know I violated it in the first place. The verbose help is: --verbose report actions to standard output so yeah, it is not respected first because the output is on by default, and second because the output is on stderr instead of stdout. Anyway it can be a separate patch or patch series to make it respect one or both of these points. I am not very much interested in doing it myself as I think it's interesting to have the output by default especially if the above patch is applied. But if people agree that it would be a good thing, I will do 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: [RFC/PATCH 1/8] update-index: add untracked cache notifications
On Mon, Dec 7, 2015 at 10:08 AM, Christian Couder wrote: > On Wed, Dec 2, 2015 at 8:16 PM, Duy Nguyen wrote: >>> --- a/builtin/update-index.c >>> +++ b/builtin/update-index.c >>> @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void) >>> if (!mkdtemp(mtime_dir.buf)) >>> die_errno("Could not make temporary directory"); >>> >>> - fprintf(stderr, _("Testing ")); >>> + fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd()); >> >> We probably should respect --verbose. I know I violated it in the first >> place. > > The verbose help is: > > --verbose report actions to standard output > > so yeah, it is not respected first because the output is on by > default, and second because the output is on stderr instead of stdout. > Anyway it can be a separate patch or patch series to make it respect > one or both of these points. > > I am not very much interested in doing it myself as I think it's > interesting to have the output by default especially if the above > patch is applied. But if people agree that it would be a good thing, I > will do it. Then we can leave it out. Not important (until people actually complain). -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 8/8] update-index: make core.untrackedCache a bool
(Sorry I already sent a private reply to Tosten by mistake.) On Sat, Dec 5, 2015 at 1:44 PM, Torsten Bögershausen wrote: > On 01.12.15 21:31, Christian Couder wrote: >> Most features in Git can be enabled or disabled using a simple >> bool config variable and it would be nice if untracked cache >> behaved the same way. >> >> This makes --[no-|force-]untracked-cache change the value of >> core.untrackedCache in the repo config file, to avoid making >> those options useless and because this avoids the untracked >> cache being disabled by a kernel change or a directory >> change. Of course this breaks some backward compatibility, >> but the simplification and increased useability is worth it. > > Some loose thinking, how the core.untrackedcache and the > command line can work together: > > core.untrackedcache = unset > everything is as today > if --test-untracked-cache is used on command line, > the test is run, additionally the result is stored > in the config variable core.untrackedcache I don't like storing the result in core.untrackedcache as it means that --test-untracked-cache would not just test. And I agree with Aevar that it's a good thing to be able to completely separate testing and configuring. > core.untrackedcache = true > same as --force-untracked-cache > if --no-untracked-cache is given on command line, > this has higher priority I guess you mean that --no-untracked-cache has priority over "core.untrackedcache = true" without removing this config variable. This means that --no-untracked-cache would only remove the untracked cache (UC) from the index. But what git should then do the next time "git status" is run? Git sees that the index does not contain any UC, but it doesn't know that "git update-index --no-untracked-cache" has just been run. It might be "git config core.untrackedcache true" that has been run last. If "git config core.untrackedcache true" has been run last, it would be logical to add the UC to the index and use it. If "git update-index --no-untracked-cache" has been run last, it would be logical to not add the UC. > core.untrackedcache = false > same as --no-untracked-cache > if --force-untracked-cache is given on command line, > this has higher priority Then the same kind of problem happens as above. There is no clear solution about what "git status" should do when the state of the index conflicts with the value of core.untrackedcache. > Does this support the workflow described by Ævar ? I don't think so. I think that when we set "core.untrackedcache = true" we want the UC to be added to the index and used as much as possible, because we know that our OS/filesystem supports it. This means that using "git update-index --no-untracked-cache" when "core.untrackedcache = true" is set can only have two possible outcomes. It could either just die saying that "core.untrackedcache" is true, or it could just unset "core.untrackedcache" or set it to false (which might mean 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
Problems installing git-for-windows
Hi, I've just downloaded git-for-windows. It uninstalled an earlier version that I had (about 5 years old) and I got some problems. 1) It wanted to install into my User directory instead of the Program Files directory, which is the best place for all programs. The installer told me that this had access right problems, so I installed it into another directory. It seemed to go OK. 2) However, the icons on the desktop pointed to the place that my previous installation had been ( C:\Program Files (x86)\Git ) - that's where I'd have hoped that the new installation would go! 3) I uninstalled it to see if a cleaner installatoin would work, and the GIT-GUI icon didn't disappear from the desktop - minor bug I suspect. Regards, Peter mailto:g...@ptoye.com www.ptoye.com -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2015, #01; Tue, 1)
On Wed, Dec 02, 2015 at 05:31:14PM -0500, Jeff King wrote: > On Wed, Dec 02, 2015 at 02:11:32PM -0800, Junio C Hamano wrote: [snip] > > "--keep-empty" has always been about keeping an originally empty > > commit, not a commit that becomes empty because of rebasing > > (i.e. what has already been applied to the updated base). The > > documentation, if it leads to any other interpretation, needs to be > > fixed. > > > > Besides, if "--keep-empty" were to mean "keep redundant ones that > > are already in the updated base", the patch must do a lot more, > > e.g. stop filtering with git-cherry patch equivalence. > > > > I'm inclined to eject this topic. > > That was my thinking too (and I notice it didn't get any review from > anybody else). [snip] Well, I kind of agree. The cherry-pick command has both --allow-empty and --keep-redundant flags, where the second one is the kind of behavior I want to achieve in my case. As an alternative to the proposed change to `--keep-empty` I could instead introduce a new flag `--keep-redundant-commits` to `git rebase` which would then pass the flag through to the cherry-pick. Any opinions on this? Patrick signature.asc Description: Digital signature
Post-receive hook for "git pull"
I have a system here where it can be quite common to have thousands of branches in the remote repository, and where I'd like to update some local state according to the appearance of new branches (or updates of pre-existing ones). Currently, I use a "git for-each-ref" after pulling and then check (for each one of those refs) if an update is warranted, but this can get slow with that many branches. Is there some way to get something like the post-receive hook to be run for "git pull", so that the script gets told directly which (remote tracking) branches have been modified/created? Stefan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
rebase has no --verify-signatures
Hi, The git merge command has a --verify-signatures flag, which, when set, checks that the commits to be merged have trusted GPG signatures. git pull also knows this flag and forwards it to the merge command. However, doing a git pull --rebase --verify-signatures silently ignores it, since rebase has no --verify-signatures flag. Is there any technical reason why rebase should not have a --verify-signatures flag? I have written a patch to git-rebase--am which enables it to do such a check. If there is no reason not to include it I'd add documentation and a test and submit it. Otherwise I think git pull should warn, or even die with an error, if both --rebase and --verify-signatures are passed. Regards, Alexander Hirsch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Friend,
Please confirm receipt of my previous mail? When and What time can I call you? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git branch in Bash subshell "$(git branch -a)" including ls output as part of return?
Hello Folks, I am Running OSX 10.10.5 Yosemite along with git 2.6.3 installed via homebrew package manager. I recently stumbled across the following bug in some scripting I was doing. "git branch -a --list" and "git branch -a" seem to include the output of an "ls" command if executed as part of a subshell in a bash script. I can't speculate to the reason. Script goal: Delete all branches aside from master in the repository Example outputs from several commands repo: ls output: ajonespro:Deploy_Script ajones$ ls -l total 0 drwxr-xr-x 11 ajones wheel 374 Dec 7 10:50 AppDeploy drwxr-xr-x 11 ajones wheel 374 Dec 7 11:15 WebDeploy git branch -a output: ajonespro:Deploy_Script ajones$ git branch -a * DWH_concurrent_api Email_No_Error_If_No_Old_Version IT/configs_in_app_support PHP_Build_Repo master remotes/origin/DWH_concurrent_api remotes/origin/Email_No_Error_If_No_Old_Version remotes/origin/IT/configs_in_app_support remotes/origin/PHP_Build_Repo remotes/origin/master echo $(git branch -a) output: ajonespro:Deploy_Script ajones$ echo $(git branch -a) AppDeploy WebDeploy DWH_concurrent_api Email_No_Error_If_No_Old_Version IT/configs_in_app_support PHP_Build_Repo master remotes/origin/DWH_concurrent_api remotes/origin/Email_No_Error_If_No_Old_Version remotes/origin/IT/configs_in_app_support remotes/origin/PHP_Build_Repo remotes/origin/master While it might be hard to see from that output, The first two "branches" in the subshell's output are actually the directories contained within the repo. If I place a file at the root it includes that in the branch list as well. Since my test file "test.txt" (not shown in example) and "WebDeploy" are sorted before all the branches, I suspect some output buffer is being accidentally being written to. Has any experienced this before, and can anyone reproduce it on a different configuration? I wouldn't be surprised if it was some weird bug with Apple's included terminal, but since I've only observed it with git branch, I thought I'd try contact git maintainers first. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git branch in Bash subshell "$(git branch -a)" including ls output as part of return?
On Mon, Dec 07, 2015 at 11:52:28AM -0500, Alex Jones wrote: > git branch -a output: > > ajonespro:Deploy_Script ajones$ git branch -a > > * DWH_concurrent_api > Email_No_Error_If_No_Old_Version > IT/configs_in_app_support > PHP_Build_Repo > master > remotes/origin/DWH_concurrent_api > remotes/origin/Email_No_Error_If_No_Old_Version > remotes/origin/IT/configs_in_app_support > remotes/origin/PHP_Build_Repo > remotes/origin/master > > echo $(git branch -a) output: > > ajonespro:Deploy_Script ajones$ echo $(git branch -a) > AppDeploy WebDeploy DWH_concurrent_api > Email_No_Error_If_No_Old_Version IT/configs_in_app_support > PHP_Build_Repo master remotes/origin/DWH_concurrent_api > remotes/origin/Email_No_Error_If_No_Old_Version > remotes/origin/IT/configs_in_app_support remotes/origin/PHP_Build_Repo > remotes/origin/master > > While it might be hard to see from that output, The first two > "branches" in the subshell's output are actually the directories > contained within the repo. Looking at the two outputs, you are seeing the shell's glob expansion of the '*' current branch marker. You probably want to quote the command expansion to prevent this: echo "$(git branch -a)" -- To unsubscribe from this list: send the line "unsubscribe 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 07/10] ref-filter: introduce align_atom_parser()
On Thu, Dec 3, 2015 at 2:53 AM, Eric Sunshine wrote: > On Wed, Nov 11, 2015 at 2:44 PM, Karthik Nayak wrote: >> Introduce align_atom_parser() which will parse 'align' atoms and store >> the required width and position into the 'used_atom' structure. While >> we're here, add support for the usage of 'width=' and 'position=' when >> using the 'align' atom (e.g. %(align:position=middle,width=30)). > > This patch is doing too much by both moving code around and modifying > that code (somewhat dramatically), thus it is difficult for reviewers > to compare the old and new behaviors. It deserves to be split apart > into at least two patches. First, the code movement patch which > introduces align_atom_parser() (and possibly get_align_position()) > without any behavior or logical change; then the patch which changes > behavior to recognize the spelled-out forms "width=" and "position=". > You may even want to spilt it into more patches, for instance by doing > the get_align_position() extraction in its own patch. > I split it into two separate patches: 1. add align_atom_parser() 2. introduce "width=" and "position=" prefixes. I think now it seems easier to follow. >> Add documentation and modify the existing tests in t6302 to reflect >> the same. >> >> Signed-off-by: Karthik Nayak >> --- >> diff --git a/Documentation/git-for-each-ref.txt >> b/Documentation/git-for-each-ref.txt >> @@ -129,14 +129,16 @@ color:: >> align:: >> Left-, middle-, or right-align the content between >> - %(align:...) and %(end). The "align:" is followed by `` >> - and `` in any order separated by a comma, where the >> - `` is either left, right or middle, default being >> - left and `` is the total length of the content with >> - alignment. If the contents length is more than the width then >> - no alignment is performed. If used with '--quote' everything >> - in between %(align:...) and %(end) is quoted, but if nested >> - then only the topmost level performs quoting. >> + %(align:...) and %(end). The "align:" is followed by >> + `width=` and `position=` in any order >> + separated by a comma, where the `` is either left, >> + right or middle, default being left and `` is the total >> + length of the content with alignment. The prefix for the >> + arguments is not mandatory. If the contents length is more > > This paragraph is so bulky that it's very easy to overlook the bit > about the "prefix for the arguments" being optional, and it's not > necessarily even clear to the casual reader what that means. It might, > therefore, be a good idea to spell it out explicitly. For instance, > you might say something like: > > For brevity, the "width=" and/or "position=" prefixes may be > omitted, and bare and used instead. > For instance, `%(align:,)`. > Added this in. >> + than the width then no alignment is performed. If used with >> + '--quote' everything in between %(align:...) and %(end) is >> + quoted, but if nested then only the topmost level performs >> + quoting. >> diff --git a/ref-filter.c b/ref-filter.c >> @@ -63,6 +69,61 @@ void color_atom_parser(struct used_atom *atom) >> +static align_type get_align_position(const char *type) > > Taken in context of the callers, this isn't a great function name, as > it implies that it is retrieving some value, when in fact it is > parsing the input argument. A better name might be > parse_align_position(). > > Likewise, 'type' isn't necessarily a great argument name. You might > instead call it 'pos' or even just short and sweet 's'. > Will change. >> +{ >> + if (!strcmp(type, "right")) >> + return ALIGN_RIGHT; >> + else if (!strcmp(type, "middle")) >> + return ALIGN_MIDDLE; >> + else if (!strcmp(type, "left")) >> + return ALIGN_LEFT; >> + return -1; >> +} >> + >> +void align_atom_parser(struct used_atom *atom) >> +{ >> + struct align *align = &atom->u.align; >> + const char *buf = NULL; >> + struct strbuf **s, **to_free; >> + int width = -1; >> + >> + match_atom_name(atom->str, "align", &buf); >> + if (!buf) >> + die(_("expected format: %%(align:,)")); > > Is this still the way you want this error message to appear, or should > it show the long-form of the arguments? (I don't care strongly.) > I think it still holds good, not too keen on changing it either. >> + s = to_free = strbuf_split_str_without_term(buf, ',', 0); >> + >> + /* By default align to ALGIN_LEFT */ > > What is ALGIN? Regardless of the answer, this comment is not > particularly useful since it merely repeats what the code itself > already states clearly. > will do. >> + align->position = ALIGN_LEFT; >> + >> + while (*s) { >> + int position; >> + buf = s[0]->buf; >> + >> + position = get_align_position(buf); >
[PATCH v7] ls-files: Add eol diagnostics
When working in a cross-platform environment, a user wants to check if text files are stored normalized in the repository and if .gitattributes are set appropriately. Make it possible to let Git show the line endings in the index and in the working tree and the effective text/eol attributes. The end of line ("eolinfo") are shown like this: "binary" binary file "text-no-eol" text file without any EOL "text-lf" text file with LF "text-crlf"text file with CRLF "text-crlf-lf" text file with mixed line endings. The effective text/eol attribute is one of these: "", "-text", "text", "text=auto", "eol=lf", "eol=crlf" git ls-files --eol gives an output like this: i/text-no-eol w/text-no-eol attr/text=auto t/t5100/empty i/binaryw/binaryattr/-text t/test-binary-2.png i/text-lf w/text-lf attr/eol=lft/t5100/rfc2047-info-0007 i/text-lf w/text-crlf attr/eol=crlf doit.bat i/text-crlf-lf w/text-crlf-lf attr/ locale/XX.po Note that the output is meant to be human-readable and may change. Helped-By: Eric Sunshine Signed-off-by: Torsten Bögershausen --- Changes since v6: - Fixed potential memory leak in convert.c, when strbuf_read_file() fails. - t0027: Cleanups (empty lines, egrep, un-needed quoting) test_when_finished 'rm e expect actual' There doesn't seem to be 100% consistency when and how to remove files. (I think if we create files, we should be able to remove them: use "rm" rather than "rm -f") Add comment about the "last test case", which removes file to run 'git ls-files -d" Documentation/git-ls-files.txt | 22 + builtin/ls-files.c | 19 convert.c | 85 convert.h | 3 ++ t/t0027-auto-crlf.sh | 108 - 5 files changed, 226 insertions(+), 11 deletions(-) diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index e26f01f..8f29c99 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -12,6 +12,7 @@ SYNOPSIS 'git ls-files' [-z] [-t] [-v] (--[cached|deleted|others|ignored|stage|unmerged|killed|modified])* (-[c|d|o|i|s|u|k|m])* + [--eol] [-x |--exclude=] [-X |--exclude-from=] [--exclude-per-directory=] @@ -147,6 +148,18 @@ a space) at the start of each line: possible for manual inspection; the exact format may change at any time. +--eol:: + Show line endings ("eolinfo") and the text/eol attributes ("texteolattr") of files. + "eolinfo" is the file content identification used by Git when + the "text" attribute is "auto", or core.autocrlf != false. + + "eolinfo" is either "" (when the the info is not available"), or one of "binary", + "text-no-eol", "text-lf", "text-crlf" or "text-crlf-lf". + The "texteolattr" can be "", "-text", "text", "text=auto", "eol=lf", "eol=crlf". + + Both the content in the index ("i/") and the content in the working tree ("w/") + are shown for regular files, followed by the "texteolattr ("attr/"). + \--:: Do not interpret any more arguments as options. @@ -161,6 +174,15 @@ which case it outputs: [ ] +'git ls-files --eol' will show + i/ w/ attr/ + +'git ls-files --eol -o' will show + i/ w/ attr/ + +'git ls-files --eol -s' will show +[ ] i/ w/ attr/ + 'git ls-files --unmerged' and 'git ls-files --stage' can be used to examine detailed information on unmerged paths. diff --git a/builtin/ls-files.c b/builtin/ls-files.c index b6a7cb0..ef892bc 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -27,6 +27,7 @@ static int show_killed; static int show_valid_bit; static int line_terminator = '\n'; static int debug_mode; +static int show_eol; static const char *prefix; static int max_prefix_len; @@ -47,6 +48,21 @@ static const char *tag_modified = ""; static const char *tag_skip_worktree = ""; static const char *tag_resolve_undo = ""; +static void write_eolinfo(const struct cache_entry *ce, const char *path) +{ + struct stat st; + const char *i_txt = ""; + const char *w_txt = ""; + if (!show_eol) + return; + if (ce && S_ISREG(ce->ce_mode)) + i_txt = get_cached_convert_stats_ascii(ce->name); + if (!lstat(path, &st) && (S_ISREG(st.st_mode))) + w_txt = get_wt_convert_stats_ascii(path); + printf("i/%-13s w/%-13s attr/%-9s ", i_txt, w_txt, +get_convert_attr_ascii(path)); +} + static void write_name(const char *name) { /* @@ -68,6 +84,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent) return; fputs(tag, stdout); + write_eolinfo(NULL, ent->name); write_name(ent->name); } @@ -170,
Re: git branch in Bash subshell "$(git branch -a)" including ls output as part of return?
On Mon, Dec 07, 2015 at 04:58:10PM +, Charles Bailey wrote: > > Looking at the two outputs, you are seeing the shell's glob expansion of > the '*' current branch marker. You probably want to quote the command > expansion to prevent this: > > echo "$(git branch -a)" Pressing send has, of course, caused me to think further. You probably don't want to parse the output of a "porcelain" command such as "git branch" at all, but instead look at using something like "git for-each-ref", perhaps with the --format=%(refname) option, grepping out master and iterating through the rest. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git branch in Bash subshell "$(git branch -a)" including ls output as part of return?
That did the trick, thanks for the help and the suggestion. On Mon, Dec 7, 2015 at 12:02 PM, Charles Bailey wrote: > On Mon, Dec 07, 2015 at 04:58:10PM +, Charles Bailey wrote: >> >> Looking at the two outputs, you are seeing the shell's glob expansion of >> the '*' current branch marker. You probably want to quote the command >> expansion to prevent this: >> >> echo "$(git branch -a)" > > Pressing send has, of course, caused me to think further. You probably > don't want to parse the output of a "porcelain" command such as "git > branch" at all, but instead look at using something like "git > for-each-ref", perhaps with the --format=%(refname) option, grepping out > master and iterating through the rest. -- Alex Jones | Software Engineer 919-238-4404 direct 336-263-2099 mobile netsertive.com -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..
Duy Nguyen writes: > On Thu, Dec 3, 2015 at 7:17 PM, Nguyễn Thái Ngọc Duy > wrote: >> The solution in d95138e is reverted in this commit. Instead we reuse the >> solution from c056261 [4]. c056261 fixes another setup-messed-up-by-alias >> by saving and restoring env for git-clone and git-init. Now I conclude >> that setup-messed-by-alias is always evil. So the env restoration is >> done for _all_ commands whenever aliases are involved. It fixes what >> d95138e tries to fix. And it does not upset git-clone-inside-hooks. > > (Reviewer hat on) I wrote env restoration is done for all commands, > but the patch is actually about all _builtin_ commands. External ones > do not receive this treatment. FIx is in the next reroll. Thanks for being careful. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems installing git-for-windows
+cc Johannes IIUC Git-for-Windows specific issues are handled in https://github.com/git-for-windows/git/issues instead of this mailing list. On Mon, Dec 7, 2015 at 4:44 AM, Peter Toye wrote: > Hi, > > I've just downloaded git-for-windows. It uninstalled an earlier > version that I had (about 5 years old) and I got some problems. > > 1) It wanted to install into my User directory instead of the > Program Files directory, which is the best place for all programs. > The installer told me that this had access right problems, so I > installed it into another directory. It seemed to go OK. > > 2) However, the icons on the desktop pointed to the place that my > previous installation had been ( C:\Program Files (x86)\Git ) - that's > where I'd have hoped that the new installation would go! > > 3) I uninstalled it to see if a cleaner installatoin would work, and > the GIT-GUI icon didn't disappear from the desktop - minor bug I > suspect. > > > Regards, > > Peter > mailto:g...@ptoye.com > www.ptoye.com > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe 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] git-p4: support multiple depot paths in p4 submit
On Sun, Dec 06, 2015, Lars Schneider wrote: > Thanks for the patch! Do you see a way to demonstrate the bug in a test case > similar to t9821 [1]? Not yet, I'm afraid. It's proving trickier than expected because for now I can only reproduce the bug when the view uses multiples depots (solution #2 on http://answers.perforce.com/articles/KB/2437), and unfortunately the test case system in Git was designed for a single depot. Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to support multiple depots be acceptable and/or welcome? I prefer to ask before I dig into the task. Regards, -- Sam. -- To unsubscribe from this list: send the line "unsubscribe 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/2] git.c: make sure we do not leak GIT_* to alias scripts
Nguyễn Thái Ngọc Duy writes: > Let's hope there will be no third report about this commit.. Hmm, why does this additional test fail only under prove but pass without it? > diff --git a/t/t0001-init.sh b/t/t0001-init.sh > index f91bbcf..19539fc 100755 > --- a/t/t0001-init.sh > +++ b/t/t0001-init.sh > @@ -87,6 +87,33 @@ test_expect_success 'plain nested in bare through aliased > command' ' > check_config bare-ancestor-aliased.git/plain-nested/.git false unset > ' > > +test_expect_success 'No extra GIT_* on alias scripts' ' > + cat <<-\EOF >expected && > + GIT_ATTR_NOSYSTEM > + GIT_AUTHOR_EMAIL > + GIT_AUTHOR_NAME > + GIT_COMMITTER_EMAIL > + GIT_COMMITTER_NAME > + GIT_CONFIG_NOSYSTEM > + GIT_EXEC_PATH > + GIT_MERGE_AUTOEDIT > + GIT_MERGE_VERBOSITY > + GIT_PREFIX > + GIT_TEMPLATE_DIR > + GIT_TEXTDOMAINDIR > + GIT_TRACE_BARE > + EOF > + cat <<-\EOF >script && > + #!/bin/sh > + env | grep GIT_ | sed "s/=.*//" | sort >actual This is more about coding discipline than style, but piping grep output to sed is wasteful. "sed -ne '/^GIT_/s/=.*//p'" or something like that, perhaps? I wondered what happens if the user has an unrelated stray variable whose name happens to begin with GIT_ in her environment, but it turns out that we cleanse them in test-lib.sh fairly early, so that would be fine. You need to tighten your "grep" pattern, though. > + exit 0 > + EOF > + chmod 755 script && > + git config alias.script \!./script && > + ( mkdir sub && cd sub && git script ) && > + test_cmp expected actual > +' > + > test_expect_success 'plain with GIT_WORK_TREE' ' > mkdir plain-wt && > test_must_fail env GIT_WORK_TREE="$(pwd)/plain-wt" git init plain-wt -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems installing git-for-windows
Hi Peter, On Mon, 7 Dec 2015, Peter Toye wrote: > 1) It wanted to install into my User directory instead of the > Program Files directory, which is the best place for all programs. There has been a regression in Git for Windows 2.6.3 (please *always* state the version when reporting bugs) that requires you to run this installer as administrator manually, else it will install into AppData by default. The other problems seem to be consequences. The regression has been fixed and the next Git for Windows version will install into Program Files again (if your account is equipped with the appropriate permissions to elevate the installer enough). Ciao, Johannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2015, #01; Tue, 1)
Patrick Steinhardt writes: > On Wed, Dec 02, 2015 at 05:31:14PM -0500, Jeff King wrote: >> On Wed, Dec 02, 2015 at 02:11:32PM -0800, Junio C Hamano wrote: > [snip] >> > "--keep-empty" has always been about keeping an originally empty >> > commit, not a commit that becomes empty because of rebasing >> > (i.e. what has already been applied to the updated base). The >> > documentation, if it leads to any other interpretation, needs to be >> > fixed. >> > >> > Besides, if "--keep-empty" were to mean "keep redundant ones that >> > are already in the updated base", the patch must do a lot more, >> > e.g. stop filtering with git-cherry patch equivalence. >> > >> > I'm inclined to eject this topic. >> >> That was my thinking too (and I notice it didn't get any review from >> anybody else). > [snip] > > Well, I kind of agree. The cherry-pick command has both > --allow-empty and --keep-redundant flags, where the second one is > the kind of behavior I want to achieve in my case. As an > alternative to the proposed change to `--keep-empty` I could > instead introduce a new flag `--keep-redundant-commits` to `git > rebase` which would then pass the flag through to the > cherry-pick. > > Any opinions on this? Honestly, I am not interested if that is only about passing that option and doing nothing else. Imagine that you have two changes from the branch being rebased already in the updated base, one was accepted verbatim, while the other one was accepted with a slight tweak. Perhaps there were some conflicts in the context, or something. When you run your rebase, the former will not even be considered because command will notice, via patch equivalence, that you do not need it, even before it attempts to replay it. But not the latter. The command will attempt to replay it, and only in this step it may notice, by seeing that the result becomes a no-op change, that the change has already been included in the updated base. I cannot quite see how adding "--keep-redundant-commits" to the command would help anybody by allowing the latter in the history but not the former. That is primarily because you can imagine a future in which the patch equivalence determination becomes smarter. With or without "--keep-redundant-commits", both of these two changes would not appear in the resulting history when that happens. The "--keep-redundant" option to "cherry-pick" makes sense, as the user will be the one who is deciding which commit to replay on top of a new base. If the user fed a commit that is already in the new base, and the command is run with the option, that is a deliberate request to leave a no-op cruft. We cannot say the same thing for "rebase", as it tries to avoid redundant cruft, and it cannot do as good a job as humans in doing so. The "--keep-redundant-commits" option will become a way to make a distinction between what got dropped by the command automatically and what didn't get dropped because the command did not look deeply enough. That distinction is not at all useful, as that can change as the tool improves. A "--keep-redundant-commits" to "rebase" that also disables the patch equivalence filtering (not just keeping empty crufts in the resulting history) might make sense, but I do not see myself (or anybody sane) using it. "In what situation would such a feature be useful?" is a question I cannot answer offhand. -- To unsubscribe from this list: send the line "unsubscribe 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] format-patch: add an option to suppress commit hash
"brian m. carlson" writes: > Oftentimes, patches created by git format-patch will be stored in > version control or compared with diff. In these cases, two otherwise > identical patches can have different commit hashes, leading to diff > noise. Teach git format-patch a --no-hash option that instead produces > an all-zero hash to avoid this diff noise. > > Signed-off-by: brian m. carlson > --- > Documentation/git-format-patch.txt | 4 > builtin/log.c | 5 + > log-tree.c | 3 ++- > revision.h | 1 + > t/t4014-format-patch.sh| 6 ++ > 5 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/Documentation/git-format-patch.txt > b/Documentation/git-format-patch.txt > index 40356491..1266f135 100644 > --- a/Documentation/git-format-patch.txt > +++ b/Documentation/git-format-patch.txt > @@ -256,6 +256,10 @@ you can use `--suffix=-patch` to get > `0001-description-of-my-change-patch`. > using this option cannot be applied properly, but they are > still useful for code review. > > +--no-hash:: > + Output an all-zero hash in each patch's From header instead > + of the hash of the commit. > + Two (big) problems with the option name. - "--no-something" would mislead people to think you are removing something, not replacing it with something else. This option does the latter (i.e. the first line of your output still has 40-hex; it's just it no longer has a useful 40-hex). - There are many places we use hexadecimal strings in format-patch output and you are not removing or replacing all of them, only the commit object name on the fake "From " line. Saying "hash" would mislead readers. > +test_expect_success 'format-patch --no-hash' ' > + git format-patch --no-hash --stdout v2..v1 >patch2 && > + cnt=$(egrep "^From 0+ Mon Sep 17 00:00:00 2001" patch2 | wc -l) && Don't test "any number of '0'"; test 40 '0's. This is because the line format was designed to be usable by things like /etc/magic to detect format-patch output, and we want to notice if/when we break that aspect of our output format. > + test $cnt = 3 > +' > + > test_done 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: Problems installing git-for-windows
Hi Peter, On Mon, 7 Dec 2015, Peter Toye wrote: > Thanks - sorry I didn't report the version number - it was 2.6.3 as you > suggested. I didn't realise that development was so active. Do yo know > when the next version will be stable or should I go for an earlier > version? I am "side-tracked" with a couple of tickets but plan to do a 2.6.3(2) soon unless Junio releases a 2.6.4 first. If you need a fixed installer *real fast*, you can follow these easy steps: https://github.com/git-for-windows/git/wiki/Making-an-installer Ciao, Johannes -- To unsubscribe from this list: send the line "unsubscribe 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] clean: new option --exclude-from
On Sat, Dec 05, 2015 at 07:51:03PM -0800, Junio C Hamano wrote: > In any case, what we've been discussing may be an interesting and > potentially important tangent, but it still is a tangent while > evaluating the patch in question. I do not think I'd be using the > new "--exclude-from=" option to "clean" (simply because I do > not think I've ever used the existing "-e" option to it unless I am > merely testing the command to make sure it works as advertised) > myself, but I do not immediately see how it would hurt us in the > future to add it now. So... I think that is OK. The reason I brought it up was "let's stop and make sure we don't want to go this other route before we add more potentially redundant options that we cannot get rid of later". But based on this discussion it is not at all clear that "clean --exclude-from" would become redundant, so let's cross that off as an objection. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: add an option to suppress commit hash
Junio C Hamano writes: >> +--no-hash:: >> + Output an all-zero hash in each patch's From header instead >> + of the hash of the commit. >> + > > Two (big) problems with the option name. > > - "--no-something" would mislead people to think you are removing >something, not replacing it with something else. This option >does the latter (i.e. the first line of your output still has >40-hex; it's just it no longer has a useful 40-hex). > > - There are many places we use hexadecimal strings in format-patch >output and you are not removing or replacing all of them, only >the commit object name on the fake "From " line. Saying "hash" >would mislead readers. I am not good at bikeshedding, but you used 'zero_commit' elsewhere in the code. I think that would be a much better name--perhaps use that consistently throughout, as the local variable in options[] array and end-user facing option name? > >> +test_expect_success 'format-patch --no-hash' ' >> +git format-patch --no-hash --stdout v2..v1 >patch2 && >> +cnt=$(egrep "^From 0+ Mon Sep 17 00:00:00 2001" patch2 | wc -l) && > > Don't test "any number of '0'"; test 40 '0's. This is because the > line format was designed to be usable by things like /etc/magic to > detect format-patch output, and we want to notice if/when we break > that aspect of our output format. > >> +test $cnt = 3 >> +' >> + >> test_done > > 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: Post-receive hook for "git pull"
Stefan Monnier writes: > I have a system here where it can be quite common to have thousands of > branches in the remote repository, and where I'd like to update some > local state according to the appearance of new branches (or updates of > pre-existing ones). > > Currently, I use a "git for-each-ref" after pulling and then check (for > each one of those refs) if an update is warranted, but this can get slow > with that many branches. Is there some way to get something like the > post-receive hook to be run for "git pull", so that the script gets told > directly which (remote tracking) branches have been modified/created? I do not think there is. But you could easily script along the lines of... #!/bin/sh git for-each-ref | sort >prestate git pull "$@" git for-each-ref | sort >poststate comm -12 prestate poststate ... or something like that, no? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Multiple fetches when unshallowing a shallow clone
I should point out that I never saw a "deepen 0" line. Rather, I saw git send "option depth 0" to the remote helper. Duy, you mentioned that depth=0 means "do not change depth". I assume that means the server should use exactly the shallows that the client sent, and it does not need to traverse the tree or modify the shallow or unshallow sets at all. Right? Duy, you also mentioned that "those lines should be rejected any way". You just mean that a "deepen 0" line should be rejected, right? And that's because the right way to tell git-upload-pack not to change the depth is to omit the "deepen" line after the "shallow" lines, so there's never a need to send "deepen 0"? Thanks! On Sun, Dec 6, 2015 at 5:46 AM, Duy Nguyen wrote: > On Sun, Dec 6, 2015 at 8:01 AM, Jeff King wrote: >> On Sun, Dec 06, 2015 at 01:37:18AM -0500, Jeff King wrote: >> >>> And indeed, replacing the logic with what I wrote does make the backfill >>> go away in my test case. But it's so far from what is there that I feel >>> like I must be missing something. >> >> I think one thing I was missing is that we need to just grab the >> _object_, but we need to realize that the ref needs updating[1]. So we >> cannot skip backfill of any tag that we do not already have, even if we >> already have the tag object. > > It's probably worth adding a few comment lines about this. I did > search back commit history but did not get this. > >> Which made me wonder why this: >> >> git init parent && >> git -C parent commit --allow-empty -m one && >> git clone parent child && >> git -C parent commit --allow-empty -m two && >> git -C parent tag -m mytag foo && >> git -C parent commit --allow-empty -m three && >> git -C child fetch >> >> does not appear to need to backfill to pick up refs/tags/foo. But it >> does. It's just that it hits the quickfetch() code path and does not >> have to ask the other side for a pack. And that explains why it does hit >> in the --shallow case: we explicitly disable quickfetch in such cases. >> >> For the unshallow case, of course we could use it (but only for the >> second, backfill fetch). Something like this seems to work for me: >> >> diff --git a/builtin/fetch.c b/builtin/fetch.c >> index ed84963..b33b90f 100644 >> --- a/builtin/fetch.c >> +++ b/builtin/fetch.c >> @@ -881,6 +881,8 @@ static void backfill_tags(struct transport *transport, >> struct ref *ref_map) >> >> transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); >> transport_set_option(transport, TRANS_OPT_DEPTH, "0"); >> + if (unshallow) >> + depth = NULL; >> fetch_refs(transport, ref_map); >> >> if (gsecondary) { >> >> But I admit I am not at all confident that it doesn't cause other >> problems, or that it covers all cases. Even in a shallow repo, we should >> be able to quickfetch individual tags, shouldn't we? > > Yes. depth is only non-NULL when you pass --depth (or --unshallow). > quickfetch should happen when you fetch without those options. > >> I wonder if we could just always set "depth = NULL" here. > > --unshallow is essentially --depth=, so I don't see why > --unshallow should be singled out here. We probably want to restore > depth back (or pass a flag to explicitly ignore the "depth" exception > in quickfetch). For multiple fetches, we spawn new commands so depth > being NULL does not harm. Just in case somebody tries to fetch a > couple more times in the same process in future. > -- > Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] ls-files: Add eol diagnostics
From: "Torsten Bögershausen" When working in a cross-platform environment, a user wants to check if text files are stored normalized in the repository and if .gitattributes are set appropriately. The need for this came up again on the Git Users list (https://groups.google.com/d/msg/git-users/jC-mngwVYo4/Sr4JXgpGJVYJ). It will definitely be useful in mixed environments wher users can get rather confused. I've added a couple of bikeshed comments on some of the wider issues this bumps into. Make it possible to let Git show the line endings in the index and in the working tree and the effective text/eol attributes. The end of line ("eolinfo") are shown like this: "binary" binary file "text-no-eol" text file without any EOL "text-lf" text file with LF "text-crlf"text file with CRLF "text-crlf-lf" text file with mixed line endings. This "text-crlf-lf" can easily occur for eol=crlf files which have merge conflict markers, which are always eol=lf. It may be that the conflict markers should use the eol setting that is in place for the file being merged. e.g. http://stackoverflow.com/questions/17832616/make-git-use-crlf-on-its-head-merge-lines (which links to https://github.com/git/git/blob/master/xdiff/xmerge.c#L173, and 3 other places in fill_conflict_hunk). The issue of how to mark EOL conflicts in-text is tricky as it's a non-obvious white space issue - it doesn't fit well with the normal >>> === <<< conflict markers. The effective text/eol attribute is one of these: "", "-text", "text", "text=auto", "eol=lf", "eol=crlf" the "-text" attribute isn't explained in the gitattributes(5) page, except by implication from one of the examples. git ls-files --eol gives an output like this: i/text-no-eol w/text-no-eol attr/text=auto t/t5100/empty i/binaryw/binaryattr/-text t/test-binary-2.png i/text-lf w/text-lf attr/eol=lft/t5100/rfc2047-info-0007 i/text-lf w/text-crlf attr/eol=crlf doit.bat i/text-crlf-lf w/text-crlf-lf attr/ locale/XX.po Note that the output is meant to be human-readable and may change. Helped-By: Eric Sunshine Signed-off-by: Torsten Bögershausen --- Changes since v6: - Fixed potential memory leak in convert.c, when strbuf_read_file() fails. - t0027: Cleanups (empty lines, egrep, un-needed quoting) test_when_finished 'rm e expect actual' There doesn't seem to be 100% consistency when and how to remove files. (I think if we create files, we should be able to remove them: use "rm" rather than "rm -f") Add comment about the "last test case", which removes file to run 'git ls-files -d" Documentation/git-ls-files.txt | 22 + builtin/ls-files.c | 19 convert.c | 85 convert.h | 3 ++ t/t0027-auto-crlf.sh | 108 - 5 files changed, 226 insertions(+), 11 deletions(-) diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index e26f01f..8f29c99 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -12,6 +12,7 @@ SYNOPSIS 'git ls-files' [-z] [-t] [-v] (--[cached|deleted|others|ignored|stage|unmerged|killed|modified])* (-[c|d|o|i|s|u|k|m])* + [--eol] [-x |--exclude=] [-X |--exclude-from=] [--exclude-per-directory=] @@ -147,6 +148,18 @@ a space) at the start of each line: possible for manual inspection; the exact format may change at any time. +--eol:: + Show line endings ("eolinfo") and the text/eol attributes ("texteolattr") of files. + "eolinfo" is the file content identification used by Git when + the "text" attribute is "auto", or core.autocrlf != false. + + "eolinfo" is either "" (when the the info is not available"), or one of "binary", + "text-no-eol", "text-lf", "text-crlf" or "text-crlf-lf". + The "texteolattr" can be "", "-text", "text", "text=auto", "eol=lf", "eol=crlf". perhaps add 'see also linkgit:gitattributes[5] .' though it may be too tangential. + + Both the content in the index ("i/") and the content in the working tree ("w/") + are shown for regular files, followed by the "texteolattr ("attr/"). + \--:: Do not interpret any more arguments as options. @@ -161,6 +174,15 @@ which case it outputs: [ ] +'git ls-files --eol' will show + i/ w/ attr/ + +'git ls-files --eol -o' will show + i/ w/ attr/ + +'git ls-files --eol -s' will show +[ ] i/ w/ attr/ + 'git ls-files --unmerged' and 'git ls-files --stage' can be used to examine detailed information on unmerged paths. diff --git a/builtin/ls-files.c b/builtin/ls-files.c index b6a7cb0..ef892bc 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -27,6 +27,7 @@ static int show_killed; static int show_valid_bit; static int line_terminator = '\n'; static int debug_mode; +static int show_eol; static const char *prefix; static int max_prefix_len; @@ -47,6 +48,2
Re: Git 2.3.7 hangs on fetch but not clone
On Sun, Dec 06, 2015 at 05:12:12PM -0500, Randall S. Becker wrote: > I have some strange behaviour I am trying to diagnose on the NonStop port of > git 2.3.7. The situation is we have a *LARGE* cloned repository with some > local modifications of openssl, which we are trying to clone again for a > Jenkins build. The command: > git clone /local/openssl openssl Because the two repositories are on the same local filesystem, git will skip the usual pack transfer and just copy or hardlink the object files. You can add "--no-local" to make it more like the fetch that is failing (I suspect it will then fail, too, but if it doesn't, that would be an interesting data point). > remote: Counting objects: 113436, done. > remote: Compressing objects: 100% (23462/23462), done. > then hangs forever without creating any files in the working directory. > There are also no files or directories modified since the init operation. > Processes left around, and without consuming resources, are: > 1493172290 2030043151 - 15:58:29 00:15 git pack-objects --revs --thin > --stdout --progress --delta-base-offset --include-tag > 452984908 402653262 - 15:58:29 00:00 git -c core.askpass=true fetch > --verbose --progress /local/git/openssl +refs/heads/*:refs/remotes/origin/* > 402653262 1694498903 - 15:58:28 00:00 git -c core.askpass=true fetch > --verbose --progress /local/git/openssl +refs/heads/*:refs/remotes/origin/* > 2030043151 402653262 - 15:58:29 00:00 git-upload-pack > /local/git/openssl What are the processes doing? Upload-pack on the "server" side spawns pack-objects, writes the set of wanted objects to its stdin, and then waits for it to produce pack data to stdout (which it then multiplexes back to the client). You clearly got past the write (since pack-objects finished "Counting objects..."). I'd expect it to be waiting for input in poll() for input from the stdout or stderr of pack-objects. It may also be blocking on write() back to the client. Pack-objects clearly completed delta compression, and so should be writing out the final pack. It doesn't look like it ever wrote the "writing objects..." progress line (or perhaps upload-pack got stuck and failed to relay it). It's probably blocking on write() to upload-pack. You have two fetch processes. Presumably you build with NO_PTHREADS and one of them is a fork()ed copy to demux the data from upload-pack. It clearly runs at least a little while (it relays the "Counting" message). It should either be blocking on read() from upload-pack, or perhaps blocking on write() back to main fetch. The main fetch hasn't yet spawned index-pack or unpack-objects, which means it hasn't gotten enough of the pack header to do so. So it's probably blocking on read() from the sideband demuxer process. Do you have some equivalent of strace on your system? Or better yet, a debugger? > This does not happen for our smaller repositories. Any pointers on where to > look would be appreciated. My wild guess would be some pipe deadlock between the processes, but I don't think we've had any of those lately. And v2.3.7 is not _too_ old (there are some fixes for http deadlocks in v2.3.7..v2.6.3, but that should not affect you here). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] ls-files: Add eol diagnostics
On Monday, December 7, 2015, Torsten Bögershausen wrote: > When working in a cross-platform environment, a user wants to > check if text files are stored normalized in the repository and if > .gitattributes are set appropriately.[...] A few style comments this time around... > Helped-By: Eric Sunshine > Signed-off-by: Torsten Bögershausen > --- > diff --git a/builtin/ls-files.c b/builtin/ls-files.c > @@ -47,6 +48,21 @@ static const char *tag_modified = ""; > +static void write_eolinfo(const struct cache_entry *ce, const char *path) > +{ > + struct stat st; > + const char *i_txt = ""; > + const char *w_txt = ""; > + if (!show_eol) > + return; > + if (ce && S_ISREG(ce->ce_mode)) > + i_txt = get_cached_convert_stats_ascii(ce->name); > + if (!lstat(path, &st) && (S_ISREG(st.st_mode))) Style: unnecessary parentheses around S_ISREG(), which is inconsistent with S_ISREG() two lines above. > + w_txt = get_wt_convert_stats_ascii(path); > + printf("i/%-13s w/%-13s attr/%-9s ", i_txt, w_txt, > +get_convert_attr_ascii(path)); > +} > diff --git a/convert.c b/convert.c > @@ -95,6 +100,62 @@ static int is_binary(unsigned long size, struct text_stat > *stats) > +static unsigned int gather_convert_stats(const char *data, unsigned long > size) > +{ > + struct text_stat stats; > + if (!data || !size) > + return 0; > + gather_stats(data, size, &stats); > + if (is_binary(size, &stats) || stats.cr != stats.crlf) > + return CONVERT_STAT_BITS_BIN; > + else if (stats.crlf && (stats.crlf == stats.lf)) Style: unnecessary parentheses around 'foo == bar' > + return CONVERT_STAT_BITS_TXT_CRLF; > + else if (stats.crlf && stats.lf) > + return CONVERT_STAT_BITS_TXT_CRLF | CONVERT_STAT_BITS_TXT_LF; > + else if (stats.lf) > + return CONVERT_STAT_BITS_TXT_LF; > + else > + return 0; > +} > + > +static const char *gather_convert_stats_ascii(const char *data, unsigned > long size) > +{ > + unsigned int convert_stats = gather_convert_stats(data, size); > + > + if (convert_stats & CONVERT_STAT_BITS_BIN) > + return "binary"; > + switch (convert_stats) { > + case CONVERT_STAT_BITS_TXT_LF: > + return("text-lf"); Style: space after "return". Also, can we lose the unnecessary noisy parentheses? (I recall mentioning this in my first review.) Same for "return" statements below. > + case CONVERT_STAT_BITS_TXT_CRLF: > + return("text-crlf"); > + case CONVERT_STAT_BITS_TXT_LF | CONVERT_STAT_BITS_TXT_CRLF: > + return("text-crlf-lf"); > + default: > + return ("text-no-eol"); > + } > +} > diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh > @@ -214,6 +239,20 @@ checkout_files () { > fi > done > > + test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" " > + test_when_finished 'rm e expect actual' && Style: test_when_finished is incorrectly indented with tab+spaces rather than only tabs > + cat >e <<-EOF && > + i/text-crlf w/$(stats_ascii $crlfname) ${src}CRLF.txt > + i/text-crlf-lf w/$(stats_ascii $lfmixcrlf) > ${src}CRLF_mix_LF.txt > + i/text-lf w/$(stats_ascii $lfname) ${src}LF.txt > + i/binary w/$(stats_ascii $lfmixcr) ${src}LF_mix_CR.txt > + i/binary w/$(stats_ascii $crlfnul) ${src}CRLF_nul.txt > + i/binary w/$(stats_ascii $crlfnul) ${src}LF_nul.txt > + EOF > + sort expect && > + git ls-files --eol $src* | sed -e 's!attr/[=a-z-]*!!g' -e 's/ > */ /g' | sort >actual && > + test_cmp expect actual > + " > @@ -480,4 +550,20 @@ checkout_filesnative true "lf" LFCRLF > CRLF_mix_LF LF_mix_CR > checkout_filesnative false "crlf"CRLF CRLF CRLF > CRLF_mix_CR CRLF_nul > checkout_filesnative true "crlf"CRLF CRLF CRLF > CRLF_mix_CR CRLF_nul > > + Style: unnecessary blank line > +# Should be the last test case: remove some files from the worktree > +# run 'git ls-files -d' This seems kind of fragile. Might it be possible to either recreate those files when this test finishes or instead provide a function which creates them on-demand for tests which need them? My concern is that there is a good chance that someone later adding tests will overlook this comment, especially since most people just plop new tests at the bottom of the file. > +test_expect_success 'ls-files --eol -d' " > + rm crlf_false_attr__CRLF.txt crlf_false_attr__CRLF_mix_LF.txt > crlf_false_attr__LF.txt .gitattributes && Style: too many spaces following 'rm' Same issue raised in my prev
Re: [PATCH v2] revision.c: fix possible null pointer access
Stefan Naewe writes: > mark_tree_uninteresting dereferences a tree pointer before checking > if the pointer is valid. Fix that by doing the check first. > > Signed-off-by: Stefan Naewe > --- I still have a problem with "dereferences", as "dereference" is about computing an address and accessing memory based on the result, and only the first half is happening here. I can live with "The function does a pointer arithmetic on 'tree' before it makes sure that 'tree' is not NULL", but in any case, let's queue this as-is for now and wait for a while to see if others can come up with a more appropriate phrases. Thanks. > revision.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/revision.c b/revision.c > index 0fbb684..8c569cc 100644 > --- a/revision.c > +++ b/revision.c > @@ -135,10 +135,12 @@ static void mark_tree_contents_uninteresting(struct > tree *tree) > > void mark_tree_uninteresting(struct tree *tree) > { > - struct object *obj = &tree->object; > + struct object *obj; > > if (!tree) > return; > + > + obj = &tree->object; > if (obj->flags & UNINTERESTING) > return; > obj->flags |= UNINTERESTING; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Can't get 'git archive' to work
On Sun, Dec 06, 2015 at 09:31:06PM -0800, Yuri wrote: > Why "operation is not supported" through http and https? 'archive' is > supposed to be the most efficient operation to get the specific snapshot of > the repository, and it should be available without authentication. Remote git-archive is not well-supported. It does work over git-daemon, but the server side has to explicitly turn it on in their config. It was never plumbed into the git-over-http protocol at all. ssh is generally the only place it works out-of-the-box (but see below). > In case of ssh with the key there is the strange error message about > "core.gitProxy". What is this about? I am not using a proxy. > [...] > # through ssh with an active ssh key > $ git archive --format=tar --remote=g...@github.com:yurivict/vm-to-tor.git > Invalid command: 'git-upload-archive 'yurivict/vm-to-tor.git'' > You appear to be using ssh to clone a git:// URL. > Make sure your core.gitProxy config option and the > GIT_PROXY_COMMAND environment variable are NOT set. > fatal: The remote end hung up unexpectedly This message isn't from git, but rather from GitHub's ssh server. You can see it here without running git locally at all: $ ssh g...@github.com git-upload-archive Invalid command: 'git-upload-archive' You appear to be using ssh to clone a git:// URL. Make sure your core.gitProxy config option and the GIT_PROXY_COMMAND environment variable are NOT set. The short of it is that GitHub does not support remote git-archive at all, not even over ssh. Part of the reason is that it maintains a separate caching layer for grabbing zips and tarballs, which is available at an http endpoint: https://github.com/yurivict/vm-to-tor/archive/master.tar.gz Clearly the advice above is not helpful in this case; I don't recall the case that it was originally written to help. You may want to write to GitHub support about it. > # through ssh without an ssh key > $ git archive --format=tar --remote=g...@github.com:yurivict/vm-to-tor.git > Permission denied (publickey). > fatal: The remote end hung up unexpectedly GitHub's ssh server only accepts key authentication, so this is not surprising. > # through https > $ git archive --format=tar > --remote=https://github.com/yurivict/vm-to-tor.git > fatal: Operation not supported by protocol. > > # through http > $ git archive --format=tar > --remote=http://github.com/yurivict/vm-to-tor.git > fatal: Operation not supported by protocol. This comes from the local git client. There's no scheme defined for hitting git-upload-archive (the server side of a remote git-archive) over HTTP. You'd have to define it, implement the client side of it, and then convince the server side to configure their HTTP servers to handle it as a CGI. That might be a nice project to work on, but it will not get you tarballs anytime soon. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] filter-branch: pass tag name via stdin without newline
On Sun, Dec 06, 2015 at 06:55:21PM -0800, Junio C Hamano wrote: > "Eric N. Vander Weele" writes: > > > "git filter-branch --tag-name-filter" fails when the user-provided > > command attempts to trivially append text to the originally tag name, > > passed via stdin, due to an unexpected newline ('\n'). The newline is > > introduced due to "echo" piping the original tag name to the > > user-provided tag name filter command. > > Is there any other place where we feed such an incomplete line > (i.e. a line without the terminating LF) to the filters in this > command? I suspect we would break filters if we did. On some systems, tools like `sed` and `tr` often behave funny when there is no trailing newline, but I do not recall all of the specific instances (if one felt like engaging in some light masochism, searching the mailing list archive will probably turn up results). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2015, #02; Fri, 4)
On Sun, Dec 06, 2015 at 10:12:18AM -0600, Edmundo Carmona Antoranz wrote: > Hi, Junio, Jeff! > > On Fri, Dec 4, 2015 at 5:15 PM, Junio C Hamano wrote: > > * ec/annotate-deleted (2015-11-20) 1 commit > > - annotate: skip checking working tree if a revision is provided > > > > Usability fix for annotate-specific " " syntax with deleted > > files. > > > > Waiting for review. > > Is there something I have to do about it? A gentle ping is sometimes helpful. :) I did not see anything wrong with it, but I think Junio would be a good person to give it a look, as he knows all of the sordid history of blame versus annotate, and the intended allowed command-line orderings. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] contrib/subtree: fix "subtree split" skipped-merge bug.
A bug occurs in 'git-subtree split' where a merge is skipped even when both parents act on the subtree, provided the merge results in a tree identical to one of the parents. Fix by copying the merge if at least one parent is non-identical, and the non-identical parent is not an ancestor of the identical parent. Also adding a test case, this checks that a descendant can be pushed to it's ancestor in this case. Signed-off-by: Dave Ware --- contrib/subtree/git-subtree.sh | 12 +++-- contrib/subtree/t/t7900-subtree.sh | 52 ++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 9f06571..b837531 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -479,8 +479,16 @@ copy_or_skip() p="$p -p $parent" fi done - - if [ -n "$identical" ]; then + + copycommit= + if [ -n "$identical" ] && [ -n "$nonidentical" ]; then + extras=$(git rev-list --boundary $identical..$nonidentical) + if [ -n "$extras" ]; then + # we need to preserve history along the other branch + copycommit=1 + fi + fi + if [ -n "$identical" ] && [ -z "$copycommit" ]; then echo $identical else copy_commit $rev $tree "$p" || exit $? diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index 9051982..ea991eb 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh @@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' ' )) ' +test_expect_success 'subtree descendent check' ' + mkdir git_subtree_split_check && + ( +cd git_subtree_split_check && +git init && + +mkdir folder && + +echo a >folder/a && +git add . && +git commit -m "first commit" && + +git branch branch && + +echo 0 >folder/0 && +git add . && +git commit -m "adding 0 to folder" && + +echo b >folder/b && +git add . && +git commit -m "adding b to folder" && +cherry=$(git rev-list HEAD -1) && + +git checkout branch && +echo text >textBranch.txt && +git add . && +git commit -m "commit to fiddle with branch: branch" && + +git cherry-pick $cherry && +git checkout master && +git merge -m "merge" branch && + +git branch noop_branch && + +echo d >folder/d && +git add . && +git commit -m "adding d to folder" && + +git checkout noop_branch && +echo moreText >anotherText.txt && +git add . && +git commit -m "irrelevant" && + +git checkout master && +git merge -m "second merge" noop_branch && + +git subtree split --prefix folder/ --branch subtree_tip master && +git subtree split --prefix folder/ --branch subtree_branch branch && +git push . subtree_tip:subtree_branch + ) + ' + test_done -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] clean: new option --exclude-from
On Sun, Dec 06, 2015 at 09:58:26AM -0500, James wrote: > +test_expect_success 'git clean -e --exclude-from' ' > + rm -fr repo && > + mkdir repo && > + cd repo && > + git init && > + touch known 1 2 3 && > + git add known && > + echo 1 >> .git/clean-exclude && > + git clean -f -e 2 --exclude-from=.git/clean-exclude && > + test_path_is_file 1 && > + test_path_is_file 2 && > + test_path_is_missing 3 && > + test_path_is_file known > +' This checks that we exclude the union of the "-e" and "--exclude-from" parameters. What happens if one excludes a path and the other negates the exclusion? How is the precedence handled? Is it based on order on the command-line, or something else? I do not have much of a preference myself, but it probably makes sense to document and test it. > +test_expect_success 'git clean --exclude-from --exclude-from' ' > + rm -fr repo && > + mkdir repo && > + git init && > + touch known 1 2 3 && > + git add known && > + cat >.git/clean-exclude1 <<-\EOF && > + 1 > + EOF > + cat >.git/clean-exclude2 <<-\EOF && > + 2 > + EOF > + git clean -f --exclude-from=.git/clean-exclude1 > --exclude-from=.git/clean-exclude2 && > + test_path_is_file 1 && > + test_path_is_file 2 && > + test_path_is_missing 3 && > + test_path_is_file known > +' Ditto here (as the same "type", the only precedence that would make any sense is command-line order here). > +test_expect_success 'git clean --exclude-from=BADFILE' ' > + rm -fr repo && > + mkdir repo && > + cd repo && > + git init && > + test_expect_code 128 git clean -f > --exclude-from=.git/clean-exclude-not-there > +' Do you actually care about the 128 here, or is just "it should exit with failure"? If the latter, we usually use test_must_fail. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git subtree bug produces divergent descendants
Thanks for taking the time to look over it. I'm not familiar with the process here. On Mon, Dec 7, 2015 at 5:53 PM, Eric Sunshine wrote: > Tests don't automatically return to the directory prior to the 'cd', > so when this test ends, the current directory will still be > 'git_subtree_split_check'. If someone later adds a test following > this one, that test will execute within 'git_subtree_split_check', > which might not be expected by the test writer. > > To ensure that the prior working directory is restored at the end of > the test (regardless of success or failure), tests typically employ a > subshell using this idiom: > > mkdir foo && > ( > cd foo && > ... && > ... > ) > I'm not at all familiar with this test harness so I had a few problems here (like this, and the bash variable). Thank you for the advice. Cheers, Dave Ware -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: best practices against long git rebase times?
On Sun, Dec 06, 2015 at 05:43:45PM +0100, Andreas Krey wrote: > On Fri, 04 Dec 2015 15:31:03 +, John Keeping wrote: > ... > > I'm pretty sure that you're right and the cherry-pick analysis is where > > the time is spent. > > But I'm pretty surprised as to the amount of CPU time that goes there. > > I'm now rebasing a single commit with a single blank line added, > and for 3000 new commits to rebase over (with 64 MByte of git log - > for them) it takes twelve minutes, or about for commits per second, > and all user CPU, no I/O. It's pretty linear in number of commits, too. You're computing the patch against the parent for each of those 3000 commits (to get a hash of it to compare against the single hash on the other side). Twelve minutes sounds long, but if you have a really gigantic tree, it might not be unreasonable. You can also try compiling with "make XDL_FAST_HASH=" (i.e., setting that option to the empty string). Last year I found there were some pretty suboptimal corner cases, and you may be hitting one (we should probably turn that option off by default; I got stuck on trying to find a hash that would perform faster and never followed up[1]. I doubt that is your problem, but it's possible). -Peff [1] http://thread.gmane.org/gmane.comp.version-control.git/261638 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Why does send-pack call pack-objects for all remote refs?
I have a repository which has ~2000 branches on the remote, and it takes ~8 seconds to push a change to one ref. The majority of this time is spent in pack-object. I wrote a hack so that only the ref being updated would be packed (the normal behavior is to pack for every ref on the remote). The push time dropped to <1 second with (seemingly) no consequences. This raised a couple of questions: 1) Are there consequences for not packing refs that are not being updated? Can all operations in send-pack which operate on other refs be skipped? 2) Why isn't git more selective about what it packs? Is it simply a performance optimization not worth the implementation complexity? 3) Is something about my repository strange? ex: 2000 is too many branches, packing for each ref takes me an unusually long time, etc. Thanks in advance to anyone who takes the time to clarify this for me. Daniel -- To unsubscribe from this list: send the line "unsubscribe 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] ls-files/dir: use is_git_repo to detect submodules
On Sun, Dec 06, 2015 at 05:59:26PM +0100, Andreas Krey wrote: > On Sat, 05 Dec 2015 02:37:44 +, Jeff King wrote: > ... > > Hrm. Weird. You'd think it would break with the existing code if I do > > this, then: > > > ... > > - (cd a/b/c; git init) && > > + (cd a/b/c; git init && git commit --allow-empty -m foo) && > > git config remote.origin.url ../foo/bar.git && > > git submodule add ../bar/a/b/c ./a/b/c && > > I tried a -f here instead; did not work either. > > I guess I will first wade through the other failures my 'fix' > causes to see the total damage. The only other one I saw was in the completion tests. And it looked like `git add` failing in a way similar to what I dug into here. So I think it's "just" the one bug. > > We know it is a git dir, but there is no sha1 for us to actually add as > > the gitlink entry. > > > > If that is the case, then there is either some very tricky refactoring > > required, > > Yes, it looks like the return code delivered need to be slightly different > dependent on the user. Maybe. From your patch it looks like the "git-add" code will return it as "untracked". Which makes sense if we then want to add it. But if it has no HEAD commit we _cannot_ add it, as we have no sha1 to stick in the index. It should probably be ignored totally in that case. But that means you have to actually find out if HEAD is valid or not. Which is what the current code is doing. :-/ > > or what we are trying to do here is simply wrong. Maybe it > > would be simpler to just speed up resolve_gitlink_ref with a better data > > structure. > > Which is what I did on square one, but now we already have a real fix > for git clean, and now it's even less advantageous the fix the consequence > (the submodule cache blowing up) instead of the cause (asking for it > in the first place). I think "clean" is a much simpler case. It only wants to know "can I skip this entry as an untracked sub-repo?". And that is similar to what "git status" or "git ls-files" wants to know. But "git add" wants to know "can I _add_ this entry to the index", and that is a different question (but I think answered by the same code that powers ls-files). > PS: I seem to not quite have send-email under control, the envelope from > seems to made the patches not reach the mailing list (nor me in the CC). Hmm, yeah. Obviously they made it to me directly, but the list is a little bit picky. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Multiple fetches when unshallowing a shallow clone
On Mon, Dec 7, 2015 at 8:57 PM, Jason Paller-Rzepka wrote: > Duy, you mentioned that depth=0 means "do not change depth". I assume that > means the server should use exactly the shallows that the client sent, and > it does not need to traverse the tree or modify the shallow or unshallow > sets at all. Right? Correct. The server might send new shallow lines anyway though, if the server repo is also shallow and the new fetched ref needs to be cut. But I don't know if Dulwich supports that yet. > Duy, you also mentioned that "those lines should be rejected any way". You > just mean that a "deepen 0" line should be rejected, right? And that's > because the right way to tell git-upload-pack not to change the depth is to > omit the "deepen" line after the "shallow" lines, so there's never a need to > send "deepen 0"? Also correct. I didn't check the code when I wrote that. But I have checked and upload-pack does reject "deepen 0" if (starts_with(line, "deepen ")) { char *end; depth = strtol(line + 7, &end, 0); if (end == line + 7 || depth <= 0) die("Invalid deepen: %s", line); -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Multiple fetches when unshallowing a shallow clone
Jeff King writes: > I think one thing I was missing is that we need to just grab the > _object_, but we need to realize that the ref needs updating[1]. So we > cannot skip backfill of any tag that we do not already have, even if we > already have the tag object. > ... > [1] I'm still puzzled why find_non_local_tags uses has_sha1_file() on > the tag object at all, then. The designed semantics of auto-following tags (not necessarily as implemented or documented, i.e. there may be implementation or documentation bugs), I think, is to arrive at the same state as doing a fetch (or a push) without the auto-following and then doing a separate fetch (or a push) of tags that point at the objects that are reachable from the tips of refs after finishing the first (i.e. without auto-follow) fetch (or a push). In a scenario where we already have a commit reachable from existing remote-tracking branch and the current transfer (be it a fetch or a push, with or without auto-follow) does not update any remote-tracking branch (because the source side did not have any changes), if the source side added a tag that refers to that commit that the receiving end lacks, that tag needs to be transferred and then stored. So has_sha1_file() is not the right test---if anything, it needs to be checking if the object being checked is reachable from a tip of some ref. But of course, that test is rather expensive, so perhaps the implementation cheated and uses has_sha1_file() instead? The only case it would misidentify would be after an aborted fetch (or push) left unconnected island of objects and some of these objects that are not reachable are pointed at by tags the receiving end does not have. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] modernize t7300
James writes: > From: James Rouzier > > --- Missing sign-off. > t/t7300-clean.sh | 382 > +++ > 1 file changed, 190 insertions(+), 192 deletions(-) > > diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh > index 86ceb38..d555bb6 100755 > --- a/t/t7300-clean.sh > +++ b/t/t7300-clean.sh > @@ -28,15 +28,15 @@ test_expect_success 'git clean with skip-worktree > .gitignore' ' > mkdir -p build docs && > touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && > git clean && > - test -f Makefile && > - test -f README && > - test -f src/part1.c && > - test -f src/part2.c && > - test ! -f a.out && > - test ! -f src/part3.c && > - test -f docs/manual.txt && > - test -f obj.o && > - test -f build/lib.so && > + test_path_is_file Makefile && > + test_path_is_file README && > + test_path_is_file src/part1.c && > + test_path_is_file src/part2.c && OK. > + test_path_is_missing a.out && > + test_path_is_missing src/part3.c && This is better than the original, which may have said "OK" upon seeing a directory called "a.out". > + test_path_is_file docs/manual.txt && > + test_path_is_file obj.o && > + test_path_is_file build/lib.so && OK. > git update-index --no-skip-worktree .gitignore && > git checkout .gitignore > ' > @@ -46,15 +46,15 @@ test_expect_success 'git clean' ' > mkdir -p build docs && > touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && > git clean && > - test -f Makefile && > - test -f README && > - test -f src/part1.c && > - test -f src/part2.c && > - test ! -f a.out && > - test ! -f src/part3.c && > - test -f docs/manual.txt && > - test -f obj.o && > - test -f build/lib.so > + test_path_is_file Makefile && > + test_path_is_file README && > + test_path_is_file src/part1.c && > + test_path_is_file src/part2.c && > + test_path_is_missing a.out && > + test_path_is_missing src/part3.c && > + test_path_is_file docs/manual.txt && > + test_path_is_file obj.o && > + test_path_is_file build/lib.so The verbosity of this conversion makes me wonder if we want to have "test_paths_are_files" and "test_paths_are_missing". For that matter, this test does not really care about the distinction between files and directories (e.g. some tests said "test ! -d docs" and would have passed if there were a 'docs' regular file, but what we really care about is the path 'docs' is _gone_), so what we want may be test_paths_exist and test_paths_are_missing. With that, the above hunk would become test_paths_exist Makefile README src/part1.c src/part2.c \ obj.o build/lib.so && test_paths_are_missing a.out src/part3.c I dunno. > test_expect_success 'git clean -e' ' > rm -fr repo && > mkdir repo && > - ( > - cd repo && > - git init && > - touch known 1 2 3 && > - git add known && > - git clean -f -e 1 -e 2 && > - test -e 1 && > - test -e 2 && > - ! (test -e 3) && > - test -e known > - ) > + cd repo && > + git init && > + touch known 1 2 3 && > + git add known && > + git clean -f -e 1 -e 2 && > + test_path_is_file 1 && > + test_path_is_file 2 && > + test_path_is_missing 3 && > + test_path_is_file known > ' I think this is wrong. The next test piece will be run inside "repo" directory with this patch applied. Don't lose the subshell here. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Multiple fetches when unshallowing a shallow clone
On Mon, Dec 07, 2015 at 01:27:51PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > I think one thing I was missing is that we need to just grab the > > _object_, but we need to realize that the ref needs updating[1]. So we > > cannot skip backfill of any tag that we do not already have, even if we > > already have the tag object. > > ... > > [1] I'm still puzzled why find_non_local_tags uses has_sha1_file() on > > the tag object at all, then. > > The designed semantics of auto-following tags (not necessarily as > implemented or documented, i.e. there may be implementation or > documentation bugs), I think, is to arrive at the same state as > doing a fetch (or a push) without the auto-following and then doing > a separate fetch (or a push) of tags that point at the objects that > are reachable from the tips of refs after finishing the first > (i.e. without auto-follow) fetch (or a push). In a scenario where > we already have a commit reachable from existing remote-tracking > branch and the current transfer (be it a fetch or a push, with or > without auto-follow) does not update any remote-tracking branch > (because the source side did not have any changes), if the source > side added a tag that refers to that commit that the receiving end > lacks, that tag needs to be transferred and then stored. > > So has_sha1_file() is not the right test---if anything, it needs to > be checking if the object being checked is reachable from a tip of > some ref. > > But of course, that test is rather expensive, so perhaps the > implementation cheated and uses has_sha1_file() instead? The only > case it would misidentify would be after an aborted fetch (or push) > left unconnected island of objects and some of these objects that > are not reachable are pointed at by tags the receiving end does not > have. I may have confused myself. There are actually two has_sha1_file() calls in find_non_local_tags. I agree it is the only sensible test for "do we have the commit this tag peels to, and if so, we want to grab the tag". Reachability is too expensive to compute. But for the other one ("do we have the tag object itself"), I initially claimed "if we have the tag object already, we do not have to do the backfill fetch". Which is not quite true. We have to update the ref even if we have the tag object. But then, what if we have the tag object for other reasons (e.g., because another tag points at it?). E.g. in this sequence: git -C parent commit --allow-empty -m base git -C parent tag -m mytag foo git clone parent child git -C parent update-ref refs/tags/bar foo git -C child fetch we must backfill refs/tags/bar during the fetch, even though we already have the object. I don't see any point in checking has_sha1_file() for the tagged object at all. If we don't have it, we obviously must fetch. And if we do have it, we must fetch the ref, even if that results in no objects transferred. It's entirely possible I'm just confused, and AFAICT nobody has noticed any breakage here, so please don't feel you need to spend a lot of time humoring me. I'm just writing up my confusion for posterity. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] clean: new option --exclude-from
James writes: > +static int exclude_from_cb(const struct option *opt, > + const char *arg, int unset) > +{ > + struct dir_struct *dir = opt->value; > + add_excludes_from_file(dir, arg); I suspect this is wrong. add_excludes_from_file() creates a new excludes_list that is separate from the command line level and pushes that down to the exclude stack. You'd instead need to add each line of the input at the same EXC_CMDL level like -e option does from the command line, 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 v2] revision.c: fix possible null pointer access
Am 07.12.2015 um 21:31 schrieb Junio C Hamano: Stefan Naewe writes: mark_tree_uninteresting dereferences a tree pointer before checking if the pointer is valid. Fix that by doing the check first. Signed-off-by: Stefan Naewe --- I still have a problem with "dereferences", as "dereference" is about computing an address and accessing memory based on the result, and only the first half is happening here. I can live with "The function does a pointer arithmetic on 'tree' before it makes sure that 'tree' is not NULL", but in any case, let's queue this as-is for now and wait for a while to see if others can come up with a more appropriate phrases. Don't shoo away language lawyers, because this is a pure C language rule patch. If this were only about pointer arithmetic, a change would not be necessary. But it isn't. The patch corrects a case where the compiler can remove a NULL pointer check that we actually want to remain. The language rule that gives sufficient room for interpretation to the compiler is about dereferencing a pointer. It is irrelevant that an address of an object is taken after the dereference and then only pointer arithmetic remains---the dereference has already taken place, and that cannot occur for a NULL pointer in a valid program. So, the phrase "dereference" is precise and correct here. -- Hannes Thanks. revision.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/revision.c b/revision.c index 0fbb684..8c569cc 100644 --- a/revision.c +++ b/revision.c @@ -135,10 +135,12 @@ static void mark_tree_contents_uninteresting(struct tree *tree) void mark_tree_uninteresting(struct tree *tree) { - struct object *obj = &tree->object; + struct object *obj; if (!tree) return; + + obj = &tree->object; if (obj->flags & UNINTERESTING) return; obj->flags |= UNINTERESTING; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Why does send-pack call pack-objects for all remote refs?
Daniel Koverman writes: > I have a repository which has ~2000 branches on the remote, and it > takes ~8 seconds to push a change to one ref. The majority of this > time is spent in pack-object. I wrote a hack so that only the ref > being updated would be packed (the normal behavior is to pack for > every ref on the remote). I am having a hard time understanding what you are trying to say, as nobody's pack-objects "packs for a ref" or "packs a ref", so my response has to be based on my best guess---I think you are talking about feeding the object names of the tips of all remote refs as the bottoms of the revision range to pack-objects. When you are pushing your 'topic' branch to update the 'topic' branch at the remote, it is true that we compute git rev-list --objects $your_topic --not $all_of_the_remote_refs to produce a packfile. And by tweaking this to git rev-list --objects $your_topic --not $their_topic you will cut down the processing time of 'rev-list', especially if you have insane number of refs at the remote end. There is a price you would pay for doing so, though. An obvious one is what if the 'topic' branch does not exist yet at the remote. Without the "--not ..." part, you would end up sending the entire history behind $your_topic, and the way you prevent that from happening is to give what are known to exist at the remote end. Even when there already is 'topic' at the remote, the contents at the paths that are different between your 'topic' and the 'topic' as exists at the remote may already exist on some other branches that are already at the remote (e.g. you may have merged some branches that are common between your repository and the remote, and the only object missing from the remote that your repository has to send may be a merge commit and the top-level tree object), but limiting the bottoms of the revision range only to "--not $their_topic" would rob this obvious optimization opportunity from you. There has to be some way to limit the list of remote-refs that are used as bottoms of the revision range. For example, if you know that the remote has all the tags, and that everything in the v1.0 tag is contained in the v2.0 tag, then a single "--not v2.0" should give the same result as "--not v1.0 v2.0" that lists both. But the computation that is needed to figure out which tags and branches are not worth listing as bottoms would need to look at all of them at least once anyway, so a naive implementation of such would end up spending the same cycles, I would suspect. Also it was unclear if you are working with a shallow repository. The performance trade-off made between the packsize and the cycles is somewhat different between a normal and a shallow repository, e.g. 2dacf26d (pack-objects: use --objects-edge-aggressive for shallow repos, 2014-12-24) might be a good starting point to think about this issue. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] modernize t7300
On Sun, Dec 6, 2015 at 9:58 AM, James wrote: > From: James Rouzier This would be a good place to explain how you are modernizing the test script. Right now, you're just updating to take advantage of test_path_is_foo(), but some other modernizations you could do include: * using '>' rather than 'touch' to create empty files when the timestamp doesn't matter (which is all cases in this script) * (optional) replace unnecessarily complex 'mkdir -p foo' with simpler 'mkdir foo' (but leave "mkdir -p foo/bar" as is) * drop blank lines before and after test body; for instance: test_expect_success 'foo' ' blah && bloo ' becomes: test_expect_success 'foo' ' blah && bloo ' > --- > diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh > @@ -609,32 +609,30 @@ test_expect_success 'force removal of nested git work > tree' ' > test_expect_success 'git clean -e' ' > rm -fr repo && > mkdir repo && > - ( > - cd repo && > - git init && > - touch known 1 2 3 && > - git add known && > - git clean -f -e 1 -e 2 && > - test -e 1 && > - test -e 2 && > - ! (test -e 3) && > - test -e known > - ) > + cd repo && The previous working directory is not automatically restored at the end of the test, so this "cd" without corresponding "cd .." causes following tests to execute at the incorrect location. Unfortunately, you can't just place "cd .." at the end of a test since it will be skipped if something before the "cd .." fails. The way the existing code deals with this is by using a subshell: mkdir repo && ( cd repo && ... ) The "cd repo" is inside the subshell, so it doesn't affect the parent shell, and when the subshell exits, the parent shell remains at the correct working directory (regardless of whether the test succeeded or failed). Therefore, you don't want to drop the subshell. > + git init && > + touch known 1 2 3 && > + git add known && > + git clean -f -e 1 -e 2 && > + test_path_is_file 1 && > + test_path_is_file 2 && > + test_path_is_missing 3 && > + test_path_is_file known > ' -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] modernize t7300
On Mon, Dec 7, 2015 at 4:40 PM, Junio C Hamano wrote: > James writes: >> @@ -46,15 +46,15 @@ test_expect_success 'git clean' ' >> mkdir -p build docs && >> touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && >> git clean && >> - test -f Makefile && >> - test -f README && >> - test -f src/part1.c && >> - test -f src/part2.c && >> - test ! -f a.out && >> - test ! -f src/part3.c && >> - test -f docs/manual.txt && >> - test -f obj.o && >> - test -f build/lib.so >> + test_path_is_file Makefile && >> + test_path_is_file README && >> + test_path_is_file src/part1.c && >> + test_path_is_file src/part2.c && >> + test_path_is_missing a.out && >> + test_path_is_missing src/part3.c && >> + test_path_is_file docs/manual.txt && >> + test_path_is_file obj.o && >> + test_path_is_file build/lib.so > > The verbosity of this conversion makes me wonder if we want to have > "test_paths_are_files" and "test_paths_are_missing". For that > matter, this test does not really care about the distinction between > files and directories (e.g. some tests said "test ! -d docs" and > would have passed if there were a 'docs' regular file, but what we > really care about is the path 'docs' is _gone_), so what we want may > be test_paths_exist and test_paths_are_missing. With that, the > above hunk would become > > test_paths_exist Makefile README src/part1.c src/part2.c \ > obj.o build/lib.so && > test_paths_are_missing a.out src/part3.c > > I dunno. Alternately, update test_path_foo() functions to accept multiple pathnames, or is that too ugly? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] modernize t7300
Eric Sunshine writes: > Alternately, update test_path_foo() functions to accept multiple > pathnames, or is that too ugly? That actually would have been my first choice, except (1) that path_is_missing has a cruft whose usefulness is dubious, and (2) that "path_exists A B C" and "path_is_missing D E F" would be gramatically incorrect. I think we should first see if we can remove that "customized message" that appears only in path_is_missing and remove it if we can. Extending "path_is_missing A B C" and friends to work would then become trivial. 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 2/2] clean: new option --exclude-from
In addition to Peff's and Junio's review comments... On Sun, Dec 6, 2015 at 9:58 AM, James wrote: > From: James Rouzier > > Specify a file to read for exclude patterns. Missing Signed-off-by:. > --- > diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh > @@ -628,6 +628,66 @@ test_expect_success 'git clean -e' ' > +test_expect_success 'git clean --exclude-from' ' > + rm -fr repo && > + mkdir repo && > + cd repo && See my review comments for patch 1/2 as to why you want to wrap 'cd' and remaining statements in a subshell. > + git init && > + touch known 1 2 3 && Likewise, use '>' rather than 'touch' to create empty files when the timestamp isn't significant. >1 && >2 && >3 && > + git add known && > + cat >.git/clean-exclude <<-\EOF && > + 1 > + 2 > + EOF > + git clean -f --exclude-from=.git/clean-exclude && > + test_path_is_file 1 && > + test_path_is_file 2 && > + test_path_is_missing 3 && > + test_path_is_file known > +' > + > +test_expect_success 'git clean -e --exclude-from' ' > + rm -fr repo && > + mkdir repo && > + cd repo && > + git init && > + touch known 1 2 3 && > + git add known && > + echo 1 >> .git/clean-exclude && > + git clean -f -e 2 --exclude-from=.git/clean-exclude && > + test_path_is_file 1 && > + test_path_is_file 2 && > + test_path_is_missing 3 && > + test_path_is_file known > +' > + > +test_expect_success 'git clean --exclude-from --exclude-from' ' > + rm -fr repo && > + mkdir repo && > + git init && > + touch known 1 2 3 && > + git add known && > + cat >.git/clean-exclude1 <<-\EOF && > + 1 > + EOF > + cat >.git/clean-exclude2 <<-\EOF && > + 2 > + EOF Creation of these single-line files probably would be more readable using 'echo', as you do in the test just above (for .git/clean-exclude): echo 1 >.git/clean-exclude1 && echo 2 >.git/clean-exclude2 && > + git clean -f --exclude-from=.git/clean-exclude1 > --exclude-from=.git/clean-exclude2 && > + test_path_is_file 1 && > + test_path_is_file 2 && > + test_path_is_missing 3 && > + test_path_is_file known > +' > + > +test_expect_success 'git clean --exclude-from=BADFILE' ' > + rm -fr repo && > + mkdir repo && > + cd repo && > + git init && > + test_expect_code 128 git clean -f > --exclude-from=.git/clean-exclude-not-there > +' > + > test_expect_success SANITY 'git clean -d with an unreadable empty directory' > ' > mkdir foo && > chmod a= foo && > -- > 2.3.6 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: best practices against long git rebase times?
Jeff King writes: > You're computing the patch against the parent for each of those 3000 > commits (to get a hash of it to compare against the single hash on the > other side). Twelve minutes sounds long, but if you have a really > gigantic tree, it might not be unreasonable. > > You can also try compiling with "make XDL_FAST_HASH=" (i.e., setting > that option to the empty string). Last year I found there were some > pretty suboptimal corner cases, and you may be hitting one (we should > probably turn that option off by default; I got stuck on trying to find > a hash that would perform faster and never followed up[1]. > > I doubt that is your problem, but it's possible). > > -Peff > > [1] http://thread.gmane.org/gmane.comp.version-control.git/261638 I vaguely recall having discussed caching the patch-ids somewhere so that this does not have to be done every time. Would such an extension help here, I wonder? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Why does send-pack call pack-objects for all remote refs?
On Mon, Dec 07, 2015 at 02:41:00PM -0800, Junio C Hamano wrote: > Also it was unclear if you are working with a shallow repository. > The performance trade-off made between the packsize and the cycles > is somewhat different between a normal and a shallow repository, > e.g. 2dacf26d (pack-objects: use --objects-edge-aggressive for > shallow repos, 2014-12-24) might be a good starting point to think > about this issue. Also note that for a while the "aggressive" form was used everywhere. I think that started in fbd4a70 (list-objects: mark more commits as edges in mark_edges_uninteresting - 2013-08-16), and was fixed in 1684c1b (rev-list: add an option to mark fewer edges as uninteresting, 2014-12-24). So it was present from v1.8.4.2 up to v2.3.0. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: best practices against long git rebase times?
On Mon, Dec 07, 2015 at 02:56:33PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > You're computing the patch against the parent for each of those 3000 > > commits (to get a hash of it to compare against the single hash on the > > other side). Twelve minutes sounds long, but if you have a really > > gigantic tree, it might not be unreasonable. > > > > You can also try compiling with "make XDL_FAST_HASH=" (i.e., setting > > that option to the empty string). Last year I found there were some > > pretty suboptimal corner cases, and you may be hitting one (we should > > probably turn that option off by default; I got stuck on trying to find > > a hash that would perform faster and never followed up[1]. > > > > I doubt that is your problem, but it's possible). > > > > -Peff > > > > [1] http://thread.gmane.org/gmane.comp.version-control.git/261638 > > I vaguely recall having discussed caching the patch-ids somewhere so > that this does not have to be done every time. Would such an > extension help here, I wonder? I think you missed John's earlier response which gave several pointers to such caching schemes. :) I used to run with patch-id-caching in my personal fork (I frequently use "git log --cherry-mark" to see what has made it upstream), but I haven't for a while. It did make a big difference in speed, but I never resolved the corner cases around cache invalidation. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: best practices against long git rebase times?
Jeff King writes: > I think you missed John's earlier response which gave several pointers > to such caching schemes. :) Yeah, you're right. > > I used to run with patch-id-caching in my personal fork (I frequently > use "git log --cherry-mark" to see what has made it upstream), but I > haven't for a while. It did make a big difference in speed, but I never > resolved the corner cases around cache invalidation. > > -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2015, #02; Fri, 4)
Jeff King writes: > On Sun, Dec 06, 2015 at 10:12:18AM -0600, Edmundo Carmona Antoranz wrote: > >> Hi, Junio, Jeff! >> >> On Fri, Dec 4, 2015 at 5:15 PM, Junio C Hamano wrote: >> > * ec/annotate-deleted (2015-11-20) 1 commit >> > - annotate: skip checking working tree if a revision is provided >> > >> > Usability fix for annotate-specific " " syntax with deleted >> > files. >> > >> > Waiting for review. >> >> Is there something I have to do about it? > > A gentle ping is sometimes helpful. :) > > I did not see anything wrong with it, but I think Junio would be a good > person to give it a look, as he knows all of the sordid history of blame > versus annotate, and the intended allowed command-line orderings. If you bring "intended allowed command-line orderings" into the picture, then I would have to say that any effort that encourages the use of paths before revs like this patch does should be frowned upon---if anything, we should actively discouraging people from feeding paths before revs, and this change goes in a wrong direction by adding a special case only to annotate, giving another excuse to people who think "git log path rev" should work "because annotate does". If the change is just to 'annotate path rev', and does not change 'blame path rev', it would be less distasteful, though. I'd have to think about 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 13/16] init: allow alternate backends to be set for new repos
On Sat, 2015-12-05 at 02:44 -0500, Jeff King wrote: > On Sat, Dec 05, 2015 at 07:30:13AM +0100, Duy Nguyen wrote: > > > On Thu, Dec 3, 2015 at 1:35 AM, David Turner > > wrote: > > > git init learns a new argument --refs-backend-type. Presently, only > > > "files" is supported, but later we will add other backends. > > > > > > When this argument is used, the repository's core.refsBackendType > > > configuration value is set, and the refs backend's initdb function is > > > used to set up the ref database. > > > > git-init can also be used on existing repos. What happens in that case > > if we use --refs-backend-type. Will existing refs be migrated to the > > new backend or hidden away? I'd like to migrate, but I don't want to write that code prematurely. I figure we can get everything else right first. > It would be neat if it migrated, but I suspect that may introduce > complications. It's probably OK in the initial implementation to bail if > the option is used in an existing repo. Will fix. > I think the config option needs to be extensions.refsBackendType, too, > per the logic in 00a09d5 (introduce "extensions" form of > core.repositoryformatversion, 2015-06-23). And I guess it needs to bump > core.repositoryformatversion to "1". I did that in a later patch, but now is a better time; will fix. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: add an option to suppress commit hash
On Mon, Dec 07, 2015 at 11:34:59AM -0800, Junio C Hamano wrote: > Two (big) problems with the option name. > > - "--no-something" would mislead people to think you are removing >something, not replacing it with something else. This option >does the latter (i.e. the first line of your output still has >40-hex; it's just it no longer has a useful 40-hex). Right. I originally considered removing that line altogether, but other parts of Git rely on that header to process patches correctly. > - There are many places we use hexadecimal strings in format-patch >output and you are not removing or replacing all of them, only >the commit object name on the fake "From " line. Saying "hash" >would mislead readers. I'll reroll with the name --zero-commit, unless somebody comes up with a better suggestion. > > +test_expect_success 'format-patch --no-hash' ' > > + git format-patch --no-hash --stdout v2..v1 >patch2 && > > + cnt=$(egrep "^From 0+ Mon Sep 17 00:00:00 2001" patch2 | wc -l) && > > Don't test "any number of '0'"; test 40 '0's. This is because the > line format was designed to be usable by things like /etc/magic to > detect format-patch output, and we want to notice if/when we break > that aspect of our output format. My idea was to future-proof it against changes in the hash function, although that's in the distant future. I'll reroll with the change you suggested. I might drop in another patch to improve the existing tests to cover the case without this option, as I think we just look for "From " currently. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: PGP signature
Re: rebase has no --verify-signatures
On Mon, Dec 07, 2015 at 03:00:15PM +0100, Alexander 'z33ky' Hirsch wrote: > Is there any technical reason why rebase should not have a > --verify-signatures flag? I have written a patch to git-rebase--am > which enables it to do such a check. If there is no reason not to > include it I'd add documentation and a test and submit it. As far as I know, there is no technical reason that it shouldn't. It's probably that nobody has implemented it yet. I'd certainly be interested in such a patch. For a thorough change, you'd probably want to make it work with git-rebase--merge and git-rebase--interactive as well. I'm sure I'm not the only person who frequently uses rebase -m. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: PGP signature
Re: [PATCH] contrib/subtree: fix "subtree split" skipped-merge bug.
On Mon, Dec 7, 2015 at 3:50 PM, Dave Ware wrote: > [PATCH] contrib/subtree: fix "subtree split" skipped-merge bug. As an aid for reviewers, please indicate the version of this patch submission. For instance, this is the second attempt, so the subject would be decorated as [PATCH v2], and the next one (if submitted) will be v3. The -v option of git-format-patch can help automate this. Style: drop the full-stop (period) from the subject line > A bug occurs in 'git-subtree split' where a merge is skipped even when > both parents act on the subtree, provided the merge results in a tree > identical to one of the parents. Fix by copying the merge if at least > one parent is non-identical, and the non-identical parent is not an > ancestor of the identical parent. > > Also adding a test case, this checks that a descendant can be pushed to s/Also adding/Also, add/ s/, this/which/ > it's ancestor in this case. s/it's/its/ > Signed-off-by: Dave Ware > --- Right here below the "---" line is a good place to describe what changed since the previous version. For instance, in v2, you made minor improvements to the commit message, added your sign-off, folded the new test into the existing t7900-subtree.sh, added a subshell around 'cd', and assigned the output of git-rev-list to a shell variable rather than dumping it to a file. Including a link to the previous version, like this[1], is also reviewer-friendly. [1]: http://thread.gmane.org/gmane.comp.version-control.git/282065 As before, I'm not a git-subtree user, so this review is superficial. More below... > contrib/subtree/git-subtree.sh | 12 +++-- > contrib/subtree/t/t7900-subtree.sh | 52 > ++ > 2 files changed, 62 insertions(+), 2 deletions(-) > > diff --git a/contrib/subtree/t/t7900-subtree.sh > b/contrib/subtree/t/t7900-subtree.sh > index 9051982..ea991eb 100755 > --- a/contrib/subtree/t/t7900-subtree.sh > +++ b/contrib/subtree/t/t7900-subtree.sh > @@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' ' > )) > ' > > +test_expect_success 'subtree descendent check' ' > + mkdir git_subtree_split_check && > + ( > +cd git_subtree_split_check && Style: indent with tabs rather than spaces > +git init && > + > +mkdir folder && > + > +echo a >folder/a && > +git add . && > +git commit -m "first commit" && > + > +git branch branch && > + > +echo 0 >folder/0 && > +git add . && > +git commit -m "adding 0 to folder" && > + > +echo b >folder/b && > +git add . && > +git commit -m "adding b to folder" && > +cherry=$(git rev-list HEAD -1) && git-rev-parse would probably be more idiomatic: cherry=$(git rev-parse HEAD) > +git checkout branch && > +echo text >textBranch.txt && > +git add . && > +git commit -m "commit to fiddle with branch: branch" && > + > +git cherry-pick $cherry && > +git checkout master && > +git merge -m "merge" branch && > + > +git branch noop_branch && > + > +echo d >folder/d && > +git add . && > +git commit -m "adding d to folder" && > + > +git checkout noop_branch && > +echo moreText >anotherText.txt && > +git add . && > +git commit -m "irrelevant" && > + > +git checkout master && > +git merge -m "second merge" noop_branch && > + > +git subtree split --prefix folder/ --branch subtree_tip master && > +git subtree split --prefix folder/ --branch subtree_branch branch && > +git push . subtree_tip:subtree_branch > + ) > + ' > + > test_done > -- > 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] gitk: let .bleft.mid widgets 'breathe'
The widgets on top of the diff window are very tightly packed. Make them breathe a little by adding an 'i'-spaced padding between them. Signed-off-by: Giuseppe Bilotta --- gitk | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gitk b/gitk index e04264b..b621762 100755 --- a/gitk +++ b/gitk @@ -2361,6 +2361,9 @@ proc makewindow {} { ${NS}::frame .bleft.mid ${NS}::frame .bleft.bottom +# gap between sub-widgets +set wgap [font measure uifont "i"] + ${NS}::button .bleft.top.search -text [mc "Search"] -command dosearch pack .bleft.top.search -side left -padx 5 set sstring .bleft.top.sstring @@ -2375,8 +2378,9 @@ proc makewindow {} { -command changediffdisp -variable diffelide -value {0 1} ${NS}::radiobutton .bleft.mid.new -text [mc "New version"] \ -command changediffdisp -variable diffelide -value {1 0} + ${NS}::label .bleft.mid.labeldiffcontext -text " [mc "Lines of context"]: " -pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left +pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left -ipadx $wgap spinbox .bleft.mid.diffcontext -width 5 \ -from 0 -increment 1 -to 1000 \ -validate all -validatecommand "diffcontextvalidate %P" \ @@ -2384,7 +2388,7 @@ proc makewindow {} { .bleft.mid.diffcontext set $diffcontext trace add variable diffcontextstring write diffcontextchange lappend entries .bleft.mid.diffcontext -pack .bleft.mid.labeldiffcontext .bleft.mid.diffcontext -side left +pack .bleft.mid.labeldiffcontext .bleft.mid.diffcontext -side left -ipadx $wgap ${NS}::checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \ -command changeignorespace -variable ignorespace pack .bleft.mid.ignspace -side left -padx 5 -- 2.6.3.659.gfdd8f28 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] gitk: match ttk fonts to gitk fonts
The fonts set in setoptions aren't consistently picked up by ttk, who uses its own predefined fonts. This is noticeable when switching between using and not using ttk with custom fonts or in HiDPI settings (where the default TTK fonts do _not_ respect tk sclaing). Fix by mapping the ttk fontset to the one used by gitk internally. Signed-off-by: Giuseppe Bilotta --- gitk | 18 ++ 1 file changed, 18 insertions(+) diff --git a/gitk b/gitk index fcc606e..e04264b 100755 --- a/gitk +++ b/gitk @@ -1943,6 +1943,8 @@ proc confirm_popup {msg {owner .}} { } proc setoptions {} { +global use_ttk + if {[tk windowingsystem] ne "win32"} { option add *Panedwindow.showHandle 1 startupFile option add *Panedwindow.sashRelief raised startupFile @@ -1965,6 +1967,18 @@ proc setoptions {} { option add *Listbox.font mainfont startupFile } +proc setttkstyle {} { +eval font configure TkDefaultFont [fontflags mainfont] +eval font configure TkTextFont [fontflags textfont] +eval font configure TkHeadingFont [fontflags mainfont] +eval font configure TkCaptionFont [fontflags mainfont] -weight bold +eval font configure TkTooltipFont [fontflags uifont] +eval font configure TkFixedFont [fontflags textfont] +eval font configure TkIconFont[fontflags uifont] +eval font configure TkMenuFont[fontflags uifont] +eval font configure TkSmallCaptionFont [fontflags uifont] +} + # Make a menu and submenus. # m is the window name for the menu, items is the list of menu items to add. # Each item is a list {mc label type description options...} @@ -12356,6 +12370,10 @@ if {![info exists have_ttk]} { set use_ttk [expr {$have_ttk && $want_ttk}] set NS [expr {$use_ttk ? "ttk" : ""}] +if {$use_ttk} { +setttkstyle +} + regexp {^git version ([\d.]*\d)} [exec git version] _ git_version set show_notes {} -- 2.6.3.659.gfdd8f28 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] gitk spacing/sizing tuning for HiDPI
On Mon, Dec 7, 2015 at 5:09 AM, Eric Sunshine wrote: > > Both patches are missing your Signed-off-by:. Doh sorry, resending now. -- Giuseppe "Oblomov" Bilotta -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html