[PATCH] files-backend.c: fix build error on Solaris
This function files_reflog_path returns void, which usually means "return;" not returning "void value" from another function. Reported-by: Ævar Arnfjörð Bjarmason Signed-off-by: Nguyễn Thái Ngọc Duy --- refs/files-backend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 9183875dad..dd8abe9185 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -180,7 +180,8 @@ static void files_reflog_path(struct files_ref_store *refs, break; case REF_TYPE_OTHER_PSEUDOREF: case REF_TYPE_MAIN_PSEUDOREF: - return files_reflog_path_other_worktrees(refs, sb, refname); + files_reflog_path_other_worktrees(refs, sb, refname); + break; case REF_TYPE_NORMAL: strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname); break; -- 2.19.1.1327.g328c130451.dirty
Re: What's cooking in git.git (Sep 2018, #01; Tue, 4)
On Sat, Nov 24, 2018 at 08:33:37PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> On Wed, Sep 05 2018, Ævar Arnfjörð Bjarmason wrote:
>
> > On Wed, Sep 05 2018, Eric Sunshine wrote:
[]
> > SunCC used to be ahead of GCC & Clang when it came to certain classes of
> > warnings, but e.g. now everything it complains about is because it
> > doesn't understand C as well, e.g. we have quite a few compile warnings
> > due to code like this, which it claims is unreachable (but isn't):
> > https://github.com/git/git/blob/v2.19.0-rc2/read-cache.c#L950-L955
>
Wait a second - even if the compiler claims something (wrong)...
there a still 1+1/2 questions from my side:
int verify_path(const char *path, unsigned mode)
{
char c;
^
/* Q1: should "c" be initialized like this: */
char c = *path;
if (has_dos_drive_prefix(path))
return 0;
goto inside;
/* Q2: and why do we need the "goto" here ? */
for (;;) {
if (!c)
return 1;
if (is_dir_sep(c)) {
inside:
Re: git overwriting local ignored files?
On 11/24/18 10:41 AM, Konstantin Khomoutov wrote: On Sat, Nov 24, 2018 at 05:57:24PM +0300, Konstantin Khomoutov wrote: On Sat, Nov 24, 2018 at 09:37:06AM -0500, David Mandelberg wrote: It seems that git is overwriting my local files on merge if they're in .gitignore. [...] The .gitignore file is to list "ignored and expendable" class of files; there is no "ignored but precious class" in Git. Ok. Would a patch be welcome? I have three ideas for how to implement it, and I'm not sure which is better. [...] You might want to first investigate this recent thread [1] which AFAIK was dedicated to exactly this problem. Well, actually the thread is old, but its continuation [2] is recent. The crux is that it discusses certain approaches to solve the apparent problem and patches to do that. 1. https://public-inbox.org/git/4c6a1c5b.4030...@workspacewhiz.com/ 2. https://public-inbox.org/git/871s8qdzph@evledraar.gmail.com/ Thanks for the pointers, and sorry to start a new thread. It looks like most of the work to do is in finding consensus on the right way forward, rather than in writing a patch. While I really would like there be a way to prevent git from overwriting ignored files, I don't have any strong opinions on what that way should look like. So I think I'll just wait and hope somebody else does the work. -- https://david.mandelberg.org/
Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)
Jeff King writes: > I do also think in the long run we should be fixing the "unreachable > always become loose" issues. I think I've seen an idea of collecting them into a garbage pack floated for at least a few times here. What are the downsides? We no longer will know when these unreachable ones were last accessed individually so we need to come up with a different policy around their expiration? As the common traits among objects in such a garbage pack (iow the way we discover the objects that need to be placed in there) does not involve path information and we lose the ability to compress them well?
Re: [PATCH v3 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
Ævar Arnfjörð Bjarmason writes: > On Sun, Oct 21 2018, Nguyễn Thái Ngọc Duy wrote: > > This change has a regression in 2.20: > >> [...] >> static void files_reflog_path(struct files_ref_store *refs, >>struct strbuf *sb, >>const char *refname) >> @@ -158,6 +178,9 @@ static void files_reflog_path(struct files_ref_store >> *refs, >> case REF_TYPE_PSEUDOREF: >> strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname); >> break; >> +case REF_TYPE_OTHER_PSEUDOREF: >> +case REF_TYPE_MAIN_PSEUDOREF: >> +return files_reflog_path_other_worktrees(refs, sb, refname); >> case REF_TYPE_NORMAL: >> strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname); >> break; > > SunCC on Solaris hard errors on this: > > "refs/files-backend.c", line 183: void function cannot return value > > Needs to be files...(); break; instead. True. The caller itself returns "void", so it would be nice if this were a mere warning() from practical usabliity's point of view, though ;-)
Re: How to efficiently backup a bare repository?
Ævar Arnfjörð Bjarmason writes: > There's no easy out of the box way to do exactly what you've > described. A few things come to mind: > ... Wouldn't it suffice to have a cron job that runs something like D=$(date +"%Y-%m-%d") git fetch $serving "refs/*:refs/backup-$D/*" on the back-up box to fetch from the repository on the box the end-users push into once a day? In the back-up repository, the refs/backup-2018-11-25/heads/master reference would be today's tip of the master branch of the serving repository. You can set the expiry timeout to "now" (i.e. "gc" will immediately drop unreachable objects, and that is fine because you expicitly have refs to pin these objects anyway), get the dedup from "git fetch" for free, repack the backup repository as a whole, and dropping the whole refs/backup-2018-10-25/* hierarcy on 2018-11-25 is all you need to expire the refs. You may want to play with the ref-advertisement limiting options in the recent Git, if it is too much to grow the amount of "have"s by 30x for the common ancestry negotiation. But that is a small implementation detail.
Re: [ANNOUNCE] Git v2.20.0-rc1
Ævar Arnfjörð Bjarmason writes:
>> * "git rebase" and "git rebase -i" have been reimplemented in C.
>
> Here's another regression in the C version (and rc1),...
> I wasn't trying to stress test rebase. I was just wanting to rebase a
> history I was about to force-push after cleaning it up, hardly an
> obscure use-case. So [repeat last transmission in
> https://public-inbox.org/git/87y39w1wc2@evledraar.gmail.com/ ]
which, to those who are reading from sidelines:
Given that we're still finding regressions bugs in the rebase-in-C
version should we be considering reverting 5541bd5b8f ("rebase: default
to using the builtin rebase", 2018-08-08)?
I love the feature, but fear that the current list of known regressions
serve as a canary for a larger list which we'd discover if we held off
for another major release (and would re-enable rebase.useBuiltin=true in
master right after 2.20 is out the door).
I am fine with the proposed flip, but I'll have to see the extent of
damage this late in the game so that I won't miss anything. In
addition to the one-liner below, we'd need to update the quoted
release notes entry, and possibly adjust some tests (even though the
"reimplementation" ought to be bug-to-bug compatible, it may not be).
diff --git b/builtin/rebase.c a/builtin/rebase.c
index 9dc8475cd3..60e357c735 100644
--- b/builtin/rebase.c
+++ a/builtin/rebase.c
@@ -54,7 +54,7 @@ static int use_builtin_rebase(void)
cp.git_cmd = 1;
if (capture_command(, , 6)) {
strbuf_release();
- return 1;
+ return 0;
}
strbuf_trim();
Re: How to efficiently backup a bare repository?
On Fri, Nov 23 2018, Guilhem Bonnefille wrote: > I'm managing many bare repositories for development teams. > > One service we want to offer is to let developers retrieve old state > of the repository up to 30 days. For example, one developer > (accidently) removed (push -f) a branch/tag and realize few days later > (after vacations) that it was an error. > > What is the best approach to do this? > > Currently, we use a classical approach, backuping all the repo every > day. But this is far from efficient as: > - we accumulate 30th copies of the repository > - due to packing logic of Git, even if the content is mostly similar, > from one backup to another, there is no way to deduplicate. > > Is there any tricks based on reflog? Even for deleted refs (branch/tags)? > Is there any tooling playing with the internal of git to offer such > feature, like copying all refs in a timestamped refs directory to > retain objects? > > Thanks in advance for any tips letting improve the backup. There's no easy out of the box way to do exactly what you've described. A few things come to mind: a) If you can simply deny non-fast-forwards that's ideal. E.g. for some branches you care about, or tags. This is how most of us deal with this issue in practice. I.e. have some "blessed" refs that matter, and if someone rewinds their own topic branch that's their own problem. b) You could as you touched upon have a post-receive hook that detects non-fast-forwards, and e.g. pushes a clobberd "master" or "v1.0" to some backup repo's 2018-11-24-23-39-04-master or whatever. Then users could grab old versions of refs from that repo. I do a similar thing at work to archive certain refs (old tags), but without renaming them. The advantage is that you get all refs ever, the disadvantage is that you're not going to get a copy of the repo as it was N days ago, it'll need to be manually pieced together. c) Git could be made block-level de-duplication friendly. I was planning to work on it, but it's a small enough itch that I didn't care, but initial results look promising: https://public-inbox.org/git/20180125002942.ga21...@sigill.intra.peff.net/ d) Note that if you're e.g. rsyncing repos that are actively being pushed into you're likely to sometimes end up with corrupt repos unless you're very careful about what you grab and in what order. Best to backup repos with "git fetch". e) If you're burned by one-off cases like this dev going away for 30 days you could bump the default expiry that comes with git from 2 weeks to e.g. 6 weeks. It's still a manual process to recover data (with fsck etc), but at least it's there.
GCC Compile farm (Linux, Solaris, AIX etc.) testing of git.git
I've had access to the GCC Compile Farm for testing on various
architectures for a while. Around the 2.19.0 release I submitted some
patches / bug reports found there.
I've now improved this to run it via GitLab CI & made the output
accessible. Outline of how this works at
https://gitlab.com/git-vcs/git-gitlab-ci/commit/497805b18f
https://gitlab.com/git-vcs/git-ci/branches is a repo that houses a merge
of {master,next,pu} from git.git and that git-gitlab-ci.git repo.
There's still plenty of rough edges to it, but
https://gitlab.com/git-vcs/git-ci/-/jobs already has some useful
results.
Right now this is all pushed out manually. But I'm planning to get it to
a point where soon after Junio pushes changes out a run will kick off on
all these platforms.
The machines it's running on & their build / test configuration is also
something I set up as a one-off. The full list of available machines is
at https://cfarm.tetaneutral.net/machines/list/ and I picked some subset
that looked interesting (outlier platforms) at
https://gitlab.com/git-vcs/git-gitlab-ci/blob/master/.gitlab-ci.yml
Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF
Am 24.11.18 um 15:51 schrieb Frank Schäfer: Am 23.11.18 um 22:47 schrieb Johannes Sixt: Am 23.11.18 um 19:19 schrieb Frank Schäfer: The CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF. It shows up as expected in '-' lines when the ending of the removed line is CR only. It also always shows up as expected in '+' lines. Is your repository configured to (1) highlight whitespace errors in diff output and (2) to leave CRLF alone in text files? I'm using the default configuration, so whitespace is set to trailing-space, but cr-at-eol is not set. If so, then it is just a side-effect of this combination, an illusion, so to say: The CR in the CRLF combo is trailing whitespace. The 'git diff' marks it by inserting an escape sequence to switch the color before ^M and another escape sequence to reset to color after ^M. This breaks the CRLF combination apart, so that the pager does not process it as a combined CRLF sequence; it displays the lone CR as ^M. Urghh... so that needs to be fixed. Why does it work correctly with '+' lines ? I don't think that there is anything to fix. If you have a file with CRLF in it, but you did not declare to Git that CRLF is the expected end-of-line indicator, then the CR *is* trailing whitespace (because the line ends at LF), and 'git diff' highlights it. -- Hannes
Re: [ANNOUNCE] Git v2.20.0-rc1
On Wed, Nov 21 2018, Junio C Hamano wrote: > * "git rebase" and "git rebase -i" have been reimplemented in C. Here's another regression in the C version (and rc1), note: the sha1collisiondetection is just a stand in for "some repo": ( rm -rf /tmp/repo && git init /tmp/repo && cd /tmp/repo && for c in 1 2 do touch $c && git add $c && git commit -m"add $c" done && git remote add origin https://github.com/cr-marcstevens/sha1collisiondetection.git && git fetch && git branch --set-upstream-to origin/master && git rebase -i ) The C version will die with "fatal: unable to read tree ". Running this with rebase.useBuiltin=false does the right thing and rebases as of the merge base of the two (which here is the root of the history). I wasn't trying to stress test rebase. I was just wanting to rebase a history I was about to force-push after cleaning it up, hardly an obscure use-case. So [repeat last transmission in https://public-inbox.org/git/87y39w1wc2@evledraar.gmail.com/ ]
Re: What's cooking in git.git (Sep 2018, #01; Tue, 4)
On Wed, Sep 05 2018, Ævar Arnfjörð Bjarmason wrote:
> On Wed, Sep 05 2018, Eric Sunshine wrote:
>
>> On Wed, Sep 5, 2018 at 4:29 AM Ævar Arnfjörð Bjarmason
>> wrote:
>>> I recently gained access to a Solaris 10 SPARC (5.10) box and discovered
>>> that the chainlint.sed implementation in 2.19.0 has more sed portability
>>> issues.
>>>
>>> First, whoever implemented the /bin/sed on Solaris apparently read the
>>> POSIX requirements for a max length label of 8 to mean that 8 characters
>>> should include the colon, so a bunch of things fail because of that, but
>>> are fixed with a shorter 7 character label.
>>
>> I'm pretty sure that Solaris 'sed' predates POSIX by a good bit, but
>> that's neither here nor there.
>>
>>> Then GIT_TEST_CHAIN_LINT=1 still fails because 878f988350 ("t/test-lib:
>>> teach --chain-lint to detect broken &&-chains in subshells", 2018-07-11)
>>> added a "grep -q" invocation. The /bin/grep on that version of Solaris
>>> doesn't have -q.
>>
>> I knew that '-q' was potentially problematic on some platforms, so I
>> checked and saw that existing tests were already using it, thus went
>> ahead and used it. Dropping '-q' here and redirecting stderr to
>> /dev/null is a perfectly fine alternative.
>>
>>> We fixed a similar issue way back in 80700fde91
>>> ("t/t1304: make a second colon optional in the mask ACL check",
>>> 2010-03-15) by redirecting to /dev/null instead.
>>>
>>> A bunch of other tests in the test suite rely on "grep -q", but nothing
>>> as central as chainlint, so it makes everything break. Do we want to
>>> away with "grep -q" entirely because of old Solaris /bin/grep?
>>
>> I count 132 instances in existing tests (though, I may have missed some).
>>
>>> At this point those familiar with Solaris are screaming ("why are you
>>> using anything in /bin!"). Okey fine, but it mostly worked before, so
>>> are we OK with breaking it? "Mostly" here is "test suite would fail
>>> 20-30 tests for various reasons, but at least no failures in test-lib.sh
>>> and the like".
>>>
>>> However, if as config.mak.uname does we run the tests with
>>> PATH=/usr/xpg6/bin:/usr/xpg4/bin:$PATH, at this point sed is fine with 8
>>> character labels [...]
>>
>> So, if you run the tests via 'make test' (or whatever), then you get
>> /usr/xpg?/bin in PATH, but if you run an individual test script (and
>> haven't set your PATH appropriately), then you encounter the problems
>> you report above?
>
> You need to manually set the PATH before running the tests, the
> SANE_TOOL_PATH just affects git-sh-setup. We should probably fix that,
> i.e. now if you compile git our shellscripts will use the fixed paths,
> but not the tests.
>
>>> [...] but starts complaining about this (also in
>>> chainlint.sed):
>>>
>>> sed: Too many commands, last: s/\n//
>>
>> Erm. Any additional context to help narrow this down?
>
> I tried playing around with it a bit, but honestly don't understand
> enough of this advanced sed syntax to make any headway, it's complaining
> about e.g. the "check for multi-line double-quoted string" rule, but
> then if you remove the s/\n// from there it complains about the next
> rule in that format.
>
> If you want to dig into this yourself I think the best way forward is
> the last two paragraphs of
> https://public-inbox.org/git/20180824152016.20286-1-ava...@gmail.com/
>
>>> diff --git a/config.mak.uname b/config.mak.uname
>>> @@ -163,6 +163,10 @@ ifeq ($(uname_S),SunOS)
>>> BASIC_CFLAGS += -D__EXTENSIONS__ -D__sun__
>>> + # t/chainlint.sed is hopelessly broken all known (tested
>>> + # Solaris 10 & 11) versions of Solaris, both /bin/sed and
>>> + # /usr/xpg4/bin/sed
>>> + GIT_TEST_CHAIN_LINT = 0
>>> endif
>>>
>>> It slightly sucks to not have chainlint on
>>> Solaris, but it would also suck to revert chainlint.sed back to 2.18.0
>>> (there were some big improvements). So I think the patch above is the
>>> best way forward, especially since we're on rc2. What do you think?
>>
>> Keeping in mind that the main goal of 'chainlint' is to prevent _new_
>> breakage from entering the test suite, disabling 'chainlint' on
>> Solaris is an entirely reasonable way forward. In present day, it
>> seems quite unlikely that we'll see someone develop new tests on
>> Solaris, so having 'chainlint' disabled there isn't likely to be a big
>> deal. Moreover, if someone does write a new test on Solaris which has
>> a broken &&-chain in a subshell, that breakage will be caught very
>> quickly once the test is run by anyone on Linux or MacOS (or Windows
>> or BSD or AIX), so it's not like the broken test will make it into the
>> project undetected.
>
> Since writing my initial message I see that the CSW package packaging
> git on Solaris just uses GNU userland:
> https://sourceforge.net/p/gar/code/HEAD/tree/csw/mgar/pkg/git/trunk/Makefile
>
> I.e. it puts /opt/csw/gnu in its $PATH, so this whole thing is probably
> a non-issue. I.e. if the
Re: [PATCH v3 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
On Sun, Oct 21 2018, Nguyễn Thái Ngọc Duy wrote: This change has a regression in 2.20: > [...] > static void files_reflog_path(struct files_ref_store *refs, > struct strbuf *sb, > const char *refname) > @@ -158,6 +178,9 @@ static void files_reflog_path(struct files_ref_store > *refs, > case REF_TYPE_PSEUDOREF: > strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname); > break; > + case REF_TYPE_OTHER_PSEUDOREF: > + case REF_TYPE_MAIN_PSEUDOREF: > + return files_reflog_path_other_worktrees(refs, sb, refname); > case REF_TYPE_NORMAL: > strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname); > break; SunCC on Solaris hard errors on this: "refs/files-backend.c", line 183: void function cannot return value Needs to be files...(); break; instead.
How to add edges to visualize cherry-picks and ported patches
Hi, I hope this is the right place for this answer. If not, plaese point me to a more appropriate place. A project "P2" [2] forked from another project "P1" [1] quite some time ago, both repos share a common history up to some point. After this point, P2 cherry-picked commits from P1, but did not merge P1 any longer. Unfortunately the author of P2 did not use any mechanism (e.g. an intermediate branch) to allow tracking up to which point P1 commits are considered. Thus the graph looks like this: P1: --A--B--C--X--X--D--E--X--X--X--F-- P2: --Y--Y--A--B--C--D--E--Y--F-- I would like to add edges (say: another branch and merge-commits) to the graph to make it look something like this: P1: --A--B--C--X--X--D--E--X--X--X--F-- \ \ \ o---o---o---new branch / / / P2: --Y--Y--A--B--C---D--EY--F-- Of course the new branch should contain the same code as the respective P2-commit. So at the end, the new branch couls become the P2's new "master" branch. How can I achieve this (which commands to use)? Is there some way to automate this? (e.g. based on `git cherry`) Thanks in advance for any answer [1] https://github.com/siacs/Conversations [2] https://github.com/kriztan/Pix-Art-Messenger -- Regards Hartmut Goebel | Hartmut Goebel | h.goe...@crazy-compilers.com | | www.crazy-compilers.com | compilers which you thought are impossible |
Re: git overwriting local ignored files?
On Sat, Nov 24, 2018 at 05:57:24PM +0300, Konstantin Khomoutov wrote: > On Sat, Nov 24, 2018 at 09:37:06AM -0500, David Mandelberg wrote: > > > > > It seems that git is overwriting my local files on merge if they're in > > > > .gitignore. > [...] > > > The .gitignore file is to list "ignored and expendable" class of > > > files; there is no "ignored but precious class" in Git. > > Ok. Would a patch be welcome? I have three ideas for how to implement it, > > and I'm not sure which is better. > [...] > > You might want to first investigate this recent thread [1] which AFAIK > was dedicated to exactly this problem. Well, actually the thread is old, but its continuation [2] is recent. The crux is that it discusses certain approaches to solve the apparent problem and patches to do that. 1. https://public-inbox.org/git/4c6a1c5b.4030...@workspacewhiz.com/ 2. https://public-inbox.org/git/871s8qdzph@evledraar.gmail.com/
Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF
On Sat, Nov 24, 2018 at 03:51:26PM +0100, Frank Schäfer wrote: [] > > Hmm... is CR-only line termination supported at all ? > E.g. 'eol' can be set to 'lf' or 'crlf' but not 'cr'... > No, CR-only is not supported, because: Nobody was implementing it, and that is probably because the only question abou CR-only (at least what I remember) was a when an old Mac OS (not the Mac OS X) was used (which used to use CR instead of LF). And, such feature may be implemented by writing a filter, replace CR with LF as "clean" and "LF" with "CR" for smudge.
Re: git overwriting local ignored files?
On Sat, Nov 24, 2018 at 09:37:06AM -0500, David Mandelberg wrote: > > > It seems that git is overwriting my local files on merge if they're in > > > .gitignore. [...] > > The .gitignore file is to list "ignored and expendable" class of > > files; there is no "ignored but precious class" in Git. > Ok. Would a patch be welcome? I have three ideas for how to implement it, > and I'm not sure which is better. [...] You might want to first investigate this recent thread [1] which AFAIK was dedicated to exactly this problem. 1. https://public-inbox.org/git/4c6a1c5b.4030...@workspacewhiz.com/
Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF
Am 23.11.18 um 22:47 schrieb Johannes Sixt: > Am 23.11.18 um 19:19 schrieb Frank Schäfer: >> The CR marker ^M doesn't show up in '-' lines of diffs when the ending >> of the removed line is CR+LF. >> It shows up as expected in '-' lines when the ending of the removed line >> is CR only. >> It also always shows up as expected in '+' lines. > > Is your repository configured to (1) highlight whitespace errors in > diff output and (2) to leave CRLF alone in text files? I'm using the default configuration, so whitespace is set to trailing-space, but cr-at-eol is not set. > > If so, then it is just a side-effect of this combination, an illusion, > so to say: The CR in the CRLF combo is trailing whitespace. The 'git > diff' marks it by inserting an escape sequence to switch the color > before ^M and another escape sequence to reset to color after ^M. This > breaks the CRLF combination apart, so that the pager does not process > it as a combined CRLF sequence; it displays the lone CR as ^M. Urghh... so that needs to be fixed. Why does it work correctly with '+' lines ? > > It is easy to achieve the opposite effect, i.e., that ^M is not > displayed. For example with these lines in .git/info/attributes or > .gitattributes: > > *.cpp > whitespace=trailing-space,cr-at-eol,indent-with-non-tab,space-before-tab > *.h > whitespace=trailing-space,cr-at-eol,indent-with-non-tab,space-before-tab > > Note the cr-at-eol. (There may be shorter versions to achieve a > similar effect.) With cr-at-eol, ^M indeed doesn't show up anymore in '+' lines with CR+LF line endings. That's correct/expected. '-' lines with CR+LF line endings are displayed correctly in this case, too. However, ^M still shows up in '+' and '-' lines with CR line endings. Hmm... is CR-only line termination supported at all ? E.g. 'eol' can be set to 'lf' or 'crlf' but not 'cr'... Regards, Frank > > -- Hannes
Re: git overwriting local ignored files?
On 11/23/18 11:22 PM, Junio C Hamano wrote: David Mandelberg writes: It seems that git is overwriting my local files on merge if they're in .gitignore. See command transcript below. I searched `git help config` and Google, but I couldn't find any way to prevent it. Am I missing something? (The reason I care about ignored files is that I'm using git with a working directory of $HOME to manage my dotfiles, and most files in my $HOME are not tracked by git but are still important.) The .gitignore file is to list "ignored and expendable" class of files; there is no "ignored but precious class" in Git. Ok. Would a patch be welcome? I have three ideas for how to implement it, and I'm not sure which is better. 1. Add a boolean config option called core.preserveIgnore (or similar). I'm guessing this is the least amount of work, but it's not very flexible. I'm also not sure how it should work with `git clean`. 2. Add some syntax to the .gitignore format so the same file can list both "ignored and expendable" and "ignored and precious". I could see using this to mark compiled files as ignored+expendable and per-repo editor/IDE config files as ignored+precious. Do you have any idea how big of a patch this would be? Or any idea for the new syntax? I think starting a line with "\x" where x is any character not currently escaped by a backslash would cause the least disruption to existing .gitignore files. 3. Like (2), but use a separate file instead of .gitignore (and the other git-ignore files). Any thoughts? -- https://david.mandelberg.org/
[PATCH] http-backend: enable cleaning up forked upload/receive-pack on exit
If http-backend dies because of errors, started upload-pack or receive-pack are not killed and waited, but rather stay running for somtime until they exits because of closed stdin. It may be undesirable in working environment, and it also causes occasional failure of t5562, because the processes keep opened act.err, and sometimes write there errors after next test started using the file. Fix by enabling cleaning of the command at http-backed exit. Reported-by: Carlo Arenas Helped-by: Carlo Arenas Signed-off-by: Max Kirillov --- This seems to fix the issue at NetBSD. I verified it manually with strace but could not catch the visible timing effect in tests at Linux. So no tests for it. the "t5562: do not reuse output files" patches are not needed then http-backend.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/http-backend.c b/http-backend.c index 9e894f197f..29e68e38b5 100644 --- a/http-backend.c +++ b/http-backend.c @@ -486,6 +486,8 @@ static void run_service(const char **argv, int buffer_input) if (buffer_input || gzipped_request || req_len >= 0) cld.in = -1; cld.git_cmd = 1; + cld.clean_on_exit = 1; + cld.wait_after_clean = 1; if (start_command()) exit(1); -- 2.19.0.1202.g68e1e8f04e
Re: [PATCH] t5562: do not reuse output files
On Sat, Nov 24, 2018 at 04:47:19PM +0900, Junio C Hamano wrote: > I do agree that forcing the parent to wait, like you described in > the comment, would be far more preferrable, It looks like it can be done as simple as: --- a/http-backend.c +++ b/http-backend.c @@ -486,6 +486,8 @@ static void run_service(const char **argv, int buffer_input) if (buffer_input || gzipped_request || req_len >= 0) cld.in = -1; cld.git_cmd = 1; + cld.clean_on_exit = 1; + cld.wait_after_clean = 1; if (start_command()) exit(1); at least according to strate it does what it should.
Re: [PATCH] t5562: do not reuse output files
On Sat, Nov 24, 2018 at 04:47:19PM +0900, Junio C Hamano wrote: > I do agree that forcing the parent to wait, like you described in > the comment, would be far more preferrable, [...] Stray processes can sometimes have funny effects on an outer test harness, too. E.g., I think I've seen hangs running t5562 under prove, because some process is holding open a pipe descriptor. This would probably fix that, too. > but until that happens,[...] But if we can't do that immediately for some reason, I do agree with everything else you said here. ;) -Peff
Re: [PATCH] t5562: fix perl path
On Fri, Nov 23, 2018 at 01:38:21AM +0200, Max Kirillov wrote: > From: Jeff King > > Some systems do not have perl installed to /usr/bin. Use the variable > from the build settiings, and call perl directly than via shebang. > > Signed-off-by: Max Kirillov > --- > Submitting. Could you sign-off? Also removed shebang from the script as it is > not needed Yep: Signed-off-by: Jeff King As Carlos mentioned, I think you could leave the shebang as documentation, but I'm OK either way. -Peff
Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)
On Thu, Nov 22, 2018 at 07:36:54PM +0100, Ævar Arnfjörð Bjarmason wrote: > > Yeah, my intent had been to circle back around to this, but I just > > hadn't gotten to it. I'm still pondering a config option or similar, > > though I remain unconvinced that the cases in which you've showed it > > being slow are actually realistic or worth worrying about > > FWIW those "used to be 2ms are now 20-40ms" pushes on ext4 are > representative of the actual prod setup I'm mainly targeting. Now, I > don't run on ext4 this patch helps there, but it seems plausible that it > matters to someone who's counting on that performance. Those are actually the more interesting cases, I think. The ones I'm most skeptical of are the multi-second delays due to having 250K+ objects. I do also think in the long run we should be fixing the "unreachable always become loose" issues. > Buh yeah, it's certainly obscure. I don't blame you if you don't want to > hack on it, and not ejecting this out before 2.20 isn't going to break > anything for me. But do you mind if I make it configurable as part of my > post-2.20 "disable collisions?" No, not at all. > > (and certainly having an obscure config option is not enough to help > > most people). If we could have it kick in heuristically, that would be > > better. > > Aside from this specific scenario. I'd really prefer if we avoid having > heuristic performance optimizations at all costs. > > Database servers tend to do that sort of thing with their query planner, > and it results in cases where your entire I/O profile changes overnight > because you're now on the wrong side of some if/else heuristic about > whather to use some index or not. Yes, I agree that's a downside. I just think if we make everybody's performance 10% slower, they're not really going to notice and seek out a config option to flip to fix it. > > However, note that the cache-load for finding abbreviations _must_ have > > the complete list. And has been loading it for some time. So if you run > > "git-fetch", for example, you've already been running this code for > > months (and using the cache in more places is now a free speedup). > > This is reminding me that I need to get around to re-submitting my > core.validateAbbrev series, which addresses this part of the problem: > https://public-inbox.org/git/20180608224136.20220-21-ava...@gmail.com/ Yeah, I still agree that is a reasonable thing to do. > > At the very least, we'd want this patch on top, too. I also think René's > > suggestion use access() is worth pursuing (though to some degree is > > orthogonal to the cache). > > I haven't had time to test that, and wasn't prioritizing it since I > figured this was post-2.20. My hunch is it doesn't matter much if at all > on NFS. The roundtrip time is what matters, whether that roundtrip is > fstat() or access() probably not. Yes, that line of reasoning makes sense to me (but I think an easy 2-3% speedup on local filesystems may be worth it). -Peff
Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)
On Sat, Nov 24, 2018 at 11:11:36AM +0900, Junio C Hamano wrote: > > However, note that the cache-load for finding abbreviations _must_ have > > the complete list. And has been loading it for some time. So if you run > > "git-fetch", for example, you've already been running this code for > > months (and using the cache in more places is now a free speedup). > > > > At the very least, we'd want this patch on top, too. I also think René's > > suggestion use access() is worth pursuing (though to some degree is > > orthogonal to the cache). > > OK, will queue, but I wonder where 66f0 comes from before silently > doing s/66f04152be/3a2e08245c/ myself. Whoops, I thought I made double-sure to pull the commit id from origin/jk/loose-object-cache instead of my original local commits, but obviously I didn't. Thanks for catching it. -Peff
Re: [PATCH 1/2] format-patch: add a more exhaustive --range-diff test
On Sat, Nov 24 2018, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason writes:
>
>> Change the narrow test added in 31e2617a5f ("format-patch: add
>> --range-diff option to embed diff in cover letter", 2018-07-22) to
>> test the full output. This test would have spotted a regression in the
>> output if it wasn't beating around the bush and tested the full
>> output, let's do that.
>
> This looks wrong, given that we are declaring that showing the
> broken --stat in the output is wrong. It makes us to expect a
> known-wrong output from the command.
>
> If the theme of the topic were "what we are giving by default is a
> sensible output when --stat is asked for, but it is rather too noisy
> and our default should be quieter---let's tone it down", then this
> patch in 1/2 and then a behaviour-change patch with updated
> expectation would make sense, but I do not think that is what you
> are aiming for with this series.
>
> Perhaps squash this into the real "fix" to show what the desired
> output should look like with the same patch?
I see you did that in 'pu'. I don't mind, looks good to me.
FWIW I split this up for ease of review, i.e. showing what the output
was before and what effect the code change had, but maybe that was
overdoing it and it's simpler just to have this all in one go.
>> Signed-off-by: Ævar Arnfjörð Bjarmason
>> ---
>> t/t3206-range-diff.sh | 27 ++-
>> 1 file changed, 22 insertions(+), 5 deletions(-)
>>
>> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
>> index e497c1358f..0235c038be 100755
>> --- a/t/t3206-range-diff.sh
>> +++ b/t/t3206-range-diff.sh
>> @@ -249,11 +249,28 @@ for prev in topic master..topic
>> do
>> test_expect_success "format-patch --range-diff=$prev" '
>> git format-patch --stdout --cover-letter --range-diff=$prev \
>> -master..unmodified >actual &&
>> -grep "= 1: .* s/5/A" actual &&
>> -grep "= 2: .* s/4/A" actual &&
>> -grep "= 3: .* s/11/B" actual &&
>> -grep "= 4: .* s/12/B" actual
>> +master..unmodified >actual.raw &&
>> +sed -e "s|^:||" -e "s|:$||" >expect <<-\EOF &&
>> +:1: 4de457d = 1: 35b9b25 s/5/A/
>> +: a => b | 0
>> +: 1 file changed, 0 insertions(+), 0 deletions(-)
>> +::
>> +:2: fccce22 = 2: de345ab s/4/A/
>> +: a => b | 0
>> +: 1 file changed, 0 insertions(+), 0 deletions(-)
>> +::
>> +:3: 147e64e = 3: 9af6654 s/11/B/
>> +: a => b | 0
>> +: 1 file changed, 0 insertions(+), 0 deletions(-)
>> +::
>> +:4: a63e992 = 4: 2901f77 s/12/B/
>> +: a => b | 0
>> +: 1 file changed, 0 insertions(+), 0 deletions(-)
>> +::
>> +:-- :
>> +EOF
>> +sed -ne "/^1:/,/^--/p" actual &&
>> +test_cmp expect actual
>> '
>> done
[PATCH v3] t5562: do not reuse output files
Some expected failures of git-http-backend leaves running its children
(receive-pack or upload-pack) which still hold opened descriptors
to act.err and with some probability they live long enough to write
there their failure messages after next test has already truncated
the files. This causes occasional failures of the test script.
Avoid the issue by using separated output and error file for each test,
apprending the test number to their name.
Reported-by: Carlo Arenas
Helped-by: Carlo Arenas
Helped-by: Junio C Hamano
Signed-off-by: Max Kirillov
---
Use another output and error files for each test
t/t5562-http-backend-content-length.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t5562-http-backend-content-length.sh
b/t/t5562-http-backend-content-length.sh
index 90d890d02f..9ebbd77bbb 100755
--- a/t/t5562-http-backend-content-length.sh
+++ b/t/t5562-http-backend-content-length.sh
@@ -8,12 +8,12 @@ test_lazy_prereq GZIP 'gzip --version'
verify_http_result() {
# some fatal errors still produce status 200
# so check if there is the error message
- if grep 'fatal:' act.err
+ if grep 'fatal:' act.err.$test_count
then
return 1
fi
- if ! grep "Status" act.out >act
+ if ! grep "Status" act.out.$test_count >act
then
printf "Status: 200 OK\r\n" >act
fi
@@ -33,7 +33,7 @@ test_http_env() {
REQUEST_METHOD=POST \
"$PERL_PATH" \
"$TEST_DIRECTORY"/t5562/invoke-with-content-length.pl \
- "$request_body" git http-backend >act.out 2>act.err
+ "$request_body" git http-backend >act.out.$test_count
2>act.err.$test_count
}
ssize_b100dots() {
@@ -161,7 +161,7 @@ test_expect_success 'empty CONTENT_LENGTH' '
GIT_HTTP_EXPORT_ALL=TRUE \
REQUEST_METHOD=GET \
CONTENT_LENGTH="" \
- git http-backend act.out 2>act.err &&
+ git http-backend act.out.$test_count
2>act.err.$test_count &&
verify_http_result "200 OK"
'
--
2.19.0.1202.g68e1e8f04e
[PATCH] doc: update diff-format.txt for removed ellipses in --raw
Since 7cb6ac1e4b ("diff: diff_aligned_abbrev: remove ellipsis after
abbreviated SHA-1 value", 2017-12-03), the "--raw" format of diff
does not add ellipses in an attempt to align the output, but the
documentation was not updated to reflect this.
Signed-off-by: Greg Hurrell
---
The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now,
to bring back the old output format, but that is already documented in
git.txt, so I am not mentioning it here.
Documentation/diff-format.txt | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
index 706916c94c..cdcc17f0ad 100644
--- a/Documentation/diff-format.txt
+++ b/Documentation/diff-format.txt
@@ -26,12 +26,12 @@ line per changed file.
An output line is formatted this way:
-in-place edit :100644 100644 bcd1234... 0123456... M file0
-copy-edit :100644 100644 abcd123... 1234567... C68 file1 file2
-rename-edit:100644 100644 abcd123... 1234567... R86 file1 file3
-create :00 100644 000... 1234567... A file4
-delete :100644 00 1234567... 000... D file5
-unmerged :00 00 000... 000... U file6
+in-place edit :100644 100644 bcd1234 0123456 M file0
+copy-edit :100644 100644 abcd123 1234567 C68 file1 file2
+rename-edit:100644 100644 abcd123 1234567 R86 file1 file3
+create :00 100644 000 1234567 A file4
+delete :100644 00 1234567 000 D file5
+unmerged :00 00 000 000 U file6
That is, from the left to the right:
@@ -75,7 +75,7 @@ and it is out of sync with the index.
Example:
-:100644 100644 5be4a4.. 00.. M file.c
+:100644 100644 5be4a4a 000 M file.c
Without the `-z` option, pathnames with "unusual" characters are
@@ -100,7 +100,7 @@ from the format described above in the following way:
Example:
-::100644 100644 100644 fabadb8... cc95eb0... 4866510... MM describe.c
+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM describe.c
Note that 'combined diff' lists only files which were modified from
--
2.19.0
