* [RFC][Outreachy] Seeking Git Community Feedback on My Application
From: Isoken Ibizugbe @ 2023-10-19 9:25 UTC (permalink / raw)
To: git, Christian Couder
Dear Git Community and Mentors,
I hope you're doing well. I'm excited to share my application draft
for the Outreachy program with the Git project. Your feedback is
invaluable, and I'm eager to align the project with the community's
needs. Please review the attached draft and share your insights.
Thank you for your support.
Project Application
----
About Me:
My name is Isoken June Ibizugbe, my language is primarily English, and
I am a resident of Nigeria. I am a student at an online coding school
called African Leadership Xcelerator (ALX), participating in the
software engineering program.
What project am I applying for?
Moving Existing Tests to a Unit Testing Framework
Why am I interested in working with the Git chosen project?
Git has been a cornerstone for software development, enabling
developers worldwide to collaborate, innovate, and create exceptional
software. I would say without Git, my journey to pursuing my software
engineering career would be impossible, as I use it almost every day.
Yet, in this constantly evolving landscape, there is always room for
improvement, even in a well-established project. The Git project
currently relies on end-to-end tests, and this is where I see an
opportunity to make a profound impact. Being able to test libraries in
isolation via unit tests or mocks speeds up determining the root cause
of bugs. I am deeply passionate about contributing to this project and
firmly believe in the power of open-source software and the collective
intelligence of the community. A successful completion of this project
will significantly improve Git's testing capabilities and bring the
benefits of fewer errors, faster work and better testing for all
parts.
My motivation for joining the Git community stemmed from my desire to
immerse myself in the world of open-source software and, ultimately,
to become a part of the Outreachy program. My time spent contributing
to Git has been nothing short of transformative. It has been a
remarkable learning experience that has introduced me to a new form of
collaboration using a mailing list and contributing through patches
rather than the typical Pull Request (PR). This collaborative
atmosphere has been pivotal in my growth as a developer, and I am
eager to continue this journey, making meaningful contributions to
this remarkable open-source project.
Contributions to Git
I have actively participated in Git's mailing list discussions and
contributed to a micro-project;
- builtin/branch.c: Adjust error messages such as die(), error(), and
warning() messages used in branch, to conform to coding guidelines
(https://lore.kernel.org/git/20231019084052.567922-1-isokenjune@gmail.com/)
- Implemented changes to fix broken tests based on reviews from the
community (https://lore.kernel.org/git/20231019084052.567922-1-isokenjune@gmail.com/)
- In review.
Project Goals:
- Improve Testing Efficiency: Transitioning from end-to-end tests to
unit tests will enable more efficient testing of error conditions.
- Codebase Stability: Unit tests enhance code stability and facilitate
easier debugging through isolation.
- Simplify Testing: Writing unit tests in pure C simplifies test
setup, data passing, and reduces testing runtime by eliminating
separate processes for each test.
Project Milestones:
- Add useful tests of library-like code
- Integrate with stdlib work
- Run alongside regular make test target
Project Timeline:
1. Oct 2 - Nov 20: Community Bonding
- Understanding the structure of Git
- Getting familiar with the code
2. Dec 4 - Jan 15: Add useful tests of library-like code
- Identify and document the current state of the tests in the Git
t/helper directory.
- Confirm the licensing and compatibility requirements for the chosen
unit testing framework.
- Develop unit tests for these library-like components.
- Execute the tests and ensure they cover various scenarios, including
error conditions.
- Run the tests and address any initial issues or bugs to ensure they
work as intended.
- Document the new tests and their coverage.
- Seek feedback and support from mentors and the Git community
3. Jan 15 - Feb 15: Integrate with Stdlib Work
- Collaborate with the team working on standard library integration.
- Ensure that the tests for library-like code align with stdlib work.
- Verify that the tests effectively check the compatibility and
interaction of the code with standard libraries.
- Gather feedback and insights from the Git community on the
integrated tests, addressing any concerns or suggestions.
4. Feb 15 - March 1: Run Alongside Regular 'make test' Target and finalize
- Configure the testing framework to run alongside the regular 'make
test' target.
- Ensure that the new tests are included in the standard testing suite.
- Execute 'make test' with the new tests and verify that they pass successfully.
- Document the integration process and how the new tests are included
in the standard testing procedure.
- Perform comprehensive testing of the entire unit testing framework.
- Ensure all migrated tests are working correctly within the new framework.
- Document the entire process of migrating Git's tests
- Prepare a final project report
Technical Requirements
According to the documentation on the unit test project
(https://github.com/steadmon/git/blob/unit-tests-asciidoc/Documentation/technical/unit-tests.adoc),
the suggested best framework for the Git project is the "Custom TAP
framework" (Phillip Wood's TAP implementation), as it aligns with
Git's licensing requirements, is vendorable, and can be customized by
Git's developers as needed, but it may require some additional
development work for features like parallel execution and mock
support, but it offers a strong foundation for unit testing within the
Git project.
Relevant Projects
Simple shell - A project based on emulating a shell. It was a
collaborative project which we managed using Git.
(https://github.com/Junie06/simple_shell).
This project was written in C, which allowed me to apply my C language
knowledge, essential for Git projects.
I'm proficient in using Shell for scripting, redirections, and
permissions, as shown in my work
(https://github.com/Junie06/alx-system_engineering-devops).
Creating the simple shell project deepened my understanding of how
shells work, and I even attempted to replicate a shell environment.
Collaborating on the Simple Shell project reinforced my Git skills.
^ permalink raw reply
* [PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Isoken June Ibizugbe @ 2023-10-19 8:40 UTC (permalink / raw)
To: git; +Cc: Isoken June Ibizugbe, rjusto, christian.couder, gitster
In-Reply-To: <e08b2ec4-786a-4c18-b7af-0a6a250ae0f0@gmail.com>
As per the CodingGuidelines document, it is recommended that a single-line
message provided to error messages such as die(), error() and warning(),
should start with a lowercase letter and should not end with a period.
Also this patch fixes the tests broken by the changes.
Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
---
builtin/branch.c | 66 +++++++++++++++++++--------------------
t/t2407-worktree-heads.sh | 2 +-
t/t3200-branch.sh | 16 +++++-----
t/t3202-show-branch.sh | 10 +++---
4 files changed, 47 insertions(+), 47 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 2ec190b14a..e7ee9bd0f1 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -173,11 +173,11 @@ static int branch_merged(int kind, const char *name,
(head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0) != merged) {
if (merged)
warning(_("deleting branch '%s' that has been merged to\n"
- " '%s', but not yet merged to HEAD."),
+ " '%s', but not yet merged to HEAD"),
name, reference_name);
else
warning(_("not deleting branch '%s' that is not yet merged to\n"
- " '%s', even though it is merged to HEAD."),
+ " '%s', even though it is merged to HEAD"),
name, reference_name);
}
free(reference_name_to_free);
@@ -190,13 +190,13 @@ static int check_branch_commit(const char *branchname, const char *refname,
{
struct commit *rev = lookup_commit_reference(the_repository, oid);
if (!force && !rev) {
- error(_("Couldn't look up commit object for '%s'"), refname);
+ error(_("couldn't look up commit object for '%s'"), refname);
return -1;
}
if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
- error(_("The branch '%s' is not fully merged.\n"
+ error(_("the branch '%s' is not fully merged.\n"
"If you are sure you want to delete it, "
- "run 'git branch -D %s'."), branchname, branchname);
+ "run 'git branch -D %s'"), branchname, branchname);
return -1;
}
return 0;
@@ -207,7 +207,7 @@ static void delete_branch_config(const char *branchname)
struct strbuf buf = STRBUF_INIT;
strbuf_addf(&buf, "branch.%s", branchname);
if (git_config_rename_section(buf.buf, NULL) < 0)
- warning(_("Update of config-file failed"));
+ warning(_("update of config-file failed"));
strbuf_release(&buf);
}
@@ -260,7 +260,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
if (kinds == FILTER_REFS_BRANCHES) {
const char *path;
if ((path = branch_checked_out(name))) {
- error(_("Cannot delete branch '%s' "
+ error(_("cannot delete branch '%s' "
"used by worktree at '%s'"),
bname.buf, path);
ret = 1;
@@ -275,7 +275,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
&oid, &flags);
if (!target) {
if (remote_branch) {
- error(_("remote-tracking branch '%s' not found."), bname.buf);
+ error(_("remote-tracking branch '%s' not found"), bname.buf);
} else {
char *virtual_name = mkpathdup(fmt_remotes, bname.buf);
char *virtual_target = resolve_refdup(virtual_name,
@@ -290,7 +290,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
"Did you forget --remote?"),
bname.buf);
else
- error(_("branch '%s' not found."), bname.buf);
+ error(_("branch '%s' not found"), bname.buf);
FREE_AND_NULL(virtual_target);
}
ret = 1;
@@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,
continue;
if (is_worktree_being_rebased(wt, target))
- die(_("Branch %s is being rebased at %s"),
+ die(_("branch %s is being rebased at %s"),
target, wt->path);
if (is_worktree_being_bisected(wt, target))
- die(_("Branch %s is being bisected at %s"),
+ die(_("branch %s is being bisected at %s"),
target, wt->path);
}
}
@@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if (ref_exists(oldref.buf))
recovery = 1;
else
- die(_("Invalid branch name: '%s'"), oldname);
+ die(_("invalid branch name: '%s'"), oldname);
}
for (int i = 0; worktrees[i]; i++) {
@@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {
if (oldref_usage & IS_HEAD)
- die(_("No commit on branch '%s' yet."), oldname);
+ die(_("no commit on branch '%s' yet"), oldname);
else
- die(_("No branch named '%s'."), oldname);
+ die(_("no branch named '%s'"), oldname);
}
/*
@@ -624,32 +624,32 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if (!copy && !(oldref_usage & IS_ORPHAN) &&
rename_ref(oldref.buf, newref.buf, logmsg.buf))
- die(_("Branch rename failed"));
+ die(_("branch rename failed"));
if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
- die(_("Branch copy failed"));
+ die(_("branch copy failed"));
if (recovery) {
if (copy)
- warning(_("Created a copy of a misnamed branch '%s'"),
+ warning(_("created a copy of a misnamed branch '%s'"),
interpreted_oldname);
else
- warning(_("Renamed a misnamed branch '%s' away"),
+ warning(_("renamed a misnamed branch '%s' away"),
interpreted_oldname);
}
if (!copy && (oldref_usage & IS_HEAD) &&
replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,
logmsg.buf))
- die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+ die(_("branch renamed to %s, but HEAD is not updated"), newname);
strbuf_release(&logmsg);
strbuf_addf(&oldsection, "branch.%s", interpreted_oldname);
strbuf_addf(&newsection, "branch.%s", interpreted_newname);
if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
- die(_("Branch is renamed, but update of config-file failed"));
+ die(_("branch is renamed, but update of config-file failed"));
if (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
- die(_("Branch is copied, but update of config-file failed"));
+ die(_("branch is copied, but update of config-file failed"));
strbuf_release(&oldref);
strbuf_release(&newref);
strbuf_release(&oldsection);
@@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
head = resolve_refdup("HEAD", 0, &head_oid, NULL);
if (!head)
- die(_("Failed to resolve HEAD as a valid ref."));
+ die(_("failed to resolve HEAD as a valid ref"));
if (!strcmp(head, "HEAD"))
filter.detached = 1;
else if (!skip_prefix(head, "refs/heads/", &head))
@@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!argc) {
if (filter.detached)
- die(_("Cannot give description to detached HEAD"));
+ die(_("cannot give description to detached HEAD"));
branch_name = head;
} else if (argc == 1) {
strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
@@ -878,8 +878,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
if (!ref_exists(branch_ref.buf))
error((!argc || branch_checked_out(branch_ref.buf))
- ? _("No commit on branch '%s' yet.")
- : _("No branch named '%s'."),
+ ? _("no commit on branch '%s' yet")
+ : _("no branch named '%s'"),
branch_name);
else if (!edit_branch_description(branch_name))
ret = 0; /* happy */
@@ -892,8 +892,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!argc)
die(_("branch name required"));
else if ((argc == 1) && filter.detached)
- die(copy? _("cannot copy the current branch while not on any.")
- : _("cannot rename the current branch while not on any."));
+ die(copy? _("cannot copy the current branch while not on any")
+ : _("cannot rename the current branch while not on any"));
else if (argc == 1)
copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);
else if (argc == 2)
@@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!branch) {
if (!argc || !strcmp(argv[0], "HEAD"))
die(_("could not set upstream of HEAD to %s when "
- "it does not point to any branch."),
+ "it does not point to any branch"),
new_upstream);
die(_("no such branch '%s'"), argv[0]);
}
if (!ref_exists(branch->refname)) {
if (!argc || branch_checked_out(branch->refname))
- die(_("No commit on branch '%s' yet."), branch->name);
+ die(_("no commit on branch '%s' yet"), branch->name);
die(_("branch '%s' does not exist"), branch->name);
}
@@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!branch) {
if (!argc || !strcmp(argv[0], "HEAD"))
die(_("could not unset upstream of HEAD when "
- "it does not point to any branch."));
+ "it does not point to any branch"));
die(_("no such branch '%s'"), argv[0]);
}
if (!branch_has_merge_config(branch))
- die(_("Branch '%s' has no upstream information"), branch->name);
+ die(_("branch '%s' has no upstream information"), branch->name);
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.remote", branch->name);
@@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
const char *start_name = argc == 2 ? argv[1] : head;
if (filter.kind != FILTER_REFS_BRANCHES)
- die(_("The -a, and -r, options to 'git branch' do not take a branch name.\n"
+ die(_("the -a, and -r, options to 'git branch' do not take a branch name.\n"
"Did you mean to use: -a|-r --list <pattern>?"));
if (track == BRANCH_TRACK_OVERRIDE)
- die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
+ die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead"));
if (recurse_submodules) {
create_branches_recursively(the_repository, branch_name,
diff --git a/t/t2407-worktree-heads.sh b/t/t2407-worktree-heads.sh
index 469443d8ae..f6835c91dc 100755
--- a/t/t2407-worktree-heads.sh
+++ b/t/t2407-worktree-heads.sh
@@ -45,7 +45,7 @@ test_expect_success 'refuse to overwrite: checked out in worktree' '
grep "cannot force update the branch" err &&
test_must_fail git branch -D wt-$i 2>err &&
- grep "Cannot delete branch" err || return 1
+ grep "cannot delete branch" err || return 1
done
'
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 080e4f24a6..3182abde27 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -291,10 +291,10 @@ test_expect_success 'git branch -M topic topic should work when main is checked
test_expect_success 'git branch -M and -C fail on detached HEAD' '
git checkout HEAD^{} &&
test_when_finished git checkout - &&
- echo "fatal: cannot rename the current branch while not on any." >expect &&
+ echo "fatal: cannot rename the current branch while not on any" >expect &&
test_must_fail git branch -M must-fail 2>err &&
test_cmp expect err &&
- echo "fatal: cannot copy the current branch while not on any." >expect &&
+ echo "fatal: cannot copy the current branch while not on any" >expect &&
test_must_fail git branch -C must-fail 2>err &&
test_cmp expect err
'
@@ -943,7 +943,7 @@ test_expect_success 'deleting currently checked out branch fails' '
git worktree add -b my7 my7 &&
test_must_fail git -C my7 branch -d my7 &&
test_must_fail git branch -d my7 2>actual &&
- grep "^error: Cannot delete branch .my7. used by worktree at " actual &&
+ grep "^error: cannot delete branch .my7. used by worktree at " actual &&
rm -r my7 &&
git worktree prune
'
@@ -954,7 +954,7 @@ test_expect_success 'deleting in-use branch fails' '
git -C my7 bisect start HEAD HEAD~2 &&
test_must_fail git -C my7 branch -d my7 &&
test_must_fail git branch -d my7 2>actual &&
- grep "^error: Cannot delete branch .my7. used by worktree at " actual &&
+ grep "^error: cannot delete branch .my7. used by worktree at " actual &&
rm -r my7 &&
git worktree prune
'
@@ -1024,7 +1024,7 @@ test_expect_success '--set-upstream-to fails on multiple branches' '
test_expect_success '--set-upstream-to fails on detached HEAD' '
git checkout HEAD^{} &&
test_when_finished git checkout - &&
- echo "fatal: could not set upstream of HEAD to main when it does not point to any branch." >expect &&
+ echo "fatal: could not set upstream of HEAD to main when it does not point to any branch" >expect &&
test_must_fail git branch --set-upstream-to main 2>err &&
test_cmp expect err
'
@@ -1072,7 +1072,7 @@ test_expect_success 'use --set-upstream-to modify a particular branch' '
'
test_expect_success '--unset-upstream should fail if given a non-existent branch' '
- echo "fatal: Branch '"'"'i-dont-exist'"'"' has no upstream information" >expect &&
+ echo "fatal: branch '"'"'i-dont-exist'"'"' has no upstream information" >expect &&
test_must_fail git branch --unset-upstream i-dont-exist 2>err &&
test_cmp expect err
'
@@ -1094,7 +1094,7 @@ test_expect_success 'test --unset-upstream on HEAD' '
test_must_fail git config branch.main.remote &&
test_must_fail git config branch.main.merge &&
# fail for a branch without upstream set
- echo "fatal: Branch '"'"'main'"'"' has no upstream information" >expect &&
+ echo "fatal: branch '"'"'main'"'"' has no upstream information" >expect &&
test_must_fail git branch --unset-upstream 2>err &&
test_cmp expect err
'
@@ -1108,7 +1108,7 @@ test_expect_success '--unset-upstream should fail on multiple branches' '
test_expect_success '--unset-upstream should fail on detached HEAD' '
git checkout HEAD^{} &&
test_when_finished git checkout - &&
- echo "fatal: could not unset upstream of HEAD when it does not point to any branch." >expect &&
+ echo "fatal: could not unset upstream of HEAD when it does not point to any branch" >expect &&
test_must_fail git branch --unset-upstream 2>err &&
test_cmp expect err
'
diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
index b17f388f56..2cdb834b37 100755
--- a/t/t3202-show-branch.sh
+++ b/t/t3202-show-branch.sh
@@ -10,7 +10,7 @@ GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
test_expect_success 'error descriptions on empty repository' '
current=$(git branch --show-current) &&
cat >expect <<-EOF &&
- error: No commit on branch '\''$current'\'' yet.
+ error: no commit on branch '\''$current'\'' yet
EOF
test_must_fail git branch --edit-description 2>actual &&
test_cmp expect actual &&
@@ -21,7 +21,7 @@ test_expect_success 'error descriptions on empty repository' '
test_expect_success 'fatal descriptions on empty repository' '
current=$(git branch --show-current) &&
cat >expect <<-EOF &&
- fatal: No commit on branch '\''$current'\'' yet.
+ fatal: no commit on branch '\''$current'\'' yet
EOF
test_must_fail git branch --set-upstream-to=non-existent 2>actual &&
test_cmp expect actual &&
@@ -224,7 +224,7 @@ done
test_expect_success 'error descriptions on non-existent branch' '
cat >expect <<-EOF &&
- error: No branch named '\''non-existent'\'.'
+ error: no branch named '\''non-existent'\''
EOF
test_must_fail git branch --edit-description non-existent 2>actual &&
test_cmp expect actual
@@ -238,7 +238,7 @@ test_expect_success 'fatal descriptions on non-existent branch' '
test_cmp expect actual &&
cat >expect <<-EOF &&
- fatal: No branch named '\''non-existent'\''.
+ fatal: no branch named '\''non-existent'\''
EOF
test_must_fail git branch -c non-existent new-branch 2>actual &&
test_cmp expect actual &&
@@ -253,7 +253,7 @@ test_expect_success 'error descriptions on orphan branch' '
test_branch_op_in_wt() {
test_orphan_error() {
test_must_fail git $* 2>actual &&
- test_i18ngrep "No commit on branch .orphan-branch. yet.$" actual
+ test_i18ngrep "no commit on branch .orphan-branch. yet$" actual
} &&
test_orphan_error -C wt branch $1 $2 && # implicit branch
test_orphan_error -C wt branch $1 orphan-branch $2 && # explicit branch
--
2.42.0.346.g24618a8a3e.dirty
^ permalink raw reply related
* Re: [PATCH] commit: detect commits that exist in commit-graph but not in the ODB
From: Patrick Steinhardt @ 2023-10-19 8:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Karthik Nayak, Taylor Blau
In-Reply-To: <ZTDQjangLsQ1cSJl@tanuki>
[-- Attachment #1: Type: text/plain, Size: 2654 bytes --]
On Thu, Oct 19, 2023 at 08:45:34AM +0200, Patrick Steinhardt wrote:
> On Tue, Oct 17, 2023 at 11:34:53AM -0700, Junio C Hamano wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > Fair point indeed. The following is a worst-case scenario benchmark of
> > > of the change where we do a full topological walk of all reachable
> > > commits in the graph, executed in linux.git. We parse commit parents via
> > > `repo_parse_commit_gently()`, so the new code path now basically has to
> > > check for object existence of every reachable commit:
> > > ...
> > > The added check does lead to a performance regression indeed, which is
> > > not all that unexpected. That being said, the commit-graph still results
> > > in a significant speedup compared to the case where we don't have it.
> >
> > Yeah, I agree that both points are expected. An extra check that is
> > much cheaper than the full parsing is paying a small price to be a
> > bit more careful than before. The question is if the price is small
> > enough. I am still not sure if the extra carefulness is warranted
> > for all normal cases to spend 30% extra cycles.
> >
> > Thanks.
>
> Well, seen in contexts like the thread that spawned this discussion I
> think it's preferable to take a relatively smaller price compared to
> disabling the commit graph entirely in some situations. With that in
> mind, personally I'd be happy with either of two outcomes here:
>
> - We take the patch I proposed as a hardening measure, which allows
> us to use the commit graph in basically any situation where we
> could parse objects from the ODB without any downside except for a
> performance hit.
>
> - We say that corrupt repositories do not need to be accounted for
> when using metadata caches like the commit-graph. That would mean
> in consequence that for the `--missing` flag we would not have to
> disable commit graphs.
>
> The test failure that was exposed in Karthik's test only stems from the
> fact that we manually corrupt the repository and is not specific to the
> `--missing` flag. This is further stressed by the new test aded in my
> own patch, as we can trigger a similar bug when not using `--missing`.
There's another way to handle this, which is to conditionally enable the
object existence check. This would be less of a performance hit compared
to disabling commit graphs altogether with `--missing`, but still ensure
that repository corruption was detected. Second, it would not regress
performance for all preexisting users of `repo_parse_commit_gently()`.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [Outreachy][PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Isoken Ibizugbe @ 2023-10-19 7:48 UTC (permalink / raw)
To: git; +Cc: rjusto, christian.couder, gitster
In-Reply-To: <20231019074126.567638-1-isokenjune@gmail.com>
On Thu, Oct 19, 2023 at 8:41 AM Isoken June Ibizugbe
<isokenjune@gmail.com> wrote:
>
> Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
> ---
> builtin/branch.c | 66 ++++++++++++++++++++++++------------------------
> 1 file changed, 33 insertions(+), 33 deletions(-)
please ignore this patch. It didn't include the changes for the test files.
thanks
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 2ec190b14a..938c40bfaa 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -173,11 +173,11 @@ static int branch_merged(int kind, const char *name,
> (head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0) != merged) {
> if (merged)
> warning(_("deleting branch '%s' that has been merged to\n"
> - " '%s', but not yet merged to HEAD."),
> + " '%s', but not yet merged to HEAD"),
> name, reference_name);
> else
> warning(_("not deleting branch '%s' that is not yet merged to\n"
> - " '%s', even though it is merged to HEAD."),
> + " '%s', even though it is merged to HEAD"),
> name, reference_name);
> }
> free(reference_name_to_free);
> @@ -190,13 +190,13 @@ static int check_branch_commit(const char *branchname, const char *refname,
> {
> struct commit *rev = lookup_commit_reference(the_repository, oid);
> if (!force && !rev) {
> - error(_("Couldn't look up commit object for '%s'"), refname);
> + error(_("couldn't look up commit object for '%s'"), refname);
> return -1;
> }
> if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
> - error(_("The branch '%s' is not fully merged.\n"
> + error(_("the branch '%s' is not fully merged.\n"
> "If you are sure you want to delete it, "
> - "run 'git branch -D %s'."), branchname, branchname);
> + "run 'git branch -D %s'"), branchname, branchname);
> return -1;
> }
> return 0;
> @@ -207,7 +207,7 @@ static void delete_branch_config(const char *branchname)
> struct strbuf buf = STRBUF_INIT;
> strbuf_addf(&buf, "branch.%s", branchname);
> if (git_config_rename_section(buf.buf, NULL) < 0)
> - warning(_("Update of config-file failed"));
> + warning(_("update of config-file failed"));
> strbuf_release(&buf);
> }
>
> @@ -260,7 +260,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
> if (kinds == FILTER_REFS_BRANCHES) {
> const char *path;
> if ((path = branch_checked_out(name))) {
> - error(_("Cannot delete branch '%s' "
> + error(_("cannot delete branch '%s' "
> "used by worktree at '%s'"),
> bname.buf, path);
> ret = 1;
> @@ -275,7 +275,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
> &oid, &flags);
> if (!target) {
> if (remote_branch) {
> - error(_("remote-tracking branch '%s' not found."), bname.buf);
> + error(_("remote-tracking branch '%s' not found"), bname.buf);
> } else {
> char *virtual_name = mkpathdup(fmt_remotes, bname.buf);
> char *virtual_target = resolve_refdup(virtual_name,
> @@ -290,7 +290,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
> "Did you forget --remote?"),
> bname.buf);
> else
> - error(_("branch '%s' not found."), bname.buf);
> + error(_("branch '%s' not found"), bname.buf);
> FREE_AND_NULL(virtual_target);
> }
> ret = 1;
> @@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,
> continue;
>
> if (is_worktree_being_rebased(wt, target))
> - die(_("Branch %s is being rebased at %s"),
> + die(_("branch %s is being rebased at %s"),
> target, wt->path);
>
> if (is_worktree_being_bisected(wt, target))
> - die(_("Branch %s is being bisected at %s"),
> + die(_("branch %s is being bisected at %s"),
> target, wt->path);
> }
> }
> @@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
> if (ref_exists(oldref.buf))
> recovery = 1;
> else
> - die(_("Invalid branch name: '%s'"), oldname);
> + die(_("invalid branch name: '%s'"), oldname);
> }
>
> for (int i = 0; worktrees[i]; i++) {
> @@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
>
> if ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {
> if (oldref_usage & IS_HEAD)
> - die(_("No commit on branch '%s' yet."), oldname);
> + die(_("no commit on branch '%s' yet"), oldname);
> else
> - die(_("No branch named '%s'."), oldname);
> + die(_("no branch named '%s'"), oldname);
> }
>
> /*
> @@ -624,32 +624,32 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
>
> if (!copy && !(oldref_usage & IS_ORPHAN) &&
> rename_ref(oldref.buf, newref.buf, logmsg.buf))
> - die(_("Branch rename failed"));
> + die(_("branch rename failed"));
> if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
> - die(_("Branch copy failed"));
> + die(_("branch copy failed"));
>
> if (recovery) {
> if (copy)
> - warning(_("Created a copy of a misnamed branch '%s'"),
> + warning(_("created a copy of a misnamed branch '%s'"),
> interpreted_oldname);
> else
> - warning(_("Renamed a misnamed branch '%s' away"),
> + warning(_("renamed a misnamed branch '%s' away"),
> interpreted_oldname);
> }
>
> if (!copy && (oldref_usage & IS_HEAD) &&
> replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,
> logmsg.buf))
> - die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
> + die(_("branch renamed to %s, but HEAD is not updated"), newname);
>
> strbuf_release(&logmsg);
>
> strbuf_addf(&oldsection, "branch.%s", interpreted_oldname);
> strbuf_addf(&newsection, "branch.%s", interpreted_newname);
> if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
> - die(_("Branch is renamed, but update of config-file failed"));
> + die(_("branch is renamed, but update of config-file failed"));
> if (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
> - die(_("Branch is copied, but update of config-file failed"));
> + die(_("branch is copied, but update of config-file failed"));
> strbuf_release(&oldref);
> strbuf_release(&newref);
> strbuf_release(&oldsection);
> @@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>
> head = resolve_refdup("HEAD", 0, &head_oid, NULL);
> if (!head)
> - die(_("Failed to resolve HEAD as a valid ref."));
> + die(_("failed to resolve HEAD as a valid ref"));
> if (!strcmp(head, "HEAD"))
> filter.detached = 1;
> else if (!skip_prefix(head, "refs/heads/", &head))
> @@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>
> if (!argc) {
> if (filter.detached)
> - die(_("Cannot give description to detached HEAD"));
> + die(_("cannot give description to detached HEAD"));
> branch_name = head;
> } else if (argc == 1) {
> strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
> @@ -878,8 +878,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
> if (!ref_exists(branch_ref.buf))
> error((!argc || branch_checked_out(branch_ref.buf))
> - ? _("No commit on branch '%s' yet.")
> - : _("No branch named '%s'."),
> + ? _("no commit on branch '%s' yet")
> + : _("no branch named '%s'"),
> branch_name);
> else if (!edit_branch_description(branch_name))
> ret = 0; /* happy */
> @@ -892,8 +892,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> if (!argc)
> die(_("branch name required"));
> else if ((argc == 1) && filter.detached)
> - die(copy? _("cannot copy the current branch while not on any.")
> - : _("cannot rename the current branch while not on any."));
> + die(copy? _("cannot copy the current branch while not on any")
> + : _("cannot rename the current branch while not on any"));
> else if (argc == 1)
> copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);
> else if (argc == 2)
> @@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> if (!branch) {
> if (!argc || !strcmp(argv[0], "HEAD"))
> die(_("could not set upstream of HEAD to %s when "
> - "it does not point to any branch."),
> + "it does not point to any branch"),
> new_upstream);
> die(_("no such branch '%s'"), argv[0]);
> }
>
> if (!ref_exists(branch->refname)) {
> if (!argc || branch_checked_out(branch->refname))
> - die(_("No commit on branch '%s' yet."), branch->name);
> + die(_("no commit on branch '%s' yet"), branch->name);
> die(_("branch '%s' does not exist"), branch->name);
> }
>
> @@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> if (!branch) {
> if (!argc || !strcmp(argv[0], "HEAD"))
> die(_("could not unset upstream of HEAD when "
> - "it does not point to any branch."));
> + "it does not point to any branch"));
> die(_("no such branch '%s'"), argv[0]);
> }
>
> if (!branch_has_merge_config(branch))
> - die(_("Branch '%s' has no upstream information"), branch->name);
> + die(_("branch '%s' has no upstream information"), branch->name);
>
> strbuf_reset(&buf);
> strbuf_addf(&buf, "branch.%s.remote", branch->name);
> @@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> const char *start_name = argc == 2 ? argv[1] : head;
>
> if (filter.kind != FILTER_REFS_BRANCHES)
> - die(_("The -a, and -r, options to 'git branch' do not take a branch name\n"
> + die(_("the -a, and -r, options to 'git branch' do not take a branch name.\n"
> "Did you mean to use: -a|-r --list <pattern>?"));
>
> if (track == BRANCH_TRACK_OVERRIDE)
> - die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
> + die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead"));
>
> if (recurse_submodules) {
> create_branches_recursively(the_repository, branch_name,
> --
> 2.42.0.346.g24618a8a3e.dirty
>
^ permalink raw reply
* [Outreachy][PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Isoken June Ibizugbe @ 2023-10-19 7:41 UTC (permalink / raw)
To: git; +Cc: rjusto, christian.couder, gitster, Isoken June Ibizugbe
Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
---
builtin/branch.c | 66 ++++++++++++++++++++++++------------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 2ec190b14a..938c40bfaa 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -173,11 +173,11 @@ static int branch_merged(int kind, const char *name,
(head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0) != merged) {
if (merged)
warning(_("deleting branch '%s' that has been merged to\n"
- " '%s', but not yet merged to HEAD."),
+ " '%s', but not yet merged to HEAD"),
name, reference_name);
else
warning(_("not deleting branch '%s' that is not yet merged to\n"
- " '%s', even though it is merged to HEAD."),
+ " '%s', even though it is merged to HEAD"),
name, reference_name);
}
free(reference_name_to_free);
@@ -190,13 +190,13 @@ static int check_branch_commit(const char *branchname, const char *refname,
{
struct commit *rev = lookup_commit_reference(the_repository, oid);
if (!force && !rev) {
- error(_("Couldn't look up commit object for '%s'"), refname);
+ error(_("couldn't look up commit object for '%s'"), refname);
return -1;
}
if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
- error(_("The branch '%s' is not fully merged.\n"
+ error(_("the branch '%s' is not fully merged.\n"
"If you are sure you want to delete it, "
- "run 'git branch -D %s'."), branchname, branchname);
+ "run 'git branch -D %s'"), branchname, branchname);
return -1;
}
return 0;
@@ -207,7 +207,7 @@ static void delete_branch_config(const char *branchname)
struct strbuf buf = STRBUF_INIT;
strbuf_addf(&buf, "branch.%s", branchname);
if (git_config_rename_section(buf.buf, NULL) < 0)
- warning(_("Update of config-file failed"));
+ warning(_("update of config-file failed"));
strbuf_release(&buf);
}
@@ -260,7 +260,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
if (kinds == FILTER_REFS_BRANCHES) {
const char *path;
if ((path = branch_checked_out(name))) {
- error(_("Cannot delete branch '%s' "
+ error(_("cannot delete branch '%s' "
"used by worktree at '%s'"),
bname.buf, path);
ret = 1;
@@ -275,7 +275,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
&oid, &flags);
if (!target) {
if (remote_branch) {
- error(_("remote-tracking branch '%s' not found."), bname.buf);
+ error(_("remote-tracking branch '%s' not found"), bname.buf);
} else {
char *virtual_name = mkpathdup(fmt_remotes, bname.buf);
char *virtual_target = resolve_refdup(virtual_name,
@@ -290,7 +290,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
"Did you forget --remote?"),
bname.buf);
else
- error(_("branch '%s' not found."), bname.buf);
+ error(_("branch '%s' not found"), bname.buf);
FREE_AND_NULL(virtual_target);
}
ret = 1;
@@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,
continue;
if (is_worktree_being_rebased(wt, target))
- die(_("Branch %s is being rebased at %s"),
+ die(_("branch %s is being rebased at %s"),
target, wt->path);
if (is_worktree_being_bisected(wt, target))
- die(_("Branch %s is being bisected at %s"),
+ die(_("branch %s is being bisected at %s"),
target, wt->path);
}
}
@@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if (ref_exists(oldref.buf))
recovery = 1;
else
- die(_("Invalid branch name: '%s'"), oldname);
+ die(_("invalid branch name: '%s'"), oldname);
}
for (int i = 0; worktrees[i]; i++) {
@@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {
if (oldref_usage & IS_HEAD)
- die(_("No commit on branch '%s' yet."), oldname);
+ die(_("no commit on branch '%s' yet"), oldname);
else
- die(_("No branch named '%s'."), oldname);
+ die(_("no branch named '%s'"), oldname);
}
/*
@@ -624,32 +624,32 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if (!copy && !(oldref_usage & IS_ORPHAN) &&
rename_ref(oldref.buf, newref.buf, logmsg.buf))
- die(_("Branch rename failed"));
+ die(_("branch rename failed"));
if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
- die(_("Branch copy failed"));
+ die(_("branch copy failed"));
if (recovery) {
if (copy)
- warning(_("Created a copy of a misnamed branch '%s'"),
+ warning(_("created a copy of a misnamed branch '%s'"),
interpreted_oldname);
else
- warning(_("Renamed a misnamed branch '%s' away"),
+ warning(_("renamed a misnamed branch '%s' away"),
interpreted_oldname);
}
if (!copy && (oldref_usage & IS_HEAD) &&
replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,
logmsg.buf))
- die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+ die(_("branch renamed to %s, but HEAD is not updated"), newname);
strbuf_release(&logmsg);
strbuf_addf(&oldsection, "branch.%s", interpreted_oldname);
strbuf_addf(&newsection, "branch.%s", interpreted_newname);
if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
- die(_("Branch is renamed, but update of config-file failed"));
+ die(_("branch is renamed, but update of config-file failed"));
if (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
- die(_("Branch is copied, but update of config-file failed"));
+ die(_("branch is copied, but update of config-file failed"));
strbuf_release(&oldref);
strbuf_release(&newref);
strbuf_release(&oldsection);
@@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
head = resolve_refdup("HEAD", 0, &head_oid, NULL);
if (!head)
- die(_("Failed to resolve HEAD as a valid ref."));
+ die(_("failed to resolve HEAD as a valid ref"));
if (!strcmp(head, "HEAD"))
filter.detached = 1;
else if (!skip_prefix(head, "refs/heads/", &head))
@@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!argc) {
if (filter.detached)
- die(_("Cannot give description to detached HEAD"));
+ die(_("cannot give description to detached HEAD"));
branch_name = head;
} else if (argc == 1) {
strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
@@ -878,8 +878,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
if (!ref_exists(branch_ref.buf))
error((!argc || branch_checked_out(branch_ref.buf))
- ? _("No commit on branch '%s' yet.")
- : _("No branch named '%s'."),
+ ? _("no commit on branch '%s' yet")
+ : _("no branch named '%s'"),
branch_name);
else if (!edit_branch_description(branch_name))
ret = 0; /* happy */
@@ -892,8 +892,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!argc)
die(_("branch name required"));
else if ((argc == 1) && filter.detached)
- die(copy? _("cannot copy the current branch while not on any.")
- : _("cannot rename the current branch while not on any."));
+ die(copy? _("cannot copy the current branch while not on any")
+ : _("cannot rename the current branch while not on any"));
else if (argc == 1)
copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);
else if (argc == 2)
@@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!branch) {
if (!argc || !strcmp(argv[0], "HEAD"))
die(_("could not set upstream of HEAD to %s when "
- "it does not point to any branch."),
+ "it does not point to any branch"),
new_upstream);
die(_("no such branch '%s'"), argv[0]);
}
if (!ref_exists(branch->refname)) {
if (!argc || branch_checked_out(branch->refname))
- die(_("No commit on branch '%s' yet."), branch->name);
+ die(_("no commit on branch '%s' yet"), branch->name);
die(_("branch '%s' does not exist"), branch->name);
}
@@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!branch) {
if (!argc || !strcmp(argv[0], "HEAD"))
die(_("could not unset upstream of HEAD when "
- "it does not point to any branch."));
+ "it does not point to any branch"));
die(_("no such branch '%s'"), argv[0]);
}
if (!branch_has_merge_config(branch))
- die(_("Branch '%s' has no upstream information"), branch->name);
+ die(_("branch '%s' has no upstream information"), branch->name);
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.remote", branch->name);
@@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
const char *start_name = argc == 2 ? argv[1] : head;
if (filter.kind != FILTER_REFS_BRANCHES)
- die(_("The -a, and -r, options to 'git branch' do not take a branch name\n"
+ die(_("the -a, and -r, options to 'git branch' do not take a branch name.\n"
"Did you mean to use: -a|-r --list <pattern>?"));
if (track == BRANCH_TRACK_OVERRIDE)
- die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
+ die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead"));
if (recurse_submodules) {
create_branches_recursively(the_repository, branch_name,
--
2.42.0.346.g24618a8a3e.dirty
^ permalink raw reply related
* Re: [PATCH] commit: detect commits that exist in commit-graph but not in the ODB
From: Patrick Steinhardt @ 2023-10-19 6:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Karthik Nayak, Taylor Blau
In-Reply-To: <xmqqjzrlhzci.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2627 bytes --]
On Tue, Oct 17, 2023 at 11:34:53AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Fair point indeed. The following is a worst-case scenario benchmark of
> > of the change where we do a full topological walk of all reachable
> > commits in the graph, executed in linux.git. We parse commit parents via
> > `repo_parse_commit_gently()`, so the new code path now basically has to
> > check for object existence of every reachable commit:
> > ...
> > The added check does lead to a performance regression indeed, which is
> > not all that unexpected. That being said, the commit-graph still results
> > in a significant speedup compared to the case where we don't have it.
>
> Yeah, I agree that both points are expected. An extra check that is
> much cheaper than the full parsing is paying a small price to be a
> bit more careful than before. The question is if the price is small
> enough. I am still not sure if the extra carefulness is warranted
> for all normal cases to spend 30% extra cycles.
>
> Thanks.
Well, seen in contexts like the thread that spawned this discussion I
think it's preferable to take a relatively smaller price compared to
disabling the commit graph entirely in some situations. With that in
mind, personally I'd be happy with either of two outcomes here:
- We take the patch I proposed as a hardening measure, which allows
us to use the commit graph in basically any situation where we
could parse objects from the ODB without any downside except for a
performance hit.
- We say that corrupt repositories do not need to be accounted for
when using metadata caches like the commit-graph. That would mean
in consequence that for the `--missing` flag we would not have to
disable commit graphs.
The test failure that was exposed in Karthik's test only stems from the
fact that we manually corrupt the repository and is not specific to the
`--missing` flag. This is further stressed by the new test aded in my
own patch, as we can trigger a similar bug when not using `--missing`.
For end users, object pruning would happen either via git-gc(1) or via
git-maintenance(1), and both of them do know to update commit graphs
already. This should significantly decrease the chance that such stale
commit graphs would be found out there in the wild, but it's still not
impossible of course. Especially in cases where you use lower-level
commands like git-repack(1) with cruft packs or git-prune(1), which is
often the case for Git hosters, the risk is higher though.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: Is there any interest in localizing term delimiters in git messages?
From: Jiang Xin @ 2023-10-19 5:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: Alexander Shopov, Git List, jmas, alexhenrie24, ralf.thielow,
matthias.ruester, phillip.szelat, vyruss, christopher.diaz.riv,
jn.avila, flashcode, bagasdotme,
Ævar Arnfjörð Bjarmason, alessandro.menti,
elongbug, cwryu, uneedsihyeon, arek_koz, dacs.git,
insolor@gmail.com, peter, bitigchi, ark, kate,
vnwildman@gmail.com, pclouds, dyroneteng@gmail.com,
oldsharp@gmail.com, lilydjwg@gmail.com, me, pan93412@gmail.com,
franklin@goodhorse.idv.tw
In-Reply-To: <xmqqwmvkve83.fsf@gitster.g>
On Wed, Oct 18, 2023 at 10:47 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jiang Xin <worldhello.net@gmail.com> writes:
>
> > Starting with the release of git 2.34.0 two years ago, we had a new
> > l10n pipeline and the git-po-helper tool as part of our l10n workflow.
> > The first version of git-po-helper introduced a validator to protect
> > git command parameters and variable names in megid.
>
> Ahh, that is the piece I was missing. I didn't know you guys are
> doing extra checks that could trigger false positives.
>
> > E.g. In pull
> > request 541 (https://github.com/git-l10n/git-po/pull/541), a
> > mismatched variable name "new_index" was reported in bg.po as below:
> >
> > level=warning msg="mismatch variable names in msgstr: new_index"
> > level=warning msg=">> msgid: unable to write new_index file"
> > level=warning msg=">> msgstr: новият индекс не може да бъде записан"
> >
> > And po/bg.po changed as below:
> >
> > msgid "unable to write new_index file"
> > msgstr "новият индекс (new_index) не може да бъде записан"
>
> Wait. Is this supposed to be a good example of validator working
> well? We use this exact message three times in builtin/commit.c; is
> the validator insisting on the translated message to have verbatim
> string "new_index" in it so that the end-users will see it?
>
> I may still be confused, but if that is what is going on, I think it
> is a wrong validation in this particular case. I can understand if
> we were creating say .git/new_index file and it helps the end users
> to diagnose a troubled repository by running "ls .git" to see if a
> file called "new_index" exists and getting in the way, but I do not
> think it is the case. A new file ".git/index.lock" is created via
> repo_hold_locked_index() and I do not think it helps the end-user to
> know that we may be calling it "new_index" internally among the
> developers' circle. If the message were about "index.lock", it
> might be a different story, but such an error would probably have
> been issued long before write_locked_index() gets called.
>
> I'd suggest doing s/new_index/new index/ to msgid string for these
> anyway.
I tried to find similar patterns in `po/bg.po` using:
$ git grep -h -B5 '([a-zA-Z_\.]*_[a-zA-Z_\.]\+)' po/bg.po
And find other translated variable names in Bulgarian as follows:
* cookie_result in builtin/fsmonitor--daemon.c:
error(_("fsmonitor: cookie_result '%d' != SEEN"),
* run_command in builtin/submodule--helper.c:
die(_("run_command returned non-zero status for %s\n."),
die(_("run_command returned non-zero status while "
* crlf_action in convert.c:
warning(_("illegal crlf_action %d"), (int)crlf_action);
* lazy_dir in name-hash.c:
die(_("unable to create lazy_dir thread: %s"),
* lazy_name in name-hash.c:
die(_("unable to create lazy_name thread: %s"),
die(_("unable to join lazy_name thread: %s"),
* load_cache_entries in read-cache.c:
die(_("unable to create load_cache_entries thread: %s"),
die(_("unable to join load_cache_entries thread: %s"),
* load_index_extensions in read-cache.c:
die(_("unable to create load_index_extensions thread: %s"),
die(_("unable to join load_index_extensions thread: %s"),
Apart from "new_index", it seems that none of the above sentences can
be rewritten simply by removing the underscores in variable names
without breaking the grammar, and I suppose it would be better to keep
those variable names unchanged.
^ permalink raw reply
* [PATCH v2] git-p4 shouldn't attempt to store symlinks in LFS
From: Matthew McClain @ 2023-10-19 0:25 UTC (permalink / raw)
To: git; +Cc: sandals, gitster, Matthew McClain
In-Reply-To: <ZTBbqobbqQpxHPI2@tapette.crustytoothpaste.net>
git-p4.py would attempt to put a symlink in LFS if its file extension
matched git-p4.largeFileExtensions.
Git LFS doesn't store symlinks because smudge/clean filters don't handle
symlinks. They never get passed to the filter process nor the
smudge/clean filters, nor could that occur without a change to the
protocol or command-line interface. Unless Git learned how to send them
to the filters, Git LFS would have a hard time using them in any useful
way.
Git LFS's goal is to move large files out of the repository history, and
symlinks are functionally limited to 4 KiB or a similar size on most
systems.
Signed-off-by: Matthew McClain <mmcclain@noprivs.com>
---
git-p4.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/git-p4.py b/git-p4.py
index d26a980e5a..0eb3bb4c47 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1522,6 +1522,10 @@ def processContent(self, git_mode, relPath, contents):
file is stored in the large file system and handles all necessary
steps.
"""
+ # symlinks aren't processed by smudge/clean filters
+ if git_mode == "120000":
+ return (git_mode, contents)
+
if self.exceedsLargeFileThreshold(relPath, contents) or self.hasLargeFileExtension(relPath):
contentTempFile = self.generateTempFile(contents)
pointer_git_mode, contents, localLargeFile = self.generatePointer(contentTempFile)
--
2.39.3
^ permalink raw reply related
* Re: [PATCH] doc: update list archive reference to use lore.kernel.org
From: Dragan Simic @ 2023-10-19 0:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq7cnz741s.fsf@gitster.g>
On 2023-10-07 00:57, Junio C Hamano wrote:
> No disrespect to other mailing list archives, but the local part of
> their URLs will become pretty much meaningless once the archives go
> out of service, and we learned the lesson hard way when $gmane
> stopped serving.
>
> Let's point into https://lore.kernel.org/ for an article that can be
> found there, because the local part of the URL has the Message-Id:
> that can be used to find the same message in other archives, even if
> lore goes down.
You asked for this patch to be reviewed, so I did it. It looks
perfectly fine to me.
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
> Documentation/CodingGuidelines | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/CodingGuidelines
> b/Documentation/CodingGuidelines
> index 65af8d82ce..71afc5b259 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -24,7 +24,7 @@ code. For Git in general, a few rough rules are:
>
> "Once it _is_ in the tree, it's not really worth the patch noise to
> go and fix it up."
> - Cf. http://lkml.iu.edu/hypermail/linux/kernel/1001.3/01069.html
> + Cf.
> https://lore.kernel.org/all/20100126160632.3bdbe172.akpm@linux-foundation.org/
>
> - Log messages to explain your changes are as important as the
> changes themselves. Clearly written code and in-code comments
^ permalink raw reply
* Re: [Outreachy] [PATCH v2] t/t7601: use "test_path_is_file"etc. instead of "test -f"
From: Junio C Hamano @ 2023-10-18 23:59 UTC (permalink / raw)
To: Dorcas AnonoLitunya; +Cc: christian.couder, git
In-Reply-To: <20231018124538.68549-2-anonolitunya@gmail.com>
Dorcas AnonoLitunya <anonolitunya@gmail.com> writes:
> Some tests in t7601 use "test -f" and "test ! -f" to see if a path
> exists or is missing.
>
> Use test_path_is_file and test_path_is_missing helper functions to
> clarify these tests a bit better. This especially matters for the
> "missing" case because "test ! -f F" will be happy if "F" exists as a
> directory, but the intent of the test is that "F" should not exist, even
> as a directory. The updated code expresses this better.
>
> Signed-off-by: Dorcas AnonoLitunya <anonolitunya@gmail.com>
> ---
> t/t7601-merge-pull-config.sh | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
Nicely done; it seems that the title line got garbled whitespace for
some reason but I'll fix them up while queuing.
Thanks. Will queue.
> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
> index bd238d89b0..e08767df66 100755
> --- a/t/t7601-merge-pull-config.sh
> +++ b/t/t7601-merge-pull-config.sh
> @@ -349,13 +349,13 @@ test_expect_success 'Cannot rebase with multiple heads' '
>
> test_expect_success 'merge c1 with c2' '
> git reset --hard c1 &&
> - test -f c0.c &&
> - test -f c1.c &&
> - test ! -f c2.c &&
> - test ! -f c3.c &&
> + test_path_is_file c0.c &&
> + test_path_is_file c1.c &&
> + test_path_is_missing c2.c &&
> + test_path_is_missing c3.c &&
> git merge c2 &&
> - test -f c1.c &&
> - test -f c2.c
> + test_path_is_file c1.c &&
> + test_path_is_file c2.c
> '
>
> test_expect_success 'fast-forward pull succeeds with "true" in pull.ff' '
> @@ -411,8 +411,8 @@ test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '
> git reset --hard c1 &&
> git config pull.twohead ours &&
> git merge c2 &&
> - test -f c1.c &&
> - ! test -f c2.c
> + test_path_is_file c1.c &&
> + test_path_is_missing c2.c
> '
>
> test_expect_success 'merge c1 with c2 and c3 (recursive in pull.octopus)' '
> @@ -431,10 +431,10 @@ test_expect_success 'merge c1 with c2 and c3 (recursive and octopus in pull.octo
> test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
> test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&
> git diff --exit-code &&
> - test -f c0.c &&
> - test -f c1.c &&
> - test -f c2.c &&
> - test -f c3.c
> + test_path_is_file c0.c &&
> + test_path_is_file c1.c &&
> + test_path_is_file c2.c &&
> + test_path_is_file c3.c
> '
>
> conflict_count()
^ permalink raw reply
* Re: [PATCH v3 05/17] t/helper/test-read-graph.c: extract `dump_graph_info()`
From: Junio C Hamano @ 2023-10-18 23:56 UTC (permalink / raw)
To: Taylor Blau
Cc: Patrick Steinhardt, git, Jonathan Tan, Jeff King,
SZEDER Gábor
In-Reply-To: <ZTAXyMWeQLRWcpuu@nand.local>
Taylor Blau <me@ttaylorr.com> writes:
> On Tue, Oct 17, 2023 at 10:45:18AM +0200, Patrick Steinhardt wrote:
>> On Tue, Oct 10, 2023 at 04:33:33PM -0400, Taylor Blau wrote:
>> > Prepare for the 'read-graph' test helper to perform other tasks besides
>> > dumping high-level information about the commit-graph by extracting its
>> > main routine into a separate function.
>> >
>> > Signed-off-by: Taylor Blau <me@ttaylorr.com>
>> > Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
>> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> > Signed-off-by: Taylor Blau <me@ttaylorr.com>
>>
>> Nit: your signoff is duplicated here. This is also still the case for
>> some of the other commits.
>
> Yeah, this is an artifact of having tossed these patches back and forth
> (originally Jonathan sent some of these, then I sent another round, then
> Jonathan, now me again). It's a little verbose, but accurately tracks
> the DCO across multiple rounds.
Well, between you and Jonathan Tan, that might be true, but my
involvement in the chain is only that I happened to have looked at
an earlier round and without doing anything else. Even if you
refetched from my tree and based the new round on that version, it
would have been the same if you started from what you sent out
earlier without even looking at my tree. So I do not think that
chain of DCO adds very little value by recording my sign-off there.
^ permalink raw reply
* Re: [PATCH 00/11] t: reduce direct disk access to data structures
From: Junio C Hamano @ 2023-10-18 23:40 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys
In-Reply-To: <ZS9vp9dnoVVUsMIt@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> As usual, I forgot to mention that this applies on top of current
> master, which is at a9ecda2788 (The eighteenth batch, 2023-10-13) at the
> time of writing. I look forward to the day where I remember to include
> this information in the cover letter...
I heard rumors that we have a --base option that records such a
piece of information in the format-patch output (I haven't used it
myself). Would it have helped, or perhaps it still needs some
usability tweak?
^ permalink raw reply
* What's cooking in git.git (Oct 2023, #07; Wed, 18)
From: Junio C Hamano @ 2023-10-18 23:36 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
There are too many topics marked with "Needs review" label. I can
help reviewing some of them myself, but obviously it will not scale.
For now, I'll let unreviewed new topics accumulating in the mailing
list archive without picking them up in 'seen' to encourage folks
help reduce the review backlog.
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* jc/merge-ort-attr-index-fix (2023-10-09) 1 commit
(merged to 'next' on 2023-10-10 at b139b87502)
+ merge-ort: initialize repo in index state
Fix "git merge-tree" to stop segfaulting when the --attr-source
option is used.
source: <pull.1583.v3.git.git.1696857660374.gitgitgadget@gmail.com>
* sn/cat-file-doc-update (2023-10-09) 1 commit
(merged to 'next' on 2023-10-10 at 6719ba7bd4)
+ doc/cat-file: make synopsis and description less confusing
"git cat-file" documentation updates.
source: <20231009175604.2361700-1-stepnem@smrk.net>
* tb/repack-max-cruft-size (2023-10-09) 5 commits
(merged to 'next' on 2023-10-09 at 38f039e880)
+ repack: free existing_cruft array after use
(merged to 'next' on 2023-10-06 at b3ca6df3b9)
+ builtin/repack.c: avoid making cruft packs preferred
+ builtin/repack.c: implement support for `--max-cruft-size`
+ builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDE
+ t7700: split cruft-related tests to t7704
"git repack" learned "--max-cruft-size" to prevent cruft packs from
growing without bounds.
source: <cover.1696293862.git.me@ttaylorr.com>
source: <20231007172031.GA1524950@coredump.intra.peff.net>
source: <035393935108d02aaf8927189b05102f4f74f340.1696370003.git.me@ttaylorr.com>
* xz/commit-title-soft-limit-doc (2023-10-09) 1 commit
(merged to 'next' on 2023-10-10 at 0bf3d9ed51)
+ doc: correct the 50 characters soft limit (+)
Doc update.
source: <pull.1585.v2.git.git.1696778367151.gitgitgadget@gmail.com>
--------------------------------------------------
[New Topics]
* jc/commit-new-underscore-index-fix (2023-10-17) 1 commit
- commit: do not use cryptic "new_index" in end-user facing messages
source: <xmqqo7gwvd8c.fsf_-_@gitster.g>
* js/bugreport-in-the-same-minute (2023-10-16) 1 commit
- bugreport: include +i in outfile suffix as needed
source: <20231016214045.146862-2-jacob@initialcommit.io>
* kh/pathspec-error-wo-repository-fix (2023-10-17) 1 commit
- grep: die gracefully when outside repository
source: <087c92e3904dd774f672373727c300bf7f5f6369.1697317276.git.code@khaugsbakk.name>
* kh/t7900-cleanup (2023-10-17) 9 commits
- t7900: fix register dependency
- t7900: factor out packfile dependency
- t7900: fix `print-args` dependency
- t7900: fix `pfx` dependency
- t7900: factor out common schedule setup
- t7900: factor out inheritance test dependency
- t7900: create commit so that branch is born
- t7900: setup and tear down clones
- t7900: remove register dependency
source: <cover.1697319294.git.code@khaugsbakk.name>
* ni/die-message-fix-for-git-add (2023-10-17) 1 commit
- builtin/add.c: clean up die() messages
source: <20231017113946.747-1-naomi.ibeh69@gmail.com>
* ps/git-repack-doc-fixes (2023-10-16) 2 commits
- doc/git-repack: don't mention nonexistent "--unpacked" option
- doc/git-repack: fix syntax for `-g` shorthand option
source: <cover.1697440686.git.ps@pks.im>
* tb/merge-tree-write-pack (2023-10-17) 7 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: factor our `finalize_checkpoint()`
- bulk-checkin: factor out `truncate_checkpoint()`
- bulk-checkin: factor out `prepare_checkpoint()`
- bulk-checkin: factor out `format_object_header_hash()`
source: <cover.1697560266.git.me@ttaylorr.com>
--------------------------------------------------
[Stalled]
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Expecting a reroll.
cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
- diff-lib: fix check_removed() when fsmonitor is active
- Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
- Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
It is unknown if the optimization is worth resurrecting, but in case...
source: <xmqqr0n0h0tw.fsf@gitster.g>
--------------------------------------------------
[Cooking]
* tb/format-pack-doc-update (2023-10-12) 2 commits
- Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
- Documentation/gitformat-pack.txt: fix typo
Doc update.
Expecting a reroll.
cf. <xmqq5y3b4id2.fsf@gitster.g>
source: <cover.1697144959.git.me@ttaylorr.com>
* ps/do-not-trust-commit-graph-blindly-for-existence (2023-10-13) 1 commit
- commit: detect commits that exist in commit-graph but not in the ODB
(this branch is used by kn/rev-list-missing-fix.)
The codepath to traverse the commit-graph learned to notice that a
commit is missing (e.g., corrupt repository lost an object), even
though it knows something about the commit (like its parents) from
what is in commit-graph.
Comments?
source: <b0bf576c51a706367a758b8e30eca37edb9c2734.1697200576.git.ps@pks.im>
* tb/pair-chunk-expect-size (2023-10-14) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- midx: read `OIDF` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDF` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
(this branch uses jk/chunk-bounds.)
Code clean-up for jk/chunk-bounds topic.
Comments?
source: <45cac29403e63483951f7766c6da3c022c68d9f0.1697225110.git.me@ttaylorr.com>
source: <cover.1697225110.git.me@ttaylorr.com>
* jc/fail-stash-to-store-non-stash (2023-10-11) 1 commit
(merged to 'next' on 2023-10-16 at e26db57315)
+ stash: be careful what we store
Feeding "git stash store" with a random commit that was not created
by "git stash create" now errors out.
Will merge to 'master'.
source: <xmqqbkd4lwj0.fsf_-_@gitster.g>
* bc/racy-4gb-files (2023-10-13) 2 commits
(merged to 'next' on 2023-10-16 at c60962dfee)
+ Prevent git from rehashing 4GiB files
+ t: add a test helper to truncate files
The index file has room only for lower 32-bit of the file size in
the cached stat information, which means cached stat information
will have 0 in its sd_size member for a file whose size is multiple
of 4GiB. This is mistaken for a racily clean path. Avoid it by
storing a bogus sd_size value instead for such files.
Will merge to 'master'.
source: <20231012160930.330618-1-sandals@crustytoothpaste.net>
* jc/grep-f-relative-to-cwd (2023-10-12) 1 commit
- grep: -f <path> is relative to $cwd
"cd sub && git grep -f patterns" tried to read "patterns" file at
the top level of the working tree; it has been corrected to read
"sub/patterns" instead.
Needs review.
source: <xmqqedhzg37z.fsf@gitster.g>
* tb/path-filter-fix (2023-10-10) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs (hopefully final and quick) review.
source: <cover.1696969994.git.me@ttaylorr.com>
* ak/pretty-decorate-more-fix (2023-10-09) 1 commit
(merged to 'next' on 2023-10-12 at 3cbb4c2268)
+ pretty: fix ref filtering for %(decorate) formats
Unlike "git log --pretty=%D", "git log --pretty="%(decorate)" did
not auto-initialize the decoration subsystem, which has been
corrected.
Will merge to 'master'.
source: <20231008202307.1568477-1-andy.koppe@gmail.com>
* en/docfixes (2023-10-09) 25 commits
(merged to 'next' on 2023-10-17 at 1e3cdeb427)
+ documentation: add missing parenthesis
+ documentation: add missing quotes
+ documentation: add missing fullstops
+ documentation: add some commas where they are helpful
+ documentation: fix whitespace issues
+ documentation: fix capitalization
+ documentation: fix punctuation
+ documentation: use clearer prepositions
+ documentation: add missing hyphens
+ documentation: remove unnecessary hyphens
+ documentation: add missing article
+ documentation: fix choice of article
+ documentation: whitespace is already generally plural
+ documentation: fix singular vs. plural
+ documentation: fix verb vs. noun
+ documentation: fix adjective vs. noun
+ documentation: fix verb tense
+ documentation: employ consistent verb tense for a list
+ documentation: fix subject/verb agreement
+ documentation: remove extraneous words
+ documentation: add missing words
+ documentation: fix apostrophe usage
+ documentation: fix typos
+ documentation: fix small error
+ documentation: wording improvements
Documentation typo and grammo fixes.
Will merge to 'master'.
source: <pull.1595.git.1696747527.gitgitgadget@gmail.com>
* kn/rev-list-missing-fix (2023-10-16) 4 commits
. rev-list: add commit object support in `--missing` option
. rev-list: move `show_commit()` to the bottom
. revision: rename bit to `do_not_die_on_missing_objects`
. Merge branch 'ps/do-not-trust-commit-graph-blindly-for-existence' into kn/rev-list-missing-fix
(this branch uses ps/do-not-trust-commit-graph-blindly-for-existence.)
"git rev-list --missing" did not work for missing commit objects,
which has been corrected.
cf. <xmqqbkcyo7ro.fsf@gitster.g>
source: <20231016103830.56486-1-karthik.188@gmail.com>
* jk/chunk-bounds (2023-10-14) 21 commits
(merged to 'next' on 2023-10-16 at 68c9e37980)
+ t5319: make corrupted large-offset test more robust
(merged to 'next' on 2023-10-10 at 21139603ce)
+ chunk-format: drop pair_chunk_unsafe()
+ commit-graph: detect out-of-order BIDX offsets
+ commit-graph: check bounds when accessing BIDX chunk
+ commit-graph: check bounds when accessing BDAT chunk
+ commit-graph: bounds-check generation overflow chunk
+ commit-graph: check size of generations chunk
+ commit-graph: bounds-check base graphs chunk
+ commit-graph: detect out-of-bounds extra-edges pointers
+ commit-graph: check size of commit data chunk
+ midx: check size of revindex chunk
+ midx: bounds-check large offset chunk
+ midx: check size of object offset chunk
+ midx: enforce chunk alignment on reading
+ midx: check size of pack names chunk
+ commit-graph: check consistency of fanout table
+ midx: check size of oid lookup chunk
+ commit-graph: check size of oid fanout chunk
+ midx: stop ignoring malformed oid fanout chunk
+ t: add library for munging chunk-format files
+ chunk-format: note that pair_chunk() is unsafe
(this branch is used by tb/pair-chunk-expect-size.)
The codepaths that read "chunk" formatted files have been corrected
to pay attention to the chunk size and notice broken files.
Will merge to 'master'.
source: <20231009205544.GA3281950@coredump.intra.peff.net>
* ps/rewritten-is-per-worktree-doc (2023-10-10) 1 commit
(merged to 'next' on 2023-10-12 at 501b960e8c)
+ doc/git-worktree: mention "refs/rewritten" as per-worktree refs
Doc update.
Will merge to 'master'.
source: <985ac850eb6e60ae76601acc8bfbcd56f99348b4.1696935657.git.ps@pks.im>
* sn/typo-grammo-phraso-fixes (2023-10-05) 5 commits
(merged to 'next' on 2023-10-18 at 575d767f9a)
+ t/README: fix multi-prerequisite example
+ doc/gitk: s/sticked/stuck/
+ git-jump: admit to passing merge mode args to ls-files
+ doc/diff-options: improve wording of the log.diffMerges mention
+ doc: fix some typos, grammar and wording issues
Many typos, ungrammatical sentences and wrong phrasing have been
fixed.
Will merge to 'master'.
source: <20231003082107.3002173-1-stepnem@smrk.net>
* so/diff-merges-dd (2023-10-09) 3 commits
(merged to 'next' on 2023-10-16 at 71b5e29625)
+ completion: complete '--dd'
+ diff-merges: introduce '--dd' option
+ diff-merges: improve --diff-merges documentation
"git log" and friends learned "--dd" that is a short-hand for
"--diff-merges=first-parent -p".
Will merge to 'master'.
source: <20231009160535.236523-1-sorganov@gmail.com>
* vd/loose-ref-iteration-optimization (2023-10-09) 4 commits
(merged to 'next' on 2023-10-12 at 99e2f83855)
+ files-backend.c: avoid stat in 'loose_fill_ref_dir'
+ dir.[ch]: add 'follow_symlink' arg to 'get_dtype'
+ dir.[ch]: expose 'get_dtype'
+ ref-cache.c: fix prefix matching in ref iteration
The code to iterate over loose references have been optimized to
reduce the number of lstat() system calls.
Will merge to 'master'.
source: <pull.1594.v2.git.1696888736.gitgitgadget@gmail.com>
* jc/update-list-references-to-lore (2023-10-06) 1 commit
- doc: update list archive reference to use lore.kernel.org
Doc update.
Needs review.
source: <xmqq7cnz741s.fsf@gitster.g>
* cc/git-replay (2023-10-10) 14 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: change rev walking options
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
Introduce "git replay", a tool meant on the server side without
working tree to recreate a history.
Needs (hopefully final and quick) review.
source: <20231010123847.2777056-1-christian.couder@gmail.com>
* ak/color-decorate-symbols (2023-10-03) 1 commit
- decorate: add color.decorate.symbols config option
A new config for coloring.
Needs review.
source: <20231003205442.22963-1-andy.koppe@gmail.com>
* jc/attr-tree-config (2023-10-13) 2 commits
- attr: add attr.tree for setting the treeish to read attributes from
- attr: read attributes from HEAD when bare repo
The attribute subsystem learned to honor `attr.tree` configuration
that specifies which tree to read the .gitattributes files from.
Will merge to 'next'?
source: <pull.1577.v5.git.git.1697218770.gitgitgadget@gmail.com>
* js/update-urls-in-doc-and-comment (2023-09-26) 4 commits
- doc: refer to internet archive
- doc: update links for andre-simon.de
- doc: update links to current pages
- doc: switch links to https
Stale URLs have been updated to their current counterparts (or
archive.org) and HTTP links are replaced with working HTTPS links.
Needs review.
source: <pull.1589.v2.git.1695553041.gitgitgadget@gmail.com>
* la/trailer-cleanups (2023-09-26) 4 commits
- trailer: only use trailer_block_* variables if trailers were found
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Needs review.
source: <pull.1563.v4.git.1695709372.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-10-04) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1696432593.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-10-04) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Needs review.
source: <cover.1696425168.git.zhiyou.jx@alibaba-inc.com>
* ty/merge-tree-strategy-options (2023-10-11) 2 commits
(merged to 'next' on 2023-10-12 at 9b873601df)
+ merge: introduce {copy|clear}_merge_options()
(merged to 'next' on 2023-09-29 at aa65b54416)
+ merge-tree: add -X strategy option
"git merge-tree" learned to take strategy backend specific options
via the "-X" option, like "git merge" does.
Will merge to 'master'.
source: <pull.1565.v6.git.1695522222723.gitgitgadget@gmail.com>
* js/config-parse (2023-09-21) 5 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split do_event() into start and flush operations
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1695330852.git.steadmon@google.com>
* jc/fake-lstat (2023-09-15) 1 commit
- cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Needs review.
source: <xmqqcyykig1l.fsf@gitster.g>
* rs/parse-options-value-int (2023-09-18) 2 commits
- parse-options: use and require int pointer for OPT_CMDMODE
- parse-options: add int value pointer to struct option
A bit of type safety for the "value" pointer used in the
parse-options API.
Not ready yet.
cf. <4014e490-c6c1-453d-b0ed-645220e3e614@web.de>
source: <e6d8a291-03de-cfd3-3813-747fc2cad145@web.de>
* js/doc-unit-tests (2023-10-16) 5 commits
- fixup! ci: run unit tests in CI
- fixup! unit tests: add TAP unit test framework
- ci: run unit tests in CI
- unit tests: add TAP unit test framework
- unit tests: add a project plan document
(this branch is used by js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Expecting a (hopefully final and minor) reroll.
cf. <20231016134421.21659-1-phillip.wood123@gmail.com>
source: <cover.1696889529.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-10-16) 7 commits
- cmake: handle also unit tests
- cmake: use test names instead of full paths
- cmake: fix typo in variable name
- artifacts-tar: when including `.dll` files, don't forget the unit-tests
- unit-tests: do show relative file paths
- unit-tests: do not mistake `.pdb` files for being executable
- cmake: also build unit tests
(this branch uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
Needs review.
source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
--------------------------------------------------
[Discarded]
* jc/doc-unit-tests-fixup (2023-10-11) 1 commit
. SQUASH???
(this branch uses js/doc-unit-tests and js/doc-unit-tests-with-cmake.)
Quick fix for jc/doc-unit-tests topic to unbreak CI running on 'seen'.
Superseded by a fixup! in the base series.
source: <xmqqwmvskf8t.fsf@gitster.g>
^ permalink raw reply
* [PATCH] am: align placeholder for --whitespace option with apply
From: Junio C Hamano @ 2023-10-18 23:35 UTC (permalink / raw)
To: git
`git am` passes the value given to its `--whitespace` option through
to the underlying `git apply`, and the value is called <action> over
there. Fix the documentation for the command that calls the value
<option> to say <action> instead.
Note that the option help given by `git am -h` already calls the
value <action>, so there is no need to make a matching change there.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-am.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git i/Documentation/git-am.txt w/Documentation/git-am.txt
index 0c1dfb3c98..de3d5dde99 100644
--- i/Documentation/git-am.txt
+++ w/Documentation/git-am.txt
@@ -12,7 +12,7 @@ SYNOPSIS
'git am' [--signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8] [--no-verify]
[--[no-]3way] [--interactive] [--committer-date-is-author-date]
[--ignore-date] [--ignore-space-change | --ignore-whitespace]
- [--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]
+ [--whitespace=<action>] [-C<n>] [-p<n>] [--directory=<dir>]
[--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
[--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]
[--quoted-cr=<action>]
@@ -116,7 +116,7 @@ include::rerere-options.txt[]
--ignore-space-change::
--ignore-whitespace::
---whitespace=<option>::
+--whitespace=<action>::
-C<n>::
-p<n>::
--directory=<dir>::
^ permalink raw reply related
* Re: [PATCH v4 00/17] bloom: changed-path Bloom filters v2 (& sundries)
From: Junio C Hamano @ 2023-10-18 23:26 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Elijah Newren, Eric W. Biederman, Jeff King,
Patrick Steinhardt
In-Reply-To: <cover.1697653929.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> (Rebased onto the tip of 'master', which is 3a06386e31 (The fifteenth
> batch, 2023-10-04), at the time of writing).
Judging from 17/17 that has a free_commit_graph() call in
close_commit_graph(), that was merged in the eighteenth batch,
the above is probably untrue. I'll apply to the current master and
see how it goes instead.
> Thanks to Jonathan, Peff, and SZEDER who have helped a great deal in
> assembling these patches. As usual, a range-diff is included below.
> Thanks in advance for your
> review!
Thanks.
^ permalink raw reply
* Re: [PATCH v3 08/10] bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
From: Junio C Hamano @ 2023-10-18 23:18 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Elijah Newren, Eric W. Biederman, Jeff King,
Patrick Steinhardt
In-Reply-To: <8667b763652ffa71b52b7bd78821e46a6e5fe5a9.1697648864.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> Now that we have factored out many of the common routines necessary to
> index a new object into a pack created by the bulk-checkin machinery, we
> can introduce a variant of `index_blob_bulk_checkin()` that acts on
> blobs whose contents we can fit in memory.
Hmph.
Doesn't the duplication between the main loop of the new
deflate_obj_contents_to_pack() with existing deflate_blob_to_pack()
bother you?
A similar duplication in the previous round resulted in a nice
refactoring of patches 5 and 6 in this round. Compared to that,
the differences in the set-up code between the two functions may be
much larger, but subtle and unnecessary differences between the code
that was copied and pasted (e.g., we do not check the errors from
the seek_to method here, but the original does) is a sign that over
time a fix to one will need to be carried over to the other, adding
unnecessary maintenance burden, isn't it?
> +static int deflate_obj_contents_to_pack_incore(struct bulk_checkin_packfile *state,
> + git_hash_ctx *ctx,
> + struct hashfile_checkpoint *checkpoint,
> + struct object_id *result_oid,
> + const void *buf, size_t size,
> + enum object_type type,
> + const char *path, unsigned flags)
> +{
> + struct pack_idx_entry *idx = NULL;
> + off_t already_hashed_to = 0;
> + struct bulk_checkin_source source = {
> + .type = SOURCE_INCORE,
> + .buf = buf,
> + .size = size,
> + .read = 0,
> + .path = path,
> + };
> +
> + /* Note: idx is non-NULL when we are writing */
> + if (flags & HASH_WRITE_OBJECT)
> + CALLOC_ARRAY(idx, 1);
> +
> + while (1) {
> + prepare_checkpoint(state, checkpoint, idx, flags);
> +
> + if (!stream_obj_to_pack(state, ctx, &already_hashed_to, &source,
> + type, flags))
> + break;
> + truncate_checkpoint(state, checkpoint, idx);
> + bulk_checkin_source_seek_to(&source, 0);
> + }
> +
> + finalize_checkpoint(state, ctx, checkpoint, idx, result_oid);
> +
> + return 0;
> +}
> +
> +static int deflate_blob_to_pack_incore(struct bulk_checkin_packfile *state,
> + struct object_id *result_oid,
> + const void *buf, size_t size,
> + const char *path, unsigned flags)
> +{
> + git_hash_ctx ctx;
> + struct hashfile_checkpoint checkpoint = {0};
> +
> + format_object_header_hash(the_hash_algo, &ctx, &checkpoint, OBJ_BLOB,
> + size);
> +
> + return deflate_obj_contents_to_pack_incore(state, &ctx, &checkpoint,
> + result_oid, buf, size,
> + OBJ_BLOB, path, flags);
> +}
> +
> static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
> struct object_id *result_oid,
> int fd, size_t size,
> @@ -456,6 +509,17 @@ int index_blob_bulk_checkin(struct object_id *oid,
> return status;
> }
>
> +int index_blob_bulk_checkin_incore(struct object_id *oid,
> + const void *buf, size_t size,
> + const char *path, unsigned flags)
> +{
> + int status = deflate_blob_to_pack_incore(&bulk_checkin_packfile, oid,
> + buf, size, path, flags);
> + if (!odb_transaction_nesting)
> + flush_bulk_checkin_packfile(&bulk_checkin_packfile);
> + return status;
> +}
> +
> void begin_odb_transaction(void)
> {
> odb_transaction_nesting += 1;
> diff --git a/bulk-checkin.h b/bulk-checkin.h
> index aa7286a7b3..1b91daeaee 100644
> --- a/bulk-checkin.h
> +++ b/bulk-checkin.h
> @@ -13,6 +13,10 @@ int index_blob_bulk_checkin(struct object_id *oid,
> int fd, size_t size,
> const char *path, unsigned flags);
>
> +int index_blob_bulk_checkin_incore(struct object_id *oid,
> + const void *buf, size_t size,
> + const char *path, unsigned flags);
> +
> /*
> * Tell the object database to optimize for adding
> * multiple objects. end_odb_transaction must be called
^ permalink raw reply
* Re: [PATCH v3 05/10] bulk-checkin: extract abstract `bulk_checkin_source`
From: Junio C Hamano @ 2023-10-18 23:10 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Elijah Newren, Eric W. Biederman, Jeff King,
Patrick Steinhardt
In-Reply-To: <da52ec838025a59a3f4f4ffaf2e6f9098a37547e.1697648864.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> A future commit will want to implement a very similar routine as in
> `stream_blob_to_pack()` with two notable changes:
>
> - Instead of streaming just OBJ_BLOBs, this new function may want to
> stream objects of arbitrary type.
>
> - Instead of streaming the object's contents from an open
> file-descriptor, this new function may want to "stream" its contents
> from memory.
>
> To avoid duplicating a significant chunk of code between the existing
> `stream_blob_to_pack()`, extract an abstract `bulk_checkin_source`. This
> concept currently is a thin layer of `lseek()` and `read_in_full()`, but
> will grow to understand how to perform analogous operations when writing
> out an object's contents from memory.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
> bulk-checkin.c | 61 +++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 53 insertions(+), 8 deletions(-)
> diff --git a/bulk-checkin.c b/bulk-checkin.c
> index f4914fb6d1..fc1d902018 100644
> --- a/bulk-checkin.c
> +++ b/bulk-checkin.c
> @@ -140,8 +140,41 @@ static int already_written(struct bulk_checkin_packfile *state, struct object_id
> return 0;
> }
>
> +struct bulk_checkin_source {
> + enum { SOURCE_FILE } type;
> +
> + /* SOURCE_FILE fields */
> + int fd;
> +
> + /* common fields */
> + size_t size;
> + const char *path;
> +};
Looks OK, even though I expected to see a bit more involved object
orientation with something like
struct bulk_checkin_source {
off_t (*read)(struct bulk_checkin_source *, void *, size_t);
off_t (*seek)(struct bulk_checkin_source *, off_t);
union {
struct {
int fd;
size_t size;
const char *path;
} from_fd;
struct {
...
} incore;
} data;
};
As there will only be two subclasses of this thing, it may not
matter all that much right now, but it would be much nicer as your
methods do not have to care about "switch (enum) { case FILE: ... }".
^ permalink raw reply
* Re: [PATCH] git-p4 shouldn't attempt to store symlinks in LFS
From: Junio C Hamano @ 2023-10-18 22:46 UTC (permalink / raw)
To: brian m. carlson; +Cc: Matthew McClain, git
In-Reply-To: <ZTBbqobbqQpxHPI2@tapette.crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> Also, as a minor request, it would be great if you could continue to
> email me at my personal address, since that's the address with which I
> read the list. My work address appearing on series is more of a
> reflection that I'm more frequently finding time to work on things on
> work time (hence the work address) versus personal time (where I'd be
> using my personal email for patches). I've fixed it up above.
Thanks.
With
[alias]
who = !sh -c 'git log -1 --format=\"%an <%ae>\" --author=\"$1\"' -
I say "git who sandals" to get your e-mail address, but I guess I
should use %aE instead to let .mailmap kick in.
^ permalink raw reply
* Re: [PATCH] git-p4 shouldn't attempt to store symlinks in LFS
From: brian m. carlson @ 2023-10-18 22:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matthew McClain, git
In-Reply-To: <xmqqil73sp8l.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2591 bytes --]
On 2023-10-18 at 19:30:02, Junio C Hamano wrote:
> Matthew McClain <mmcclain@noprivs.com> writes:
>
> > If a symlink in your Perforce repository matches
> > git-p4.largeFileExtensions, git-p4.py will attempt to put the symlink in
> > LFS but will blow up when it passes a string to generateTempFile.
> >
> > Git LFS normal behavior does not stash symlinks in LFS.
>
> I am not a "git p4" (or "git lfs") user, but if nobody uses LFS to
> store symbolic links (which is very much understandable given what
> it was invented for), then that alone is a good enough reason to
> avoid the regular blob codepath, and that is true even if
> generateTempFile accepted a str and silently converted it to bytes
> to help callers, no?
Git LFS doesn't store symlinks because smudge/clean filters don't handle
symlinks. They never get passed to the filter process nor the
smudge/clean filters, nor could that occur without a change to the
protocol or command-line interface. Unless Git learned how to send them
to the filters, Git LFS would have a hard time using them in any useful
way.
Also, for Git LFS, whose goal is to move large files out of the
repository history, symlinks are typically not an interesting thing to
process because they are functionally limited to 4 KiB or a similar size
on most systems.
> So "but will blow up ..." and the stacktrace below are more or less
> irrelevant details and do not help convince readers why this change
> is desirable. I'd suggest removing it (and perhaps place more stress
> on explaining why storing symbolic links in LFS is a bad practice
> nobody uses, but if it is so obvious perhaps the single sentence
> explanation you have above would be sufficient).
Hopefully my explanation above can be part of that commit message.
> I know I can ask brian to take a look at this change from LFS angle,
> but who's authoritatively reviewing "git p4" related changes these
> days (Matthew, this is not a question for you, but to the list)?
It looks fine to me from an LFS angle, but I know nothing about Perforce
or git-p4.
Also, as a minor request, it would be great if you could continue to
email me at my personal address, since that's the address with which I
read the list. My work address appearing on series is more of a
reflection that I'm more frequently finding time to work on things on
work time (hence the work address) versus personal time (where I'd be
using my personal email for patches). I've fixed it up above.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH] Include gettext.h in MyFirstContribution tutorial
From: Junio C Hamano @ 2023-10-18 21:35 UTC (permalink / raw)
To: git, Emily Shaffer; +Cc: Jacob Stopak
In-Reply-To: <20231017041503.3249-1-jacob@initialcommit.io>
Jacob Stopak <jacob@initialcommit.io> writes:
> The tutorial in Documentation/MyFirstContribution.txt has steps to print
> some text using the "_" function. However, this leads to compiler errors
> when running "make" since "gettext.h" is not #included.
>
> Update docs with a note to #include "gettext.h" in "builtin/psuh.c".
>
> Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
> ---
> Documentation/MyFirstContribution.txt | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
Who's the first responder on this document these days? I think the
"psuh" was Emily's invention, so sending it in her direction.
Thanks.
> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
> index 62d11a5cd7..7cfed60c2e 100644
> --- a/Documentation/MyFirstContribution.txt
> +++ b/Documentation/MyFirstContribution.txt
> @@ -160,10 +160,11 @@ in order to keep the declarations alphabetically sorted:
> int cmd_psuh(int argc, const char **argv, const char *prefix);
> ----
>
> -Be sure to `#include "builtin.h"` in your `psuh.c`.
> +Be sure to `#include "builtin.h"` in your `psuh.c`. You'll also need to
> +`#include "gettext.h"` to use functions related to printing output text.
>
> -Go ahead and add some throwaway printf to that function. This is a decent
> -starting point as we can now add build rules and register the command.
> +Go ahead and add some throwaway printf to the `cmd_psuh` function. This is a
> +decent starting point as we can now add build rules and register the command.
>
> NOTE: Your throwaway text, as well as much of the text you will be adding over
> the course of this tutorial, is user-facing. That means it needs to be
^ permalink raw reply
* Re: [PATCH 04/11] t: convert tests to not write references via the filesystem
From: Junio C Hamano @ 2023-10-18 21:18 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys
In-Reply-To: <c79431c0bf117d756e1d584f4c9415888d9ff9eb.1697607222.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> @@ -434,7 +432,7 @@ test_expect_success 'Query "main@{2005-05-28}" (past end of history)' '
> test_i18ngrep -F "warning: log for ref $m unexpectedly ended on $ld" e
> '
>
> -rm -f .git/$m .git/logs/$m expect
> +git update-ref -d $m
We are not clearing "expect" file. I do not know if it matters
here, but I am only recording what I noticed.
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index 10a539158c4..5cce24f1006 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -115,15 +115,16 @@ test_expect_success 'zlib corrupt loose object output ' '
> '
>
> test_expect_success 'branch pointing to non-commit' '
> - git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
> + tree_oid=$(git rev-parse --verify HEAD^{tree}) &&
> + test-tool ref-store main update-ref msg refs/heads/invalid $tree_oid $ZERO_OID REF_SKIP_OID_VERIFICATION &&
I have mixed feelings on this.
In olden days, plumbing commands tended to allow to pass anything
the user told them to use, but in more recent versions of Git, we,
probably by mistake, managed to butcher some of the plumbing
commands to make them unable to deliberately "break" repositories,
one victim being "update-ref", i.e.
$ git update-ref refs/heads/invalid HEAD^{tree}
is rejected these days (I just checked with v1.3.0 and it allows me
to do this), and that is one of the reasons why we manually broke
the repository in these tests. We need to have a warning message in
comments near the implementation of "ref-store update-ref" that says
never ever attempt to share code with the production version of
update-ref---otherwise this (or the "safety" given to the plumbing
command, possibly by mistake) will be broken, depending on which
direction such a sharing goes. On the other hand, forcing us to
keep two separate implementations, one deliberately loose to allow
us corrupt repositories, the other for production and actively used,
would mean the former one that is only used for validation would risk
bitrotting.
> test_when_finished "git update-ref -d refs/heads/invalid" &&
Not a problem this patch introduces, but I think it is a better
discipline to have when_finished clean-up routine prepared before we
do actual damage, i.e. if I were writing this test today from scratch,
I would expect it to be before "git rev-parse >.git/refs/heads/invalid"
is done.
> test_must_fail git fsck 2>out &&
> test_i18ngrep "not a commit" out
> '
A #leftoverbit that is not relevant to the topic; we should clean
these test_i18ngrep and replace them with a plain "grep".
> test_expect_success 'HEAD link pointing at a funny object' '
> - test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
> - mv .git/HEAD .git/SAVED_HEAD &&
> + saved_head=$(git rev-parse --verify HEAD) &&
> + test_when_finished "git update-ref HEAD ${saved_head}" &&
> echo $ZERO_OID >.git/HEAD &&
Are you sure .git/HEAD when this test is entered is a detached HEAD?
The title of the test says "HEAD link", and I take it to mean HEAD
is a symlink, and we save it away, while we create a loose ref that
points at 0{40} in a detached HEAD state. Actually, the original
would also work if HEAD is detached on entry. In either case,
moving SAVED_HEAD back to HEAD would restore the original state.
But the updated one only works if HEAD upon entry is already
detached. Is this intended?
> @@ -131,8 +132,8 @@ test_expect_success 'HEAD link pointing at a funny object' '
> '
>
> test_expect_success 'HEAD link pointing at a funny place' '
> - test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
> - mv .git/HEAD .git/SAVED_HEAD &&
> + saved_head=$(git rev-parse --verify HEAD) &&
> + test_when_finished "git update-ref --no-deref HEAD ${saved_head}" &&
Likewise. Use of "update-ref" in the previous one vs "update-ref
--no-deref" in this one to recover from the damage the tests make
makes me feel that we may be assuming too much.
> echo "ref: refs/funny/place" >.git/HEAD &&
Even though "git symbolic-ref" refuses to point HEAD outside refs/,
as plumbing command should, it allows it to point it outside refs/heads/.
so this line should probably become
git symbolic-ref HEAD refs/funny/place
in the same spirit as the rest of the series.
> @@ -391,7 +393,7 @@ test_expect_success 'tag pointing to nonexistent' '
>
> tag=$(git hash-object -t tag -w --stdin <invalid-tag) &&
> test_when_finished "remove_object $tag" &&
> - echo $tag >.git/refs/tags/invalid &&
> + git update-ref refs/tags/invalid $tag &&
Good (not just this one, but similar ones throughout this patch).
^ permalink raw reply
* [PATCH v1 4/4] maintenance: use XDG config if it exists
From: Kristoffer Haugsbakk @ 2023-10-18 20:28 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, ps, stolee
In-Reply-To: <cover.1697660181.git.code@khaugsbakk.name>
`git maintenance register` registers the repository in the user's global
config. `$XDG_CONFIG_HOME/git/config` is supposed to be used if
`~/.gitconfig` does not exist. However, this command creates a
`~/.gitconfig` file and writes to that one even though the XDG variant
exists.
This used to work correctly until 50a044f1e4 (gc: replace config
subprocesses with API calls, 2022-09-27), when the command started calling
the config API instead of git-config(1).
Also change `unregister` accordingly.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
builtin/gc.c | 23 +++++------------------
t/t7900-maintenance.sh | 21 +++++++++++++++++++++
2 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 17fc031f63a..7b780f2ab38 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1526,19 +1526,12 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
if (!found) {
int rc;
- char *user_config = NULL, *xdg_config = NULL;
- if (!config_file) {
- git_global_config_paths(&user_config, &xdg_config);
- config_file = user_config;
- if (!user_config)
- die(_("$HOME not set"));
- }
+ if (!config_file)
+ config_file = git_global_config();
rc = git_config_set_multivar_in_file_gently(
config_file, "maintenance.repo", maintpath,
CONFIG_REGEX_NONE, 0);
- free(user_config);
- free(xdg_config);
if (rc)
die(_("unable to add '%s' value of '%s'"),
@@ -1595,18 +1588,12 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
if (found) {
int rc;
- char *user_config = NULL, *xdg_config = NULL;
- if (!config_file) {
- git_global_config_paths(&user_config, &xdg_config);
- config_file = user_config;
- if (!user_config)
- die(_("$HOME not set"));
- }
+
+ if (!config_file)
+ config_file = git_global_config();
rc = git_config_set_multivar_in_file_gently(
config_file, key, NULL, maintpath,
CONFIG_FLAGS_MULTI_REPLACE | CONFIG_FLAGS_FIXED_VALUE);
- free(user_config);
- free(xdg_config);
if (rc &&
(!force || rc == CONFIG_NOTHING_SET))
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 487e326b3fa..a11e6c61520 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -67,6 +67,27 @@ test_expect_success 'maintenance.auto config option' '
test_subcommand ! git maintenance run --auto --quiet <false
'
+test_expect_success 'register uses XDG_CONFIG_HOME config if it exists' '
+ XDG_CONFIG_HOME=.config &&
+ test_when_finished rm -r "$XDG_CONFIG_HOME"/git/config &&
+ export "XDG_CONFIG_HOME" &&
+ mkdir -p "$XDG_CONFIG_HOME"/git &&
+ touch "$XDG_CONFIG_HOME"/git/config &&
+ git maintenance register &&
+ git config --file="$XDG_CONFIG_HOME"/git/config --get maintenance.repo >actual &&
+ pwd >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'register does not need XDG_CONFIG_HOME config to exist' '
+ test_when_finished git maintenance unregister &&
+ test_path_is_missing "$XDG_CONFIG_HOME"/git/config &&
+ git maintenance register &&
+ git config --global --get maintenance.repo >actual &&
+ pwd >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'maintenance.<task>.enabled' '
git config maintenance.gc.enabled false &&
git config maintenance.commit-graph.enabled true &&
--
2.42.0.2.g879ad04204
^ permalink raw reply related
* [PATCH v1 3/4] config: factor out global config file retrieval
From: Kristoffer Haugsbakk @ 2023-10-18 20:28 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, ps, stolee
In-Reply-To: <cover.1697660181.git.code@khaugsbakk.name>
Factor out code that retrieves the global config file so that we can use
it in `gc.c` as well.
Use the old name from the previous commit since this function acts
functionally the same as `git_system_config` but for “global”.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
builtin/config.c | 25 ++-----------------------
config.c | 24 ++++++++++++++++++++++++
config.h | 1 +
3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 6fff2655816..df06b766fad 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -708,30 +708,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)
}
if (use_global_config) {
- char *user_config, *xdg_config;
-
- git_global_config_paths(&user_config, &xdg_config);
- if (!user_config)
- /*
- * It is unknown if HOME/.gitconfig exists, so
- * we do not know if we should write to XDG
- * location; error out even if XDG_CONFIG_HOME
- * is set and points at a sane location.
- */
- die(_("$HOME not set"));
-
+ given_config_source.file = git_global_config();
given_config_source.scope = CONFIG_SCOPE_GLOBAL;
-
- if (access_or_warn(user_config, R_OK, 0) &&
- xdg_config && !access_or_warn(xdg_config, R_OK, 0)) {
- given_config_source.file = xdg_config;
- free(user_config);
- } else {
- given_config_source.file = user_config;
- free(xdg_config);
- }
- }
- else if (use_system_config) {
+ } else if (use_system_config) {
given_config_source.file = git_system_config();
given_config_source.scope = CONFIG_SCOPE_SYSTEM;
} else if (use_local_config) {
diff --git a/config.c b/config.c
index d2cdda96edd..2ff766c56ff 100644
--- a/config.c
+++ b/config.c
@@ -2111,6 +2111,30 @@ char *git_system_config(void)
return system_config;
}
+char *git_global_config(void)
+{
+ char *user_config, *xdg_config;
+
+ git_global_config_paths(&user_config, &xdg_config);
+ if (!user_config)
+ /*
+ * It is unknown if HOME/.gitconfig exists, so
+ * we do not know if we should write to XDG
+ * location; error out even if XDG_CONFIG_HOME
+ * is set and points at a sane location.
+ */
+ die(_("$HOME not set"));
+
+ if (access_or_warn(user_config, R_OK, 0) && xdg_config &&
+ !access_or_warn(xdg_config, R_OK, 0)) {
+ free(user_config);
+ return xdg_config;
+ } else {
+ free(xdg_config);
+ return user_config;
+ }
+}
+
void git_global_config_paths(char **user_out, char **xdg_out)
{
char *user_config = xstrdup_or_null(getenv("GIT_CONFIG_GLOBAL"));
diff --git a/config.h b/config.h
index 9f04de8ee3e..5cf961b548d 100644
--- a/config.h
+++ b/config.h
@@ -394,6 +394,7 @@ int config_error_nonbool(const char *);
#endif
char *git_system_config(void);
+char *git_global_config(void);
void git_global_config_paths(char **user, char **xdg);
int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
--
2.42.0.2.g879ad04204
^ permalink raw reply related
* [PATCH v1 2/4] config: rename global config function
From: Kristoffer Haugsbakk @ 2023-10-18 20:28 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, ps, stolee
In-Reply-To: <cover.1697660181.git.code@khaugsbakk.name>
Rename this function to a more descriptive name since we want to use the
existing name for a new function.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
builtin/config.c | 2 +-
builtin/gc.c | 4 ++--
builtin/var.c | 2 +-
config.c | 4 ++--
config.h | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 87d0dc92d99..6fff2655816 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -710,7 +710,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
if (use_global_config) {
char *user_config, *xdg_config;
- git_global_config(&user_config, &xdg_config);
+ git_global_config_paths(&user_config, &xdg_config);
if (!user_config)
/*
* It is unknown if HOME/.gitconfig exists, so
diff --git a/builtin/gc.c b/builtin/gc.c
index 5c4315f0d81..17fc031f63a 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1529,7 +1529,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
char *user_config = NULL, *xdg_config = NULL;
if (!config_file) {
- git_global_config(&user_config, &xdg_config);
+ git_global_config_paths(&user_config, &xdg_config);
config_file = user_config;
if (!user_config)
die(_("$HOME not set"));
@@ -1597,7 +1597,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
int rc;
char *user_config = NULL, *xdg_config = NULL;
if (!config_file) {
- git_global_config(&user_config, &xdg_config);
+ git_global_config_paths(&user_config, &xdg_config);
config_file = user_config;
if (!user_config)
die(_("$HOME not set"));
diff --git a/builtin/var.c b/builtin/var.c
index 74161bdf1c6..8e18b50b1e5 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -90,7 +90,7 @@ static char *git_config_val_global(int ident_flag UNUSED)
char *user, *xdg;
size_t unused;
- git_global_config(&user, &xdg);
+ git_global_config_paths(&user, &xdg);
if (xdg && *xdg) {
normalize_path_copy(xdg, xdg);
strbuf_addf(&buf, "%s\n", xdg);
diff --git a/config.c b/config.c
index 19f832818f1..d2cdda96edd 100644
--- a/config.c
+++ b/config.c
@@ -2111,7 +2111,7 @@ char *git_system_config(void)
return system_config;
}
-void git_global_config(char **user_out, char **xdg_out)
+void git_global_config_paths(char **user_out, char **xdg_out)
{
char *user_config = xstrdup_or_null(getenv("GIT_CONFIG_GLOBAL"));
char *xdg_config = NULL;
@@ -2186,7 +2186,7 @@ static int do_git_config_sequence(const struct config_options *opts,
data, CONFIG_SCOPE_SYSTEM,
NULL);
- git_global_config(&user_config, &xdg_config);
+ git_global_config_paths(&user_config, &xdg_config);
if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))
ret += git_config_from_file_with_options(fn, xdg_config, data,
diff --git a/config.h b/config.h
index 6332d749047..9f04de8ee3e 100644
--- a/config.h
+++ b/config.h
@@ -394,7 +394,7 @@ int config_error_nonbool(const char *);
#endif
char *git_system_config(void);
-void git_global_config(char **user, char **xdg);
+void git_global_config_paths(char **user, char **xdg);
int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
--
2.42.0.2.g879ad04204
^ permalink raw reply related
* [PATCH v1 1/4] config: format newlines
From: Kristoffer Haugsbakk @ 2023-10-18 20:28 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, ps, stolee
In-Reply-To: <cover.1697660181.git.code@khaugsbakk.name>
Remove unneeded newlines according to `clang-format`.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
Honestly the formatter changing these lines over and over again was just
annoying. And we're visiting the file anyway.
builtin/config.c | 1 -
config.c | 2 --
2 files changed, 3 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 11a4d4ef141..87d0dc92d99 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -760,7 +760,6 @@ int cmd_config(int argc, const char **argv, const char *prefix)
given_config_source.scope = CONFIG_SCOPE_COMMAND;
}
-
if (respect_includes_opt == -1)
config_options.respect_includes = !given_config_source.file;
else
diff --git a/config.c b/config.c
index 3846a37be97..19f832818f1 100644
--- a/config.c
+++ b/config.c
@@ -96,7 +96,6 @@ static long config_file_ftell(struct config_source *conf)
return ftell(conf->u.file);
}
-
static int config_buf_fgetc(struct config_source *conf)
{
if (conf->u.buf.pos < conf->u.buf.len)
@@ -3564,7 +3563,6 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
write_err_out:
ret = write_error(get_lock_file_path(&lock));
goto out_free;
-
}
void git_config_set_multivar_in_file(const char *config_filename,
--
2.42.0.2.g879ad04204
^ permalink raw reply related
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