Re: BUG: fetch in certain repo always gives "did not send all necessary objects"
On Wed, Feb 7, 2018 at 3:08 AM, Duy Nguyenwrote: > On Wed, Feb 7, 2018 at 7:00 AM, Elijah Newren wrote: >> and knew they had been using it, then I might have guessed that "HEAD" >> meant "not your actual HEAD but the HEAD of the vestige of some other >> worktree"). >> >> Does anyone have pointers about what might be doable in terms of >> providing a more useful error message to allow users to recover? > > I noticed this too. I was working on improving this message a bit but > got side tracked and since I figured this did not happen often > anymore, this fix got lower priority than others. I'll resume that > work. Sweet, that'd be great. Let me know if I can help. >> And/or ideas of what steps could cause corruption so I can send out a >> PSA to help users avoid it? > > There is another thing we could do. One bad HEAD should not abort the > entire operation (at least if it's not the current worktree's HEAD). > We could still give a warning and move on, or don't warn at all and > let "git worktree prune" collect it (which I see from your message > that it also fails to do). > > I guess that's two more items on my todo list :) Cool, if you're taking a look at those, I'll take a look at the other one mentioned by Peff in this thread -- "make fsck pay attention to other worktree HEADs" :-) > Sorry for all the trouble because of this bug of mine. Let me apologize if my tone came across too harshly in the report; I sometimes accidentally do that. Anyway, bugs happen. People were able to get back up and running with "just reclone somewhere else" pretty quickly, and no one lost any important data here, it was just jarring or perplexing enough that I felt the need to dig and figure out an improvement before more reports come in. Besides, you've made git a lot better. Thanks for that.
Re: [PATCH] files-backend: unlock packed store only if locked
On Wed, 7 Feb 2018 09:42:51 -0500 Jeff Kingwrote: > But this all seemed strangely familiar... I think this is the same bug > as: > > https://public-inbox.org/git/20180118143841.1a4c674d@novascotia/ > > which is queued as mr/packed-ref-store-fix. It's listed as "will merge > to next" in the "what's cooking" from Jan 31st. Ah...thanks, I didn't notice that. > I actually like this double-label a bit more than what is queued on > mr/packed-ref-store-fix, though I am OK with either solution. Same here.
Re: [PATCH] t0050: remove the unused $test_case variable
On Wed, Feb 07 2018, Johannes Sixt jotted: > Am 07.02.2018 um 09:07 schrieb Ævar Arnfjörð Bjarmason: >> >> On Wed, Feb 07 2018, Johannes Sixt jotted: >> >>> Am 07.02.2018 um 00:13 schrieb Ævar Arnfjörð Bjarmason: The $test_case variable hasn't been used since decd3c0c28 ("t0050-*.sh: mark the rename (case change) test as passing", 2014-11-28) when its last user went away. Let's remove the "say" as well, since it's obvious from subsequent output that we're testing on a case sensitive filesystem. >>> >>> Am I misunderstanding the message? I think it reports properties of >>> the test environment. And the tests do run on case-insensitive >>> filesystems. IMO, the message should be kept. >> >> It's obvious from subsequent output whether the FS is case sensitive or >> not, so I thought it was redundant to keep this report at the top since >> we didn't have the variable setting anymore. > > There are test cases that do different things depending on whether the > CASE_INSENSITIVE_FS prerequisite is set. I think it was the intent to > report whether it is set and not whether one or the other value of the > (now unused) variable is used somewhere. > > BTW, the message texts do not show which variant is taken (these are > without your patch): > > On Windows: > > t>sh t0050-filesystem.sh > will test on a case insensitive filesystem > will test on a filesystem lacking symbolic links > ok 1 - detection of case insensitive filesystem during repo init > ok 2 - detection of filesystem w/o symlink support during repo init > ok 3 - setup case tests > ok 4 - rename (case change) > ok 5 - merge (case change) > not ok 6 - add (with different case) # TODO known breakage > ok 7 - setup unicode normalization tests > ok 8 - rename (silent unicode normalization) > ok 9 - merge (silent unicode normalization) > # still have 1 known breakage(s) > # passed all remaining 8 test(s) > 1..9 > > On Linux: > > t@master:1002> ./t0050-filesystem.sh > ok 1 - detection of case insensitive filesystem during repo init > ok 2 - detection of filesystem w/o symlink support during repo init > ok 3 - setup case tests > ok 4 - rename (case change) > ok 5 - merge (case change) > ok 6 # skip add (with different case) (missing CASE_INSENSITIVE_FS) > ok 7 - setup unicode normalization tests > ok 8 - rename (silent unicode normalization) > ok 9 - merge (silent unicode normalization) > # passed all 9 test(s) > 1..9 > > I'd even argue that there should be messages on Linux, too. Thanks. Let's just drop this patch. I thought it would still print out something similar to that "missing CASE_INSENSITIVE_FS" at a quick glance last night, but was obviously wrong.
Re: [PATCH v2 00/41] Automate updating git-completion.bash a bit
Nguyễn Thái Ngọc Duywrites: > I posted a proof of concept a while back [1]. This is the full version. > > This series lets "git" binary help git-completion.bash to complete > -- so that when a new option is added, we don't have to update > git-completion.bash manually too (people often forget it). As a side > effect, about 180 more options are now completable. > > parse-options is updated to allow developers to flag certain options > not to be completable if they want finer control over it. But by > default, new non-hidden options are completable. Negative forms must > be handled manually. That's for the next step. Everybody seems to be in favour of the approach taken by this series. Is it in a good enough shape that we can merge it to 'next' and then go incremental from now on? Or do we want to keep it in 'pu' to give easier access to volunteer guinea pigs and wait until the way to handle '--no-foo' options are figured out?
[PATCH] send-email: have default batch size when relogin delay is given
When the batch size is neither configured nor given on the command line, but the relogin delay is given, then the user is not using the the feature as intended. But as the user gave a relogin delay, there is clearly the intention to delay sending out emails. Assume a batch size of 1 instead of silently ignoring the given relogin delay. Signed-off-by: Stefan Beller--- git-send-email.perl | 6 ++ 1 file changed, 6 insertions(+) diff --git a/git-send-email.perl b/git-send-email.perl index 340b5c8482..5672e05b98 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -379,6 +379,12 @@ unless ($rc) { die __("Cannot run git format-patch from outside a repository\n") if $format_patch and not $repo; +if (defined $relogin_delay) { + if (not defined $batch_size) { + $batch_size = 1; + } +} + # Now, let's fill any that aren't set in with defaults: sub read_config { -- 2.15.1.433.g936d1b9894.dirty
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
Duy Nguyenwrites: > ... > Then we still need to decide the new keyword for this feature, I feel > compact is a bit too vague (I read --stat=compact as "it's compact > stat", not "stat with compact summary"), so perhaps > --stat=compact-summary, or just --stat=summary? Yup, this is about giving summary in a compact way, not about giving a compact stat information. I agree with all the above reasoning, and that is why I said that your "compact-summary" was a good way to refer to the feature.
Re: [PATCH v2] rebase: add --allow-empty-message option
Johannes Schindelinwrites: > Very nice. I looked over the patch (sorry, I have too little time to test > this thoroughly, but then, it is the custom on this here mailing list to > just review the patch as per the mail) and it looks very good to me. > > Junio, if you like, please add a "Reviewed-by:" line for me. Will do. You do not have to apologize for not "testing" it; nobody expects _you_ to test every change you come across on the list, and other people (including the CI) are testing without advertising ;-).
Re: "git branch" issue in 2.16.1
> On 07 Feb 2018, at 19:09, Jason Raceywrote: > > Hi Lars, > > Here’s what I’m certain of: > > 1. Just set up a new MacBook Pro at work. Git version 2.16.1 installed via > Homebrew. “git branch” command always displays the list of branches in the > less pager, regardless of number of branches or screen size. I’ve never seen > this happen before. > 2. Checked the “git branch” behavior on my personal MacBook Pro. Git version > there is 2.16.0. Does not have this always paging behavior. > 3. Ran "brew upgrade git" on personal MacBook Pro. Git upgraded to 2.16.1. > 4. Checked the “git branch” behavior on my personal MacBook Pro after version > upgrade. Pager now always used regardless of number of branches or screen > size. Oh no! Seems like there might be a bug in the new version, better email > the git bug list in just to be safe. > 5. Meanwhile research problem on Stack Overflow. Mitigated (though not really > fixed) issue on both machines with this command: git config --global > core.pager ‘’ > That's great! Thank you. Can you share your exact OS version running on your work and personal machine? Plus, what shell do you use and what terminal application? Thanks, Lars PS: Please don't top post on the git mailing list :-) https://en.wikipedia.org/wiki/Posting_style > Thanks! > > - Jason > > >> On Feb 7, 2018, at 9:54 AM, Lars Schneider wrote: >> >> >>> On 06 Feb 2018, at 21:05, Stefan Beller wrote: >>> >>> On Tue, Feb 6, 2018 at 11:57 AM, Todd Zullinger wrote: Hi Jason, Jason Racey wrote: > After upgrading git from 2.16.0 to 2.16.1 (via Homebrew - > I’m on macOS) I noticed that the “git branch” command > appears to display the branch listing in something similar > to a vi editor - though not quite the same. I don’t know > the technical term for this state. You can’t actually edit > the output of the command, but you’re in a state where you > have to type “q” to exit and then the list disappears. > It’s very inconvenient and it doesn’t seem like it was by > design. I’m using zsh in iTerm2 if that helps. Thanks. In 2.16.0 `git branch --list` is sent to a pager by default. (Without arguments, --list is the default, so this applies to `git branch`). You can set pager.branch to false to disable this in the config, or use git --no-pager branch to do so for a single invocation. I can't say why you're seeing this with 2.16.1 and not 2.16.0, but I'm not familiar with homebrew, so perhaps something didn't work as intended in 2.16.0. >>> >>> Maybe the number of branches changed since then? >>> As the pager only comes to life when the output fills >>> more than your screen. Quick workarounds: >>> * buy a bigger screen >>> * have fewer branches. >> >> Hmmm... there might be more to it. I just noticed the >> pager behavior on macOS, too. Consider this call: >> >> $ git diff --shortstat >> >> This should generate at most one line of output. On Linux >> the pager is never used. On macOS the pager is always used. >> >> I tried older versions of Git on macOS and experienced the >> same behavior. >> >> @Jason: That might be a bug on macOS. However, I am surprised >> you only noticed it after upgrading from 2.16.0 to 2.16.1. >> Do you recall anything else you've changed? >> >> - Lars >> >
Re: [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding
Torsten Bögershausenwrites: >> the user explicitly tells us it is in UTF-16, right? Is there such a >> thing as UTF-16 binary? > > I don't think so, by definiton UTF-16 is ment to be text. > (this means that git ls-files --eol needs some update, I can have a look) > > Do we agree that UTF-16 is text ? > If yes, could Git assume that the "text" attribute is set when > working-tree-encoding is set ? These are two different questions. It seems that between the two of us, we established that "UTF-16 binary" is a nonsense, and a path that is given working-tree-encoding=UTF-16 must be treated as holding a text file. But that does not have direct relevance to the other question you are asking: "is a question 'does this path have text attribute set?' be answered with 'yes' if the path has wte attribute set to UTF-16?" I think the answer to that latter question ought to be "no". By the way, a related tangent is if it makes sense to give working-tree-encoding to anything that is binary, regardless of the value---I am inclined to say it is not, so the issue here is not "by definition UTF-16 is text", but "any path that has wte set to some enconding should be treated the same way as if the path also has text attribute set as far as convert machinery is concerned.".
Re: categorization, documentation and packaging of "git core" commands
On Wed, Feb 07, 2018 at 03:03:31PM -0500, Robert P. J. Day wrote: > huh ... well, that raises the question, if tla has been unbuildable > for that long (possibly for other distros), what is the value in > continuing to support git-archimport? > > https://www.kernel.org/pub/software/scm/git/docs/git-archimport.html I don't think that we do really support archimport. It's just that we tend to leave things sitting around if they're not actively causing us work, in the off chance that somebody might find them useful. I'd be OK with declaring archimport dead and unmaintained, and dropping it totally (I almost did so a few months ago when it _did_ cause me extra work, because it contained a bunch of shell injections that I turned up while auditing the whole code base). -Peff
Re: [PATCH] send-email: have default batch size when relogin delay is given
On Wed, Feb 7, 2018 at 2:45 PM, Stefan Bellerwrote: > When the batch size is neither configured nor given on the command > line, but the relogin delay is given, then the user is not using the > the feature as intended. But as the user gave a relogin delay, there is > clearly the intention to delay sending out emails. Assume a batch size > of 1 instead of silently ignoring the given relogin delay. > > Signed-off-by: Stefan Beller > --- > diff --git a/git-send-email.perl b/git-send-email.perl > @@ -379,6 +379,12 @@ unless ($rc) { > +if (defined $relogin_delay) { > + if (not defined $batch_size) { > + $batch_size = 1; > + } > +} Maybe also print a message that this batch size has been used as default lest the user wonder why it's sending "slowly" without apparently batching anything. Alternately, complain and die if both options are not specified.
Re: categorization, documentation and packaging of "git core" commands
On Wed, 7 Feb 2018, Todd Zullinger wrote: > Robert P. J. Day wrote: ... snip ... > > finally, from fedora, i am utterly unable to find a package that > > provides git-archimport. pretty sure fedora used to have a > > "git-arch" package but it's not there now. > > It hasn't been in Fedora since 2011. The tla command which is > required for git-archimport was retired, thus we removed the > git-arch package. The rpm changelog shows this: > > * Tue Jul 26 2011 Todd Zullinger- 1.7.6-4 > - Drop git-arch on fedora >= 16, the tla package has been retired > > As does the git history for the package: > > https://src.fedoraproject.org/rpms/git/c/3f0dc974fa > > The tla package was retired because it failed to build for > several releases: > > https://src.fedoraproject.org/rpms/tla/blob/master/f/dead.package huh ... well, that raises the question, if tla has been unbuildable for that long (possibly for other distros), what is the value in continuing to support git-archimport? https://www.kernel.org/pub/software/scm/git/docs/git-archimport.html i don't really care one way or the other, but perhaps git-archimport should be broken out as a "non-core" component of git. related post coming shortly ... rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
Re: [PATCH] t0050: remove the unused $test_case variable
Am 07.02.2018 um 09:07 schrieb Ævar Arnfjörð Bjarmason: On Wed, Feb 07 2018, Johannes Sixt jotted: Am 07.02.2018 um 00:13 schrieb Ævar Arnfjörð Bjarmason: The $test_case variable hasn't been used since decd3c0c28 ("t0050-*.sh: mark the rename (case change) test as passing", 2014-11-28) when its last user went away. Let's remove the "say" as well, since it's obvious from subsequent output that we're testing on a case sensitive filesystem. Am I misunderstanding the message? I think it reports properties of the test environment. And the tests do run on case-insensitive filesystems. IMO, the message should be kept. It's obvious from subsequent output whether the FS is case sensitive or not, so I thought it was redundant to keep this report at the top since we didn't have the variable setting anymore. There are test cases that do different things depending on whether the CASE_INSENSITIVE_FS prerequisite is set. I think it was the intent to report whether it is set and not whether one or the other value of the (now unused) variable is used somewhere. BTW, the message texts do not show which variant is taken (these are without your patch): On Windows: t>sh t0050-filesystem.sh will test on a case insensitive filesystem will test on a filesystem lacking symbolic links ok 1 - detection of case insensitive filesystem during repo init ok 2 - detection of filesystem w/o symlink support during repo init ok 3 - setup case tests ok 4 - rename (case change) ok 5 - merge (case change) not ok 6 - add (with different case) # TODO known breakage ok 7 - setup unicode normalization tests ok 8 - rename (silent unicode normalization) ok 9 - merge (silent unicode normalization) # still have 1 known breakage(s) # passed all remaining 8 test(s) 1..9 On Linux: t@master:1002> ./t0050-filesystem.sh ok 1 - detection of case insensitive filesystem during repo init ok 2 - detection of filesystem w/o symlink support during repo init ok 3 - setup case tests ok 4 - rename (case change) ok 5 - merge (case change) ok 6 # skip add (with different case) (missing CASE_INSENSITIVE_FS) ok 7 - setup unicode normalization tests ok 8 - rename (silent unicode normalization) ok 9 - merge (silent unicode normalization) # passed all 9 test(s) 1..9 I'd even argue that there should be messages on Linux, too. -- Hannes
Re: categorization, documentation and packaging of "git core" commands
Robert P. J. Day wrote: > On Wed, 7 Feb 2018, Todd Zullinger wrote: > >> Robert P. J. Day wrote: >>> first, here are the executables under /usr/libexec/git-core/ that >>> are unreferenced by that web page, but that should be fine as >>> almost all of them would be considered underlying helpers or >>> utilities (except for things like git-subtree, but we're still >>> unclear on its status, right?): >> >> I don't think there's anything unclear about git subtree's status. >> It's in contrib/ within the source, so it's not part of the core git >> suite. Some distributions (Fedora being one of them) ship a >> git-subtree package to provide it for users who want it. > > not true, "git-subtree" is part of fedora's lowest-level > "git-core" package. Eek, my apologies for providing bad information. I really should know the Fedora git packaging better than that. :/ Amusingly, I did suggest packaging it as a subpackage specifically to avoid any confusion that it was a core command in the Fedora bug which requested it be included in the git packaging: https://bugzilla.redhat.com/show_bug.cgi?id=864651 I'll see about changing that going forward in the Fedora packaging. I think it deserves to be in a subpackage. -- Todd ~~ Doing a job RIGHT the first time gets the job done. Doing the job WRONG fourteen times gives you job security.
[ANNOUNCE] Git for Windows 2.16.1(4)
Dear Git users, It is my pleasure to announce that Git for Windows 2.16.1(4) is available from: https://git-for-windows.github.io/ Changes since Git for Windows v2.16.1(3) (February 6th 2018) Bug Fixes * When called from TortoiseGit, git.exe can now spawn processes again . Filename | SHA-256 | --- Git-2.16.1.4-64-bit.exe | 5664d42d6ea636f760e6f926bf2239c30782742af6b10d5fb7b6541ba4356c8c Git-2.16.1.4-32-bit.exe | 742234e477afdfa07c446758adf7234ab2cd7231ba3bc17642073dd1267cc55a PortableGit-2.16.1.4-64-bit.7z.exe | a2191f676d77f8b8ba88501b8d373dc5418845c52dca86313ec449881af14cbd PortableGit-2.16.1.4-32-bit.7z.exe | 5d3e89163cb8b88484d906f8ba53604279e3d7b8c170e39d89116aa5c7274f26 MinGit-2.16.1.4-64-bit.zip | 4185d4510a82ca1a4f27297b800e9851832a183adb5a52644ae129b395c60fa6 MinGit-2.16.1.4-32-bit.zip | d8bd7f8d8145a0ba961a91884acb6fdeadd580b28b77218a833c1ade498b6aff MinGit-2.16.1.4-busybox-64-bit.zip | 28fbbc5c9d8ae587b332484dca8a8bee275560e40f5ebf41e9767c0cdadd40db MinGit-2.16.1.4-busybox-32-bit.zip | 170b4dffd84f781c03d98d16e89b772785a3081c4b85c91d46bfb53f5659c84f Git-2.16.1.4-64-bit.tar.bz2 | edf22fd92f414a930a8dab4916930fe0b74bd0cf6ae664aa9600360a2ed52bd1 Git-2.16.1.4-32-bit.tar.bz2 | b5c0991479a29369841da78e713a850244cff6a600acc16803cf35c5dfe5fcdd Ciao, Johannes
Re: "git branch" issue in 2.16.1
On Wed, Feb 07, 2018 at 06:54:23PM +0100, Lars Schneider wrote: > > Maybe the number of branches changed since then? > > As the pager only comes to life when the output fills > > more than your screen. Quick workarounds: > > * buy a bigger screen > > * have fewer branches. > > Hmmm... there might be more to it. I just noticed the > pager behavior on macOS, too. Consider this call: > > $ git diff --shortstat > > This should generate at most one line of output. On Linux > the pager is never used. On macOS the pager is always used. > > I tried older versions of Git on macOS and experienced the > same behavior. Keep in mind that we always run the pager, since we don't know ahead of time how much output will be generated. It's just that with certain configurations of "less", it may exit if it sees EOF before there's a whole screen worth of data. This is controlled by the "-F" option. By default, Git will set LESS=FRX in the environment if you do not already have a $LESS environment. So some other possibilities are: 1. You have $LESS in your environment (without "F") on one platform but not the other. 2. Git was built with a different PAGER_ENV Makefile variable on one platform versus the other (that's what controls the baked-in LESS defaults). 3. "less" somehow behaves differently on macOS. The "F" behavior is quite old, but possibly there's some platform-specific bug. -Peff
Re: "git branch" issue in 2.16.1
> On 06 Feb 2018, at 21:05, Stefan Bellerwrote: > > On Tue, Feb 6, 2018 at 11:57 AM, Todd Zullinger wrote: >> Hi Jason, >> >> Jason Racey wrote: >>> After upgrading git from 2.16.0 to 2.16.1 (via Homebrew - >>> I’m on macOS) I noticed that the “git branch” command >>> appears to display the branch listing in something similar >>> to a vi editor - though not quite the same. I don’t know >>> the technical term for this state. You can’t actually edit >>> the output of the command, but you’re in a state where you >>> have to type “q” to exit and then the list disappears. >>> It’s very inconvenient and it doesn’t seem like it was by >>> design. I’m using zsh in iTerm2 if that helps. Thanks. >> >> In 2.16.0 `git branch --list` is sent to a pager by default. >> (Without arguments, --list is the default, so this applies >> to `git branch`). >> >> You can set pager.branch to false to disable this in the >> config, or use git --no-pager branch to do so for a single >> invocation. >> >> I can't say why you're seeing this with 2.16.1 and not >> 2.16.0, but I'm not familiar with homebrew, so perhaps >> something didn't work as intended in 2.16.0. >> > > Maybe the number of branches changed since then? > As the pager only comes to life when the output fills > more than your screen. Quick workarounds: > * buy a bigger screen > * have fewer branches. Hmmm... there might be more to it. I just noticed the pager behavior on macOS, too. Consider this call: $ git diff --shortstat This should generate at most one line of output. On Linux the pager is never used. On macOS the pager is always used. I tried older versions of Git on macOS and experienced the same behavior. @Jason: That might be a bug on macOS. However, I am surprised you only noticed it after upgrading from 2.16.0 to 2.16.1. Do you recall anything else you've changed? - Lars
Re: [RFC PATCH 000/194] Moving global state into the repository object
On Wed, Feb 7, 2018 at 3:48 AM, Duy Nguyenwrote: > On Tue, Feb 6, 2018 at 6:51 AM, Stefan Beller wrote: >> This series moves a lot of global state into the repository struct. >> It applies on top of 2512f15446149235156528dafbe75930c712b29e (2.16.0) >> It can be found at https://github.com/stefanbeller/git/tree/object-store >> >> Motivation for this series: >> * Easier to reason about code when all state is stored in one place, >> for example for multithreading >> * Easier to move to processing submodules in-process instead of >> calling a processes for the submodule handling. >> The last patch demonstrates this. > > I have a mixed feeling about this. The end game is to keep > "the_repository" references as minimum as possible, right? Like we > only need to mention in in cmd_xxx() and not all over the place. If > so, yay!! Yes. And the super-end-game long after this series is to have cmd_xxx(struct repository *r, argc, argv) or so. > Something else.. maybe "struct repository *" should not be a universal > function argument to avoid global states. Like sha1_file.c is mostly about the > object store, and I see that you added object store struct (nice!). > These sha1 related function should take the object_store* (which > probably is a combination of both packed and loose stores since they > depend on each other), not repository*. This way if a function needs > both access to object store and ref store, we can see the two > dependencies from function arguments. I tried that in the beginning and it blew up a couple of times when I realized that I forgot to pass through one of these dependencies. Maybe we can go to the repository and shrink the scope in a follow up? > The alternate object store, if modeled right, could share the same > object store interface. But I don't think we should do anything that > big right now, just put alternate object store inside object_store. yup that is the case, see struct raw_object_store at the end of the series https://github.com/stefanbeller/git/blob/object-store-v2/object-store.h > Similarly those functions in blob.c, commit.c, tree.c... should take > object_parser* as argument instead of repository*. Maybe there's a > better name for this than object_parser. parsed_object_store I guess. > Or maybe just object_pool. Note that the initial few patches are misleading in the name, https://public-inbox.org/git/20180205235735.216710-59-sbel...@google.com/ which splits up the object handling into two layers, the "physical" layer (where to get the data from, mmaping pack files, alternates, streams of bytes), which is "struct raw_object_store objects;" in the repository, and the "object" layer, which is about parsing the objects and making sense of the data (which tree belongs to a commit, dereferencing tags) So maybe I'll try to make the first layer into its own series, such that we'll have a smaller series. Thanks, Stefan > -- > Duy
Re: BUG: fetch in certain repo always gives "did not send all necessary objects"
On Wed, Feb 07, 2018 at 09:25:42AM -0800, Elijah Newren wrote: > > So other_head_refs knows that it's looking at the worktrees. And it > > passes the alternate ref-store to refs_head_ref(), with "add_one_ref" as > > the callback. But the knowledge that we're not talking about the real > > "HEAD" is lost as we cross that callback boundary. We'd need to either > > add another parameter to the callback, or have some way of talking about > > "HEAD in this worktree" as a refname (which AFAIK we don't have). > > Can we use "worktrees/${WORKTREE}/HEAD"? It already satisfies all the > necessary rev-parse rules... True, but it's mostly an accident that it works. And once we have ref backends besides the filesystem, it will probably stop working. I think there was discussion at some point of embedding worktree refs into the normal ref namespace, but I don't know what came of it (it's not a feature I've followed very closely). > (And on a slight tangent...do we want to start disallowing the > creation of branches/tags whose name starts with "worktrees/", > "refs/", "hooks/", or other paths that exists under gitdir? Making a > branch named "refs/heads/foo" so that it fully-qualifies as > "refs/heads/refs/heads/foo" is always fun) We recently taught the porcelain to disallow a branch named "HEAD". Though I think there are actually two related problems with different solutions. One is saying something like: git checkout -b HEAD or: git checkout -b refs/heads/foo both of which will not do what you want, and leave you with a funnily-named branch in the ref namespace. But that's separate from the fact that: git rev-parse info/refs will look at a file that is not a ref at all. Long-term I think the solution is storage formats that don't mingle with other files. But we could probably teach even the files-backend that any ref at the top-level is supposed to be either in refs/, or to consist only of "[A-Z_]". -Peff
Re: "git branch" issue in 2.16.1
Hi Lars, Here’s what I’m certain of: 1. Just set up a new MacBook Pro at work. Git version 2.16.1 installed via Homebrew. “git branch” command always displays the list of branches in the less pager, regardless of number of branches or screen size. I’ve never seen this happen before. 2. Checked the “git branch” behavior on my personal MacBook Pro. Git version there is 2.16.0. Does not have this always paging behavior. 3. Ran "brew upgrade git" on personal MacBook Pro. Git upgraded to 2.16.1. 4. Checked the “git branch” behavior on my personal MacBook Pro after version upgrade. Pager now always used regardless of number of branches or screen size. Oh no! Seems like there might be a bug in the new version, better email the git bug list in just to be safe. 5. Meanwhile research problem on Stack Overflow. Mitigated (though not really fixed) issue on both machines with this command: git config --global core.pager ‘’ Thanks! - Jason > On Feb 7, 2018, at 9:54 AM, Lars Schneiderwrote: > > >> On 06 Feb 2018, at 21:05, Stefan Beller wrote: >> >> On Tue, Feb 6, 2018 at 11:57 AM, Todd Zullinger wrote: >>> Hi Jason, >>> >>> Jason Racey wrote: After upgrading git from 2.16.0 to 2.16.1 (via Homebrew - I’m on macOS) I noticed that the “git branch” command appears to display the branch listing in something similar to a vi editor - though not quite the same. I don’t know the technical term for this state. You can’t actually edit the output of the command, but you’re in a state where you have to type “q” to exit and then the list disappears. It’s very inconvenient and it doesn’t seem like it was by design. I’m using zsh in iTerm2 if that helps. Thanks. >>> >>> In 2.16.0 `git branch --list` is sent to a pager by default. >>> (Without arguments, --list is the default, so this applies >>> to `git branch`). >>> >>> You can set pager.branch to false to disable this in the >>> config, or use git --no-pager branch to do so for a single >>> invocation. >>> >>> I can't say why you're seeing this with 2.16.1 and not >>> 2.16.0, but I'm not familiar with homebrew, so perhaps >>> something didn't work as intended in 2.16.0. >>> >> >> Maybe the number of branches changed since then? >> As the pager only comes to life when the output fills >> more than your screen. Quick workarounds: >> * buy a bigger screen >> * have fewer branches. > > Hmmm... there might be more to it. I just noticed the > pager behavior on macOS, too. Consider this call: > > $ git diff --shortstat > > This should generate at most one line of output. On Linux > the pager is never used. On macOS the pager is always used. > > I tried older versions of Git on macOS and experienced the > same behavior. > > @Jason: That might be a bug on macOS. However, I am surprised > you only noticed it after upgrading from 2.16.0 to 2.16.1. > Do you recall anything else you've changed? > > - Lars >
Re: categorization, documentation and packaging of "git core" commands
On Wed, 7 Feb 2018, Todd Zullinger wrote: > Robert P. J. Day wrote: > > first, here are the executables under /usr/libexec/git-core/ that > > are unreferenced by that web page, but that should be fine as > > almost all of them would be considered underlying helpers or > > utilities (except for things like git-subtree, but we're still > > unclear on its status, right?): > > I don't think there's anything unclear about git subtree's status. > It's in contrib/ within the source, so it's not part of the core git > suite. Some distributions (Fedora being one of them) ship a > git-subtree package to provide it for users who want it. not true, "git-subtree" is part of fedora's lowest-level "git-core" package. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
Re: BUG: On some Linux systems, "Stage To Commit" hotkey in git-gui stages one file only, even if multiple files are selected
On Wed, Feb 7, 2018 at 8:29 AM, Birger Skogeng Pedersenwrote: > Steps to reproduce (on Ubuntu 17.10): > - Open git-gui in a repository with two or more unstaged, changed files > - Select two or more files in the "Unstaged Changes" > - Click CTRL+T to stage the files that you have selected > (Only a single file will get staged) This was fixed in Git 2.16.0. 76756d6706 (git-gui: allow Ctrl+T to toggle multiple paths, 2018-01-09)
Re: [PATCH v3 22/35] upload-pack: support shallow requests
On Tue, Feb 6, 2018 at 5:12 PM, Brandon Williamswrote: > Add the 'shallow' feature to the protocol version 2 command 'fetch' > which indicates that the server supports shallow clients and deepen > requets. > > Signed-off-by: Brandon Williams > --- > Documentation/technical/protocol-v2.txt | 67 +++- > serve.c | 2 +- > t/t5701-git-serve.sh| 2 +- > upload-pack.c | 138 > +++- > upload-pack.h | 3 + > 5 files changed, 173 insertions(+), 39 deletions(-) > > diff --git a/Documentation/technical/protocol-v2.txt > b/Documentation/technical/protocol-v2.txt > index 4d5096dae..fedeb6b77 100644 > --- a/Documentation/technical/protocol-v2.txt > +++ b/Documentation/technical/protocol-v2.txt > @@ -201,12 +201,42 @@ packet-lines: > to its base by position in pack rather than by an oid. That is, > they can read OBJ_OFS_DELTA (ake type 6) in a packfile. > > +shallow > + A client must notify the server of all objects for which it only s/all objects/all commits/ for preciseness > + has shallow copies of (meaning that it doesn't have the parents > + of a commit) by supplying a 'shallow ' line for each such > + object so that the serve is aware of the limitations of the > + client's history. > + > +deepen > + Request that the fetch/clone should be shallow having a commit depth > of > +relative to the remote side. What does depth mean? number of commits, or number of edges? Are there any special numbers (-1, 0, 1, max int) ? >From reading ahead: "Cannot be used with deepen-since, but can be combined with deepen-relative" ? > + > +deepen-relative > + Requests that the semantics of the "deepen" command be changed > + to indicate that the depth requested is relative to the clients > + current shallow boundary, instead of relative to the remote > + refs. > + > +deepen-since > + Requests that the shallow clone/fetch should be cut at a > + specific time, instead of depth. Internally it's equivalent of > + doing "rev-list --max-age=". Cannot be used with > + "deepen". > + > +deepen-not > + Requests that the shallow clone/fetch should be cut at a > + specific revision specified by '', instead of a depth. > + Internally it's equivalent of doing "rev-list --not ". > + Cannot be used with "deepen", but can be used with > + "deepen-since". What happens if those are given in combination?
Re: feature-request: git "cp" like there is git mv.
Stefan Mochwrites: > * Jonathan Nieder [2017-12-15T17:31:30-0800]: >> This sounds like a reasonable thing to add. See builtin/mv.c for how >> "git mv" works if you're looking for inspiration. >> >> cmd_mv in that file looks rather long, so I'd also be happy if someone >> interested refactors to break it into multiple self-contained pieces >> for easier reading (git mostly follows >> https://www.kernel.org/doc/html/latest/process/coding-style.html#functions). > > I looked at builtin/mv.c and have a rough idea how to split it > up to support both mv and cp commands. > > But first I noticed and removed a redundant check in cmd_mv, > also added a test case to check if mv --dry-run does not move > the file. I guess these two patches went unnoticed when posted at the end of last year. Reading them again, I think they are good changes. As a no-op clean-up of a127331c ("mv: allow moving nested submodules", 2016-04-19), the attached would also make sense, I would think. Thanks. builtin/mv.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index 9662804d23..9cb07990fd 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -266,10 +266,11 @@ int cmd_mv(int argc, const char **argv, const char *prefix) const char *src = source[i], *dst = destination[i]; enum update_mode mode = modes[i]; int pos; - if (show_only || verbose) - printf(_("Renaming %s to %s\n"), src, dst); - if (show_only) + if (show_only) { + if (verbose) + printf(_("Renaming %s to %s\n"), src, dst); continue; + } if (mode != INDEX && rename(src, dst) < 0) { if (ignore_errors) continue;
Re: Fetch-hooks
On 02/07/2018 11:51 PM, Ævar Arnfjörð Bjarmason wrote: > > On Wed, Feb 07 2018, Leo Gaspard jotted: > >> Hello, >> >> tl;dr: Is there currently a way to have fetch hooks, and if not do you >> think it could be a nice feature? >> >> I was in the process of implementing hooks for git that ensure the >> repository is always cleanly signed by someone allowed to by the >> repository itself. I think I've completed the signature-checking part >> [1] and the push hook [2] (even though it isn't really configurable at >> the moment). >> >> However, I was starting to think about handling the fetch step, and >> couldn't find any fetch hook. Is there one? >> >> If not, would you think it is would be a good idea to add one, that >> would eg. be passed the commit-before, commit-after and could block the >> changing of the reference if it failed? >> >> The only other solution I could think of is using a separate script for >> fetching, but that would be fragile, as the user could always not think >> about it well and run a git fetch, breaking the objective that after the >> first clone all commits were correctly signature-checked. >> >> Thanks for reading me! >> Leo >> >> PS1: I am not subscribed to the ML. >> >> PS2: I've tried asking freenode#git, without success so far. >> >> >> [1] >> https://github.com/Ekleog/signed-git/blob/master/git-hooks/check-range-signed.sh >> >> [2] https://github.com/Ekleog/signed-git/blob/master/git-hooks/pre-push > > There is no fetch hook, however you may find that the > post-{checkout,merge} hooks are suitable for what you want to do. > > Setting those to some custom comand is a common pattern for > e.g. compiling some assets on "git pull", so you could similarly check > the commits from HEAD, of course those are post-* hooks, so they won't > stop the checkout. Hmm, I don't think these would fit the bill. For post-merge, simply because I spend my life rebasing stuff around, and very rarely merge. For post-checkout, it could work, but then I'd need to keep track manually of up to where the commits have been checked and to search the git graph for the latest checked ancestor (as otherwise checking-out another branch then checking-out the first branch again would likely trigger a failure, due to the keyring being dynamic), so it would likely be a dealbreaker, due to the hook becoming too complex to be trusted. (Just in case you wonder, by “the keyring being dynamic” I mean the PGP keys allowed to sign commits are stored directly inside the git repository) That said, I just came upon [1] (esp. the description [2] and the patch [3]), and wondered: it looks like the patch was abandoned midway in favor of a hook refactoring. Would you happen to know whether the hook refactoring eventually took place, and/or whether this patch was resubmitted later, and/or whether it would still be possible to merge this now? (not having any experience with git's internals yet, I don't really know whether these are stupid questions or not) Thanks! Leo PS: Cc'ing Joey, as you most likely know best what eventually happened, if you can remember it? [1] https://marc.info/?t=13247704151=1=2 [2] https://marc.info/?l=git=132483581218382=2 [3] https://marc.info/?l=git=132486687023893=2
Re: [PATCH v1] name-hash: properly fold directory names in adjust_dirname_case()
On Wed, 2018-02-07 at 19:41 -0500, Ben Peart wrote: > Correct the pointer arithmetic in adjust_dirname_case() so that it > calls > find_dir_entry() with the correct string length. Previously passing > in > "dir1/foo" would pass a length of 6 instead of the correct 4. This > resulted in > find_dir_entry() never finding the entry and so the subsequent memcpy > that would > fold the name to the version with the correct case never executed. > > Add a test to validate the corrected behavior with name folding of > directories. Looks good to me.
Re: [PATCH 025/194] object-store: allow prepare_alt_odb to handle arbitrary repositories
On Tue, Feb 06, 2018 at 09:48:56AM -0800, Stefan Beller wrote: > On Mon, Feb 5, 2018 at 5:19 PM, brian m. carlson >wrote: > > On Mon, Feb 05, 2018 at 03:54:46PM -0800, Stefan Beller wrote: > >> @@ -434,12 +433,12 @@ static int link_alt_odb_entry_the_repository(const > >> char *entry, > >> ent = alloc_alt_odb(pathbuf.buf); > >> > >> /* add the alternate entry */ > >> - *the_repository->objects.alt_odb_tail = ent; > >> - the_repository->objects.alt_odb_tail = &(ent->next); > >> + *r->objects.alt_odb_tail = ent; > >> + r->objects.alt_odb_tail = &(ent->next); > >> ent->next = NULL; > > > > I'm sure I'm missing something obvious, but it's not clear to me that > > this transformation is correct. Could you perhaps say a few words about > > why it is? > > This is a pretty open ended question, so I'll give it a try: I apologize. My question was about the use of ent and ent->next, but it appears I merely misread the patch as converting from ent to &(ent->next), but that's not the case. -- brian m. carlson / brian with sandals: Houston, Texas, US https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH v2 01/41] parse-options: support --git-completion-helper
> > OK how about some thing like this fixup patch? __gitcomp_builtin now > > allows to add extra options as well as remove some. > > > > -- 8< -- > > __gitcomp_builtin () > > { > > + local incl="$2" > > + local excl="$3" > > + options="$(__git ${cmd/_/ } --git-completion-helper) $incl " > > + for i in $excl; do > > + options="${options/$i /}" > > Is 'options' guaranteed to end with a space? It is, note the space before the closing double quote in: options="$(__git ${cmd/_/ } --git-completion-helper) $incl " > If not, then this > expulsion will fail for the very last option. I'd think you can get by > fine with just "${options/$i}". I would prefer a space both at the beginning and at the end of the pattern. Please excuse the contrived corner case, but it could still fail if the option to be excluded is a suffix of an other option: $ o="$(echo --foo--bar --baz --bar) " $ echo "'${o/--bar /}'" # exclude '--bar' '--foo--baz --bar ' Maybe we'll never have --opt--ions with a doubledash in them[1], but still... $ o=" $(echo --foo--bar --baz --bar) " $ echo "'${o/ --bar / }'" ' --foo--bar --baz ' [1] - Interestingly, grep shows that the German translation does contain a '--reset--author'.
[PATCH v1] name-hash: properly fold directory names in adjust_dirname_case()
Correct the pointer arithmetic in adjust_dirname_case() so that it calls find_dir_entry() with the correct string length. Previously passing in "dir1/foo" would pass a length of 6 instead of the correct 4. This resulted in find_dir_entry() never finding the entry and so the subsequent memcpy that would fold the name to the version with the correct case never executed. Add a test to validate the corrected behavior with name folding of directories. Signed-off-by: Ben Peart--- Notes: Base Ref: v2.16.1 Web-Diff: https://github.com/benpeart/git/commit/2944970f4e Checkout: git fetch https://github.com/benpeart/git adjust_dirname-v1 && git checkout 2944970f4e name-hash.c | 6 +++--- t/t0050-filesystem.sh | 12 +++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/name-hash.c b/name-hash.c index 45c98db0a0..2ddbb72647 100644 --- a/name-hash.c +++ b/name-hash.c @@ -696,12 +696,12 @@ void adjust_dirname_case(struct index_state *istate, char *name) if (*ptr == '/') { struct dir_entry *dir; - ptr++; - dir = find_dir_entry(istate, name, ptr - name + 1); + dir = find_dir_entry(istate, name, ptr - name); if (dir) { memcpy((void *)startPtr, dir->name + (startPtr - name), ptr - startPtr); - startPtr = ptr; + startPtr = ptr + 1; } + ptr++; } } } diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh index b29d749bb7..219c96594c 100755 --- a/t/t0050-filesystem.sh +++ b/t/t0050-filesystem.sh @@ -80,7 +80,17 @@ test_expect_success 'merge (case change)' ' git merge topic ' - +test_expect_success CASE_INSENSITIVE_FS 'add directory (with different case)' ' + git reset --hard initial && + mkdir -p dir1 && + mkdir -p dir1/dir2 && + touch dir1/dir2/a && + touch dir1/dir2/b && + git add dir1/dir2/a && + git add dir1/DIR2/b && + camel=$(git ls-files | grep dir2) && + test $(echo "$camel" | wc -l) = 2 +' test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' ' git reset --hard initial && base-commit: 8279ed033f703d4115bee620dccd32a9ec94d9aa -- 2.15.0.windows.1
[PATCH] docs/interpret-trailers: fix agreement error
In the description of git interpret-trailers, we describe "a group…of lines" that have certain characteristics. Because the first option uses a plural verb (referring to "lines"), the second option must also use plural verbs for parallelism. Signed-off-by: brian m. carlson--- I'm somewhat on the fence about this patch. To me, the number disagreement is very jarring. However, I'm also sympathetic to the fact that the latter sentence reads more naturally in the singular. Opinions on improvements welcome. Documentation/git-interpret-trailers.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index 9dd19a1dd9..de5011e564 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -51,8 +51,8 @@ with only spaces at the end of the commit message part, one blank line will be added before the new trailer. Existing trailers are extracted from the input message by looking for -a group of one or more lines that (i) are all trailers, or (ii) contains at -least one Git-generated or user-configured trailer and consists of at +a group of one or more lines that (i) are all trailers, or (ii) contain at +least one Git-generated or user-configured trailer and consist of at least 25% trailers. The group must be preceded by one or more empty (or whitespace-only) lines. The group must either be at the end of the message or be the last
[PATCH v2] hash: update obsolete reference to SHA1_HEADER
We moved away from SHA1_HEADER to a preprocessor if chain, but didn't update the comment discussing the platform defines. Update this comment so it reflects the current state of our codebase. Signed-off-by: brian m. carlson--- hash.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hash.h b/hash.h index eb30f59be3..7c8238bc2e 100644 --- a/hash.h +++ b/hash.h @@ -18,8 +18,8 @@ #ifndef platform_SHA_CTX /* * platform's underlying implementation of SHA-1; could be OpenSSL, - * blk_SHA, Apple CommonCrypto, etc... Note that including - * SHA1_HEADER may have already defined platform_SHA_CTX for our + * blk_SHA, Apple CommonCrypto, etc... Note that the relevant + * SHA-1 header may have already defined platform_SHA_CTX for our * own implementations like block-sha1 and ppc-sha1, so we list * the default for OpenSSL compatible SHA-1 implementations here. */
[PATCH] send-email: error out when relogin delay is missing
When the batch size is neither configured nor given on the command line, but the relogin delay is given, then the current code ignores the relogin delay setting. This is unsafe as there was some intention when setting the batch size. One workaround would be to just assume a batch size of 1 as a default. This however may be bad UX, as then the user may wonder why it is sending slowly without apparent batching. Error out for now instead of potentially confusing the user. As 5453b83bdf (send-email: --batch-size to work around some SMTP server limit, 2017-05-21) lays out, we rather want to not have this interface anyway and would rather want to react on the server throttling dynamically. Helped-by: Eric SunshineSigned-off-by: Stefan Beller --- git-send-email.perl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/git-send-email.perl b/git-send-email.perl index 340b5c8482..bc0d3ade16 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -379,6 +379,9 @@ unless ($rc) { die __("Cannot run git format-patch from outside a repository\n") if $format_patch and not $repo; +die __("When a batch size is given, the relogin delay must be set\n") + if defined $relogin_delay and not defined $batch_size; + # Now, let's fill any that aren't set in with defaults: sub read_config { -- 2.16.0.rc1.238.g530d649a79-goog
Re: categorization, documentation and packaging of "git core" commands
On Wed, 7 Feb 2018, Todd Zullinger wrote: > Robert P. J. Day wrote: > > first, here are the executables under /usr/libexec/git-core/ that > > are unreferenced by that web page, but that should be fine as > > almost all of them would be considered underlying helpers or > > utilities (except for things like git-subtree, but we're still > > unclear on its status, right?): > > I don't think there's anything unclear about git subtree's status. > It's in contrib/ within the source, so it's not part of the core git > suite. Some distributions (Fedora being one of them) ship a > git-subtree package to provide it for users who want it. > > > on the other hand (and this is not so much a git issue as a fedora > > packaging issue), there are a number of command links at that web > > page that are supplied by distinct RPM packages rather than by the > > basic fedora git package, so one would need to install the > > following packages to get some of those commands on fedora: > > > > * gitk > > * git-cvs > > * git-svn > > * git-p4 > > * git-email (provides git-send-email) > > These packages are in separate sub-packages in Fedora (and some > other distributions) because they are no required by all users and > they pull in dependencies which are not wanted on minimal installs. > In Fedora, you can install git-all to get all the available git > sub-packages. not to belabour this (and i'm sure it's *way* too late for that), but fedora has the following packaging scheme. first, there's a bunch of stuff in "git-core", which has no dependencies on any other git-related packages. then there's "git", which has the following property: $ rpm -qR git /bin/sh /usr/bin/perl emacs-filesystem >= 25.3 git-core = 2.14.3-2.fc27 git-core-doc = 2.14.3-2.fc27 ... snip ... $ rpm -ql git ... snip ... /usr/libexec/git-core/git-add--interactive /usr/libexec/git-core/git-am /usr/libexec/git-core/git-credential-libsecret /usr/libexec/git-core/git-credential-netrc /usr/libexec/git-core/git-difftool /usr/libexec/git-core/git-difftool--helper /usr/libexec/git-core/git-instaweb /usr/libexec/git-core/git-request-pull /usr/libexec/git-core/git-submodule /usr/libexec/git-core/git-submodule--helper ... snip ... /usr/share/man/man1/git-am.1.gz /usr/share/man/man1/git-difftool.1.gz /usr/share/man/man1/git-instaweb.1.gz /usr/share/man/man1/git-request-pull.1.gz /usr/share/man/man1/git-submodule.1.gz /usr/share/man/man1/gitweb.1.gz /usr/share/man/man5/gitweb.conf.5.gz $ so with fedora, "git" drags in "git-core" and a small number of additional git utilities. all of this leads one to wonder -- is there any comprehensible relationship between: 1) commands that claim to be in the "git suite" 2) commands that come from contrib/ 3) commands listed at https://www.kernel.org/pub/software/scm/git/docs/ 4) how different distros package all of the above as i think we've noticed, it's not at all clear how git decides what is and isn't part of the "official" git suite. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
Re: [PATCH 025/194] object-store: allow prepare_alt_odb to handle arbitrary repositories
On Mon, 5 Feb 2018 15:54:46 -0800 Stefan Bellerwrote: > + /* > + * Path to the alternate object database, relative to the > + * current working directory. > + */ > char path[FLEX_ARRAY]; I would prefer this to be commented: Path to the alternative object store. If this is a relative path, it is relative to the current working directory. to show that it is not necessarily relative, but the current version is fine too. > + /* > + * Paths in alt are relative to the cwd. We ignore environment > + * settings like this for all repositories except for > + * the_repository, so we don't have to worry about transforming > + * the path to be relative to another repository. > + */ > + link_alt_odb_entries(r, alt, PATH_SEP, NULL, 0); I find the comment confusing - it makes more sense for the reason for us not worrying about transforming the path is that the paths as stored in struct alternate_object_database are relative to the CWD, not that we ignore environment variables for certain repositories. I think it's best to remove this comment, and instead add a comment to read_info_alternates() before its call to link_alt_odb_entries(), explaining that paths in the alternates file are relative to "info/alternates", not to the CWD (since that is the exceptional case). All the patches prior to this look good. Thanks especially for the consistent naming convention of the patch titles.
Re: "git branch" issue in 2.16.1
> On 07 Feb 2018, at 21:08, Jeff Kingwrote: > > On Wed, Feb 07, 2018 at 06:54:23PM +0100, Lars Schneider wrote: > >>> Maybe the number of branches changed since then? >>> As the pager only comes to life when the output fills >>> more than your screen. Quick workarounds: >>> * buy a bigger screen >>> * have fewer branches. >> >> Hmmm... there might be more to it. I just noticed the >> pager behavior on macOS, too. Consider this call: >> >> $ git diff --shortstat >> >> This should generate at most one line of output. On Linux >> the pager is never used. On macOS the pager is always used. >> >> I tried older versions of Git on macOS and experienced the >> same behavior. > > Keep in mind that we always run the pager, since we don't know ahead of > time how much output will be generated. It's just that with certain > configurations of "less", it may exit if it sees EOF before there's a > whole screen worth of data. > > This is controlled by the "-F" option. By default, Git will set LESS=FRX > in the environment if you do not already have a $LESS environment. So > some other possibilities are: > > 1. You have $LESS in your environment (without "F") on one platform > but not the other. I think that's it. On my system LESS is defined to "-R". This opens the pager: $ echo "TEST" | less This does not open the pager: $ echo "TEST" | less -FRX That means "F" works on macOS but Git doesn't set it because LESS is already in my environment. Question is, why is LESS set that way on my system? I can't find it in .bashrc .bash_profile .zshrc and friends. - Lars > > 2. Git was built with a different PAGER_ENV Makefile variable on one > platform versus the other (that's what controls the baked-in LESS > defaults). > > 3. "less" somehow behaves differently on macOS. The "F" behavior is > quite old, but possibly there's some platform-specific bug. > > -Peff
Re: [PATCH v3 1/3] read-cache: fix reading the shared index for other repos
Junio C Hamanowrites: >>> This was a bit painful change, given that some changes in flight do >>> add new callsites to read_index_from() and they got the function >>> changed under their feet. >> >> Sorry about that. Is there any way to make such a change less painful >> in the future? > > One way is to do for read_index_from() what you did for the existing > users of read_cache_from(). Introduce a _new_ helper that will not > be known for any existing topics in flight, and use that to make the > existing API a thin wrapper around it. This continues to cause pain simply because read_index_from() is something new topics want to stay stable X-<. I'll be merging this to 'next' hopefully as part of today's integration run, so will need to endure the pain only until it (and other topics that conflict with it) all graduate to 'master'. Next time a patch with an internal API change like this appears, please remind me to push it back a lot stronger. I was too lenient and ended up slowing down the progress of other topics this time. Thanks.
Re: Fetch-hooks
On Wed, Feb 07 2018, Leo Gaspard jotted: > Hello, > > tl;dr: Is there currently a way to have fetch hooks, and if not do you > think it could be a nice feature? > > I was in the process of implementing hooks for git that ensure the > repository is always cleanly signed by someone allowed to by the > repository itself. I think I've completed the signature-checking part > [1] and the push hook [2] (even though it isn't really configurable at > the moment). > > However, I was starting to think about handling the fetch step, and > couldn't find any fetch hook. Is there one? > > If not, would you think it is would be a good idea to add one, that > would eg. be passed the commit-before, commit-after and could block the > changing of the reference if it failed? > > The only other solution I could think of is using a separate script for > fetching, but that would be fragile, as the user could always not think > about it well and run a git fetch, breaking the objective that after the > first clone all commits were correctly signature-checked. > > Thanks for reading me! > Leo > > PS1: I am not subscribed to the ML. > > PS2: I've tried asking freenode#git, without success so far. > > > [1] > https://github.com/Ekleog/signed-git/blob/master/git-hooks/check-range-signed.sh > > [2] https://github.com/Ekleog/signed-git/blob/master/git-hooks/pre-push There is no fetch hook, however you may find that the post-{checkout,merge} hooks are suitable for what you want to do. Setting those to some custom comand is a common pattern for e.g. compiling some assets on "git pull", so you could similarly check the commits from HEAD, of course those are post-* hooks, so they won't stop the checkout.
Re: [PATCH 038/194] pack: allow sha1_loose_object_info to handle arbitrary repositories
On Wed, Feb 7, 2018 at 2:33 PM, Jonathan Tanwrote: > On Mon, 5 Feb 2018 15:54:59 -0800 > Stefan Beller wrote: > >> From: Jonathan Nieder >> >> Signed-off-by: Stefan Beller >> Signed-off-by: Jonathan Nieder >> --- >> sha1_file.c | 11 +-- >> 1 file changed, 5 insertions(+), 6 deletions(-) > > Thanks - I can see that this has been a lot of work, and it brings a lot > of benefit. Among other things, this will probably allow me to remove > the "fetch_if_missing" global variable that we need to set and reset in > the partial clone series, replacing it with a setting in either the repo > or the object store (and when fetching a missing object, first cloning > the repo/store and then setting that setting, so that objects are not > recursively fetched). That sounds intriguing. > If we're planning to split the series up for merging in batches (which, > as a reviewer, I'm very much in favor of), I think this is a good > stopping point for the first batch, so I'll stop my review here for now. Thanks for identifying a good spot. I'll look at this part of the series closer for a reroll, and need to make sure there are no leftovers with the #define trick. > After these 38 patches, the net benefit is a better position of the > packed object variables (in struct repository) and permitting the > reading of loose objects in any repository. (Permitting the reading of > any object in any repository, I see, will only come later in patch 74.) which would then be a good portion for the next series. Thanks for the review!
Re: [PATCH 5/8] rebase: introduce the --recreate-merges option
Øyvind Rønningstadwrites: >> So no, I do not think that --recreate-merges --first-parent is a good > idea >> at all. Unless you try to do that non-interactively only, *and > disallow it >> in interactive mode*. Correct. If the original side branch has commits A, B and C, you are rebuilding the topic to have only A and C but not B and then recreate the merge of that rebuilt topic, then you absolutely do not want "cherry-pick -m1" of the original merge when recreating the merge, as that would resurrect the effect of having B. The same argument applies if you rebuilt the topic with A and C and then a new commit D. "cherry-pick -m1" of the original would do a wrong thing. When there is no such fixing up, "cherry-pick -m1" is the right thing to do, though, so it probably makes sense to pick merges that way when the side topic being merged consists of the same commits as the original. I do not think that the code structure in the topic as posted makes it impossible (or unnecessarily hard) to give an enhancement like that in the future as a follow-up series.
Re: [PATCH 5/8] rebase: introduce the --recreate-merges option
edit: Sending again, hopefully without HTML :). Sorry for spamming. Hi, I think --recreate-merges is a very exciting feature. I've also been puzzled by why we can't just pick merge commits directly including conflict resolutions, so allow me to join the discussion. On Wed, Feb 7, 2018 at 6:36 PM, Johannes Schindelin wrote: > > Hi, > > [...] > > And guess what happens if you drop that `pick` line in your todo list and > then the `merge` command simply tries to re-create the original merge > commit's changes? > > Exactly. The merge will become an evil merge, and will introduce that very > much not-wanted and therefore-dropped changes. I think I understand. Evil merges happen when we change the branch that is not the mainline..? Is there any reason why the following wouldn't work? Imagine rebase is about to pick a merge commit, and we have edited at least one commit in each branch to be merged. 1. apply patch mainline_orig..merge_orig 2. apply patch branch1_orig..branch1 ... N. apply patch branchN_orig..branchN N+1. Commit merge I do see complications, like the fact that steps 2-N can be done in any order, with possibly quite different results. Moving commits from one branch to another might not work very well. And what to do when you remove branches or create new ones? These problems might be prohibitive, but picking merge commits seems like something that should be possible to do. > > [...] > > So --preserve-merges --first-parent is probably what you were looking for. I want this as well :). I don't quite see the risk if it's not used with --interactive. > [...] > > So no, I do not think that --recreate-merges --first-parent is a good idea > at all. Unless you try to do that non-interactively only, *and disallow it > in interactive mode*. Because the entire point of the interactive rebase > is to allow reordering and dropping commits, in --recreate-merges even > moving, introducing and dropping merge commits. The --first-parent option > flies in the face of this idea. FWIW I'd be totally fine with disallowing it in --interactive. It would be incredibly useful e.g. with pull --rebase in merge-based workflows. BTW what is the difference between --recreate-merges and --preserve- merges when --interactive is not present? I apologize if you have explained this somewhere else in the patch series. > > Ciao, > Johannes Thanks, Øyvind
What's cooking in git.git (Feb 2018, #01; Wed, 7)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. I am migrating my build and integration environment to a different machine; if you notice anything out of ordinary, please let me know before I decomission and reimage my usual environment ;-) You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * bc/hash-algo (2018-02-02) 12 commits - bulk-checkin: abstract SHA-1 usage - csum-file: abstract uses of SHA-1 - csum-file: rename sha1file to hashfile - read-cache: abstract away uses of SHA-1 - pack-write: switch various SHA-1 values to abstract forms - pack-check: convert various uses of SHA-1 to abstract forms - fast-import: switch various uses of SHA-1 to the_hash_algo - sha1_file: switch uses of SHA-1 to the_hash_algo - builtin/unpack-objects: switch uses of SHA-1 to the_hash_algo - builtin/index-pack: improve hash function abstraction - hash: create union for hash context allocation - hash: move SHA-1 macros to hash.h More abstraction of hash function from the codepath. Will merge to 'next'. * bp/untracked-cache-noflush (2018-02-05) 1 commit - dir.c: don't flag the index as dirty for changes to the untracked cache Writing out the index file when the only thing that changed in it is the untracked cache information is often wasteful, and this has been optimized out. Waiting for the discussion to finish. cf.
Re: [PATCHv3] tag: add --edit option
Eric Sunshinewrites: > On Tue, Feb 6, 2018 at 3:36 AM, Nicolas Morey-Chaisemartin > wrote: >> Add a --edit option whichs allows modifying the messages provided by -m or >> -F, >> the same way git commit --edit does. >> >> Signed-off-by: Nicolas Morey-Chaisemartin >> --- >> Changes since v2 ( >> https://public-inbox.org/git/e99947cf-93ba-9376-f059-7f6a369d3...@suse.com ): >> * Add [-e] to git tag summary > > Thanks, I think this addresses all my comments from previous rounds. > Just a couple minor style issues below... > >> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh >> @@ -452,6 +452,21 @@ test_expect_success \ >> +> +test_expect_success \ >> + 'creating an annotated tag with -m message --edit should succeed' ' >> + EDITOR=./fakeeditor git tag -m "A message" --edit >> annotated-tag-edit && > > Whitespace between 'fakeeditor' and 'git' is a tab but should be a space. > >> + get_tag_msg annotated-tag-edit >actual && >> + test_cmp expect actual >> +' >> @@ -465,6 +480,21 @@ test_expect_success \ >> +test_expect_success \ >> + 'creating an annotated tag with -F messagefile --edit should >> succeed' ' >> + EDITOR=./fakeeditor git tag -F msgfile --edit >> file-annotated-tag-edit && > > Ditto. > >> + get_tag_msg file-annotated-tag-edit >actual && >> + test_cmp expect actual >> +' Also, GIT_EDITOR takes precedence over EDITOR, so these two new tests should use it instead, just like other existing tests do. Will try to amend locally before queuing, so unless I botch that (or others find other things to tweak), no need to re-send. Thanks, both.
Fetch-hooks
Hello, tl;dr: Is there currently a way to have fetch hooks, and if not do you think it could be a nice feature? I was in the process of implementing hooks for git that ensure the repository is always cleanly signed by someone allowed to by the repository itself. I think I've completed the signature-checking part [1] and the push hook [2] (even though it isn't really configurable at the moment). However, I was starting to think about handling the fetch step, and couldn't find any fetch hook. Is there one? If not, would you think it is would be a good idea to add one, that would eg. be passed the commit-before, commit-after and could block the changing of the reference if it failed? The only other solution I could think of is using a separate script for fetching, but that would be fragile, as the user could always not think about it well and run a git fetch, breaking the objective that after the first clone all commits were correctly signature-checked. Thanks for reading me! Leo PS1: I am not subscribed to the ML. PS2: I've tried asking freenode#git, without success so far. [1] https://github.com/Ekleog/signed-git/blob/master/git-hooks/check-range-signed.sh [2] https://github.com/Ekleog/signed-git/blob/master/git-hooks/pre-push
Re: [PATCH 038/194] pack: allow sha1_loose_object_info to handle arbitrary repositories
On Mon, 5 Feb 2018 15:54:59 -0800 Stefan Bellerwrote: > From: Jonathan Nieder > > Signed-off-by: Stefan Beller > Signed-off-by: Jonathan Nieder > --- > sha1_file.c | 11 +-- > 1 file changed, 5 insertions(+), 6 deletions(-) Thanks - I can see that this has been a lot of work, and it brings a lot of benefit. Among other things, this will probably allow me to remove the "fetch_if_missing" global variable that we need to set and reset in the partial clone series, replacing it with a setting in either the repo or the object store (and when fetching a missing object, first cloning the repo/store and then setting that setting, so that objects are not recursively fetched). If we're planning to split the series up for merging in batches (which, as a reviewer, I'm very much in favor of), I think this is a good stopping point for the first batch, so I'll stop my review here for now. After these 38 patches, the net benefit is a better position of the packed object variables (in struct repository) and permitting the reading of loose objects in any repository. (Permitting the reading of any object in any repository, I see, will only come later in patch 74.)
Hello
Hello, I'm Mr. Merle Butler the mega winner of $218M In Mega Millions Jackpot, I'm donating to 5 random individuals if you get this email then your email was selected after a spin ball.I have spread most of my wealth over a number of charities and organizations. I and my wife Patricia Butler have voluntarily decided to donate the sum of $2 Million USD to you as one of the selected 5, to verify my winnings please see my you tube page below. WATCH ME HERE: https://www.youtube.com/watch?v=VXDhZZFzJ34 THIS IS YOUR DONATION CODE: [ 0043034] Reply with the DONATION CODE to this email: mmerlepbut...@gmail.com Hope to make you and your family happy. Regards Merle and Patricia Butler
Re: "git branch" issue in 2.16.1
Jeff Kingwrites: > Keep in mind that we always run the pager, since we don't know ahead of > time how much output will be generated. It's just that with certain > configurations of "less", it may exit if it sees EOF before there's a > whole screen worth of data. > > This is controlled by the "-F" option. By default, Git will set LESS=FRX > in the environment if you do not already have a $LESS environment. So > some other possibilities are: > > 1. You have $LESS in your environment (without "F") on one platform > but not the other. > > 2. Git was built with a different PAGER_ENV Makefile variable on one > platform versus the other (that's what controls the baked-in LESS > defaults). > > 3. "less" somehow behaves differently on macOS. The "F" behavior is > quite old, but possibly there's some platform-specific bug. 4. Between your two runs, you do not have that many branches to fill the screen vertically in either case, but only in one case you have a branch with very long name (or perhaps "branch -l -v" made a line longer), and you have S and F in LESS environment. The case that shows branches with short names will cause pager to exit at the end, while the other one, because it wants to allow you to scroll sideways, will not.
Re: feature-request: git "cp" like there is git mv.
On Wed, Feb 7, 2018 at 11:49 AM, Junio C Hamanowrote: > Stefan Moch writes: > >> * Jonathan Nieder [2017-12-15T17:31:30-0800]: >>> This sounds like a reasonable thing to add. See builtin/mv.c for how >>> "git mv" works if you're looking for inspiration. >>> >>> cmd_mv in that file looks rather long, so I'd also be happy if someone >>> interested refactors to break it into multiple self-contained pieces >>> for easier reading (git mostly follows >>> https://www.kernel.org/doc/html/latest/process/coding-style.html#functions). >> >> I looked at builtin/mv.c and have a rough idea how to split it >> up to support both mv and cp commands. >> >> But first I noticed and removed a redundant check in cmd_mv, >> also added a test case to check if mv --dry-run does not move >> the file. > > I guess these two patches went unnoticed when posted at the end of > last year. Reading them again, I think they are good changes. > > As a no-op clean-up of a127331c ("mv: allow moving nested > submodules", 2016-04-19), the attached would also make sense, I > would think. > > Thanks. > > builtin/mv.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/builtin/mv.c b/builtin/mv.c > index 9662804d23..9cb07990fd 100644 > --- a/builtin/mv.c > +++ b/builtin/mv.c > @@ -266,10 +266,11 @@ int cmd_mv(int argc, const char **argv, const char > *prefix) > const char *src = source[i], *dst = destination[i]; > enum update_mode mode = modes[i]; > int pos; > - if (show_only || verbose) > - printf(_("Renaming %s to %s\n"), src, dst); > - if (show_only) > + if (show_only) { > + if (verbose) > + printf(_("Renaming %s to %s\n"), src, dst); > continue; > + } This is actually changing behavior to if (show_only && verbose) print(...) if show_only continue The second part is already there as is, only the printing behavior actually changes. So I might be missing the obvious here for the claim of no-op? Looking up further we have (line 177): if (show_only) printf(_("Checking rename of '%s' to '%s'\n"), src, dst); which prints regardless of verbosity.
Re: Bug? Error during commit
On Mon, Feb 05, 2018 at 08:59:52PM +0700, Duy Nguyen wrote: > On Mon, Feb 5, 2018 at 8:48 PM, Andreas Kalzwrote: > > Hello, > > > > I am using git frequently and usually it is running great. > > > > I read to write to this eMail address regarding problems and possible bugs. > > I am using git version 2.16.1.windows.2 / 64 Bit and during commit the > > following error message comes up: > > e:\Internet>git commit -m 2018-01-27 > > fatal: unable to generate diffstat for > > Thunderbird/andreas-kalz.de/Mail/pop.gmx.net/Inbox > > [master f74cf30] 2018-01-27 > > > > I also tried this before with an older git version with same problem. > > > > Can you help me with this problem please? Thanks in advance. > > I think if you add -q to that "git commit" command, diffstat is not > generated and you can get past that. If that particular commit can be > published in public, it'll help us find out why diffstat could not be > generated. I think that's the first time I've seen that particular error. :) I think the only reason that xdiff would report failure is if malloc() failed, or if one of the files exceeds MAX_XDIFF_SIZE, which is ~1GB. I think we'd usually avoid doing a text diff on anything over core.bigFileThreshold, though. But it doesn't seem to work: $ yes | head -c $((1024*1024*1024 - 10*1024*1024)) >file $ git add file $ git commit -m one $ yes | head -c $((1024*1024*1024)) >file $ git commit -am two fatal: unable to generate diffstat for file What's weird is that if I run "git show --stat" on the same commit, it works! So there's something about how commit invokes the diff that doesn't let the big-file check kick in. It looks like the logic in diff_filespec_is_binary() will only check big_file_threshold if we haven't already loaded the contents into RAM. So "commit" does that, but "diff" is more careful about not loading the file contents. I think we probably ought to consider anything over big_file_threshold to be binary, no matter what. Possibly even if the user gave us a .gitattribute that says "no really, this is text". Because that 1GB limit is a hard limit that the code can't cope with; our options are either to generate a binary diff or to die. -Peff
Re: [PATCH] files-backend: unlock packed store only if locked
Jonathan Tanwrites: > On Wed, 7 Feb 2018 09:42:51 -0500 > Jeff King wrote: > >> But this all seemed strangely familiar... I think this is the same bug >> as: >> >> https://public-inbox.org/git/20180118143841.1a4c674d@novascotia/ >> >> which is queued as mr/packed-ref-store-fix. It's listed as "will merge >> to next" in the "what's cooking" from Jan 31st. > > Ah...thanks, I didn't notice that. > >> I actually like this double-label a bit more than what is queued on >> mr/packed-ref-store-fix, though I am OK with either solution. > > Same here. I do agree that the double-label approach is more future-proof way, especially if we anticipate that there will be more code after the "attempt initial ref transaction commit" block before the packed-ref-store is unlocked. On the other hand, introduction of the locked_cleanup label can be done as part of such a change that adds new code that needs to be skipped, so I am OK with what is queued there. Thanks, all.
Re: categorization, documentation and packaging of "git core" commands
Robert P. J. Day wrote: > not to belabour this (and i'm sure it's *way* too late for that), > but fedora has the following packaging scheme. first, there's a bunch > of stuff in "git-core", which has no dependencies on any other > git-related packages. The split in Fedora between git and git-core is done to minimize the dependencies required for a minimal git install. The initial reason was to to allow installing the git-core package on systems, in containers, etc. without requiring perl and its various dependencies to be installed. The name git-core was not chosen to imply any official status as core versus contrib from upstream. (Farther back in the past, the main git package (and the upstream tarball, IIRC) was named git-core due to conflicts with another tool named git (GNU interactive tools).) > so with fedora, "git" drags in "git-core" and a small number of > additional git utilities. all of this leads one to wonder -- is there > any comprehensible relationship between: > > 1) commands that claim to be in the "git suite" > 2) commands that come from contrib/ > 3) commands listed at > https://www.kernel.org/pub/software/scm/git/docs/ > 4) how different distros package all of the above > > as i think we've noticed, it's not at all clear how git decides what > is and isn't part of the "official" git suite. I don't think there's any good reason to use the packaging of any distribution as the source for what the git project considers officially part of the suite. For that, you should look at the git source and particularly contrib/README. The first paragraph says: Although these pieces are available as part of the official git source tree, they are in somewhat different status. The intention is to keep interesting tools around git here, maybe even experimental ones, to give users an easier access to them, and to give tools wider exposure, so that they can be improved faster. If anything the Fedora packging does in the splitting or naming of the git packages is something the git project feels is incorrect or needlessly confusing, I (with my Fedora maintainer hat on) would be happy to make any changes I can to improve things. -- Todd ~~ Some god must protect drunkards and fools, there are so many of them still around.
Re: [PATCH] send-email: error out when relogin delay is missing
> 在 2018年2月8日,上午7:43,Stefan Beller写道: > > +die __("When a batch size is given, the relogin delay must be set\n") > +if defined $relogin_delay and not defined $batch_size; > + According the code, maybe you want to say “When relogin delay is given, a batch size must be set “ ?
Re: [PATCH] t0050: remove the unused $test_case variable
On Wed, Feb 07 2018, Johannes Sixt jotted: > Am 07.02.2018 um 00:13 schrieb Ævar Arnfjörð Bjarmason: >> The $test_case variable hasn't been used since >> decd3c0c28 ("t0050-*.sh: mark the rename (case change) test as >> passing", 2014-11-28) when its last user went away. >> >> Let's remove the "say" as well, since it's obvious from subsequent >> output that we're testing on a case sensitive filesystem. > > Am I misunderstanding the message? I think it reports properties of > the test environment. And the tests do run on case-insensitive > filesystems. IMO, the message should be kept. It's obvious from subsequent output whether the FS is case sensitive or not, so I thought it was redundant to keep this report at the top since we didn't have the variable setting anymore.
[PATCH v2] dir.c: ignore paths containing .git when invalidating untracked cache
read_directory() code ignores all paths named ".git" even if it's not a valid git repository. See treat_path() for details. Since ".git" is basically invisible to read_directory(), when we are asked to invalidate a path that contains ".git", we can safely ignore it because the slow path would not consider it anyway. This helps when fsmonitor is used and we have a real ".git" repo at worktree top. Occasionally .git/index will be updated and if the fsmonitor hook does not filter it, untracked cache is asked to invalidate the path ".git/index". Without this patch, we invalidate the root directory unncessarily, which: - makes read_directory() fall back to slow path for root directory (slower) - makes the index dirty (because UNTR extension is updated). Depending on the index size, writing it down could also be slow. A note about the new "safe_path" knob. Since this new check could be relatively expensive, avoid it when we know it's not needed. If the path comes from the index, it can't contain ".git". If it does contain, we may be screwed up at many more levels, not just this one. Noticed-by: Ævar Arnfjörð BjarmasonSigned-off-by: Nguyễn Thái Ngọc Duy --- My v1 was rubbish. It's no wonder Ben didn't see my intention. v2 corrects the "is .git in a given path?" logic and adds a test to verify it. dir.c | 10 ++ dir.h | 2 +- fsmonitor.c | 2 +- fsmonitor.h | 2 +- t/t7519-status-fsmonitor.sh | 39 + unpack-trees.c | 2 +- 6 files changed, 49 insertions(+), 8 deletions(-) diff --git a/dir.c b/dir.c index 7c4b45e30e..fce45fc55e 100644 --- a/dir.c +++ b/dir.c @@ -1773,7 +1773,7 @@ static enum path_treatment treat_path(struct dir_struct *dir, if (!de) return treat_path_fast(dir, untracked, cdir, istate, path, baselen, pathspec); - if (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, ".git")) + if (is_dot_or_dotdot(de->d_name) || !fspathcmp(de->d_name, ".git")) return path_none; strbuf_setlen(path, baselen); strbuf_addstr(path, de->d_name); @@ -2968,10 +2968,12 @@ static int invalidate_one_component(struct untracked_cache *uc, } void untracked_cache_invalidate_path(struct index_state *istate, -const char *path) +const char *path, int safe_path) { if (!istate->untracked || !istate->untracked->root) return; + if (!safe_path && !verify_path(path)) + return; invalidate_one_component(istate->untracked, istate->untracked->root, path, strlen(path)); } @@ -2979,13 +2981,13 @@ void untracked_cache_invalidate_path(struct index_state *istate, void untracked_cache_remove_from_index(struct index_state *istate, const char *path) { - untracked_cache_invalidate_path(istate, path); + untracked_cache_invalidate_path(istate, path, 1); } void untracked_cache_add_to_index(struct index_state *istate, const char *path) { - untracked_cache_invalidate_path(istate, path); + untracked_cache_invalidate_path(istate, path, 1); } /* Update gitfile and core.worktree setting to connect work tree and git dir */ diff --git a/dir.h b/dir.h index 11a047ba48..06df057054 100644 --- a/dir.h +++ b/dir.h @@ -350,7 +350,7 @@ static inline int dir_path_match(const struct dir_entry *ent, int cmp_dir_entry(const void *p1, const void *p2); int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in); -void untracked_cache_invalidate_path(struct index_state *, const char *); +void untracked_cache_invalidate_path(struct index_state *, const char *, int safe_path); void untracked_cache_remove_from_index(struct index_state *, const char *); void untracked_cache_add_to_index(struct index_state *, const char *); diff --git a/fsmonitor.c b/fsmonitor.c index 0af7c4edba..6d7bcd5d0e 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -130,7 +130,7 @@ static void fsmonitor_refresh_callback(struct index_state *istate, const char *n * as it could be a new untracked file. */ trace_printf_key(_fsmonitor, "fsmonitor_refresh_callback '%s'", name); - untracked_cache_invalidate_path(istate, name); + untracked_cache_invalidate_path(istate, name, 0); } void refresh_fsmonitor(struct index_state *istate) diff --git a/fsmonitor.h b/fsmonitor.h index cd3cc0ccf2..65f3743636 100644 --- a/fsmonitor.h +++ b/fsmonitor.h @@ -65,7 +65,7 @@ static inline void mark_fsmonitor_invalid(struct index_state *istate, struct cac { if (core_fsmonitor) { ce->ce_flags &= ~CE_FSMONITOR_VALID; -
Re: [PATCH 5/8] rebase: introduce the --recreate-merges option
Jacob Kellerwrites: > On Tue, Feb 6, 2018 at 10:16 PM, Sergey Organov wrote: >> Johannes Schindelin writes: >> >> [...] >> >>> +--recreate-merges:: >>> + Recreate merge commits instead of flattening the history by replaying >>> + merges. Merge conflict resolutions or manual amendments to merge >>> + commits are not preserved. >> >> I wonder why you guys still hold on replaying "merge-the-operation" >> instead of replaying "merge-the-result"? The latter, the merge commit >> itself, no matter how exactly it was created in the first place, is the >> most valuable thing git keeps about the merge, and you silently drop it >> entirely! OTOH, git keeps almost no information about >> "merge-the-operation", so it's virtually impossible to reliably replay >> the operation automatically, and yet you try to. >> > > I'm not sure I follow what you mean here? > > You mean that you'd want this to actually attempt to re-create the > original merge including conflict resolutions by taking the contents > of the result? I mean just cherry-pick the merge the same way all other commits are essentially cherry-picked during rebase. That's what Johannes Sixt did in his patch I was reffering to. > How do you handle if that result has conflicts? What UX do you present > to the user to handle such conflicts? I don't think the normal 3-way > conflicts would even be possible in this case? No problem here. It goes exactly the same way as for non-merge commits that are being rebased. You can try it right now using $ git cherry-pick -m1 that will induce conflicts. The (somewhat tricky) functional difference is only in recording correct additional parents to the final commit, but that part is hidden from the user. -- Sergey
Re: [RFC PATCH 000/194] Moving global state into the repository object
On Tue, Feb 6, 2018 at 3:25 PM, Stefan Bellerwrote: >> Any suggestions welcome! > > Eric repeatedly points out leaking memory. > > As of today we do not care about memory leaking as it is cleaned > up at the end of the program anyway, for example the objects > hash table is never cleared. Is this still true/desirable when multiple 'repos' are involved? > In a resend I will put the infrastructure in place to free the memory via > adding > > raw_object_store_clear(struct raw_object_store *) > object_parser_clear(struct object_parser *) > ref_store_clear(struct ref_store *) > > and calling these functions on repo destruction. The functions > itself will be empty code-wise and contain TODO comments listing > all variables that need care. I'm confused. If you go to the effort of inserting TODO's why not go all the way and instead insert the actual code? > Follow up patches can figure out what is best to do, such as closing > the memleaks. As repo_clear is not called for the_repository > we'd even keep the property of relying on fast cleanup by the OS.
Re: [PATCH v2 1/3] worktree: improve message when creating a new worktree
On Sun, Feb 4, 2018 at 9:12 PM, Duy Nguyenwrote: > As a former translator, I'm not thrilled to see a sentence broken into > two pieces like this. I'm not a Japanese translator, but I think this > sentence is translated differently when the translator sees the whole > line "Preparing ..., setting ...". > > I think the purpose of "Preparing..." in the first place is to show > something when git is busy checkout out the worktree. As long as we > print it before git-reset, we should be good. The original message was "Enter " which had the potential to confuse someone into thinking the working directory had changed[1], so it was changed to "Preparing...". The reason for keeping that message (rather than dropping it outright) was to provide context to messages printed after it, especially messages such as "HEAD is now at..." which might otherwise confuse the reader into thinking that HEAD in the current worktree changed rather than HEAD in the new worktree[2,3]. [1]: https://public-inbox.org/git/55a8f4b1.9060...@drmicha.warpmail.net/ [2]: https://public-inbox.org/git/capig+crshwmmf9ccubrrdccw4kvg9peouxp5vqpsgfxzmxh...@mail.gmail.com/ [3]: https://public-inbox.org/git/capig+csls4-ukicvmbsknero_fyd722hs1_u6qztrim8cio...@mail.gmail.com/
Windows: mintty.exe classified as exploit by AV software
Hi everyone, a few days ago I installed version 2.16.1.2, downloaded from https://git-scm.com/download/win on my Windows 7 system. The OS is Windows 7 Enterprise 64bit, Build 7601/SP1, in case it matters. This is a first time install, not an upgrade. Our current virus protection software is Cylance, from https://www.cylance.com/en_us/home.html During install, several executions of C:\Program Files\Git\usr\bin\bash.exe were blocked, the violation being given as "Stack Pivot". Our admins then temporarily lifted some rules for my device so that I could properly install it. But now, when I start ... "C:\Program Files\Git\git-bash.exe" --cd-to-home ... Cylance classifies it as an Exploit, and blocks execution with the following messages: Category: Exploit Event: Blocked Details: Violation: StackProtect; Application: C:\Program Files\Git\usr\bin\mintty.exe (Screenshot available if needed) If I start ... C:\Program Files\Git\usr\bin\mintty.exe directly, and choose the 64 bit version from the dialog, it is allowes to start without getting blocked. My current problem is that the security guys don't want to see this software installed on my machine because of this. And as Cylance is not a pattern-based AV, it's not something that will go away by waiting for the next daily update ... Any ideas about this? Thanks Michael
[PATCH v2] dir.c: ignore paths containing .git when invalidating untracked cache
read_directory() code ignores all paths named ".git" even if it's not a valid git repository. See treat_path() for details. Since ".git" is basically invisible to read_directory(), when we are asked to invalidate a path that contains ".git", we can safely ignore it because the slow path would not consider it anyway. This helps when fsmonitor is used and we have a real ".git" repo at worktree top. Occasionally .git/index will be updated and if the fsmonitor hook does not filter it, untracked cache is asked to invalidate the path ".git/index". Without this patch, we invalidate the root directory unncessarily, which: - makes read_directory() fall back to slow path for root directory (slower) - makes the index dirty (because UNTR extension is updated). Depending on the index size, writing it down could also be slow. A note about the new "safe_path" knob. Since this new check could be relatively expensive, avoid it when we know it's not needed. If the path comes from the index, it can't contain ".git". If it does contain, we may be screwed up at many more levels, not just this one. Noticed-by: Ævar Arnfjörð BjarmasonSigned-off-by: Nguyễn Thái Ngọc Duy --- My v1 was rubbish. It's no wonder Ben didn't see my intention. v2 corrects the "is .git in a given path?" logic and adds a test to verify it. dir.c | 10 ++ dir.h | 2 +- fsmonitor.c | 2 +- fsmonitor.h | 2 +- t/t7519-status-fsmonitor.sh | 39 + unpack-trees.c | 2 +- 6 files changed, 49 insertions(+), 8 deletions(-) diff --git a/dir.c b/dir.c index 7c4b45e30e..fce45fc55e 100644 --- a/dir.c +++ b/dir.c @@ -1773,7 +1773,7 @@ static enum path_treatment treat_path(struct dir_struct *dir, if (!de) return treat_path_fast(dir, untracked, cdir, istate, path, baselen, pathspec); - if (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, ".git")) + if (is_dot_or_dotdot(de->d_name) || !fspathcmp(de->d_name, ".git")) return path_none; strbuf_setlen(path, baselen); strbuf_addstr(path, de->d_name); @@ -2968,10 +2968,12 @@ static int invalidate_one_component(struct untracked_cache *uc, } void untracked_cache_invalidate_path(struct index_state *istate, -const char *path) +const char *path, int safe_path) { if (!istate->untracked || !istate->untracked->root) return; + if (!safe_path && !verify_path(path)) + return; invalidate_one_component(istate->untracked, istate->untracked->root, path, strlen(path)); } @@ -2979,13 +2981,13 @@ void untracked_cache_invalidate_path(struct index_state *istate, void untracked_cache_remove_from_index(struct index_state *istate, const char *path) { - untracked_cache_invalidate_path(istate, path); + untracked_cache_invalidate_path(istate, path, 1); } void untracked_cache_add_to_index(struct index_state *istate, const char *path) { - untracked_cache_invalidate_path(istate, path); + untracked_cache_invalidate_path(istate, path, 1); } /* Update gitfile and core.worktree setting to connect work tree and git dir */ diff --git a/dir.h b/dir.h index 11a047ba48..06df057054 100644 --- a/dir.h +++ b/dir.h @@ -350,7 +350,7 @@ static inline int dir_path_match(const struct dir_entry *ent, int cmp_dir_entry(const void *p1, const void *p2); int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in); -void untracked_cache_invalidate_path(struct index_state *, const char *); +void untracked_cache_invalidate_path(struct index_state *, const char *, int safe_path); void untracked_cache_remove_from_index(struct index_state *, const char *); void untracked_cache_add_to_index(struct index_state *, const char *); diff --git a/fsmonitor.c b/fsmonitor.c index 0af7c4edba..6d7bcd5d0e 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -130,7 +130,7 @@ static void fsmonitor_refresh_callback(struct index_state *istate, const char *n * as it could be a new untracked file. */ trace_printf_key(_fsmonitor, "fsmonitor_refresh_callback '%s'", name); - untracked_cache_invalidate_path(istate, name); + untracked_cache_invalidate_path(istate, name, 0); } void refresh_fsmonitor(struct index_state *istate) diff --git a/fsmonitor.h b/fsmonitor.h index cd3cc0ccf2..65f3743636 100644 --- a/fsmonitor.h +++ b/fsmonitor.h @@ -65,7 +65,7 @@ static inline void mark_fsmonitor_invalid(struct index_state *istate, struct cac { if (core_fsmonitor) { ce->ce_flags &= ~CE_FSMONITOR_VALID; -
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
On Tue, Feb 6, 2018 at 5:20 AM, Duy Nguyenwrote: > On Tue, Feb 6, 2018 at 1:56 AM, Junio C Hamano wrote: >> Duy Nguyen writes: >> I actually think compact-summary was a good way to phrase it. >> >> Personally, I think it was a UI mistake that --summary can be given >> independently with or without --stat (instead, there shouldn't have >> been the --summary option, and instead when it was added, --stat >> just should have gained an extra kind of output). A single option >> that can give both kinds of info may be a good way forward, so >> another possibility may be --summary-in-stat (meaning: the info >> given by summary is included in stat output). I dunno. > > +Eric maybe he has some idea (sorry I forgot to include people from > the last round). What about the earlier suggestion[1] (and minor follow-ups[2,3]) of making this another option to --stat= (for instance, --stat=compact)? Did that idea get shot down or am I misunderstanding the question here. [1]: https://public-inbox.org/git/capig+cqlgs59jyxcmk30qy307arwqjx6pnoo95z39_jj9+d...@mail.gmail.com/ [2]: https://public-inbox.org/git/cacsjy8b5qrn8t1aai3y3nfec5baqn2xkk6vzozmp5lh-mpz...@mail.gmail.com/ [3]: https://public-inbox.org/git/CACsJy8CPHk+aXHr-mkHZi27s=c3+ny8d9csuzoso8pwevib...@mail.gmail.com/
Re: [PATCHv3] tag: add --edit option
On Tue, Feb 6, 2018 at 3:36 AM, Nicolas Morey-Chaisemartinwrote: > Add a --edit option whichs allows modifying the messages provided by -m or -F, > the same way git commit --edit does. > > Signed-off-by: Nicolas Morey-Chaisemartin > --- > Changes since v2 ( > https://public-inbox.org/git/e99947cf-93ba-9376-f059-7f6a369d3...@suse.com ): > * Add [-e] to git tag summary Thanks, I think this addresses all my comments from previous rounds. Just a couple minor style issues below... > diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh > @@ -452,6 +452,21 @@ test_expect_success \ > +> +test_expect_success \ > + 'creating an annotated tag with -m message --edit should succeed' ' > + EDITOR=./fakeeditor git tag -m "A message" --edit > annotated-tag-edit && Whitespace between 'fakeeditor' and 'git' is a tab but should be a space. > + get_tag_msg annotated-tag-edit >actual && > + test_cmp expect actual > +' > @@ -465,6 +480,21 @@ test_expect_success \ > +test_expect_success \ > + 'creating an annotated tag with -F messagefile --edit should succeed' > ' > + EDITOR=./fakeeditor git tag -F msgfile --edit > file-annotated-tag-edit && Ditto. > + get_tag_msg file-annotated-tag-edit >actual && > + test_cmp expect actual > +'
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
On Wed, Feb 7, 2018 at 4:52 PM, Eric Sunshinewrote: > On Tue, Feb 6, 2018 at 5:20 AM, Duy Nguyen wrote: >> On Tue, Feb 6, 2018 at 1:56 AM, Junio C Hamano wrote: >>> Duy Nguyen writes: >>> I actually think compact-summary was a good way to phrase it. >>> >>> Personally, I think it was a UI mistake that --summary can be given >>> independently with or without --stat (instead, there shouldn't have >>> been the --summary option, and instead when it was added, --stat >>> just should have gained an extra kind of output). A single option >>> that can give both kinds of info may be a good way forward, so >>> another possibility may be --summary-in-stat (meaning: the info >>> given by summary is included in stat output). I dunno. >> >> +Eric maybe he has some idea (sorry I forgot to include people from >> the last round). > > What about the earlier suggestion[1] (and minor follow-ups[2,3]) of > making this another option to --stat= (for instance, --stat=compact)? > Did that idea get shot down or am I misunderstanding the question > here. I thought that was something like --stat[=[,[,,[compact and turning on "compact" alone would get awkward because you need to specify all those widths and counts too. --stat=compact as a separate form, with no combination with any other stat params, does not feel justified. We could just do --stat-compact then. Perhaps we can allow compact to appear anywhere in --stat=, and not just the end? The --stat= syntax would be --stat=[[,[,...]]] where option could be keyword ones like compact or anything else in future, or = form. could also be a number, but in that case the three consecutive number options will present width, name-width and count in this order. Or we could simply add new --stat= syntax _without_ " as numbers". widths and counts must be specified keywords as well, e.g. --stat=width=40,name-width=20,count=10,compact and leave the old syntax "--stat=,," alone. Then we still need to decide the new keyword for this feature, I feel compact is a bit too vague (I read --stat=compact as "it's compact stat", not "stat with compact summary"), so perhaps --stat=compact-summary, or just --stat=summary? > [1]: > https://public-inbox.org/git/capig+cqlgs59jyxcmk30qy307arwqjx6pnoo95z39_jj9+d...@mail.gmail.com/ > [2]: > https://public-inbox.org/git/cacsjy8b5qrn8t1aai3y3nfec5baqn2xkk6vzozmp5lh-mpz...@mail.gmail.com/ > [3]: > https://public-inbox.org/git/CACsJy8CPHk+aXHr-mkHZi27s=c3+ny8d9csuzoso8pwevib...@mail.gmail.com/ -- Duy
is http://git-scm.com an *official* git-affiliated site?
i ask WRT whether it should be up to date. i'm currently writing a number of git-related wiki pages, and i want to link to whatever are the canonical man pages for various git commands, but that site seems a bit off. if one follows a link to get here: https://git-scm.com/doc there is another link to the "Reference Manual" that promises "The official and comprehensive man pages that are included in the Git package itself." but upon getting there: https://git-scm.com/docs it seems clear that that page doesn't come close to covering all of the available git commands. as an example, there is a link "submodule" on that page, which does indeed take one to the page https://git-scm.com/docs/git-submodule. so far, so good. however, while there is no link for the "worktree" command, there does in fact exist a similarly-named web page https://git-scm.com/docs/git-worktree. finally, there is no reference to the git "subtree" command, either as a link *or* as an existing web page https://git-scm.com/docs/git-subtree. it all seems kind of arbitrary. is there an actual, canonical set of online web pages for all current git commands one should use? rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
Re: [PATCH v1] dir.c: don't flag the index as dirty for changes to the untracked cache
On Tue, Feb 6, 2018 at 7:55 PM, Duy Nguyenwrote: > On Tue, Feb 6, 2018 at 7:27 PM, Duy Nguyen wrote: >> On Tue, Feb 6, 2018 at 8:48 AM, Ben Peart wrote: >>> With the new behavior, making a change in dir1/, then calling status would >>> update the dir1/ untracked cache entry but not write it out. On the next >>> status, git would detect the change in dir1/ again and update the untracked >> >> Thing only missing piece here I would add is, this dir1/ detection is >> done by watchman. We have to contact watchman and ask the set of >> changed paths since $TIME where $TIME is the last time we updated >> untracked cache and invalidate those paths in core. Then it should >> work correctly. I checked the watchman query in the fsmonitor hook and >> I think it's correct so far. > > No I think I'm wrong. And worse, I think the interaction between FSNM > and UNTR extension is broken. This is partly from me guessing how > fsmonitor works so I may be double-wrong. > > UNTR extension is supposed to cover the untracked state at timestamp > $X. Where $X is stored in FSNM extension. Correct? > > When fsmonitor is used and read_directory() is executed (at timestamp > $Y), we ask watchman "what's new on worktree since $X?"). We get the > list, we invalidate relevant directories so we can see new untracked > entries (or lack of old untracked entries). We write FSNM with > timestamp $Y down. We may or may not write UNTR down because of this > patch, but let's suppose we do. All is good. UNTR now records the > state at $Y. FSNM says $Y. This is how it works (when there's no bugs) > > UNTR extension is only updated when read_directory() is called. It > does not always happen. FSNM is updated all the time (almost at every I was indeed doubly wrong. When FSMN is read, it does make calls to invalidate stuff in untracked cache data structure. If the index is written down (with updated FSMN extension and timestamp) then the UNTR extension, which is generated from in-core untracked data structure, also reflects the damages by the changed paths so next time even if those changed paths are not reported again (between $Y and $Z below), it's fine. All is good in the world again :) > git command since most of them needs to read index, many write it > down) with new timestamp. Suppose FSNM now records timestamp $Z, UNTR > still records the state at $Y because in the last index update, > read_directory() is not executed. 4 files have been added between $Y > and $Z. When we ask watchman "what's new since $Z?" we will not see > those files, so we don't invalidate relevant directories and > read_directory() will not see those files. -- Duy
Re: BUG: fetch in certain repo always gives "did not send all necessary objects"
On Wed, Feb 7, 2018 at 7:00 AM, Elijah Newrenwrote: > and knew they had been using it, then I might have guessed that "HEAD" > meant "not your actual HEAD but the HEAD of the vestige of some other > worktree"). > > Does anyone have pointers about what might be doable in terms of > providing a more useful error message to allow users to recover? I noticed this too. I was working on improving this message a bit but got side tracked and since I figured this did not happen often anymore, this fix got lower priority than others. I'll resume that work. > And/or ideas of what steps could cause corruption so I can send out a > PSA to help users avoid it? There is another thing we could do. One bad HEAD should not abort the entire operation (at least if it's not the current worktree's HEAD). We could still give a warning and move on, or don't warn at all and let "git worktree prune" collect it (which I see from your message that it also fails to do). I guess that's two more items on my todo list :) Sorry for all the trouble because of this bug of mine. > If not, I'll try to dig more, but I thought I'd ask others familiar > with this area. -- Duy
Re: is http://git-scm.com an *official* git-affiliated site?
On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Daywrote: > > i ask WRT whether it should be up to date. i'm currently writing a > number of git-related wiki pages, and i want to link to whatever are > the canonical man pages for various git commands, I think this one is updated often by Junio (Git maintainer) https://www.kernel.org/pub/software/scm/git/docs/ > but that site seems a bit off. -- Duy
Compliment of the day to you. Dear Friend
Compliment of the day to you. Dear Friend. for security reason contact me Through this email (mrschantal.sonian.k...@gmail.com) Dear Friend. I am Mrs.Chantal Sonian Kadi. am sending this brief letter to solicit your partnership to transfer $10.5 million US Dollars. I shall send you more information and procedures when I receive positive response from you. please send me a message in my Email box (mrschantal.sonian. k...@gmail.com) Coupy This Email Id (mrschantal.sonian.k...@gmail.com)To Reply Me with this Id(mrschantal.sonian.k...@gmail.com) Mrs.Chantal Sonian Kadi
Re: is http://git-scm.com an *official* git-affiliated site?
On Wed, 7 Feb 2018, Duy Nguyen wrote: > On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day> wrote: > > > > i ask WRT whether it should be up to date. i'm currently writing a > > number of git-related wiki pages, and i want to link to whatever are > > the canonical man pages for various git commands, > > I think this one is updated often by Junio (Git maintainer) > > https://www.kernel.org/pub/software/scm/git/docs/ ah, better, thanks. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
Re: is http://git-scm.com an *official* git-affiliated site?
On Wed, Feb 07 2018, Robert P. J. Day jotted: > i ask WRT whether it should be up to date. i'm currently writing a > number of git-related wiki pages, and i want to link to whatever are > the canonical man pages for various git commands, but that site seems > a bit off. > > if one follows a link to get here: > > https://git-scm.com/doc > > there is another link to the "Reference Manual" that promises "The > official and comprehensive man pages that are included in the Git > package itself." > > but upon getting there: > > https://git-scm.com/docs > > it seems clear that that page doesn't come close to covering all of > the available git commands. > > as an example, there is a link "submodule" on that page, which does > indeed take one to the page https://git-scm.com/docs/git-submodule. so > far, so good. > > however, while there is no link for the "worktree" command, there > does in fact exist a similarly-named web page > https://git-scm.com/docs/git-worktree. It is an official site, of the git project. The project is legally managed by the SFC which owns the domain, Github happens to host it (for free) for us. It's not fully auto-generated so stuff like git-worktree doesn't get added automatically, I just added a PR for that: https://github.com/git/git-scm.com/pull/1133 > finally, there is no reference to the git "subtree" command, either > as a link *or* as an existing web page > https://git-scm.com/docs/git-subtree. it all seems kind of arbitrary. Unlike git-worktree, git-subtree is not shipped as a "proper" git command, it lives in contrib/. What the status of that is I'm not sure, but that's why it's not on git-scm or kernel.org in any form. > is there an actual, canonical set of online web pages for all > current git commands one should use? You can use git-scm.com or the kernel.org link Duy noted. The thing that's the most official is the man pages we ship with any given release, and as seen above the online presence may lag behind in some ways, but that can always be fixed. Even though we may have some official online sources, it's better to direct users to the documentation that ships with git on their system, since it'll reflect the version they're using.
Re: is http://git-scm.com an *official* git-affiliated site?
On Wed, 7 Feb 2018, Duy Nguyen wrote: > On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day> wrote: > > > > i ask WRT whether it should be up to date. i'm currently writing a > > number of git-related wiki pages, and i want to link to whatever are > > the canonical man pages for various git commands, > > I think this one is updated often by Junio (Git maintainer) > > https://www.kernel.org/pub/software/scm/git/docs/ whoops, just noticed that there is still no entry for "git subtree" there; is there something about that git command that makes it so hard to track down? :-) rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
Re: [RFC PATCH 000/194] Moving global state into the repository object
On Tue, Feb 6, 2018 at 6:51 AM, Stefan Bellerwrote: > This series moves a lot of global state into the repository struct. > It applies on top of 2512f15446149235156528dafbe75930c712b29e (2.16.0) > It can be found at https://github.com/stefanbeller/git/tree/object-store > > Motivation for this series: > * Easier to reason about code when all state is stored in one place, > for example for multithreading > * Easier to move to processing submodules in-process instead of > calling a processes for the submodule handling. > The last patch demonstrates this. I have a mixed feeling about this. The end game is to keep "the_repository" references as minimum as possible, right? Like we only need to mention in in cmd_xxx() and not all over the place. If so, yay!! Something else.. maybe "struct repository *" should not be a universal function argument to avoid global states. Like sha1_file.c is mostly about the object store, and I see that you added object store struct (nice!). These sha1 related function should take the object_store* (which probably is a combination of both packed and loose stores since they depend on each other), not repository*. This way if a function needs both access to object store and ref store, we can see the two dependencies from function arguments. The alternate object store, if modeled right, could share the same object store interface. But I don't think we should do anything that big right now, just put alternate object store inside object_store. Similarly those functions in blob.c, commit.c, tree.c... should take object_parser* as argument instead of repository*. Maybe there's a better name for this than object_parser. parsed_object_store I guess. Or maybe just object_pool. -- Duy
Re: is http://git-scm.com an *official* git-affiliated site?
On Wed, Feb 7, 2018 at 6:41 PM, Robert P. J. Daywrote: > On Wed, 7 Feb 2018, Duy Nguyen wrote: > >> On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day >> wrote: >> > >> > i ask WRT whether it should be up to date. i'm currently writing a >> > number of git-related wiki pages, and i want to link to whatever are >> > the canonical man pages for various git commands, >> >> I think this one is updated often by Junio (Git maintainer) >> >> https://www.kernel.org/pub/software/scm/git/docs/ > > whoops, just noticed that there is still no entry for "git subtree" > there; is there something about that git command that makes it so hard > to track down? :-) git-subtree is not an official command, so it's not there. -- Duy
Re: is http://git-scm.com an *official* git-affiliated site?
On Wed, 7 Feb 2018, Duy Nguyen wrote: > On Wed, Feb 7, 2018 at 6:41 PM, Robert P. J. Day> wrote: > > On Wed, 7 Feb 2018, Duy Nguyen wrote: > > > >> On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day > >> wrote: > >> > > >> > i ask WRT whether it should be up to date. i'm currently writing a > >> > number of git-related wiki pages, and i want to link to whatever are > >> > the canonical man pages for various git commands, > >> > >> I think this one is updated often by Junio (Git maintainer) > >> > >> https://www.kernel.org/pub/software/scm/git/docs/ > > > > whoops, just noticed that there is still no entry for "git subtree" > > there; is there something about that git command that makes it so hard > > to track down? :-) > > git-subtree is not an official command, so it's not there. i'm going to push back a bit on that since, at the bottom of the man page for git-subtree, it states: GIT Part of the git(1) suite so either it's part of the git suite, or it isn't. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
categorization, documentation and packaging of "git core" commands
(related to earlier thread but different enough that i'll start fresh.) based on the collection of man page links here: https://www.kernel.org/pub/software/scm/git/docs/ i took a look at how git 2.14.3 is laid out on my fedora 27 system, particularly all of the executables under /usr/libexec/git-core/. after manually cross-checking all of those executables against the links on the docs page, here's what i learned. first, here are the executables under /usr/libexec/git-core/ that are unreferenced by that web page, but that should be fine as almost all of them would be considered underlying helpers or utilities (except for things like git-subtree, but we're still unclear on its status, right?): git-add--interactive git-bisect--helper git-credential-cache--daemon git-credential-libsecret git-credential-netrc git-difftool--helper git-fsck-objects git-gui--askpass git-init-db git-merge-octopus git-merge-ours git-merge-recursive git-merge-resolve git-merge-subtree git-mergetool--lib git-rebase--am git-rebase--helper git-rebase--interactive git-rebase--merge git-remote-ext git-remote-fd git-remote-ftp git-remote-ftps git-remote-http git-remote-https git-sh-i18n--envsubst git-stage git-submodule--helper git-subtree git-web--browse on the other hand (and this is not so much a git issue as a fedora packaging issue), there are a number of command links at that web page that are supplied by distinct RPM packages rather than by the basic fedora git package, so one would need to install the following packages to get some of those commands on fedora: * gitk * git-cvs * git-svn * git-p4 * git-email (provides git-send-email) finally, from fedora, i am utterly unable to find a package that provides git-archimport. pretty sure fedora used to have a "git-arch" package but it's not there now. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
Re: is http://git-scm.com an *official* git-affiliated site?
On Wed, Feb 7, 2018 at 6:58 PM, Robert P. J. Daywrote: > On Wed, 7 Feb 2018, Duy Nguyen wrote: > >> On Wed, Feb 7, 2018 at 6:41 PM, Robert P. J. Day >> wrote: >> > On Wed, 7 Feb 2018, Duy Nguyen wrote: >> > >> >> On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day >> >> wrote: >> >> > >> >> > i ask WRT whether it should be up to date. i'm currently writing a >> >> > number of git-related wiki pages, and i want to link to whatever are >> >> > the canonical man pages for various git commands, >> >> >> >> I think this one is updated often by Junio (Git maintainer) >> >> >> >> https://www.kernel.org/pub/software/scm/git/docs/ >> > >> > whoops, just noticed that there is still no entry for "git subtree" >> > there; is there something about that git command that makes it so hard >> > to track down? :-) >> >> git-subtree is not an official command, so it's not there. > > i'm going to push back a bit on that since, at the bottom of the man > page for git-subtree, it states: > > GIT >Part of the git(1) suite > > so either it's part of the git suite, or it isn't. It's a template that people use when write document for new commands. That part should be deleted, I guess, if it's that confusing. -- Duy
Re: is http://git-scm.com an *official* git-affiliated site?
On 07/02/2018, Jeff King wrote: > On Wed, Feb 07, 2018 at 12:37:32PM +0100, Ævar Arnfjörð Bjarmason wrote: > > >> It's not fully auto-generated so stuff like git-worktree doesn't get >> added automatically, I just added a PR for that: >> https://github.com/git/git-scm.com/pull/1133 > Thanks for doing that. I'm also open to auto-generating the index if we > can come up with well-organized output. > > -Peff I did not know that git-worktree is not considered ready for general consumption. It has been present in the release notes for quite some time now. If there's something available from the git repo to drive the build of the index, that would be a good way to advert the publicly available commands of git. JN
Re: BUG: fetch in certain repo always gives "did not send all necessary objects"
On Wed, Feb 07, 2018 at 06:08:40PM +0700, Duy Nguyen wrote: > > And/or ideas of what steps could cause corruption so I can send out a > > PSA to help users avoid it? > > There is another thing we could do. One bad HEAD should not abort the > entire operation (at least if it's not the current worktree's HEAD). > We could still give a warning and move on, or don't warn at all and > let "git worktree prune" collect it (which I see from your message > that it also fails to do). Whether that's safe or not depends very much on what the caller intends to do with the ref. Skipping broken refs in "git prune" is a bad idea, for instance. -Peff
BUG: On some Linux systems, "Stage To Commit" hotkey in git-gui stages one file only, even if multiple files are selected
In git-gui, multiple files from the "Unstaged Changes" widget can be selected. Once multiple files are selected, they can be staged by clicking (toolbar) "Commit"->"Stage To Commit". All the files that were selected then gets staged for the commit. The "Stage To Commit" hotkey (CTRL+T) behaves like the command itself, staging all files that are selected (Tested on Windows 10). However, on some linux systems (currently tested on Ubuntu 17.10), only one file (the most recently selected) gets staged when using the "Stage To Commit" hotkey (CTRL+T), while the other, selected files remain unstaged. Steps to reproduce (on Ubuntu 17.10): - Open git-gui in a repository with two or more unstaged, changed files - Select two or more files in the "Unstaged Changes" - Click CTRL+T to stage the files that you have selected (Only a single file will get staged)
Re: [PATCH v1] dir.c: don't flag the index as dirty for changes to the untracked cache
On 2/7/2018 5:59 AM, Duy Nguyen wrote: On Tue, Feb 6, 2018 at 7:55 PM, Duy Nguyenwrote: On Tue, Feb 6, 2018 at 7:27 PM, Duy Nguyen wrote: On Tue, Feb 6, 2018 at 8:48 AM, Ben Peart wrote: With the new behavior, making a change in dir1/, then calling status would update the dir1/ untracked cache entry but not write it out. On the next status, git would detect the change in dir1/ again and update the untracked Thing only missing piece here I would add is, this dir1/ detection is done by watchman. We have to contact watchman and ask the set of changed paths since $TIME where $TIME is the last time we updated untracked cache and invalidate those paths in core. Then it should work correctly. I checked the watchman query in the fsmonitor hook and I think it's correct so far. No I think I'm wrong. And worse, I think the interaction between FSNM and UNTR extension is broken. This is partly from me guessing how fsmonitor works so I may be double-wrong. UNTR extension is supposed to cover the untracked state at timestamp $X. Where $X is stored in FSNM extension. Correct? When fsmonitor is used and read_directory() is executed (at timestamp $Y), we ask watchman "what's new on worktree since $X?"). We get the list, we invalidate relevant directories so we can see new untracked entries (or lack of old untracked entries). We write FSNM with timestamp $Y down. We may or may not write UNTR down because of this patch, but let's suppose we do. All is good. UNTR now records the state at $Y. FSNM says $Y. This is how it works (when there's no bugs) UNTR extension is only updated when read_directory() is called. It does not always happen. FSNM is updated all the time (almost at every I was indeed doubly wrong. When FSMN is read, it does make calls to invalidate stuff in untracked cache data structure. If the index is written down (with updated FSMN extension and timestamp) then the UNTR extension, which is generated from in-core untracked data structure, also reflects the damages by the changed paths so next time even if those changed paths are not reported again (between $Y and $Z below), it's fine. All is good in the world again :) Thank you for looking into this closely. It's always good to have someone else examine the logic to make sure there aren't errors that were missed. Sorry my responses have been slow, my day job has had me busy lately. git command since most of them needs to read index, many write it down) with new timestamp. Suppose FSNM now records timestamp $Z, UNTR still records the state at $Y because in the last index update, read_directory() is not executed. 4 files have been added between $Y and $Z. When we ask watchman "what's new since $Z?" we will not see those files, so we don't invalidate relevant directories and read_directory() will not see those files.
Re: [PATCH 02/10] t5812: add 'test_i18ngrep's missing filename parameter
On Fri, Jan 26, 2018 at 7:27 PM, Jeff Kingwrote: > On Fri, Jan 26, 2018 at 01:37:00PM +0100, SZEDER Gábor wrote: > >> The second 'test_i18ngrep' invocation in the test 'curl redirects >> respect whitelist' is missing its filename parameter. This has >> remained unnoticed since its introduction in f4113cac0 (http: limit >> redirection to protocol-whitelist, 2015-09-22), because it would only >> cause the test to fail if Git was built with a sufficiently old >> libcurl version. The test's two ||-chained 'test_i18ngrep' >> invocations are supposed to check that either one of the two patterns >> is present in 'git clone's error message. As it happens, the first >> invocation covers the error message from any reasonably up-to-date >> libcurl, thus the second invocation, the one without the filename >> parameter, isn't executed at all. Apparently no one has run the test >> suite's httpd tests with such an old libcurl in the last 2+ years, or >> at least they haven't bothered to notify us about the failed test. > > Interesting find. > > The "too old" curl is older than 7.19.4, which we actually fail to build > with since v2.12.0. So they probably did not even get as far as the > tests. ;) Oh, OK, I was not aware of that. The oldest non-maintenance release with the missing filename parameter is v2.7.0, so that's still a 5 releases time frame to notice it. Anyway, I'm preparing v2 of this series, and I'm not sure what to do about this. - Should I simply drop the "your curl version is too old" pattern? It would make sense, but it just doesn't feel quite right to remove it while the corresponding printf() is still there, even if it can't be triggered anymore. However, cleaning up the curl version checks in http.c to remove this message is beyond the scope of this patch series. - Or leave it almost-as-is, only dropping the now unnecessary curly braces as Simon pointed out. And perhaps a bit of update to the commit message. I'd prefer the second option. >> Fix this by consolidating the two patterns into a single extended >> regexp, eliminating the need for an ||-chained second 'test_i18ngrep' >> invocation. > > OK. Once upon a time I think we had trouble with "grep -E", since some > older systems had only "egrep". But I see we've introduced some "grep > -E" invocations as far back as 2013 and nobody has complained, so it's > probably fine. Yeah, first I went with the more traditional "\(this\|that\)" pattern, but then noticed that 'grep -E' is already used in a couple of places, and picked the format that uses less escape characters.
Re: [bug report]: error doing_rebase
Hi Bulat, Please make sure to keep the Git mailing list in Cc: (I get *very* prickly when Git users treat me as a free-of-cost help desk, and when I get that annoyed, I stop helping). On Tue, 6 Feb 2018, Bulat Musin wrote: > Yes, I tested again. > > With built 2.16... and it shows error message. git rebase --abort restores > > state before rebase. You misunderstood me. I am convinced that that error message *is correct*. It shows an incorrect usage. You cannot start off an interactive rebase by a `squash` command. > With git 2.14 from Ubuntu's repo it works - 3 commits are squashed into first > one Yes, but you called `git rebase -i HEAD~2`, which means that only two commits were up for rebasing. The third commit is outside the range `HEAD~2..` which the command `git rebase -i HEAD~2` wants to let you rebase. If v2.14 indeed modified `HEAD~2` (as I suspected in my earlier mail), then you successfully confirmed that we fixed a bug, and that you expected the buggy behavior. > - with change SHA. > > It seems to be bug in recent version. > > Should I provide additional information? Ciao, Johannes > On 02/06/2018 12:47 PM, Johannes Schindelin wrote: > > Hi, > > > > On Mon, 5 Feb 2018, Bulat Musin wrote: > > > > > Now there are 3 sequential commits, I want to squash them into 1: > > > > > > git rebase -i HEAD~2 > > > > > > In editor I changed all "pick" to "squash", saved file, I got: > > > > > > error: cannot 'squash' without a previous commit > > You cannot start with a squash. You have to pick the first one, then > > squash the second into the first. > > > > > However, 2.14.1 from Ubuntu's repo does the job - squashes 3 commits into > > > 1. > > It may be careless enough to do that, however, it might now have modified > > the *wrong* commit, i.e. squashed the two patches *into HEAD~2*. > > > > Please verify that your HEAD~2 is still intact and part of the rebased > > history, otherwise you will have a problem. > > > > Ciao, > > Johannes > >
Re: [PATCH v1] dir.c: don't flag the index as dirty for changes to the untracked cache
On 2/6/2018 7:27 AM, Duy Nguyen wrote: This is another thing that bugs me. I know you're talking about huge index files, but at what size should we start this sort of optimization? Writing down a few MBs on linux is cheap enough that I won't bother optimizing (and I get my UNTR extension repaired all the time, so reduced lstat calls and stuff). This "typically" only comes at certain size, what size? It's important to identify what we're trading off here. With the proposed optimization, we'll end up doing less writes of the index but potentially more lstat calls. Even with a small index, writing the index is much more expensive than calling lstat on a file. Exactly how much more expensive depends on a lot of variables but even with a SSD its still orders of magnitude difference. That means we could potentially lstat hundreds or thousands of files and still come out ahead. Given the untracked cache works at the directory level, the lstat cost actually scales with the number of directories that have had changes (ie changing a 2nd file in the same directory doesn't add any additional cost). In "typical" workflows, developers don't often change hundreds of files across different directories so we'd "typically" still come out ahead. We have internal performance tests based on common developer sequences of commands (status, edit a file, status, add, status, commit, log, push, etc) that show that having the untracked cache turned on actually makes these sequences slower. At the time, we didn't have time to investigate why so we just turned off the untracked cache. When writing the fsmonitor patch series which can utilize the untracked cache, I did some investigation into why the untracked cache was slowing things down in these tests and discovered the cause was the additional index writes. That is what led to this patch. I've been sitting on it for months now because I didn't have the time to write some performance tests for the git perf framework to demonstrate the problem and how this helps solve it. With the discussion about the fsmonitor extension, I thought this might be a good time to send it out there. If you have the cycles, time a typical series of commands with and without the untracked cache (ie don't just call status over and over in a loop) and you should see the same results. Given my limited time right now, I'm OK putting this on the back burner again until a git perf test can be written to ensure it actually speeds things up as advertised.
Re: [PATCH 02/10] t5812: add 'test_i18ngrep's missing filename parameter
On Wed, Feb 07, 2018 at 02:53:17PM +0100, SZEDER Gábor wrote: > > The "too old" curl is older than 7.19.4, which we actually fail to build > > with since v2.12.0. So they probably did not even get as far as the > > tests. ;) > > Oh, OK, I was not aware of that. The oldest non-maintenance release > with the missing filename parameter is v2.7.0, so that's still a 5 > releases time frame to notice it. Actually, I'm wrong. It looks like we did finally fix it in f18777ba6e (http: fix handling of missing CURLPROTO_*, 2017-08-11), which is in v2.15. So: > Anyway, I'm preparing v2 of this series, and I'm not sure what to do > about this. > > - Should I simply drop the "your curl version is too old" pattern? It > would make sense, but it just doesn't feel quite right to remove it > while the corresponding printf() is still there, even if it can't be > triggered anymore. However, cleaning up the curl version checks in > http.c to remove this message is beyond the scope of this patch > series. > > - Or leave it almost-as-is, only dropping the now unnecessary curly > braces as Simon pointed out. And perhaps a bit of update to the > commit message. > > I'd prefer the second option. Yeah, I think just leave it as-is. Thanks. -Peff
Re: [PATCH] files-backend: unlock packed store only if locked
On Tue, Feb 06, 2018 at 12:36:15PM -0800, Jonathan Tan wrote: > In commit 42c7f7ff9685 ("commit_packed_refs(): remove call to > `packed_refs_unlock()`", 2017-06-23), a call to packed_refs_unlock() was > added to files_initial_transaction_commit() in order to compensate for > removing that call from commit_packed_refs(). However, that call was > added in the cleanup section, which is run even if the packed_ref_store > was never locked (which happens if an error occurs earlier in the > function). > > Create a new cleanup goto target which runs packed_refs_unlock(), and > ensure that only goto statements after a successful invocation of > packed_refs_lock() jump there. The explanation and patch look good to me. But this all seemed strangely familiar... I think this is the same bug as: https://public-inbox.org/git/20180118143841.1a4c674d@novascotia/ which is queued as mr/packed-ref-store-fix. It's listed as "will merge to next" in the "what's cooking" from Jan 31st. > diff --git a/refs/files-backend.c b/refs/files-backend.c > index f75d960e1..89bc5584a 100644 > --- a/refs/files-backend.c > +++ b/refs/files-backend.c > @@ -2931,13 +2931,14 @@ static int files_initial_transaction_commit(struct > ref_store *ref_store, > > if (initial_ref_transaction_commit(packed_transaction, err)) { > ret = TRANSACTION_GENERIC_ERROR; > - goto cleanup; > + goto locked_cleanup; > } > > +locked_cleanup: > + packed_refs_unlock(refs->packed_ref_store); > cleanup: > if (packed_transaction) > ref_transaction_free(packed_transaction); > - packed_refs_unlock(refs->packed_ref_store); I actually like this double-label a bit more than what is queued on mr/packed-ref-store-fix, though I am OK with either solution. -Peff
Bug report: Subtree split including extra commits
Apologies if this is the wrong place to send a bug report for Contributed software. I've run into what seems like an issue/bug with git subtree. I am trying to split a single directory of our repo into its own repo using git subtree. I ran the the following command from our project root: git subtree split --prefix=geekui2 -b geekui2-split where geekui2 is the name of the subdir. For commits before mid 2017, the created split branch contains our entire commit history, regardless whether or not they include changes in geekui2. For commits after that point, the commits are properly filtered to include only commits that contain changes in geekui2. The upshot is that if I push the branch to a new repo, and check out one of those earlier commits, I can essentially recover the entire codebase of our application (at that point), not just the content in geekui2. Since our goal was to share part of our repo while keeping the rest of it private, this is obviously a problem. I've also tried using git-subrepo for this split--it seems to correctly filter the commits, excluding all commits without changes to geekui2. So something seems to be going wrong with the way git-subtree handles this relative to git subrepo. Unfortunately, there is a lot going on in our repo--I have no idea how I would generate a minimal reproduction of this. While our repo is private, I'm happy to help try to help debug this if someone wants to take a look at this issue, although it is not my area of expertise. My git version is 2.16.1, and I am running it on macOS 10.12.6. I am not subscribed to the git mailing list, so if anyone wants to get in touch with me about this, I'm most likely to see it if you send an email. -- Daniel Karp
Re: [PATCH v2 04/14] commit-graph: implement construct_commit_graph()
On Mon, Feb 5, 2018 at 5:06 PM, Derrick Stoleewrote: > On 2/2/2018 10:32 AM, SZEDER Gábor wrote: >> In my git repo, with 9 pack files at the moment, i.e. not that big a >> repo and not that many pack files: >> >>$ time ./git commit-graph --write --update-head >>4df41a3d1cc408b7ad34bea87b51ec4ccbf4b803 >> >>real0m27.550s >>user0m27.113s >>sys 0m0.376s >> >> In comparison, performing a good old revision walk to gather all the >> info that is written into the graph file: >> >>$ time git log --all --topo-order --format='%H %T %P %cd' |wc -l >>52954 >> >>real0m0.903s >>user0m0.972s >>sys 0m0.058s > > > Two reasons this is in here: > > (1) It's easier to get the write implemented this way and add the reachable > closure later (which I do). > > (2) For GVFS, we want to add all commits that arrived in a "prefetch pack" > to the graph even if we do not have a ref that points to the commit yet. We > expect many commits to become reachable soon and having them in the graph > saves a lot of time in merge-base calculations. > > So, (1) is for patch simplicity, and (2) is why I want it to be an option in > the final version. See the --stdin-packs argument later for a way to do this > incrementally. > > I expect almost all users to use the reachable closure method with > --stdin-commits (and that's how I will integrate automatic updates with > 'fetch', 'repack', and 'gc' in a later patch). I see. I was about to ask about the expected use-cases of the '--stdin-packs' option, considering how much slower it is to enumerate all objects in pack files, but run out of time after patch 10. The run-time using '--stdin-commits' is indeed great: $ time { git for-each-ref --format='%(objectname)' refs/heads/ | ./git commit-graph --write --update-head --stdin-commits ; } 82fe9a5cd715ff578f01f7f44e0611d7902d20c8 real 0m0.985s user 0m0.916s sys 0m0.024s Considering the run-time difference, I think in the end it would be a better default for a plain 'git commit-graph --write' to traverse history from all refs, and it should enumerate pack files only if explicitly told so via '--stdin-packs'. To be clear: I'm not saying that traversing history should already be the default when introducing construct_commit_graph() and '--write'. If enumerating pack files keeps the earlier patches simpler and easier to review, then by all means stick with it, and only change the '--stdin-*'-less behavior near the end of the series, when all the building blocks are already in place (but then mention this in the early commit messages). I have also noticed a segfault when feeding non-commit object names to '--stdin-commits', i.e. when I run the above command without restricting 'git for-each-ref' to branches and it listed object names of tags as well. $ git rev-parse v2.16.1 |./git commit-graph --write --update-head --stdin-commits error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit Segmentation fault (gdb) bt #0 __memcpy_avx_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-avx-unaligned.S:126 #1 0x004ea97c in sha1write (f=0x356bbf0, buf=0x4, count=20) at csum-file.c:104 #2 0x004d98e1 in write_graph_chunk_data (f=0x356bbf0, hash_len=20, commits=0x3508de0, nr_commits=50615) at commit-graph.c:506 #3 0x004da9ca in construct_commit_graph ( pack_dir=0x8ff360 ".git/objects/pack", pack_indexes=0x0, nr_packs=0, commit_hex=0x8ff790, nr_commits=1) at commit-graph.c:818 #4 0x0044184e in graph_write () at builtin/commit-graph.c:149 #5 0x00441a8c in cmd_commit_graph (argc=0, argv=0x7fffe310, prefix=0x0) at builtin/commit-graph.c:224 #6 0x00405a0a in run_builtin (p=0x8ad950 , argc=4, argv=0x7fffe310) at git.c:346 #7 0x00405ce4 in handle_builtin (argc=4, argv=0x7fffe310) at git.c:555 #8 0x00405ec8 in run_argv (argcp=0x7fffe1cc, argv=0x7fffe1c0) at git.c:607 #9 0x00406079 in cmd_main (argc=4, argv=0x7fffe310) at git.c:684 #10 0x004a85c8 in main (argc=5, argv=0x7fffe308) at common-main.c:43
Re: [PATCH v2 04/14] commit-graph: implement construct_commit_graph()
On 2/7/2018 10:08 AM, SZEDER Gábor wrote: On Mon, Feb 5, 2018 at 5:06 PM, Derrick Stoleewrote: On 2/2/2018 10:32 AM, SZEDER Gábor wrote: In my git repo, with 9 pack files at the moment, i.e. not that big a repo and not that many pack files: $ time ./git commit-graph --write --update-head 4df41a3d1cc408b7ad34bea87b51ec4ccbf4b803 real0m27.550s user0m27.113s sys 0m0.376s In comparison, performing a good old revision walk to gather all the info that is written into the graph file: $ time git log --all --topo-order --format='%H %T %P %cd' |wc -l 52954 real0m0.903s user0m0.972s sys 0m0.058s Two reasons this is in here: (1) It's easier to get the write implemented this way and add the reachable closure later (which I do). (2) For GVFS, we want to add all commits that arrived in a "prefetch pack" to the graph even if we do not have a ref that points to the commit yet. We expect many commits to become reachable soon and having them in the graph saves a lot of time in merge-base calculations. So, (1) is for patch simplicity, and (2) is why I want it to be an option in the final version. See the --stdin-packs argument later for a way to do this incrementally. I expect almost all users to use the reachable closure method with --stdin-commits (and that's how I will integrate automatic updates with 'fetch', 'repack', and 'gc' in a later patch). I see. I was about to ask about the expected use-cases of the '--stdin-packs' option, considering how much slower it is to enumerate all objects in pack files, but run out of time after patch 10. The run-time using '--stdin-commits' is indeed great: $ time { git for-each-ref --format='%(objectname)' refs/heads/ | ./git commit-graph --write --update-head --stdin-commits ; } 82fe9a5cd715ff578f01f7f44e0611d7902d20c8 real 0m0.985s user 0m0.916s sys 0m0.024s Considering the run-time difference, I think in the end it would be a better default for a plain 'git commit-graph --write' to traverse history from all refs, and it should enumerate pack files only if explicitly told so via '--stdin-packs'. To be clear: I'm not saying that traversing history should already be the default when introducing construct_commit_graph() and '--write'. If enumerating pack files keeps the earlier patches simpler and easier to review, then by all means stick with it, and only change the '--stdin-*'-less behavior near the end of the series, when all the building blocks are already in place (but then mention this in the early commit messages). I will consider this. I have also noticed a segfault when feeding non-commit object names to '--stdin-commits', i.e. when I run the above command without restricting 'git for-each-ref' to branches and it listed object names of tags as well. $ git rev-parse v2.16.1 |./git commit-graph --write --update-head --stdin-commits error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit Segmentation fault (gdb) bt #0 __memcpy_avx_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-avx-unaligned.S:126 #1 0x004ea97c in sha1write (f=0x356bbf0, buf=0x4, count=20) at csum-file.c:104 #2 0x004d98e1 in write_graph_chunk_data (f=0x356bbf0, hash_len=20, commits=0x3508de0, nr_commits=50615) at commit-graph.c:506 #3 0x004da9ca in construct_commit_graph ( pack_dir=0x8ff360 ".git/objects/pack", pack_indexes=0x0, nr_packs=0, commit_hex=0x8ff790, nr_commits=1) at commit-graph.c:818 #4 0x0044184e in graph_write () at builtin/commit-graph.c:149 #5 0x00441a8c in cmd_commit_graph (argc=0, argv=0x7fffe310, prefix=0x0) at builtin/commit-graph.c:224 #6 0x00405a0a in run_builtin (p=0x8ad950 , argc=4, argv=0x7fffe310) at git.c:346 #7 0x00405ce4 in handle_builtin (argc=4, argv=0x7fffe310) at git.c:555 #8 0x00405ec8 in run_argv (argcp=0x7fffe1cc, argv=0x7fffe1c0) at git.c:607 #9 0x00406079 in cmd_main (argc=4, argv=0x7fffe310) at git.c:684 #10 0x004a85c8 in main (argc=5, argv=0x7fffe308) at common-main.c:43 I noticed this while preparing v3. I have a fix, but you now remind me that I need to add tags to the test script. Thanks, -Stolee
Re: Shawn Pearce has died
On Tue, Jan 30, 2018 at 01:49:08PM -0500, Jeff King wrote: > On Mon, Jan 29, 2018 at 11:15:55PM -0800, Chris DiBona wrote: > > > That's a fantastic idea. When is the contrib summit and is it open to > > others? I could send someone ... > > It's March 7th in Barcelona. Details are here: > > https://public-inbox.org/git/20180119001034.ga29...@sigill.intra.peff.net/ > > There will be GitHub video folks on premises the following day for the > Git Merge main conference, but I'm looking into whether they'll be > around to record a few interviews on the contributor summit day. Unfortunately I just learned that we (GitHub) are not actually sending video folks to Barcelona this year. So we (Git) are on our own for recording anything. Certainly we can record something with one of our cameras, though I wonder if it might be simpler to just have people record themselves and send it to Chris, then. -Peff
Re: [RFC PATCH 000/194] Moving global state into the repository object
On 2/5/2018 6:51 PM, Stefan Beller wrote: This series moves a lot of global state into the repository struct. It applies on top of 2512f15446149235156528dafbe75930c712b29e (2.16.0) It can be found at https://github.com/stefanbeller/git/tree/object-store Motivation for this series: * Easier to reason about code when all state is stored in one place, for example for multithreading * Easier to move to processing submodules in-process instead of calling a processes for the submodule handling. The last patch demonstrates this. [...] Very nice. My eyes glazed over a few times, but I like the direction you're heading here. Thanks for tackling this! Jeff
Re: git: CVE-2018-1000021: client prints server sent ANSI escape codes to the terminal, allowing for unverified messages to potentially execute arbitrary commands
On Feb 06 2018, "Randall S. Becker"wrote: > What I don't know - and it's not explicitly in the CVE - is just how many > other terminal types with similar vulnerabilities are out there, but I'm > suspecting it's larger than one would guess - mostly, it seems like this > particular sequence is intended to be used for writing status line output > (line 25?) instead of sticking it in a prompt. This can be used prettifies a > lengthy bash prompt to display the current branch and repository at the > bottom of the screen instead of in the inline prompt, but that's the user's > choice and not something git has to deal with. There were some green-screen > terminals with other weird ESC sequences back in the day that could really > get into trouble with this, including loading/executing programs in terminal > memory via output - really. I'm sure it seemed like a good idea at the time, > but I can see how it could have been used for evil. Do you also want to block "+++AT"? :-) Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
[PATCHv4] tag: add --edit option
Add a --edit option whichs allows modifying the messages provided by -m or -F, the same way git commit --edit does. Signed-off-by: Nicolas Morey-Chaisemartin--- Fixes since v3 ( https://public-inbox.org/git/88e7c122-599f-4ab1-6d65-c75f7a3ae...@suse.com/ ): * Replace tab by space in t/t7004-tag.sh Documentation/git-tag.txt | 8 +++- builtin/tag.c | 11 +-- t/t7004-tag.sh| 30 ++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 956fc019f984..1d17101bac39 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -9,7 +9,7 @@ git-tag - Create, list, delete or verify a tag object signed with GPG SYNOPSIS [verse] -'git tag' [-a | -s | -u ] [-f] [-m | -F ] +'git tag' [-a | -s | -u ] [-f] [-m | -F ] [-e] [ | ] 'git tag' -d ... 'git tag' [-n[]] -l [--contains ] [--no-contains ] @@ -167,6 +167,12 @@ This option is only applicable when listing tags without annotation lines. Implies `-a` if none of `-a`, `-s`, or `-u ` is given. +-e:: +--edit:: + The message taken from file with `-F` and command line with + `-m` are usually used as the tag message unmodified. + This option lets you further edit the message taken from these sources. + --cleanup=:: This option sets how the tag message is cleaned up. The '' can be one of 'verbatim', 'whitespace' and 'strip'. The diff --git a/builtin/tag.c b/builtin/tag.c index a7e6a5b0f234..ce5cac3dd23f 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -194,6 +194,7 @@ static int build_tag_object(struct strbuf *buf, int sign, struct object_id *resu struct create_tag_options { unsigned int message_given:1; + unsigned int use_editor:1; unsigned int sign; enum { CLEANUP_NONE, @@ -224,7 +225,7 @@ static void create_tag(const struct object_id *object, const char *tag, tag, git_committer_info(IDENT_STRICT)); - if (!opt->message_given) { + if (!opt->message_given || opt->use_editor) { int fd; /* write the template message before editing: */ @@ -233,7 +234,10 @@ static void create_tag(const struct object_id *object, const char *tag, if (fd < 0) die_errno(_("could not create file '%s'"), path); - if (!is_null_oid(prev)) { + if (opt->message_given) { + write_or_die(fd, buf->buf, buf->len); + strbuf_reset(buf); + } else if (!is_null_oid(prev)) { write_tag_body(fd, prev); } else { struct strbuf buf = STRBUF_INIT; @@ -372,6 +376,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) static struct ref_sorting *sorting = NULL, **sorting_tail = struct ref_format format = REF_FORMAT_INIT; int icase = 0; + int edit_flag = 0; struct option options[] = { OPT_CMDMODE('l', "list", , N_("list tag names"), 'l'), { OPTION_INTEGER, 'n', NULL, , N_("n"), @@ -386,6 +391,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPT_CALLBACK('m', "message", , N_("message"), N_("tag message"), parse_msg_arg), OPT_FILENAME('F', "file", , N_("read message from file")), + OPT_BOOL('e', "edit", _flag, N_("force edit of tag message")), OPT_BOOL('s', "sign", , N_("annotated and GPG-signed tag")), OPT_STRING(0, "cleanup", _arg, N_("mode"), N_("how to strip spaces and #comments from message")), @@ -524,6 +530,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) die(_("tag '%s' already exists"), tag); opt.message_given = msg.given || msgfile; + opt.use_editor = edit_flag; if (!cleanup_arg || !strcmp(cleanup_arg, "strip")) opt.cleanup_mode = CLEANUP_ALL; diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index a9af2de9960b..0630f2dee24b 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -452,6 +452,21 @@ test_expect_success \ test_cmp expect actual ' +get_tag_header annotated-tag-edit $commit commit $time >expect +echo "An edited message" >>expect +test_expect_success 'set up editor' ' + write_script fakeeditor <<-\EOF + sed -e "s/A message/An edited message/g" <"$1" >"$1-" + mv "$1-" "$1" + EOF +' +test_expect_success \ + 'creating an annotated tag with -m message --edit should succeed' ' + EDITOR=./fakeeditor git tag -m "A message" --edit annotated-tag-edit && + get_tag_msg annotated-tag-edit >actual && + test_cmp expect actual +' + cat >msgfile
Re: [PATCH v2] dir.c: ignore paths containing .git when invalidating untracked cache
On 2/7/2018 4:21 AM, Nguyễn Thái Ngọc Duy wrote: read_directory() code ignores all paths named ".git" even if it's not a valid git repository. See treat_path() for details. Since ".git" is basically invisible to read_directory(), when we are asked to invalidate a path that contains ".git", we can safely ignore it because the slow path would not consider it anyway. This helps when fsmonitor is used and we have a real ".git" repo at worktree top. Occasionally .git/index will be updated and if the fsmonitor hook does not filter it, untracked cache is asked to invalidate the path ".git/index". Without this patch, we invalidate the root directory unncessarily, which: - makes read_directory() fall back to slow path for root directory (slower) - makes the index dirty (because UNTR extension is updated). Depending on the index size, writing it down could also be slow. Thank you again, this patch makes much more sense to me. A note about the new "safe_path" knob. Since this new check could be relatively expensive, avoid it when we know it's not needed. If the path comes from the index, it can't contain ".git". If it does contain, we may be screwed up at many more levels, not just this one. I do have a simplifying suggestion to make. I noticed that other uses of verify_path() check when the potentially erroneous path is passed in and then all the underlying code can assume it is valid. I think that makes sense here as well and it makes for a smaller patch. diff --git a/fsmonitor.h b/fsmonitor.h index cd3cc0ccf2..65f3743636 100644 --- a/fsmonitor.h +++ b/fsmonitor.h @@ -65,7 +65,7 @@ static inline void mark_fsmonitor_invalid(struct index_state *istate, struct cac { if (core_fsmonitor) { ce->ce_flags &= ~CE_FSMONITOR_VALID; - untracked_cache_invalidate_path(istate, ce->name); + untracked_cache_invalidate_path(istate, ce->name, 1); This test isn't needed because we're pulling the name right out of the cache entry so it doesn't need to be verified. Here is a modified version of your patch for consideration: read_directory() code ignores all paths named ".git" even if it's not a valid git repository. See treat_path() for details. Since ".git" is basically invisible to read_directory(), when we are asked to invalidate a path that contains ".git", we can safely ignore it because the slow path would not consider it anyway. This helps when fsmonitor is used and we have a real ".git" repo at worktree top. Occasionally .git/index will be updated and if the fsmonitor hook does not filter it, untracked cache is asked to invalidate the path ".git/index". Without this patch, we invalidate the root directory unnecessarily, which: - makes read_directory() fall back to slow path for root directory (slower) - makes the index dirty (because UNTR extension is updated). Depending on the index size, writing it down could also be slow. Noticed-by: Ævar Arnfjörð BjarmasonSigned-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Ben Peart --- Notes: Base Ref: master Web-Diff: https://github.com/benpeart/git/commit/218a577618 Checkout: git fetch https://github.com/benpeart/git verify_path-v1 && git checkout 218a577618 dir.c | 2 +- fsmonitor.c | 6 +- t/t7519-status-fsmonitor.sh | 39 +++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/dir.c b/dir.c index 7c4b45e30e..d431da46f5 100644 --- a/dir.c +++ b/dir.c @@ -1773,7 +1773,7 @@ static enum path_treatment treat_path(struct dir_struct *dir, if (!de) return treat_path_fast(dir, untracked, cdir, istate, path, baselen, pathspec); - if (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, ".git")) + if (is_dot_or_dotdot(de->d_name) || !fspathcmp(de->d_name, ".git")) return path_none; strbuf_setlen(path, baselen); strbuf_addstr(path, de->d_name); diff --git a/fsmonitor.c b/fsmonitor.c index 0af7c4edba..019576f306 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -118,8 +118,12 @@ static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *que static void fsmonitor_refresh_callback(struct index_state *istate, const char *name) { - int pos = index_name_pos(istate, name, strlen(name)); + int pos; + if (!verify_path(name)) + return; + + pos = index_name_pos(istate, name, strlen(name)); if (pos >= 0) { struct cache_entry *ce = istate->cache[pos]; ce->ce_flags &= ~CE_FSMONITOR_VALID; diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index eb2d13bbcf..756beb0d8e 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -314,4 +314,43 @@ test_expect_success 'splitting the
RE: git: CVE-2018-1000021: client prints server sent ANSI escape codes to the terminal, allowing for unverified messages to potentially execute arbitrary commands
On February 7, 2018 11:53 AM, Andreas Schwab wrote: > On Feb 06 2018, "Randall S. Becker"wrote: > > > What I don't know - and it's not explicitly in the CVE - is just how > > many other terminal types with similar vulnerabilities are out there, > > but I'm suspecting it's larger than one would guess - mostly, it seems > > like this particular sequence is intended to be used for writing > > status line output (line 25?) instead of sticking it in a prompt. This > > can be used prettifies a lengthy bash prompt to display the current > > branch and repository at the bottom of the screen instead of in the > > inline prompt, but that's the user's choice and not something git has > > to deal with. There were some green-screen terminals with other weird > > ESC sequences back in the day that could really get into trouble with > > this, including loading/executing programs in terminal memory via > > output - really. I'm sure it seemed like a good idea at the time, but I can see > how it could have been used for evil. > > Do you also want to block "+++AT"? :-) Oh dear. Oh dear. You *do* know that actually could be bad. I wonder how many git users are still using dial-up to clone/push. Of course, they would probably not even see this message after trying to download it. Chuckles, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
Re: BUG: fetch in certain repo always gives "did not send all necessary objects"
On Wed, Feb 7, 2018 at 5:21 AM, Jeff Kingwrote: > On Tue, Feb 06, 2018 at 04:00:32PM -0800, Elijah Newren wrote: > >> It took me hours to figure it out, after users ran out of ideas and >> came and asked me for help. (Maybe if I was familiar with worktree, >> and knew they had been using it, then I might have guessed that "HEAD" >> meant "not your actual HEAD but the HEAD of the vestige of some other >> worktree"). > > Yeah, this is the obvious thing that seems like it ought to be improved. > Unfortunately fixing that is a little tricky. In this case the stack > looks like: > > parse_object_or_die (oid=0x7fffd690, name=0x55792880 "HEAD") at > object.c:239 > add_one_ref (path=0x55792880 "HEAD", oid=0x7fffd690, flag=0, > cb_data=0x7fffd8e0) at reachable.c:38 > refs_head_ref (refs=0x55a65430, fn=0x556b6ef5 , > cb_data=0x7fffd8e0) at refs.c:1316 > other_head_refs (fn=0x556b6ef5 , cb_data=0x7fffd8e0) > at worktree.c:404 > > So other_head_refs knows that it's looking at the worktrees. And it > passes the alternate ref-store to refs_head_ref(), with "add_one_ref" as > the callback. But the knowledge that we're not talking about the real > "HEAD" is lost as we cross that callback boundary. We'd need to either > add another parameter to the callback, or have some way of talking about > "HEAD in this worktree" as a refname (which AFAIK we don't have). Can we use "worktrees/${WORKTREE}/HEAD"? It already satisfies all the necessary rev-parse rules... (And on a slight tangent...do we want to start disallowing the creation of branches/tags whose name starts with "worktrees/", "refs/", "hooks/", or other paths that exists under gitdir? Making a branch named "refs/heads/foo" so that it fully-qualifies as "refs/heads/refs/heads/foo" is always fun)
Re: categorization, documentation and packaging of "git core" commands
Robert P. J. Day wrote: > first, here are the executables under /usr/libexec/git-core/ that > are unreferenced by that web page, but that should be fine as almost > all of them would be considered underlying helpers or utilities > (except for things like git-subtree, but we're still unclear on its > status, right?): I don't think there's anything unclear about git subtree's status. It's in contrib/ within the source, so it's not part of the core git suite. Some distributions (Fedora being one of them) ship a git-subtree package to provide it for users who want it. > on the other hand (and this is not so much a git issue as a fedora > packaging issue), there are a number of command links at that web page > that are supplied by distinct RPM packages rather than by the basic > fedora git package, so one would need to install the following > packages to get some of those commands on fedora: > > * gitk > * git-cvs > * git-svn > * git-p4 > * git-email (provides git-send-email) These packages are in separate sub-packages in Fedora (and some other distributions) because they are no required by all users and they pull in dependencies which are not wanted on minimal installs. In Fedora, you can install git-all to get all the available git sub-packages. > finally, from fedora, i am utterly unable to find a package that > provides git-archimport. pretty sure fedora used to have a "git-arch" > package but it's not there now. It hasn't been in Fedora since 2011. The tla command which is required for git-archimport was retired, thus we removed the git-arch package. The rpm changelog shows this: * Tue Jul 26 2011 Todd Zullinger- 1.7.6-4 - Drop git-arch on fedora >= 16, the tla package has been retired As does the git history for the package: https://src.fedoraproject.org/rpms/git/c/3f0dc974fa The tla package was retired because it failed to build for several releases: https://src.fedoraproject.org/rpms/tla/blob/master/f/dead.package -- Todd ~~ Going to trial with a lawyer who considers your whole life-style a Crime in Progress is not a happy prospect. -- Hunter S. Thompson
Re: [PATCH 5/8] rebase: introduce the --recreate-merges option
Hi, On Wed, 7 Feb 2018, Sergey Organov wrote: > Johannes Schindelinwrites: > > [...] > > > +--recreate-merges:: > > + Recreate merge commits instead of flattening the history by replaying > > + merges. Merge conflict resolutions or manual amendments to merge > > + commits are not preserved. > > I wonder why you guys still hold on replaying "merge-the-operation" > instead of replaying "merge-the-result"? This misses the point of rebasing: you want to replay the changes. > The latter, the merge commit itself, no matter how exactly it was > created in the first place, is the most valuable thing git keeps about > the merge, and you silently drop it entirely! You miss another very crucial point. I don't blame you, as you certainly have not used the Git garden shears for years. Let me explain the scenario which comes up plenty of times in my work with Git for Windows. We have a thicket of some 70 branches on top of git.git's latest release. These branches often include fixup! and squash! commits and even more complicated constructs that rebase cannot handle at all at the moment, such as reorder-before! and reorder-after! (for commits that really need to go into a different branch). Even if you do not have such a complicated setup, it is quite possible that you need to include a commit in your development that needs to be dropped before contributing your work. Think e.g. removing the `-O2` flag when compiling with GCC because GDB gets utterly confused with executables compiled with `-O2` while single-stepping. This could be an initial commit called `TO-DROP` or some such. And guess what happens if you drop that `pick` line in your todo list and then the `merge` command simply tries to re-create the original merge commit's changes? Exactly. The merge will become an evil merge, and will introduce that very much not-wanted and therefore-dropped changes. > OTOH, git keeps almost no information about "merge-the-operation", so > it's virtually impossible to reliably replay the operation > automatically, and yet you try to. That is true. However, the intended use case is not to allow you to recreate funny merges. Its use case is to allow you to recreate merges. At a later stage, I might introduce support to detect `-s ours` merges, because they are easy to detect. But even then, it will be an opt-in. > IMHO that was severe mistake in the original --preserve-merges, and you > bring with you to this new --recreate-merges... It's sad. Please refrain from drawing this discussion into an emotional direction. That is definitely not helpful. > Even more sad as solution is already known for years: > > bc00341838a8faddcd101da9e746902994eef38a > Author: Johannes Sixt > Date: Sun Jun 16 15:50:42 2013 +0200 > > rebase -p --first-parent: redo merge by cherry-picking first-parent > change > > and it works like a charm. It might work for you, as you probably used --preserve-merges, and dealt with the fact that you could neither drop nor reorder commits. So --preserve-merges --first-parent is probably what you were looking for. Instead, --recreate-merges is all about allowing the same level of freedom as with regular interactive rebases, but recreating the original commit topology (and allowing to change it, too). Therefore, I think that it would be even harmful to allow --recreate-merges --first-parent *because it would cause evil merges*! And I totally could see myself being vexed again about options that worked perfectly well (just like --preserve-merges) being completely messed up by allowing it to be combined with options *that they cannot work with* (just like --preserve-merges --interactive, a *huge* mistake causing so many annoying "bug" reports: I *never intended it that way because I knew it would not work as users expect*). So no, I do not think that --recreate-merges --first-parent is a good idea at all. Unless you try to do that non-interactively only, *and disallow it in interactive mode*. Because the entire point of the interactive rebase is to allow reordering and dropping commits, in --recreate-merges even moving, introducing and dropping merge commits. The --first-parent option flies in the face of this idea. Ciao, Johannes
Re: is http://git-scm.com an *official* git-affiliated site?
On Wed, Feb 07, 2018 at 12:37:32PM +0100, Ævar Arnfjörð Bjarmason wrote: > > however, while there is no link for the "worktree" command, there > > does in fact exist a similarly-named web page > > https://git-scm.com/docs/git-worktree. > > It is an official site, of the git project. The project is legally > managed by the SFC which owns the domain, Github happens to host it (for > free) for us. Actually, this isn't accurate anymore. The costs are covered by donated services from a few companies. I've been meaning to write this up (and thank them!), and will probably send something out around the contributor summit. > It's not fully auto-generated so stuff like git-worktree doesn't get > added automatically, I just added a PR for that: > https://github.com/git/git-scm.com/pull/1133 Thanks for doing that. I'm also open to auto-generating the index if we can come up with well-organized output. -Peff
Re: BUG: fetch in certain repo always gives "did not send all necessary objects"
On Wed, Feb 07, 2018 at 08:21:57AM -0500, Jeff King wrote: > The best PSA for this particular bug may be "try pruning the worktrees": > > $ git worktree prune -v > Removing worktrees/foo: gitdir file points to non-existent location > > $ git prune; echo $? > 0 Sorry, I just read Stefan's response and not your original before responding. I see that you did try that. So IMHO that's a separate bug that "worktree prune" did not do the right thing for your particular case. -Peff
Re: BUG: fetch in certain repo always gives "did not send all necessary objects"
On Tue, Feb 06, 2018 at 04:00:32PM -0800, Elijah Newren wrote: > > According to Peff this got fixed > > https://public-inbox.org/git/20171020031630.44zvzh3d2vlhg...@sigill.intra.peff.net/ > > and but you've had a corrupted repo from back when you were using an older > > version of Git. > > > > Did that repo exist before d0c39a49cc was rolled out? Then we can keep that > > hypothesis of "left-over corruption" as Peff put it. > > I'm somewhat confused by this explanation. That precise commit is the > one I bisected to that _caused_ the fetch to fail. Also, there might > be one important difference here -- in the link you provide, it > suggests that you had a corrupted working directory that made use of a > now gc'ed commit. In the case I was able to dig into, we did not. > (There was a left-over .git/worktree/ that had a now gc'ed > commit, but no working directory that used it.) If you had a corrupted .git/worktree//HEAD, then that does sound like the same problem. It's true that the commit you bisected to caused "fetch" to fail, but only because it started looking at more of your corrupted repository. The corruption happened long before (and I don't know exactly when it was fixed, but I couldn't replicate it anymore; it might even still exist). In your case it sounds like you have the extra twist that the matching working directory for "" had gone away, but I don't think that materially changes anything. Until you run "git worktree prune", that HEAD file is still there and still supposed to be valid. > I suspect you mean that there was another previous bug that induced > corruption, that this commit fixed that other bug, while also > introducing this new bug that makes folks' clones unusable because the > error doesn't provide enough information for users to know how to fix. If you want to call that last thing a bug, then I guess so. It's perhaps a matter for the philosophers whether it is the fault of the new code to start complaining about an existing on-disk corruption. > It took me hours to figure it out, after users ran out of ideas and > came and asked me for help. (Maybe if I was familiar with worktree, > and knew they had been using it, then I might have guessed that "HEAD" > meant "not your actual HEAD but the HEAD of the vestige of some other > worktree"). Yeah, this is the obvious thing that seems like it ought to be improved. > Does anyone have pointers about what might be doable in terms of > providing a more useful error message to allow users to recover? > And/or ideas of what steps could cause corruption so I can send out a > PSA to help users avoid it? Here's a minimal manual reproduction: # new repo... git init git commit --allow-empty -m one # with a worktree... git worktree add foo git -C foo commit --allow-empty -m two obj=.git/objects/$(git rev-parse foo | sed 's#..#&/#') # now we stop using that worktree git -C foo checkout --detach git branch -f -D foo rm -rf foo # and this is the corruption; this might have happened ye olden days # because of a bug in the worktree code, but we'll assume that somehow # the object went away rm -f $obj And now lots of commands may fail with confusing errors: $ git prune fatal: unable to parse object: HEAD Unfortunately fixing that is a little tricky. In this case the stack looks like: parse_object_or_die (oid=0x7fffd690, name=0x55792880 "HEAD") at object.c:239 add_one_ref (path=0x55792880 "HEAD", oid=0x7fffd690, flag=0, cb_data=0x7fffd8e0) at reachable.c:38 refs_head_ref (refs=0x55a65430, fn=0x556b6ef5 , cb_data=0x7fffd8e0) at refs.c:1316 other_head_refs (fn=0x556b6ef5 , cb_data=0x7fffd8e0) at worktree.c:404 So other_head_refs knows that it's looking at the worktrees. And it passes the alternate ref-store to refs_head_ref(), with "add_one_ref" as the callback. But the knowledge that we're not talking about the real "HEAD" is lost as we cross that callback boundary. We'd need to either add another parameter to the callback, or have some way of talking about "HEAD in this worktree" as a refname (which AFAIK we don't have). As for PSAs, my normal go-to in confusing matters like this is git-fsck. But it seems that it does not check worktree HEADs. :( $ git fsck Checking object directories: 100% (256/256), done. So that seems like another bug. The best PSA for this particular bug may be "try pruning the worktrees": $ git worktree prune -v Removing worktrees/foo: gitdir file points to non-existent location $ git prune; echo $? 0 -Peff
Re: is http://git-scm.com an *official* git-affiliated site?
On Wed, Feb 07, 2018 at 11:34:51AM +, pedro rijo wrote: > The command list under https://git-scm.com/docs doesn't show all the > commands. It's a manually curated list as we can see at > > - https://github.com/git/git-scm.com/blob/master/app/views/doc/ref.html.erb > - https://github.com/git/git-scm.com/tree/master/app/views/shared/ref > > I'm not sure if the goal ever was to list all available commands or to just > list some important existing commands (cc @Peff). > > If we want to list all available commands, there's some work that must be > done in order to automate that, since it's not feasible to manually add > each new command. I think we _could_ just add all commands, but there's some value in organizing them. I'm not sure if there's enough information in git.git to do that organization. But we could also have a curated list of some subset of the commands, and then dump the rest in an alphabetized index. -Peff