* Re: [PATCH v2 1/2] t: add a test helper to truncate files
From: Eric Sunshine @ 2023-10-12 17:49 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Junio C Hamano, Taylor Blau, Jason Hatton
In-Reply-To: <20231012160930.330618-2-sandals@crustytoothpaste.net>
On Thu, Oct 12, 2023 at 12:10 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> In a future commit, we're going to work with some large files which will
> be at least 4 GiB in size. To take advantage of the sparseness
> functionality on most Unix systems and avoid running the system out of
> disk, it would be convenient to use truncate(2) to simply create a
> sparse file of sufficient size.
>
> However, the GNU truncate(1) utility isn't portable, so let's write a
> tiny test helper that does the work for us.
>
> Signed-off-by: brian m. carlson <bk2204@github.com>
> ---
> diff --git a/t/helper/test-truncate.c b/t/helper/test-truncate.c
> @@ -0,0 +1,27 @@
> +int cmd__truncate(int argc, const char **argv)
> +{
> + char *p = NULL;
> + uintmax_t sz = 0;
> + int fd = -1;
> +
> + if (argc != 3)
> + die("expected filename and size");
> +
> + sz = strtoumax(argv[2], &p, 0);
> + if (*p)
> + die("invalid size");
Do you want to check 'errno' here, as well (probably before the '*p' check)?
Or is that being too defensive for a 'test-tool' command?
> + fd = open(argv[1], O_WRONLY | O_CREAT, 0600);
> + if (fd < 0)
> + die_errno("failed to open file %s", argv[1]);
> +
> + if (ftruncate(fd, (off_t) sz) < 0)
> + die_errno("failed to truncate file");
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH] mailmap: change primary address for Derrick Stolee
From: Junio C Hamano @ 2023-10-12 17:46 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget; +Cc: git, Derrick Stolee
In-Reply-To: <pull.1592.git.1697131834003.gitgitgadget@gmail.com>
[jc: removed @github.com address from CC]
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> -Derrick Stolee <derrickstolee@github.com> <stolee@gmail.com>
> -Derrick Stolee <derrickstolee@github.com> Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
> -Derrick Stolee <derrickstolee@github.com> <dstolee@microsoft.com>
> +Derrick Stolee <stolee@gmail.com> <derrickstolee@github.com>
> +Derrick Stolee <stolee@gmail.com> Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
> +Derrick Stolee <stolee@gmail.com> <dstolee@microsoft.com>
It has been irritating to see @github.com address bouncing. Will
apply.
Thanks.
^ permalink raw reply
* Re: Bug: Git grep -f reads the filename relative to the repository root
From: Junio C Hamano @ 2023-10-12 17:28 UTC (permalink / raw)
To: Erik Cervin Edin; +Cc: Git Mailing List
In-Reply-To: <CA+JQ7M_htKUv5=GRQEUqWmJrQmFQNfZkPjr8n12CU6x0Khr4dw@mail.gmail.com>
Erik Cervin Edin <erik@cervined.in> writes:
> In the Git repository, I ran
>
> echo tig > pattern-file &&
> echo git > xdiff/pattern-file &&
> cd xdfiff &&
> git grep -f pattern-file
>
> What did you expect to happen? (Expected behavior)
>
> Git grep -f to read the pattern-file, in the xdiff directory and
> search for lines matching `git` in the xdiff directory.
That does sound like a bug. It should use the original directory as
the base of the relative path computation, similar to the way how
OPT_FILENAME() options are handled.
Perhaps something along this line, but this is not even compile
tested yet.
----- >8 --------- >8 --------- >8 --------- >8 -----
Subject: [PATCH] grep: -f <path> is relative to $cwd
Just like OPT_FILENAME() does, "git grep -f <path>" should treat
the <path> relative to the original $cwd by paying attention to the
prefix the command is given.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/grep.c | 13 +++++++++++--
t/t7810-grep.sh | 13 +++++++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index b71222330a..fe78d4c98b 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -4,6 +4,7 @@
* Copyright (c) 2006 Junio C Hamano
*/
#include "builtin.h"
+#include "abspath.h"
#include "gettext.h"
#include "hex.h"
#include "repository.h"
@@ -812,14 +813,20 @@ static int file_callback(const struct option *opt, const char *arg, int unset)
{
struct grep_opt *grep_opt = opt->value;
int from_stdin;
+ const char *filename = arg;
FILE *patterns;
int lno = 0;
struct strbuf sb = STRBUF_INIT;
BUG_ON_OPT_NEG(unset);
- from_stdin = !strcmp(arg, "-");
- patterns = from_stdin ? stdin : fopen(arg, "r");
+ if (!*filename)
+ ; /* leave it as-is */
+ else
+ filename = prefix_filename_except_for_dash(grep_prefix, filename);
+
+ from_stdin = !strcmp(filename, "-");
+ patterns = from_stdin ? stdin : fopen(filename, "r");
if (!patterns)
die_errno(_("cannot open '%s'"), arg);
while (strbuf_getline(&sb, patterns) == 0) {
@@ -833,6 +840,8 @@ static int file_callback(const struct option *opt, const char *arg, int unset)
if (!from_stdin)
fclose(patterns);
strbuf_release(&sb);
+ if (filename != arg)
+ free((void *)filename);
return 0;
}
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 39d6d713ec..91ac66935f 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -808,6 +808,19 @@ test_expect_success 'grep -f, ignore empty lines, read patterns from stdin' '
test_cmp expected actual
'
+test_expect_success 'grep -f, use cwd relative file' '
+ test_when_finished "git rm -f sub/dir/file" &&
+ mkdir -p sub/dir &&
+ echo hit >sub/dir/file &&
+ git add sub/dir/file &&
+ echo hit >sub/dir/pattern &&
+ echo miss >pattern &&
+ (
+ cd sub/dir && git grep -f pattern file
+ ) &&
+ git -C sub/dir grep -f pattern file
+'
+
cat >expected <<EOF
y:y yy
--
--
2.42.0-345-gaab89be2eb
^ permalink raw reply related
* [PATCH] mailmap: change primary address for Derrick Stolee
From: Derrick Stolee via GitGitGadget @ 2023-10-12 17:30 UTC (permalink / raw)
To: git; +Cc: gitster, Derrick Stolee, Derrick Stolee
From: Derrick Stolee <stolee@gmail.com>
The previous primary address is no longer valid.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
mailmap: change primary address for Derrick Stolee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1592%2Fderrickstolee%2Fmailmap-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1592/derrickstolee/mailmap-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1592
.mailmap | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/.mailmap b/.mailmap
index dc31d70b8c1..82129be449f 100644
--- a/.mailmap
+++ b/.mailmap
@@ -59,9 +59,9 @@ David Reiss <dreiss@facebook.com> <dreiss@dreiss-vmware.(none)>
David S. Miller <davem@davemloft.net>
David Turner <novalis@novalis.org> <dturner@twopensource.com>
David Turner <novalis@novalis.org> <dturner@twosigma.com>
-Derrick Stolee <derrickstolee@github.com> <stolee@gmail.com>
-Derrick Stolee <derrickstolee@github.com> Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
-Derrick Stolee <derrickstolee@github.com> <dstolee@microsoft.com>
+Derrick Stolee <stolee@gmail.com> <derrickstolee@github.com>
+Derrick Stolee <stolee@gmail.com> Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
+Derrick Stolee <stolee@gmail.com> <dstolee@microsoft.com>
Deskin Miller <deskinm@umich.edu>
Đoàn Trần Công Danh <congdanhqx@gmail.com> Doan Tran Cong Danh
Dirk Süsserott <newsletter@dirk.my1.cc>
base-commit: bcb6cae2966cc407ca1afc77413b3ef11103c175
--
gitgitgadget
^ permalink raw reply related
* Re: How to combine multiple commit diffs?
From: Johannes Sixt @ 2023-10-12 16:41 UTC (permalink / raw)
To: ZheNing Hu; +Cc: Git List, Junio C Hamano
In-Reply-To: <CAOLTT8RzcENBx9NKffHReVKJAho89TCO7W2SPBX8sb2tEU84Gw@mail.gmail.com>
[For general support questions, please address only the mailing list.
It's not necessary to bother the maintainer personally. Though, I'll not
remove him from Cc, yet, as to comply with this ML's etiquette.]
Am 12.10.23 um 14:00 schrieb ZheNing Hu:
> Hi everyone,
>
> Our company wants to design a "small-batch" code review feature.
> Simply put, this "small-batch" means being able to treat multiple
> related commits within a MergeRequest as an independent "small" code
> review.
>
> Let me give you an example: We have five commits: A1, B, A2, C, A3.
> Among them, A1, A2, and A3 are multiple commits for the same feature.
> So when the user selects these commits, the page will return a
> "combine diff" that combines them together.
>
> A1 B A2 A3 C
> *--------*----*-----*-------* (branch)
> \ A1' \ A2' \ A3'
> *------------*------*------- (small branch code review)
>
> This may seem similar to cherry-picking a few commits from a pile of
> commits, but in fact, we do not expect to actually perform
> cherry-picking.
>
> Do you have any suggestions on how we can merge a few commits together
> and display the diff? The only reference we have is the non-open
> source platform, JetBrains Space CodeReview, they support selecting
> multiple commits for CodeReview. [1], .
Take a step back. Then ask: What are the consequences of the review?
What if the result is: the feature is perfect, we want it merged,
however, we cannot, because we do not want commit B. What if the result
is the opposite? You need B, but you can't merge it because the feature
is not ready, yet?
You are looking for a technical workaround for a non-optimal workflow.
If A1,A2,A3 are a feature on their own, they, and only they, should be
in their own feature branch.
So, I would say, the best solution is to reorder the commits in a better
manageable order. You do know about git rebase --interactive, don't you?
-- Hannes
^ permalink raw reply
* Re: [Outreachy]Introduction and Problem while installing Git
From: Dorcas Litunya @ 2023-10-12 17:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqmswnhkxt.fsf@gitster.g>
On Thu, Oct 12, 2023 at 09:20:14AM -0700, Junio C Hamano wrote:
> Benson Muite <benson_muite@emailplus.org> writes:
>
> > On 10/12/23 09:57, Dorcas Litunya wrote:
> >> Hello everyone,
> >> My name is Dorcas Litunya. I am excited to contribute to the git
> >> community, I am a first time contributor through the Outreachy program.
> >> I am excited to learn and grow through this project. I am currently
> >> installing Git and I have been faced with this error once I run the make
> >> command:
> >> In file included from http.c:2:
> >> git-curl-compat.h:3:10: fatal error: curl/curl.h: No such file or directory
> >> 3 | #include <curl/curl.h>
> >> | ^~~~~~~~~~~~~
> > You will need to have curl libraries and development headers.
> > https://curl.se/libcurl/
> > You maybe able to get these from a package manager, for example on Ubuntu
> > sudo apt-get install curl-dev
> > Fedora
> > sudo dnf install libcurl-devel
>
> Thanks for helping. Perhaps reading the INSTALL file at the top of
> the working tree would worth the time?
Thanks Junio. I did follow it eventually. I got stuck here since the
instructions to install some libraries was at the towards the middle and
end of the INSTALL file while the
instruction to run the make command was at the top of the file, and it
seemed a bit easy as the file had just said "it should work
normally"(without pointing out the libraries needed) was what kinda got
me lost, but I eventually found my solutions as I read through the doc. But yes,I will now try to read and search entire docs as all the good stuff might be at the bottom.
:-)
>
^ permalink raw reply
* Re: How to combine multiple commit diffs?
From: Junio C Hamano @ 2023-10-12 17:03 UTC (permalink / raw)
To: ZheNing Hu; +Cc: Git List
In-Reply-To: <CAOLTT8RzcENBx9NKffHReVKJAho89TCO7W2SPBX8sb2tEU84Gw@mail.gmail.com>
ZheNing Hu <adlternative@gmail.com> writes:
> This may seem similar to cherry-picking a few commits from a pile of
> commits, but in fact, we do not expect to actually perform
> cherry-picking.
If you said "We do not want to", I may be sympathetic, but it is
curious that you said "We do not expect to", which hints that you
already have some implementation in mind.
Whether you use a checkout of the history to a working tree and
perform a series of "git cherry-pick" to implement it, or use the
"git replay" machinery to perform them in-core, at the conceptual
level, you would need to pick a base commit and apply the effect of
those commits you would want to look at on top of that base in
sequence, before you can see the combined change, no?
Puzzled.
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Junio C Hamano @ 2023-10-12 16:58 UTC (permalink / raw)
To: Sebastian Thiel; +Cc: git, Josh Triplett, Kristoffer Haugsbakk
In-Reply-To: <0E44CB2C-57F2-4075-95BE-60FBFDD3CEE2@icloud.com>
Sebastian Thiel <sebastian.thiel@icloud.com> writes:
> ### What about a `$` syntax in `.gitignore` files?
>
> I looked into adding a new prefix, `$` to indicate the following path is
> precious or… valuable. It can be escaped with `\$` just like `\!`.
I have been regretting that I did not make the quoting syntax not
obviously extensible in f87f9497 (git-ls-files: --exclude mechanism
updates., 2005-07-24), which technically was a breaking change (as a
relative pathname that began with '!' were not special, but after
the change, it became necessary to '\'-quote it). A relative
pathname that begins with '$' would be now broken the same way, but
hopefully the fallout would be minor. I presume you picked '$'
exactly because of this reason?
I do not think it will be the end of the world if we don't do so,
but it would be really really nice if we at least explored a way (or
two) to make a big enough hole in the syntax to not just add
"precious", but leave room to later add other traits, without having
to worry about breaking the backward compatibility again. A
simplest and suboptimal way may be to declare that a path that
begins with '$' now needs '\'-quoting (just like your proposal),
reserve '$$' as the precious prefix, and '$' followed by any other
byte reserved for future use, but there may be better ideas.
> *Unfortunately*, users can't just add a local `.git/info/exclude` file with
> `$.config` in it and expect `.config` to be considered precious as the pattern
> search order will search this last as it's part of the exclude-globals.
That it nothing new and is the same for ignored files. The lower
precedence files do not override higher precedence files.
> Thus, to make this work, projects that ship the `.gitignore` files would *have
> to add patterns* that make certain files precious.
Not really. They do not have to do anything if they are content
with the current Git ecosystem. And users who have precious stuff
can mark them in the.git/info/excludes no? The only case that is
problematic is when the project says 'foo' is ignored and expendable
but the user thinks otherwise. So to make this work, projects that
ship the ".gitignore" files have to avoid adding patterns to ignore
things that it may reasonably be expected for its users to mark
precious.
> Such opted-in projects would produce `.gitignore` files like these:
>
> .*
> $.config
I would understand if you ignored "*~" or "*.o", but why ignore ".*"?
THanks.
^ permalink raw reply
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Junio C Hamano @ 2023-10-12 16:26 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Patrick Steinhardt, git
In-Reply-To: <CAOLa=ZQxNX4oGtqrgLyKenC_D8M=9q0sFJVmo4fyjSPtgw315Q@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> ... To ensure that the feature should
> work with corrupt repositories with stale commit-graph, I'm thinking of
> disabling the commit-graph whenever the `--missing` option is used.
Just to avoid giving anybody a wrong guideline, for most of the
other features, we should assume that the commit-graph accurately
represents the reality in the object database and take advantage of
it, leaving it to "git fsck" to ensure that the commit-graph is
healthy.
But "rev-list --missing", especially when used with actions other
than allow-promisor or allow-any, is a feature that is only useful
when one wants to really notice objects that are available in the
object store, and it would be a bug for it to pretend as if objects
the commit-graph has heard of exists without checking.
^ permalink raw reply
* Re: [Outreachy]Introduction and Problem while installing Git
From: Junio C Hamano @ 2023-10-12 16:20 UTC (permalink / raw)
To: Benson Muite; +Cc: Dorcas Litunya, git
In-Reply-To: <4c5fef38-a671-dd6b-4b10-a531e1ae254a@emailplus.org>
Benson Muite <benson_muite@emailplus.org> writes:
> On 10/12/23 09:57, Dorcas Litunya wrote:
>> Hello everyone,
>> My name is Dorcas Litunya. I am excited to contribute to the git
>> community, I am a first time contributor through the Outreachy program.
>> I am excited to learn and grow through this project. I am currently
>> installing Git and I have been faced with this error once I run the make
>> command:
>> In file included from http.c:2:
>> git-curl-compat.h:3:10: fatal error: curl/curl.h: No such file or directory
>> 3 | #include <curl/curl.h>
>> | ^~~~~~~~~~~~~
> You will need to have curl libraries and development headers.
> https://curl.se/libcurl/
> You maybe able to get these from a package manager, for example on Ubuntu
> sudo apt-get install curl-dev
> Fedora
> sudo dnf install libcurl-devel
Thanks for helping. Perhaps reading the INSTALL file at the top of
the working tree would worth the time?
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Junio C Hamano @ 2023-10-12 16:19 UTC (permalink / raw)
To: Dragan Simic; +Cc: Jeff King, Christoph Anton Mitterer, git
In-Reply-To: <3946c06e90604a92ad0dddf787729668@manjaro.org>
Dragan Simic <dsimic@manjaro.org> writes:
> Please note that dropping "-X" and leaving "-F" would actually
> introduce the inconsistency that I already mentioned. To reiterate,
> short outputs would then remain displayed on screen, while long
> outputs would disappear after exiting less(1).
Good point.
^ permalink raw reply
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Junio C Hamano @ 2023-10-12 16:17 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Karthik Nayak, git
In-Reply-To: <ZSfSt4tXx8sE68Bn@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> Wouldn't this have the potential to significantly regress performance
> for all those preexisting users of the `--missing` option? The commit
> graph is quite an important optimization nowadays, and especially in
> commands where we potentially walk a lot of commits (like we may do
> here) it can result in queries that are orders of magnitudes faster.
The test fails only when GIT_TEST_COMMIT_GRAPH is on, which updates
the commit-graph every time a commit is made via "git commit" or
"git merge".
I'd suggest stepping back and think a bit.
My assumption has been that the failing test emulates this scenario
that can happen in real life:
* The user creates a new commit.
* A commit graph is written (not as part of GIT_TEST_COMMIT_GRAPH
that is not realistic, but as part of "maintenance").
* The repository loses some objects due to corruption.
* Now, "--missing=print" is invoked so that the user can view what
are missing. Or "--missing=allow-primisor" to ensure that the
repository does not have missing objects other than the ones that
the promisor will give us if we asked again.
* But because the connectivity of these objects appear in the
commit graph file, we fail to notice that these objects are
missing, producing wrong results. If we disabled commit-graph
while traversal (an earlier writing of it was perfectly OK), then
"rev-list --missing" would have noticed and reported what the
user wanted to know.
In other words, the "optimization" you value is working to quickly
produce a wrong result. Is it "significantly regress"ing if we
disabled it to obtain the correct result?
My assumption also has been that there is no point in running
"rev-list --missing" if we know there is no repository corruption,
and those who run "rev-list --missing" wants to know if the objects
are really available, i.e. even if commit-graph that is out of sync
with reality says it exists, if it is not in the object store, they
would want to know that.
If you can show me that it is not the case, then I may be pursuaded
why producing a result that is out of sync with reality _quickly_,
instead of taking time to produce a result that matches reality, is
a worthy "optimization" to keep.
Thanks.
^ permalink raw reply
* [PATCH v2 0/2] Prevent re-reading 4 GiB files on every status
From: brian m. carlson @ 2023-10-12 16:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Taylor Blau, Jason Hatton
Several people have noticed that Git continually re-reads and re-hashes
files that are an exact multiple of 4 GiB every time the index is
refreshed (for example, when "git status" is called). This is slow and
expensive, especially with SHA-1 DC, and it also causes performance
problems when these files are used with Git LFS (because the same issue
occurs, just with Git LFS hashing the data instead of Git).
Jason Hatton sent a patch previously to fix this, but it lacked tests
and didn't get picked up. I've adopted their patch, making some minor
changes to the commit message and including some tests, and also
including a suitable test helper to make the tests possible. All credit
should be directed to Jason, and I'll accept all the responsibility for
any problems.
I don't anticipate this being in any way controversial, so I'm not
expecting a huge number of rerolls, but of course one or two might be
necessary.
Jason Hatton (1):
Prevent git from rehashing 4GiB files
brian m. carlson (1):
t: add a test helper to truncate files
Makefile | 1 +
statinfo.c | 20 ++++++++++++++++++--
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/helper/test-truncate.c | 27 +++++++++++++++++++++++++++
t/t7508-status.sh | 16 ++++++++++++++++
6 files changed, 64 insertions(+), 2 deletions(-)
create mode 100644 t/helper/test-truncate.c
^ permalink raw reply
* [PATCH v2 2/2] Prevent git from rehashing 4GiB files
From: brian m. carlson @ 2023-10-12 16:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Taylor Blau, Jason Hatton
In-Reply-To: <20231012160930.330618-1-sandals@crustytoothpaste.net>
From: Jason Hatton <jhatton@globalfinishing.com>
The index stores file sizes using a uint32_t. This causes any file
that is a multiple of 2^32 to have a cached file size of zero.
Zero is a special value used by racily clean. This causes git to
rehash every file that is a multiple of 2^32 every time git status
or git commit is run.
This patch mitigates the problem by making all files that are a
multiple of 2^32 appear to have a size of 1<<31 instead of zero.
The value of 1<<31 is chosen to keep it as far away from zero
as possible to help prevent things getting mixed up with unpatched
versions of git.
An example would be to have a 2^32 sized file in the index of
patched git. Patched git would save the file as 2^31 in the cache.
An unpatched git would very much see the file has changed in size
and force it to rehash the file, which is safe. The file would
have to grow or shrink by exactly 2^31 and retain all of its
ctime, mtime, and other attributes for old git to not notice
the change.
This patch does not change the behavior of any file that is not
an exact multiple of 2^32.
Signed-off-by: Jason D. Hatton <jhatton@globalfinishing.com>
Signed-off-by: brian m. carlson <bk2204@github.com>
---
statinfo.c | 20 ++++++++++++++++++--
t/t7508-status.sh | 16 ++++++++++++++++
2 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/statinfo.c b/statinfo.c
index 17bb8966c3..9367ca099c 100644
--- a/statinfo.c
+++ b/statinfo.c
@@ -2,6 +2,22 @@
#include "environment.h"
#include "statinfo.h"
+/*
+ * Munge st_size into an unsigned int.
+ */
+static unsigned int munge_st_size(off_t st_size) {
+ unsigned int sd_size = st_size;
+
+ /*
+ * If the file is an exact multiple of 4 GiB, modify the value so it
+ * doesn't get marked as racily clean (zero).
+ */
+ if (!sd_size && st_size)
+ return 0x80000000;
+ else
+ return sd_size;
+}
+
void fill_stat_data(struct stat_data *sd, struct stat *st)
{
sd->sd_ctime.sec = (unsigned int)st->st_ctime;
@@ -12,7 +28,7 @@ void fill_stat_data(struct stat_data *sd, struct stat *st)
sd->sd_ino = st->st_ino;
sd->sd_uid = st->st_uid;
sd->sd_gid = st->st_gid;
- sd->sd_size = st->st_size;
+ sd->sd_size = munge_st_size(st->st_size);
}
int match_stat_data(const struct stat_data *sd, struct stat *st)
@@ -51,7 +67,7 @@ int match_stat_data(const struct stat_data *sd, struct stat *st)
changed |= INODE_CHANGED;
#endif
- if (sd->sd_size != (unsigned int) st->st_size)
+ if (sd->sd_size != munge_st_size(st->st_size))
changed |= DATA_CHANGED;
return changed;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 6928fd89f5..6c46648e11 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1745,4 +1745,20 @@ test_expect_success 'slow status advice when core.untrackedCache true, and fsmon
)
'
+test_expect_success EXPENSIVE 'status does not re-read unchanged 4 or 8 GiB file' '
+ (
+ mkdir large-file &&
+ cd large-file &&
+ # Files are 2 GiB, 4 GiB, and 8 GiB sparse files.
+ test-tool truncate file-a 0x080000000 &&
+ test-tool truncate file-b 0x100000000 &&
+ test-tool truncate file-c 0x200000000 &&
+ # This will be slow.
+ git add file-a file-b file-c &&
+ git commit -m "add large files" &&
+ git diff-index HEAD file-a file-b file-c >actual &&
+ test_must_be_empty actual
+ )
+'
+
test_done
^ permalink raw reply related
* [PATCH v2 1/2] t: add a test helper to truncate files
From: brian m. carlson @ 2023-10-12 16:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Taylor Blau, Jason Hatton
In-Reply-To: <20231012160930.330618-1-sandals@crustytoothpaste.net>
From: "brian m. carlson" <bk2204@github.com>
In a future commit, we're going to work with some large files which will
be at least 4 GiB in size. To take advantage of the sparseness
functionality on most Unix systems and avoid running the system out of
disk, it would be convenient to use truncate(2) to simply create a
sparse file of sufficient size.
However, the GNU truncate(1) utility isn't portable, so let's write a
tiny test helper that does the work for us.
Signed-off-by: brian m. carlson <bk2204@github.com>
---
Makefile | 1 +
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/helper/test-truncate.c | 27 +++++++++++++++++++++++++++
4 files changed, 30 insertions(+)
create mode 100644 t/helper/test-truncate.c
diff --git a/Makefile b/Makefile
index 9c6a2f125f..03adcb5a48 100644
--- a/Makefile
+++ b/Makefile
@@ -852,6 +852,7 @@ TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o
TEST_BUILTINS_OBJS += test-submodule.o
TEST_BUILTINS_OBJS += test-subprocess.o
TEST_BUILTINS_OBJS += test-trace2.o
+TEST_BUILTINS_OBJS += test-truncate.o
TEST_BUILTINS_OBJS += test-urlmatch-normalization.o
TEST_BUILTINS_OBJS += test-userdiff.o
TEST_BUILTINS_OBJS += test-wildmatch.o
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 9010ac6de7..876cd2dc31 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -86,6 +86,7 @@ static struct test_cmd cmds[] = {
{ "submodule-nested-repo-config", cmd__submodule_nested_repo_config },
{ "subprocess", cmd__subprocess },
{ "trace2", cmd__trace2 },
+ { "truncate", cmd__truncate },
{ "userdiff", cmd__userdiff },
{ "urlmatch-normalization", cmd__urlmatch_normalization },
{ "xml-encode", cmd__xml_encode },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index f134f96b97..70dd4eba11 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -79,6 +79,7 @@ int cmd__submodule_config(int argc, const char **argv);
int cmd__submodule_nested_repo_config(int argc, const char **argv);
int cmd__subprocess(int argc, const char **argv);
int cmd__trace2(int argc, const char **argv);
+int cmd__truncate(int argc, const char **argv);
int cmd__userdiff(int argc, const char **argv);
int cmd__urlmatch_normalization(int argc, const char **argv);
int cmd__xml_encode(int argc, const char **argv);
diff --git a/t/helper/test-truncate.c b/t/helper/test-truncate.c
new file mode 100644
index 0000000000..bd3fde364b
--- /dev/null
+++ b/t/helper/test-truncate.c
@@ -0,0 +1,27 @@
+#include "test-tool.h"
+#include "git-compat-util.h"
+
+/*
+ * Truncate a file to the given size.
+ */
+int cmd__truncate(int argc, const char **argv)
+{
+ char *p = NULL;
+ uintmax_t sz = 0;
+ int fd = -1;
+
+ if (argc != 3)
+ die("expected filename and size");
+
+ sz = strtoumax(argv[2], &p, 0);
+ if (*p)
+ die("invalid size");
+
+ fd = open(argv[1], O_WRONLY | O_CREAT, 0600);
+ if (fd < 0)
+ die_errno("failed to open file %s", argv[1]);
+
+ if (ftruncate(fd, (off_t) sz) < 0)
+ die_errno("failed to truncate file");
+ return 0;
+}
^ permalink raw reply related
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Karthik Nayak @ 2023-10-12 13:23 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Junio C Hamano, git
In-Reply-To: <ZSfSt4tXx8sE68Bn@tanuki>
On Thu, Oct 12, 2023 at 1:04 PM Patrick Steinhardt <ps@pks.im> wrote:
> Wouldn't this have the potential to significantly regress performance
> for all those preexisting users of the `--missing` option? The commit
> graph is quite an important optimization nowadays, and especially in
> commands where we potentially walk a lot of commits (like we may do
> here) it can result in queries that are orders of magnitudes faster.
>
> If we were introducing a new option then I think this would be an
> acceptable tradeoff as we could still fix the performance issue in
> another iteration. But we don't and instead aim to change the meaning of
> `--missing` which may already have users out there. Seen in that light
> it does not seem sensible to me to just disable the graph for them.
>
Agreed, that there is a rather huge performance drop doing this. So either:
1. Add commit support for `--missing` and disable commit-graph. There is huge
performance drop here for commits.
2. Add commit support for `--missing` but disabling commit-graph is provided via
an additional `--disable-commit-graph` option. This expects the user to manually
disable commit-graph for `--missing` to work correctly with commits.
I was also thinking about this, but people who are using `--missing`
till date, also use
it with `--objects`. Using `--objects` is already slow and doesn't
benefit from the
commit-graph. So I don't know if there is much of a performance hit.
Some benchmarks on the Git repo:
With commit graphs:
$ hyperfine --warmup 2 "git rev-list --objects --missing=print HEAD > /dev/null"
Benchmark 1: git rev-list --objects --missing=print HEAD > /dev/null
Time (mean ± σ): 1.336 s ± 0.013 s [User: 1.278 s, System: 0.054 s]
Range (min … max): 1.323 s … 1.365 s 10 runs
$ hyperfine --warmup 2 "git rev-list --missing=print HEAD > /dev/null"
Benchmark 1: git rev-list --missing=print HEAD > /dev/null
Time (mean ± σ): 29.6 ms ± 1.5 ms [User: 20.2 ms, System: 9.1 ms]
Range (min … max): 27.5 ms … 35.1 ms 92 runs
Without commit graphs:
$ hyperfine --warmup 2 "git rev-list --objects --missing=print HEAD > /dev/null"
Benchmark 1: git rev-list --objects --missing=print HEAD > /dev/null
Time (mean ± σ): 1.735 s ± 0.022 s [User: 1.672 s, System: 0.057 s]
Range (min … max): 1.703 s … 1.765 s 10 runs
$ hyperfine --warmup 2 "git rev-list --missing=print HEAD > /dev/null"
Benchmark 1: git rev-list --missing=print HEAD > /dev/null
Time (mean ± σ): 426.6 ms ± 10.2 ms [User: 410.1 ms, System: 15.1 ms]
Range (min … max): 414.9 ms … 441.3 ms 10 runs
This shows that while there is definitely a performance loss when
disabling commit-graphs.
This loss is much lesser when used alongside the `--objects` flag.
Overall, I lean towards the option "1", but okay with either.
> Did you figure out what exactly the root cause of this functional
> regression is? If so, can we address that root cause instead?
Are you referring to `--missing` not working with commits? Looking at
the history
of `--missing` shows:
- caf3827e2f: This commits introduces the `--missing` option, it
basically states that
for the upcoming partial clone mechanism, we want to add support for identifying
missing objects.
- 7c0fe330d5: This commit adds support for trees in `--missing`. It
also goes on to
state that `--missing` assumed only blobs could be missing.
Both of these show that `--missing` started off support for only blobs
and eventually
added support to trees (similar to how we're adding support for
commits here). So
I guess the intention was to never work with commits. But the documentation and
the option seem generic enough. Considering that trees was an extension, commits
should be treated the same way.
^ permalink raw reply
* Re: [PATCH] send-email: add --compose-cover option
From: Ben Dooks @ 2023-10-12 13:07 UTC (permalink / raw)
To: git, gitster
In-Reply-To: <20231012112743.2756259-1-ben.dooks@codethink.co.uk>
On 12/10/2023 12:27, Ben Dooks wrote:
> Adding an option to automatically compose a cover letter would be
> helpful to put the whole process of sending an series with a cover
> into one command.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> Documentation/git-send-email.txt | 5 +++++
> git-send-email.perl | 11 ++++++++++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 41cd8cb424..f299732867 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -78,6 +78,11 @@ Missing From or In-Reply-To headers will be prompted for.
> +
> See the CONFIGURATION section for `sendemail.multiEdit`.
>
> +--compose-cover::
> + Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
> + to edit a cover letter generated by passing --cover-letter to
> + git-send-email before invoking the editor.
> +
OOPS, clearly meant git-format-patch here, not git-send-email.
> --from=<address>::
> Specify the sender of the emails. If not specified on the command line,
> the value of the `sendemail.from` configuration option is used. If
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5861e99a6e..debec088f6 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -54,6 +54,7 @@ sub usage {
> --in-reply-to <str> * Email "In-Reply-To:"
> --[no-]xmailer * Add "X-Mailer:" header (default).
> --[no-]annotate * Review each patch that will be sent in an editor.
> + --compose-cover * Open an editor on format-patch --cover-letter
> --compose * Open an editor for introduction.
> --compose-encoding <str> * Encoding to assume for introduction.
> --8bit-encoding <str> * Encoding to assume 8bit mails if undeclared
> @@ -199,7 +200,7 @@ sub format_2822_time {
> # Variables we fill in automatically, or via prompting:
> my (@to,@cc,@xh,$envelope_sender,
> $initial_in_reply_to,$reply_to,$initial_subject,@files,
> - $author,$sender,$smtp_authpass,$annotate,$compose,$time);
> + $author,$sender,$smtp_authpass,$annotate,$compose_cover,$compose,$time);
> # Things we either get from config, *or* are overridden on the
> # command-line.
> my ($no_cc, $no_to, $no_bcc, $no_identity);
> @@ -512,6 +513,7 @@ sub config_regexp {
> "no-smtp-auth" => sub {$smtp_auth = 'none'},
> "annotate!" => \$annotate,
> "no-annotate" => sub {$annotate = 0},
> + "compose-cover" => \$compose_cover,
> "compose" => \$compose,
> "quiet" => \$quiet,
> "cc-cmd=s" => \$cc_cmd,
> @@ -782,6 +784,9 @@ sub is_format_patch_arg {
> die __("Cannot run git format-patch from outside a repository\n")
> unless $repo;
> require File::Temp;
> + if ($compose_cover) {
> + push @rev_list_opts, "--cover-letter";
> + }
> push @files, $repo->command('format-patch', '-o', File::Temp::tempdir(CLEANUP => 1), @rev_list_opts);
> }
>
> @@ -854,6 +859,8 @@ sub get_patch_subject {
>
> if ($annotate) {
> do_edit($compose_filename, @files);
> + } elsif ($compose_cover) {
> + do_edit($files[0]);
> } else {
> do_edit($compose_filename);
> }
> @@ -927,6 +934,8 @@ sub get_patch_subject {
>
> } elsif ($annotate) {
> do_edit(@files);
> +} elsif ($compose_cover) {
> + do_edit($files[0]);
> }
>
> sub term {
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ permalink raw reply
* Bug: Git grep -f reads the filename relative to the repository root
From: Erik Cervin Edin @ 2023-10-12 12:38 UTC (permalink / raw)
To: Git Mailing List
Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.
What did you do before the bug happened? (Steps to reproduce your issue)
In the Git repository, I ran
echo tig > pattern-file &&
echo git > xdiff/pattern-file &&
cd xdfiff &&
git grep -f pattern-file
What did you expect to happen? (Expected behavior)
Git grep -f to read the pattern-file, in the xdiff directory and
search for lines matching `git` in the xdiff directory.
What happened instead? (Actual behavior)
Git grep -f reads the filename, relative to the Git root and searches
for lines matching `tig` in the xdiff directory.
What's different between what you expected and what actually happened?
The file that Git grep uses for patterns is read relative to the root
of the Git repository, and not the current directory.
Anything else you want to add:
Please review the rest of the bug report below.
You can delete any lines you don't wish to share.
[System Info]
git version:
git version 2.42.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27
02:56:13 UTC 2023 x86_64
compiler info: gnuc: 11.4
libc info: glibc: 2.35
$SHELL (typically, interactive shell): /bin/bash
^ permalink raw reply
* How to combine multiple commit diffs?
From: ZheNing Hu @ 2023-10-12 12:00 UTC (permalink / raw)
To: Git List, Junio C Hamano
Hi everyone,
Our company wants to design a "small-batch" code review feature.
Simply put, this "small-batch" means being able to treat multiple
related commits within a MergeRequest as an independent "small" code
review.
Let me give you an example: We have five commits: A1, B, A2, C, A3.
Among them, A1, A2, and A3 are multiple commits for the same feature.
So when the user selects these commits, the page will return a
"combine diff" that combines them together.
A1 B A2 A3 C
*--------*----*-----*-------* (branch)
\ A1' \ A2' \ A3'
*------------*------*------- (small branch code review)
This may seem similar to cherry-picking a few commits from a pile of
commits, but in fact, we do not expect to actually perform
cherry-picking.
Do you have any suggestions on how we can merge a few commits together
and display the diff? The only reference we have is the non-open
source platform, JetBrains Space CodeReview, they support selecting
multiple commits for CodeReview. [1], .
[1]: https://www.jetbrains.com/help/space/review-code.html#code-review-example
Thanks,
--
ZheNing Hu
^ permalink raw reply
* [PATCH] send-email: add --compose-cover option
From: Ben Dooks @ 2023-10-12 11:27 UTC (permalink / raw)
To: git, gitster; +Cc: Ben Dooks
Adding an option to automatically compose a cover letter would be
helpful to put the whole process of sending an series with a cover
into one command.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
Documentation/git-send-email.txt | 5 +++++
git-send-email.perl | 11 ++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 41cd8cb424..f299732867 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -78,6 +78,11 @@ Missing From or In-Reply-To headers will be prompted for.
+
See the CONFIGURATION section for `sendemail.multiEdit`.
+--compose-cover::
+ Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
+ to edit a cover letter generated by passing --cover-letter to
+ git-send-email before invoking the editor.
+
--from=<address>::
Specify the sender of the emails. If not specified on the command line,
the value of the `sendemail.from` configuration option is used. If
diff --git a/git-send-email.perl b/git-send-email.perl
index 5861e99a6e..debec088f6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -54,6 +54,7 @@ sub usage {
--in-reply-to <str> * Email "In-Reply-To:"
--[no-]xmailer * Add "X-Mailer:" header (default).
--[no-]annotate * Review each patch that will be sent in an editor.
+ --compose-cover * Open an editor on format-patch --cover-letter
--compose * Open an editor for introduction.
--compose-encoding <str> * Encoding to assume for introduction.
--8bit-encoding <str> * Encoding to assume 8bit mails if undeclared
@@ -199,7 +200,7 @@ sub format_2822_time {
# Variables we fill in automatically, or via prompting:
my (@to,@cc,@xh,$envelope_sender,
$initial_in_reply_to,$reply_to,$initial_subject,@files,
- $author,$sender,$smtp_authpass,$annotate,$compose,$time);
+ $author,$sender,$smtp_authpass,$annotate,$compose_cover,$compose,$time);
# Things we either get from config, *or* are overridden on the
# command-line.
my ($no_cc, $no_to, $no_bcc, $no_identity);
@@ -512,6 +513,7 @@ sub config_regexp {
"no-smtp-auth" => sub {$smtp_auth = 'none'},
"annotate!" => \$annotate,
"no-annotate" => sub {$annotate = 0},
+ "compose-cover" => \$compose_cover,
"compose" => \$compose,
"quiet" => \$quiet,
"cc-cmd=s" => \$cc_cmd,
@@ -782,6 +784,9 @@ sub is_format_patch_arg {
die __("Cannot run git format-patch from outside a repository\n")
unless $repo;
require File::Temp;
+ if ($compose_cover) {
+ push @rev_list_opts, "--cover-letter";
+ }
push @files, $repo->command('format-patch', '-o', File::Temp::tempdir(CLEANUP => 1), @rev_list_opts);
}
@@ -854,6 +859,8 @@ sub get_patch_subject {
if ($annotate) {
do_edit($compose_filename, @files);
+ } elsif ($compose_cover) {
+ do_edit($files[0]);
} else {
do_edit($compose_filename);
}
@@ -927,6 +934,8 @@ sub get_patch_subject {
} elsif ($annotate) {
do_edit(@files);
+} elsif ($compose_cover) {
+ do_edit($files[0]);
}
sub term {
--
2.42.0
^ permalink raw reply related
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Patrick Steinhardt @ 2023-10-12 11:04 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Junio C Hamano, git
In-Reply-To: <CAOLa=ZQxNX4oGtqrgLyKenC_D8M=9q0sFJVmo4fyjSPtgw315Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2669 bytes --]
On Thu, Oct 12, 2023 at 12:44:27PM +0200, Karthik Nayak wrote:
> On Wed, Oct 11, 2023 at 6:54 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Karthik Nayak <karthik.188@gmail.com> writes:
> >
> > > Seems like this is because of commit-graph being enabled, I think
> > > the best thing to do here would be to disable the commit graph of
> > > these tests.
> >
> > If the CI uncovered that the new code is broken and does not work
> > with commit-graph, wouldn't the above a totally wrong approach to
> > correct it? If the updated logic cannot work correctly when
> > commit-graph covers the history you intentionally break, shouldn't
> > the code, when the feature that is incompatible with commit-graph is
> > triggered, disable the commit-graph? I am assuming that the new
> > feature is meant to be used to recover from a corrupt repository,
> > and if it does not work well when commit-graph knows (now stale
> > after repository corruption) more about the objects that are corrupt
> > in the object store, we do want to disable commit-graph. After all,
> > commit-graph is a secondary information that is supposed to be
> > recoverable from the primary data that is what is in the object
> > store. Disabling commit-graph in the test means you are telling the
> > end-users "do not use commit-graph if you want to use this feature",
> > which sounds like a wrong thing to do.
>
> I agree with what you're saying. Disabling writing the commit-graph for
> only the test doesn't serve the real usage. To ensure that the feature should
> work with corrupt repositories with stale commit-graph, I'm thinking of
> disabling the commit-graph whenever the `--missing` option is used. The
> commit graph already provides an API for this, so it should be simple to do.
>
> Thanks!
Wouldn't this have the potential to significantly regress performance
for all those preexisting users of the `--missing` option? The commit
graph is quite an important optimization nowadays, and especially in
commands where we potentially walk a lot of commits (like we may do
here) it can result in queries that are orders of magnitudes faster.
If we were introducing a new option then I think this would be an
acceptable tradeoff as we could still fix the performance issue in
another iteration. But we don't and instead aim to change the meaning of
`--missing` which may already have users out there. Seen in that light
it does not seem sensible to me to just disable the graph for them.
Did you figure out what exactly the root cause of this functional
regression is? If so, can we address that root cause instead?
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Sebastian Thiel @ 2023-10-12 10:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Josh Triplett, Kristoffer Haugsbakk
In-Reply-To: <xmqqttqytnqb.fsf@gitster.g>
I liked the idea too see precious files as sub-class of ignored files, and
investigated possibilities on how to achieve that while keeping the overall
effort low and remove any potential for backwards-incompatibility as well.
Currently, `.gitignore` files only contain one pattern per line, which
optionally may be prefixed with `!` to negate it. This can be escaped with `\!`
- and that's it.
Parsing patterns that way makes for simple parsing without a need for
quoting.
### What about a `$` syntax in `.gitignore` files?
I looked into adding a new prefix, `$` to indicate the following path is
precious or… valuable. It can be escaped with `\$` just like `\!`.
Doing so has the advantage that older `git` versions simply take the
declaration as literal and would now exclude `$.config`, for example, whereas
newer `git` versions will consider them precious.
There is some potential for accidentally excluding files that previously
were untracked with older versions of git, but I'd think chances are low.
#### Example: Linux kernel
`.config` is ignored via `.gitignore:
.*
*Unfortunately*, users can't just add a local `.git/info/exclude` file with
`$.config` in it and expect `.config` to be considered precious as the pattern
search order will search this last as it's part of the exclude-globals. The
same is true for per-user git-ignore files. This means that any git would
have the `.*` pattern match before the `$.config` pattern, and stop right there
concluding that it's expendable, instead of precious. This is how users can
expect `.gitignore` files to work, and this is how `!negations` work as well -
the negation has to come after the actual exclusion to be effective.
Thus, to make this work, projects that ship the `.gitignore` files would *have
to add patterns* that make certain files precious.
Alternatively, users have to specify gitignore-overrides on the command-line,
but not all commands (if any except for `git check-ignore`) support that.
In the case of `git clean` one can already pass `--exclude=pattern`, but if
that's needed one doesn't need syntax for precious files in the first place.
**This makes the $precious syntax useful only for projects who chose to opt in,
but makes overrides for users near impossible**.
Such opted-in projects would produce `.gitignore` files like these:
.*
$.config
Note that due to the way ignore patterns are searched, the following would
consider `.config` trackable, not precious:
.*
$.config
!.config
It's up the maintainer of the repository to configure their .gitignore files
correctly, so nothing new either.
#### Benefits
* simple implementation, fast
* backwards compatible
#### Disadvantages
* cannot easily be overridden by the user as part of their local settings.
* needs repository-buy-in to be useful
* $file could clash with the file '$file' and cause older git to ignore it
### What about a `precious` attribute?
The search of `.gitattributes` works differently which makes it possible for
users to set attributes on any file or folder easily using their local files.
Using attributes has the added benefit of being extensible as one can start out
with:
```gitattributes
.config precious
```
and optionally continue with…
```gitattributes
.config precious=input
kernel precious=output
```
…to further classify kinds of precious files, probably for their personal use.
Please note that currently pathspecs can't be used to filter by attribute
for files that are igonred and untracked or I couldn't figure out how.
That even makes sense as it wasn't considered a use-case yet.
#### Benefits
* backwards compatible
* easily extendable with 'tags' or sub-classes of precious files using the
assignment syntax.
* overridable with user's local files
#### Disadvantages
* any 'exclude' query now also needs a .gitattribute query to support precious
files (and it's not easy to optimize unless there is a flag to turn precious
file support on or off)
* `precious` might be in use by some repos which now gains a possibly different
meaning in `git` as well.
### Conclusion
Weighing advantages and disadvantages of both approaches makes me prefer the
`.gitignore` extension. The `.gitattributes` version of it *could* also be
implemented on top of it at a later date. However, it should be gated behind a
configuration flag so users who need it as they want local overrides
can opt-in. Then they also pay for the feature which for most repositories
won't be an issue in the first place.
All this seems a bit too good to be true, and I hope you can show where
it wouldn't work or which dangers or possible issues haven't been
mentioned yet.
^ permalink raw reply
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Karthik Nayak @ 2023-10-12 10:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Patrick Steinhardt, git
In-Reply-To: <xmqqbkd5nlq0.fsf@gitster.g>
On Wed, Oct 11, 2023 at 6:54 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> > Seems like this is because of commit-graph being enabled, I think
> > the best thing to do here would be to disable the commit graph of
> > these tests.
>
> If the CI uncovered that the new code is broken and does not work
> with commit-graph, wouldn't the above a totally wrong approach to
> correct it? If the updated logic cannot work correctly when
> commit-graph covers the history you intentionally break, shouldn't
> the code, when the feature that is incompatible with commit-graph is
> triggered, disable the commit-graph? I am assuming that the new
> feature is meant to be used to recover from a corrupt repository,
> and if it does not work well when commit-graph knows (now stale
> after repository corruption) more about the objects that are corrupt
> in the object store, we do want to disable commit-graph. After all,
> commit-graph is a secondary information that is supposed to be
> recoverable from the primary data that is what is in the object
> store. Disabling commit-graph in the test means you are telling the
> end-users "do not use commit-graph if you want to use this feature",
> which sounds like a wrong thing to do.
I agree with what you're saying. Disabling writing the commit-graph for
only the test doesn't serve the real usage. To ensure that the feature should
work with corrupt repositories with stale commit-graph, I'm thinking of
disabling the commit-graph whenever the `--missing` option is used. The
commit graph already provides an API for this, so it should be simple to do.
Thanks!
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Josh Triplett @ 2023-10-12 9:04 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: Sebastian Thiel, git
In-Reply-To: <25b25127-aa10-4179-bc02-065fe12d01ef@app.fastmail.com>
On Tue, Oct 10, 2023 at 09:10:34PM +0200, Kristoffer Haugsbakk wrote:
> Hi Josh
>
> On Tue, Oct 10, 2023, at 16:10, Josh Triplett wrote:
> > > [snip]
> >
> > While I'd love for it to default to that and require an extra option to
> > clean away precious files, I'd expect that that would break people's
> > workflows and finger memory. If someone expects `git clean -x -d -f` to
> > clean away everything, including `.config`, and then it leaves some
> > files in place, that seems likely to cause problems. (Leaving aside that
> > it might break scripted workflows.)
> >
> > It seems safer to keep the existing behavior for existing options, and
> > add a new option for "remove everything except precious files".
>
> What's a scenario where it breaks? I'm guessing:
>
> 1. Someone clones a project
> 2. That project has precious files marked via `.gitattributes`
> 3. They later do a `clean`
> 4. The precious files are left alone even though they expected them to be
> deleted; they don't check what `clean` did (it deletes everything
> untracked (they expect) so nothing to check)
> 5. This hurts them somehow
The scenario I had in mind was:
- Project has ignored files; git doesn't have a concept of "precious"
- Users expect that `git clean -x -d -f` deletes everything that isn't
part of the latest commit.
- Git introduces the concept of "precious"
- Project adopts "precious" and marks some of its ignored files as
"precious" instead
- Users' finger-macros around `git clean` stop cleaning up files they
expected to be cleaned.
That said, given Junio's response I'm no longer concerned about this
scenario.
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Josh Triplett @ 2023-10-12 8:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, Sebastian Thiel, git
In-Reply-To: <xmqqo7h6tnib.fsf@gitster.g>
On Tue, Oct 10, 2023 at 10:07:08AM -0700, Junio C Hamano wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
>
> > While I'd love for it to default to that and require an extra option to
> > clean away precious files, I'd expect that that would break people's
> > workflows and finger memory. If someone expects `git clean -x -d -f` to
> > clean away everything, including `.config`, and then it leaves some
> > files in place, that seems likely to cause problems. (Leaving aside that
> > it might break scripted workflows.)
>
> I thought the point of introducing the new "precious" class of
> paths, in addition to the current "tracked", "ignored, untracked,
> and expendable", "not ignored and untracked", is so that people can
> do "git clean -x -d -f" and expect the ".config" that is marked as
> "precious" to stay. Before their Git learned the precious class, if
> they marked ".config" as "ignored, untracked, and expendable", then
> such an invocation of "clean" would have removed it, but if they add
> it to the new "precious" class, their expectation ought to be that
> precious ones are not removed, no? Otherwise I am not quite sure
> what the point of adding such a new protection is.
I'd expect a lot of projects to move things *from* the current "ignored"
state to "precious", once "precious" exists. Linux `.config`, for
instance.
That said, I do agree that the ideal behavior is for clean to preserve
precious files by default, and require an extra option to remove
precious files. If you think that doesn't have backwards-compatibility
considerations, then it certainly seems much easier to jump directly to
that behavior.
- Josh Triplett
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox