* [PATCH 0/3] t: improve compatibility with NixOS
From: Patrick Steinhardt @ 2023-11-08 7:29 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]
Hi,
this patch series improves compatibility of our test suite with NixOS.
NixOS is somewhat special compared to more conventional Linux distros
because it doesn't follow the Filesystem Hierarchy Specification for
most of the part. Instead, packages are installed into the Nix store
in `/nix` with hashed paths that change frequently when upgrading the
system to a newer generation. Consequentially, paths cannot be hardcoded
and must instead be computed at runtime.
We have two such issues in our test harness right now:
- t/lib-httpd searches Apache httpd and its modules directory in a
list of hardcoded paths.
- t9164 doesn't propagate PATH to a script and thus cannot find the
basename(1) utility.
Both of these issues are fixed in this patch series. In addition, this
patch series fixes an upcoming issue in httpd's passwd files caused by
the deprecation of the crypt(3) function.
Patrick
Patrick Steinhardt (3):
t/lib-httpd: dynamically detect httpd and modules path
t/lib-httpd: stop using legacy crypt(3) for authentication
t9164: fix inability to find basename(1) in hooks
t/lib-httpd.sh | 51 ++++++++++++++++++---------
t/lib-httpd/passwd | 2 +-
t/lib-httpd/proxy-passwd | 2 +-
t/t9164-git-svn-dcommit-concurrent.sh | 12 +++++--
4 files changed, 45 insertions(+), 22 deletions(-)
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 8/9] for-each-ref: add option to fully dereference tags
From: Patrick Steinhardt @ 2023-11-08 7:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Victoria Dye, Victoria Dye via GitGitGadget, git
In-Reply-To: <xmqq4jhx7x8l.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2521 bytes --]
On Wed, Nov 08, 2023 at 12:14:02PM +0900, Junio C Hamano wrote:
> Victoria Dye <vdye@github.com> writes:
>
> > I think `^{}fieldname` would be a good candidate, but it's *extremely*
>
> Gaah. Why? fieldname^{} I may understand, but in the prefix form?
>
> In any case, has anybody considered that we may be better off to
> declare that "*field" peeling a tag only once is a longstanding bug?
>
> IOW, can we not add "fully peel" command line option or a new syntax
> and instead just "fix" the bug to fully peel when "*field" is asked
> for?
I see where you're coming from, but I wonder whether this wouldn't break
scripts. To me, the documentation seems to explicitly state that this
will only deref tags once:
If fieldname is prefixed with an asterisk (*) and the ref points at
a tag object, use the value for the field in the object which the
tag object refers to (instead of the field in the tag object).
So changing that now would break both the documented and the actual
behaviour. Now whether anybody actually cares about such a breaking
change is of course a different question, and you're probably correct
that in practice nobody does.
Patrick
> An application that cares about handling a chain of annotatetd tags
> would want to be able to say "this is the outermost tag's
> information; one level down, the tag was signed by this person;
> another level down, the tag was signed by this person, etc." which
> would mean either
>
> * we have a syntax that shows the information from all levels
> (e.g., "**taggername" may say "Victoria\nPatrick\nGitster")
>
> * we have a syntax that allows to specify how many levels to peel,
> (e.g., "*0*taggername" may be the same as "taggername",
> "*1*taggername" may be the same as "*taggername") plus some
> programming construct like variables and loops.
>
> but the repertoire being proposed that consists only of "peel only
> once" and "peel all levels" is way too insufficient.
>
> Note that I do not advocate for allowing inspection of each levels
> separately. Quite the contrary. I would say that --format=<>
> placeholder should not be a programming language to satisify such a
> niche need. And my conclusion from that stance is "peel once" plus
> "peel all" are already one level too many, and "peel once" was a
> very flawed implementation from day one, when 9f613ddd (Add
> git-for-each-ref: helper for language bindings, 2006-09-15)
> introduced it.
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 2/3] tmp-objdir: introduce `tmp_objdir_repack()`
From: Patrick Steinhardt @ 2023-11-08 7:05 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Jeff King, Elijah Newren, Junio C Hamano,
Johannes Schindelin
In-Reply-To: <0f19c139ba9bb5105747f545038825d0c89f2e42.1699381371.git.me@ttaylorr.com>
[-- Attachment #1: Type: text/plain, Size: 3155 bytes --]
On Tue, Nov 07, 2023 at 01:22:58PM -0500, Taylor Blau wrote:
> In the following commit, we will teach `git replay` how to write a pack
> containing the set of new objects created as a result of the `replay`
> operation.
>
> Since `replay` needs to be able to see the object(s) written
> from previous steps in order to replay each commit, the ODB transaction
> may have multiple pending packs. Migrating multiple packs back into the
> main object store has a couple of downsides:
>
> - It is error-prone to do so: each pack must be migrated in the
> correct order (with the ".idx" file staged last), and the set of
> packs themselves must be moved over in the correct order to avoid
> racy behavior.
>
> - It is a (potentially significant) performance degradation to migrate
> a large number of packs back into the main object store.
>
> Introduce a new function that combines the set of all packs in the
> temporary object store to produce a single pack which is the logical
> concatenation of all packs created during that level of the ODB
> transaction.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
> tmp-objdir.c | 13 +++++++++++++
> tmp-objdir.h | 6 ++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/tmp-objdir.c b/tmp-objdir.c
> index 5f9074ad1c..ef53180b47 100644
> --- a/tmp-objdir.c
> +++ b/tmp-objdir.c
> @@ -12,6 +12,7 @@
> #include "strvec.h"
> #include "quote.h"
> #include "object-store-ll.h"
> +#include "run-command.h"
>
> struct tmp_objdir {
> struct strbuf path;
> @@ -277,6 +278,18 @@ int tmp_objdir_migrate(struct tmp_objdir *t)
> return ret;
> }
>
> +int tmp_objdir_repack(struct tmp_objdir *t)
> +{
> + struct child_process cmd = CHILD_PROCESS_INIT;
> +
> + cmd.git_cmd = 1;
> +
> + strvec_pushl(&cmd.args, "repack", "-a", "-d", "-k", "-l", NULL);
> + strvec_pushv(&cmd.env, tmp_objdir_env(t));
I wonder what performance of this repack would be like in a large
repository with many refs. Ideally, I would expect that the repacking
performance should scale with the number of objects we have written into
the temporary object directory. But in practice, the repack will need to
compute reachability and thus also scales with the size of the repo
itself, doesn't it?
Patrick
> + return run_command(&cmd);
> +}
> +
> const char **tmp_objdir_env(const struct tmp_objdir *t)
> {
> if (!t)
> diff --git a/tmp-objdir.h b/tmp-objdir.h
> index 237d96b660..d00e3b3e27 100644
> --- a/tmp-objdir.h
> +++ b/tmp-objdir.h
> @@ -36,6 +36,12 @@ struct tmp_objdir *tmp_objdir_create(const char *prefix);
> */
> const char **tmp_objdir_env(const struct tmp_objdir *);
>
> +/*
> + * Combines all packs in the tmp_objdir into a single pack before migrating.
> + * Removes original pack(s) after installing the combined pack into place.
> + */
> +int tmp_objdir_repack(struct tmp_objdir *);
> +
> /*
> * Finalize a temporary object directory by migrating its objects into the main
> * object database, removing the temporary directory, and freeing any
> --
> 2.42.0.446.g0b9ef90488
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: first-class conflicts?
From: Elijah Newren @ 2023-11-08 6:31 UTC (permalink / raw)
To: phillip.wood; +Cc: Sandra Snan, git, Martin von Zweigbergk, Randall S. Becker
In-Reply-To: <ba047d38-4ad1-4440-8342-3379404f430b@gmail.com>
Hi Phillip,
On Tue, Nov 7, 2023 at 3:49 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Elijah
>
> [I've cc'd Martin to see if he has anything to add about how "jj"
> manages the issues around storing conflicts.]
+1. I'll add some other questions for him too while we're at it,
separately in this thread.
[...]
> > Martin also gave us an update at the 2023 Git Contributors summit, and
> > in particular noted a significant implementation change to not have
> > per-file storage of conflicts, but rather storing at the commit level
> > the multiple conflicting trees involved. That model might be
> > something we could implement in Git. And if we did, it'd solve
> > various issues such as people wanting to be able to stash conflicts,
> > or wanting to be able to partially resolve conflicts and fix it up
> > later, or be able to collaboratively resolve conflicts without having
> > everyone have access to the same checkout.
>
> One thing to think about if we ever want to implement this is what other
> data we need to store along with the conflict trees to preserve the
> context in which the conflict was created. For example the files that
> are read by "git commit" when it commits a conflict resolution. For a
> single cherry-pick/revert it would probably be fairly straight forward
> to store CHERRY_PICK_HEAD/REVERT_HEAD and add it as a parent so it gets
> transferred along with the conflicts.
This is a great thing to think about and bring up. However, I'm not
sure what part of it actually needs to be preserved; in fact, it's not
clear to me that any of it needs preserving -- especially not the
files read by "git commit". A commit was already created, after all.
It seems that CHERRY_PICK_HEAD/REVERT_HEAD files exist primarily to
clue in that we are in-the-middle-of-<op>, and the conflict header
(the "tree A + tree B - tree C" thing; whatever that's called)
similarly provides signal that the commit still has conflicts.
Secondarily, these files contain information about the tree we came
from and its parent tree, which allows users to investigate the diff
between those...but that information is also available from the
conflict header in the recorded commit. The CHERRY_PICK_HEAD and
REVERT_HEAD files could also be used to access the commit message, but
that would have been stored in the conflicted commit as well. Are
there any other pieces of information I'm missing?
> For a sequence of cherry-picks or
> a rebase it is more complicated to preserve the context of the conflict.
I think the big piece here is whether we also want to adopt jj's
behavior of automatically rebasing all descendant commits when
checking out and amending some historical commit (or at least having
the option of doing so). That behavior allows users to amend commits
to resolve conflicts without figuring out complicated interactive
rebases to fix all the descendant commits across all relevant
branches. Without that feature, I agree this might be a bit more
difficult, but with that feature, I'm having a hard time figuring out
what context we actually need to preserve for a sequence of
cherry-picks or a rebase.
Digging into a few briefly...
Many of the state files are about the status of the in-progress
operation (todo-list, numbers of commits done and to do, what should
be done with not-yet-handled commits, temporary refs corresponding to
temporary labels that need to be deleted, rescheduling failed execs,
dropping or keeping redundant commits, etc.), but if the operation has
completed and new commits created (potentially with multiple files
with conflict headers), I don't see how this information is useful
anymore.
There are some special state files related to half-completed
operations (e.g. squash commits when we haven't yet reached the final
one in the sequence, a file to note that we want to edit a commit
message once the user has finished resolving conflicts, whether we
need to create a new root commit), but again, the operation has
completed and commits have been created with appropriate parentage and
commit messages so I don't think these are useful anymore either.
Other state files are related to things needing to be done at the end
of the operation, like invoke the post-rewrite hook or pop the
autostash (with knowledge of what was rewritten to what). But the
operation would have been completed and those things done already, so
I don't see how this is necessary either.
Some state files are for controlling how commits are created (setting
committer date to author date, gpg signing options, whether to add
signoff), but, again, commits have already been created, and can be
further amended as the user wants (hopefully including resolving the
conflicts).
The biggest issue is perhaps that REBASE_HEAD is used in the
implementation of `git rebase --show-current-patch`, but all
information stored in that is still accessible -- the commit message
is stored in the commit, the author time is stored in the commit, and
the trees involved are in the conflict header. The only thing missing
is committer timestamp, which isn't relevant anyway.
The only ones I'm pausing a bit on are the strategy and
strategy-options. Those might be useful somehow...but I can't
currently quite put my finger on explaining how they would be useful
and I'm not sure they are.
Am I missing anything?
> Even "git merge" can create several files in addition to MERGE_HEAD
> which are read when the conflict resolution is committed.
That's a good one to bring up too, but I'm not sure I understand how
these could be useful to preserve either. Am I missing something? My
breakdown:
* MERGE_HEAD: was recorded in the commit as a second parent, so we
already have that info
* MERGE_MSG: was recorded in the commit as the commit message, so
again we already have that info
* MERGE_AUTOSTASH: irrelevant since the stashed stuff isn't part of
the commit and was in fact unstashed after the
merge-commit-with-conflicts was created
* MERGE_MODE: irrelevant since it's only used for reducing heads at
time of git-commit, and git-commit has already been run
* MERGE_RR: I think this is irrelevant; the conflict record (tree A
+ tree B - tree C) lets us redo the merge if needed to get the list of
conflicted files and textual conflicts found therein
So I don't see how any of the information in these files need to be
recorded as additional auxiliary information. However, that last item
might depend upon the strategy and strategy-options, which currently
is not recorded...hmm....
> > But we'd also have to be careful and think through usecases, including
> > in the surrounding community. People would probably want to ensure
> > that e.g. "Protected" or "Integration" branches don't get accept
> > fetches or pushes of conflicted commits,
>
> I think this is a really important point, while it can be useful to
> share conflicts so they can be collaboratively resolved we don't want to
> propagate them into "stable" or production branches. I wonder how 'jj'
> handles this.
Yeah, figuring this out might be the biggest sticking point.
> > git status would probably
> > need some special warnings or notices, git checkout would probably
> > benefit from additional warnings/notices checks for those cases, git
> > log should probably display conflicted commits differently, we'd need
> > to add special handling for higher order conflicts (e.g. a merge with
> > conflicts is itself involved in a merge) probably similar to what jj
> > has done, and audit a lot of other code paths to see what would be
> > needed.
>
> As you point out there is a lot more to this than just being able to
> store the conflict data in a commit - in many ways I think that is the
> easiest part of the solution to sharing conflicts.
Yeah, another one I just thought of is that the trees referenced in
the conflicts would also need to affect reachability computations as
well, to make sure they both don't get gc'ed and that they are
transferred when appropriate. There are lots of things that would be
involved in implementing this idea.
^ permalink raw reply
* Re: [PATCH] diff: implement config.diff.renames=copies-harder
From: Elijah Newren @ 2023-11-08 4:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sam James via GitGitGadget, git, Sam James
In-Reply-To: <xmqqv8ac7utk.fsf@gitster.g>
On Tue, Nov 7, 2023 at 8:06 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> >> > We probably don't want to copy all three of those sentences here, but
> >> > I think we need to make sure users can find them, thus my suggestion
> >> > to reference the `--find-copies-harder` option to git-diff so that
> >> > affected users can get the info they need to choose.
> >>
> >> "in addition to paths that are different, will look for more copies
> >> even in unmodified paths" then?
> >
> > That's much better. I still slightly prefer referencing
> > `--find-copies-harder` so that there's a link between "copies-harder"
> > and `--find-copies-harder`; but this version would also be fine.
>
> Oh, I didn't mean "use this rewrite and do not make any external
> reference". More like "external reference is a good idea and
> necessary to help motivated readers, but we should give enough
> information inline, and I think this level of details would be
> sufficient".
Ah, gotcha. Yeah, this sounds good.
^ permalink raw reply
* Re: [PATCH] diff: implement config.diff.renames=copies-harder
From: Junio C Hamano @ 2023-11-08 4:06 UTC (permalink / raw)
To: Elijah Newren; +Cc: Sam James via GitGitGadget, git, Sam James
In-Reply-To: <CABPp-BEgxKn3QvJQ+6L3Z1RN1im=c3dfApLRCrQqum_Yim44Gw@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
>> > We probably don't want to copy all three of those sentences here, but
>> > I think we need to make sure users can find them, thus my suggestion
>> > to reference the `--find-copies-harder` option to git-diff so that
>> > affected users can get the info they need to choose.
>>
>> "in addition to paths that are different, will look for more copies
>> even in unmodified paths" then?
>
> That's much better. I still slightly prefer referencing
> `--find-copies-harder` so that there's a link between "copies-harder"
> and `--find-copies-harder`; but this version would also be fine.
Oh, I didn't mean "use this rewrite and do not make any external
reference". More like "external reference is a good idea and
necessary to help motivated readers, but we should give enough
information inline, and I think this level of details would be
sufficient".
Thanks.
^ permalink raw reply
* Re: [PATCH] diff: implement config.diff.renames=copies-harder
From: Elijah Newren @ 2023-11-08 3:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sam James via GitGitGadget, git, Sam James
In-Reply-To: <xmqqttpx828i.fsf@gitster.g>
On Tue, Nov 7, 2023 at 5:26 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > I find that marginally better; but I still don't think it answers the
> > user's question of why they should pick one option or the other. The
> > wording for the `--find-copies-harder` does explain when it's useful:
> >
> > For performance reasons, by default, `-C` option finds copies only
> > if the original file of the copy was modified in the same
> > changeset. This flag makes the command
> > inspect unmodified files as candidates for the source of
> > copy. This is a very expensive operation for large
> > projects, so use it with caution.
> >
> > We probably don't want to copy all three of those sentences here, but
> > I think we need to make sure users can find them, thus my suggestion
> > to reference the `--find-copies-harder` option to git-diff so that
> > affected users can get the info they need to choose.
>
> "in addition to paths that are different, will look for more copies
> even in unmodified paths" then?
That's much better. I still slightly prefer referencing
`--find-copies-harder` so that there's a link between "copies-harder"
and `--find-copies-harder`; but this version would also be fine.
^ permalink raw reply
* Re: [PATCH 8/9] for-each-ref: add option to fully dereference tags
From: Junio C Hamano @ 2023-11-08 3:14 UTC (permalink / raw)
To: Victoria Dye; +Cc: Patrick Steinhardt, Victoria Dye via GitGitGadget, git
In-Reply-To: <cf691b7c-288f-4cc9-a2ac-1a43972ae446@github.com>
Victoria Dye <vdye@github.com> writes:
> I think `^{}fieldname` would be a good candidate, but it's *extremely*
Gaah. Why? fieldname^{} I may understand, but in the prefix form?
In any case, has anybody considered that we may be better off to
declare that "*field" peeling a tag only once is a longstanding bug?
IOW, can we not add "fully peel" command line option or a new syntax
and instead just "fix" the bug to fully peel when "*field" is asked
for?
An application that cares about handling a chain of annotatetd tags
would want to be able to say "this is the outermost tag's
information; one level down, the tag was signed by this person;
another level down, the tag was signed by this person, etc." which
would mean either
* we have a syntax that shows the information from all levels
(e.g., "**taggername" may say "Victoria\nPatrick\nGitster")
* we have a syntax that allows to specify how many levels to peel,
(e.g., "*0*taggername" may be the same as "taggername",
"*1*taggername" may be the same as "*taggername") plus some
programming construct like variables and loops.
but the repertoire being proposed that consists only of "peel only
once" and "peel all levels" is way too insufficient.
Note that I do not advocate for allowing inspection of each levels
separately. Quite the contrary. I would say that --format=<>
placeholder should not be a programming language to satisify such a
niche need. And my conclusion from that stance is "peel once" plus
"peel all" are already one level too many, and "peel once" was a
very flawed implementation from day one, when 9f613ddd (Add
git-for-each-ref: helper for language bindings, 2006-09-15)
introduced it.
^ permalink raw reply
* Job vacancy
From: St. Thomas' Hospital UK @ 2023-11-08 2:19 UTC (permalink / raw)
To: git
St. Thomas' Hospital UK
REF: HR/MED-004/06923
St. Thomas' Hospital UK is a large NHS teaching hospital in Central
London, England. It is one of the institutions that compose the King's
Health Partners, an academic health science Center. Administratively
part of the Guy's and St Thomas' NHS Foundation Trust, together with
Guy's Hospital and King's College Hospital, it provides the location
of the King's College London GKT School of Medical Education.
It is ranked amongst the best Ten (10) hospitals in the United Kingdom
with 840 beds. The hospital has provided healthcare freely or under
charitable auspices since the 12th century. It is one of London's most
famous hospitals, associated with names such as Sir Astley Cooper,
William Cheselden, Florence Nightingale, Linda Richards, Edmund
Montgomery, Agnes Elizabeth Jones and Sir Harold Ridley. It is a
prominent London landmark =E2=80=93 largely due to its location on the
opposite bank of the River Thames to the Houses of Parliament.
The largest not-for-profit health system in the world, we provide high
quality, personalized and compassionate care to our patients through
our dedication to safety, rigorous self-assessment, performance
improvement, corporate integrity and health service management. We are
committed to being the per-eminent provider of acute inpatient and
outpatient health care services.
DESCRIPTION: Following the COVID-19 outbreak, expansion and
development in our hospital, we are currently recruiting and employing
the services of Medical Professionals (Specialists, Consultants,
General Practitioners) with relevant experiences to fill in the
following below vacancies in our health care facility in the United
Kingdom.
AREAS OF VACANCIES:
StH1. ALLERGY & IMMUNOLOGY StH2. ANAESTHESIOLOGY StH3. ANGIOLOGY StH4.
ANTHROPOSOPHIC MEDICINE StH5. BREAST SURGERY StH6. CARDIOLOGY StH7.
CRANIOSACRAL PRACTITIONER / THERAPIST StH8. CARDIOTHORACIC SURGERY
StH9. CARDIAC SURGERY
StH10. CRITICAL CARE MEDICINE StH11. DENTISTS StH12. DENTAL SURGEON
StH13. DERMATOLOGY StH14. ENDOCRINOLOGY
StH15. EMERGENCY MEDICINE StH16. GASTROENTEROLOGY StH17. GENERAL
SURGERY StH18. GENERAL PAEDIATRICS StH19. GENERAL MEDICINE StH20.
HEMATOLOGY StH21. HYPERTENSION SPECIALIST StH22. INTERNAL MEDICINE
StH23. INFECTOLOGY StH24. MORPHOLOGY StH25. NEPHROLOGY StH26.
NEUROSURGERY StH27. NEONATOLOGY StH28. ORTHOPAEDICS StH29. ORTHOPAEDIC
SURGERY StH30. OTORHINOLARYNGOLOGY StH31. ORTHODONTIST StH32.
OCCUPATIONAL MEDICINE StH33. ORAL AND MAXILLOFACIAL SURGERY StH34.
PATHOLOGY
StH35. PLASTIC & RECONSTRUCTIVE SURGERY StH36. PNEUMOLOGY StH37.
PAEDIATRIC SURGEON StH38. PSYCHOLOGIST StH39. PHYSIOTHERAPY StH40.
PEDIATRICS StH41. PUBLIC HEALTH StH42. RADIOLOGY StH43. RHEUMATOLOGY
StH44. REHABILITATION MEDICINE StH45. RESPIRATORY MEDICINE StH46.
THORACIC SURGERY StH47. TRAUMATOLOGY StH48. TRICHOLOGIST StH49.
UROLOGY
JOB LOCATION: London, United Kingdom
JOB COMMENCEMENT: 2023
EMPLOYMENT TYPE: Contract / Full-time
EMPLOYMENT BENEFITS:
Excellent Salary and Overtime Bonus, Health/life Insurance, Relocation
expenses, Research and Educational assistance, Medical, Optical and
Dental Care, Family/Single housing accommodation, 24/7 Official
Vehicle, Scholarship for employee's dependent within UK schools.
Interested applicants are to send a detailed attachment Resume via email: recruitment@gsttsnhsuk.online
NOTE: APPLICATION IS OPEN TO INTERESTED PERSONS FROM ALL INTERNATIONAL
LOCATIONS, ALL SUCCESSFUL APPLICANTS IN OUR RECRUITMENT PROCESS MUST
BE WILLING TO RELOCATE TO THE UK FOR WORK.
Coronavirus (COVID-19)- Stay at home if you feel unwell. If you have a
fever, cough and difficulty breathing, seek medical attention and
call-in advance. Follow the directions of your local health authority.
Source: World Health Organization
Sincerely,
Julie Screaton,
St. Thomas' Hospital Uk
Guy's & St. Thomas NHS Foundation Trust
London, United Kingdom @2023
St Thomas' Hospital UK incorporated in England, UK (Reg. No: 06160266)
having its registered address at Westminster Bridge Rd, London SE1
7EH, England.
^ permalink raw reply
* Re: Error when "git mv" file in a sparsed checkout
From: Elijah Newren @ 2023-11-08 2:21 UTC (permalink / raw)
To: Josef Wolf, git
In-Reply-To: <20231107130303.GS7041@raven.inka.de>
Hi,
On Tue, Nov 7, 2023 at 5:32 AM Josef Wolf <jw@raven.inka.de> wrote:
>
> Hello,
>
> I have used the procedure described below for many years. In fact,
> this procedure is part of a script which I am using for about 10 years.
> This procedure was definitely working with git-2-25-1 and git-2.26.2.
>
> Now, with git-2.34.1 (on a freshly installed ubuntu-22.04), this
> procedure fails.
>
> Here is what I do:
>
> I want to rename a file on a branch which is not currently checked out
> without messing/touching my current working directory.
>
> For this, I first create a clone of the repo with shared git-directory:
>
> $ SANDBOX=/var/tmp/manage-scans-X1pKZQiey
> $ WT=$SANDBOX/wt
> $ GIT=$SANDBOX/git
>
> $ mkdir -p $SANDBOX
> $ git --work-tree $WT --git-dir $GIT clone -qns -n ~/upstream-repo $GIT
>
> Then, I do a sparse checkout in this clone, containing only the file
> that is to be renamed:
>
> $ cd $WT
> $ echo 'path/to/old-filename' >>$GIT/info/sparse-checkout
> $ git --work-tree $WT --git-dir $GIT config core.sparsecheckout true
> $ git --work-tree $WT --git-dir $GIT checkout -b the-branch remotes/origin/the-branch
> Switched to a new branch 'the-branch'
>
> Next step would be to "git mv" the file:
>
> $ mkdir -p /path/to # already exists, but should do no harm
> $ git --work-tree $WT --git-dir $GIT mv path/to/old-filename path/to/new-filename
sparse checkouts are designed such that only files matching the
patterns in the sparse-checkout file should be present in the working
tree, so renaming to a path that should not be present is problematic.
We could possibly have "git-mv" immediately remove the path from the
working tree (while leaving the new pathname in the index), but that's
problematic in that users often overlook the index and only look at
the working tree and might think the file was deleted instead of
renamed. Not immediately removing it is potentially even worse,
because any subsequent operation (particularly ones like checkout,
reset, merge, rebase, etc.) are likely to nuke the file from the
working tree and the fact that the removal is delayed makes it much
harder for users to understand and diagnose.
So, Stolee fixed this to make it throw an error; see
https://lore.kernel.org/git/pull.1018.v4.git.1632497954.gitgitgadget@gmail.com/
for details. His description did focus on cone mode, but you'll note
that none of my explanation here did. The logic for making this an
error fully applies to non-cone mode for all the same reasons.
If you want to interact with `path/to/new-filename` as a path within
your sparse checkout (as suggested by your git-mv command), then that
path should actually be part of your sparse checkout. In other words,
you should add `path/to/new-filename` to $GIT/info/sparse-checkout and
do so _before_ attempting your `git mv` command. If you don't like
that for some reason, you are allowed to instead ignore the
problematic consequences of renaming outside the sparse-checkout by
providing the `--sparse` flag. Both of these possibilities are
documented in the hints provided along with the error message you
showed below:
> The following paths and/or pathspecs matched paths that exist
> outside of your sparse-checkout definition, so will not be
> updated in the index:
> path/to/new-filename
> hint: If you intend to update such entries, try one of the following:
> hint: * Use the --sparse option.
> hint: * Disable or modify the sparsity rules.
> hint: Disable this message with "git config advice.updateSparsePath false"
>
> This error is something I have not expected.
>
> Error message suggests, there already exists a file named "new-filename". This
> is not true at all. There is no file named "new-filename" in the entire
> repository. Not in any directory of any branch.
You are correct; the wording of the error message here is suboptimal
and seems to have been focused more on the git-add case (the error
message is shared by git-add, git-mv, and git-rm). Thanks for
pointing it out! We could improve that wording, perhaps with
something like:
The following paths and/or pathspecs match paths that are
outside of your sparse-checkout definition, so will not be
updated:
Which is still slightly slanted towards git-add and git-rm cases, but
I hope it works better than the current message. Thoughts?
^ permalink raw reply
* Re: issue unable to commit file and folder name to upper lower case
From: Junio C Hamano @ 2023-11-08 1:48 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: chengpu lee, git
In-Reply-To: <20231107173557.GA29083@tb-raspi4>
Torsten Bögershausen <tboegi@web.de> writes:
> Yes, that is a restriction in Git, call it a bug, call it a missing feature.
> Unless someone fixes it, the recommendation is still to use a workaround:
>
> tb@pc:/tmp/ttt> git mv Abc tmp
> tb@pc:/tmp/ttt> git mv tmp abc
> tb@pc:/tmp/ttt> git status
> On branch master
> Changes to be committed:
> (use "git restore --staged <file>..." to unstage)
> renamed: Abc/.keep -> abc/.keep
>
> tb@pc:/tmp/ttt>
Correct and very helpful suggestion. Or get a better filesystem ;-)
Thanks.
^ permalink raw reply
* Re: Regression: git send-email Message-Id: numbering doesn't start at 1 any more
From: Junio C Hamano @ 2023-11-08 1:34 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Michael Strawbridge, Douglas Anderson, git, entwicklung
In-Reply-To: <20231107070632.spe3cappk5b5jg3q@pengutronix.de>
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> The output of git send-email dumps the messages it sends out and then I
> pick the message-id of the last mail by cut-n-paste and call my script
> with that as a parameter.
Yikes. I was hoping that the whole "dumps the messages" output is
read by the script, so that it does not have to assume anything
about the Message-ID format (like "it has some unchanging parts,
the changing parts begin with 1, and it increments by 1 for each
message").
You could give Message-ID externally to the output of format-patch
before feeding send-email, which would just use them, and that way
you would have more control over the entire process, I guess.
^ permalink raw reply
* Re: [PATCH 0/9] for-each-ref optimizations & usability improvements
From: Victoria Dye @ 2023-11-08 1:31 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Junio C Hamano, Victoria Dye via GitGitGadget, git
In-Reply-To: <ZUoWPpFHEi-PZjoD@tanuki>
Patrick Steinhardt wrote:
> On Mon, Nov 06, 2023 at 06:48:29PM -0800, Victoria Dye wrote:
>> Junio C Hamano wrote:
>>> "Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:
> [snip]
>>>> * I'm not attached to '--full-deref' as a name - if someone has an idea for
>>>> a more descriptive name, please suggest it!
>>>
>>> Another candidate verb may be "to peel", and I have no strong
>>> opinion between it and "to dereference". But I have a mild aversion
>>> to an abbreviation that is not strongly established.
>>>
>>
>> Makes sense. I got the "deref" abbreviation for 'update-ref --no-deref', but
>> 'show-ref' has a "--dereference" option and protocol v2's "ls-refs" includes
>> a "peel" arg. "Dereference" is the term already used in the 'for-each-ref'
>> documentation, though, so if no one comes in with an especially strong
>> opinion on this I'll change the option to '--full-dereference'. Thanks!
>
> But doesn't dereferencing in the context of git-update-ref(1) refer to
> something different? It's not about tags, but it is about symbolic
> references and whether we want to update the symref or the pointee. But
> true enough, in git-show-ref(1) "dereference" actually means that we
> should peel the tag.
Since both annotated tags and symbolic refs are essentially pointers, it's
not surprising that they both use the term "dereference." Even though
"deref" refers to symbolic refs in 'update-ref', its existence as an
abbreviation for "dereference" is relevant when coming up with a way to
abbreviate "dereference" when referring to tags.
>
> To me it feels like preexisting commands are confused already. In my
> mind model:
>
> - "peel" means that an object gets resolved to one of its pointees.
> This also includes the case here, where a tag gets peeled to its
> pointee.
>
> - "dereference" means that a symbolic reference gets resolved to its
> pointee. This matches what we do in `git update-ref --no-deref`.
>
> But after reading through the code I don't think we distinguish those
> terms cleanly throughout our codebase. Still, "peeling" feels like a
> better match in my opinion.
Hmm. I think I mostly agree on your definition of "peel". In the docs, it's
used to refer to:
- recursively resolving an OID to an object of a specified type [1]
- recursively resolving a tag OID to a non-tag object [2]
Notably, there seems to be a strong association of "peeling" to "recursive
resolution". Which means it doesn't necessarily describe what "*" currently
does.
"Dereference" generally seems like a looser term than what you've suggested.
It does refer to symbolic ref resolution as you describe [3], but "recursive
dereference" is definitely also a synonym for "peel" [4]. That, combined
with the fact that "*" is the "dereference operator", leads me to believe
that "%(*fieldname)" would accurately be described as a "tag dereference"
field in the context of 'for-each-ref'.
As I mentioned in [5], I'm going to try adding this functionality with a
field specifier rather than a command line option, so the name of the option
might be moot. But, since dereferencing/peeling will still be relevant to
the changes, I'll make sure the terminology I use in the documentation is as
precise as possible (i.e., use "peel" where I previously used "fully
dereference").
Separately, this has inspired me to revisit something I've been putting off,
which is to add a definition for "peel" (and now probably "dereference" as
well) in 'gitglossary'. I'll try to send that out in the next couple days.
Thanks!
[1] https://git-scm.com/docs/git-rev-parse#Documentation/git-rev-parse.txt---verify
[2] https://git-scm.com/docs/gitprotocol-v2#_ls_refs
[3] https://git-scm.com/docs/gitglossary#Documentation/gitglossary.txt-aiddefsymrefasymref
[4] https://git-scm.com/docs/gitglossary#Documentation/gitglossary.txt-aiddefcommit-ishacommit-ishalsocommittish
[5] https://lore.kernel.org/git/cf691b7c-288f-4cc9-a2ac-1a43972ae446@github.com/
>
> Patrick
^ permalink raw reply
* Re: [PATCH] diff: implement config.diff.renames=copies-harder
From: Junio C Hamano @ 2023-11-08 1:26 UTC (permalink / raw)
To: Elijah Newren; +Cc: Sam James via GitGitGadget, git, Sam James
In-Reply-To: <CABPp-BF9iUkF+g_w7wLATFTmjfJ3f1hsBr+zXxNZEcq-XiNOWg@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
>> True. "often copies of a previous version" means that it is a
>> directory that has a collection of subdirectories, one for each
>> version? In a source tree managed in a version control system,
>> files are often rewritten in place from the previous version,
>> so I am puzzled by that justification.
>>
>> It is, in the proposed log message of our commits, a bit unusual to
>> see "This patch does X" and "I do Y", by the way, which made my
>> reading hiccup a bit, but perhaps it is just me?
>
> I think I read Sam's description a bit differently than you. My
> assumption was they'd have files with names like the following in the
> same directory:
> gcc-13.x.build.recipe
> gcc-12.x.build.recipe
> gcc-11.x.build.recipe
> gcc-10.x.build.recipe
>
> And that gcc-13.x.build.recipe was started as a copy of
> gcc-12.x.build.recipe (which was started as a copy of
> gcc-11.x.build.recipe, etc.). They keep all versions because they
> want users to be able to build and install multiple gcc versions.
OK, "previous version" is within the context of "variants of gcc",
and to us, there is no distinction among them (we do not care which
ones are older than the others---we need to keep track of them all).
Which makes sense. OK.
> I find that marginally better; but I still don't think it answers the
> user's question of why they should pick one option or the other. The
> wording for the `--find-copies-harder` does explain when it's useful:
>
> For performance reasons, by default, `-C` option finds copies only
> if the original file of the copy was modified in the same
> changeset. This flag makes the command
> inspect unmodified files as candidates for the source of
> copy. This is a very expensive operation for large
> projects, so use it with caution.
>
> We probably don't want to copy all three of those sentences here, but
> I think we need to make sure users can find them, thus my suggestion
> to reference the `--find-copies-harder` option to git-diff so that
> affected users can get the info they need to choose.
"in addition to paths that are different, will look for more copies
even in unmodified paths" then?
^ permalink raw reply
* Re: [PATCH 8/9] for-each-ref: add option to fully dereference tags
From: Victoria Dye @ 2023-11-08 1:13 UTC (permalink / raw)
To: Patrick Steinhardt, Victoria Dye via GitGitGadget; +Cc: git
In-Reply-To: <ZUoWWo7IEKsiSx-C@tanuki>
Patrick Steinhardt wrote:
> On Tue, Nov 07, 2023 at 01:26:00AM +0000, Victoria Dye via GitGitGadget wrote:
>> From: Victoria Dye <vdye@github.com>
>>
>> Add a boolean flag '--full-deref' that, when enabled, fills '%(*fieldname)'
>> format fields using the fully peeled target of tag objects, rather than the
>> immediate target.
>>
>> In other builtins ('rev-parse', 'show-ref'), "dereferencing" tags typically
>> means peeling them down to their non-tag target. Unlike these commands,
>> 'for-each-ref' dereferences only one "level" of tags in '*' format fields
>> (like "%(*objectname)"). For most annotated tags, one level of dereferencing
>> is enough, since most tags point to commits or trees. However, nested tags
>> (annotated tags whose target is another annotated tag) dereferenced once
>> will point to their target tag, different a full peel to e.g. a commit.
>>
>> Currently, if a user wants to filter & format refs and include information
>> about the fully dereferenced tag, they can do so with something like
>> 'cat-file --batch-check':
>>
>> git for-each-ref --format="%(objectname)^{} %(refname)" <pattern> |
>> git cat-file --batch-check="%(objectname) %(rest)"
>>
>> But the combination of commands is inefficient. So, to improve the
>> efficiency of this use case, add a '--full-deref' option that causes
>> 'for-each-ref' to fully dereference tags when formatting with '*' fields.
>
> I do wonder whether it would make sense to introduce this feature in the
> form of a separate field prefix, as you also mentioned in your cover
> letter. It would buy the user more flexibility, but the question is
> whether such flexibility would really ever be needed.
>
> The only thing I could really think of where it might make sense is to
> distinguish tags that peel to a commit immediately from ones that don't.
> That feels rather esoteric to me and doesn't seem to be of much use. But
> regardless of whether or not we can see the usefulness now, if this
> wouldn't be significantly more complex I wonder whether it would make
> more sense to use a new field prefix instead anyway.
>
> In any case, I think it would be helpful if this was discussed in the
> commit message.
I've been going back and forth on this, but I think a field specifier might
be the way to go after all. Using a field specifier would inherently be more
complex than the command line option (since the formatting code is a bit
complicated), but that's not an insurmountable problem. The thing I kept
getting caught up on was which symbol (or symbols?) to use to indicate a full
object peel. I mentioned `**fieldname` in the cover letter, but that looks
more like a double dereference than a recursive one.
I think `^{}fieldname` would be a good candidate, but it's *extremely*
important (for the sake of avoiding user confusion/frustration) that it
produces the same object & associated info as the standard revision parsing
machinery [1]. One notable difference (it might be the only one) from
`*fieldname` would be, if a ref points to a non-tag object, then that
object's information would printed (rather than an empty string). But maybe
that difference is what we'd want anyway, since it's a better one-for-one
replacement of 'git for-each-ref | git cat-file --batch-check'.
I'll try implementing that for V2. If it doesn't work for some reason,
though, I'll explain why in the commit message.
[1] https://git-scm.com/docs/git-rev-parse#Documentation/git-rev-parse.txt-emltrevgtemegemv0998em
>
> Patrick
>
^ permalink raw reply
* Tender Enquiry | UAE-Oman Railway Link Project
From: P.V. Soma Sekhar @ 2023-11-07 14:32 UTC (permalink / raw)
To: git
[-- Attachment #1.1: Type: text/plain, Size: 935 bytes --]
Sayın kullanıcı,
Umarım bu mesaj sizi iyi bulur.
Aşağıdaki malzemeler için teklifinizi istiyoruz,
Ekte Sipariş örneğimiz ve şirket bilgilerimiz bulunmaktadır,
Lütfen bize en son kataloğunuzu rekabetçi fiyatlarla gönderin.
1'den 4'e kadar olan maddelere acilen ihtiyacımız var, lütfen müsaitlik durumunu belirtin.
Acil bir soruşturmamız olduğu için nazik ve acil geri bildiriminizi bekliyoruz.
Hızlı yanıtınız takdir edilecektir.
Best Regards,
P.V.SOMA SEKHAR
Project Manager
Telecom Infrastructure Project Business Unit
Technology & Communication Division
Infrastructure, Technology, Industrial & Consumer Solutions
MOHSIN HAIDER DARWISH LLC
P. O. Box 880, Ruwi – 112,
Muscat, Sultanate of Oman
Office: +968 24835500
Fax : +968 24833369
Mob. : + 968-98632229
soma.s.@mhd.co.om mailto:soma.s.@mhd.co.om
www.mhdoman.com http://www.mhdoman.com/
[-- Attachment #1.2: Type: text/html, Size: 11280 bytes --]
[-- Attachment #2: Tender Enquiry UAE-Oman Railway Link Project.xlam --]
[-- Type: application/octet-stream, Size: 689376 bytes --]
^ permalink raw reply
* Re: git-send-email: Send with mutt(1)
From: Jeff King @ 2023-11-07 20:16 UTC (permalink / raw)
To: Alejandro Colomar; +Cc: git
In-Reply-To: <ZUqDwnmu9d1dD1tb@devuan>
On Tue, Nov 07, 2023 at 07:36:44PM +0100, Alejandro Colomar wrote:
> > I assume what you want out of send-email here is the actual generation
> > of patch emails. But under the hood that is all done by git-format-patch
> > anyway. So for example if you do:
>
> Yeah, most of it is done by format-patch. There are few things I
> actually need from send-email. One of them is generating the Cc from
> the sign-offs and other tags found in the patch.
>
> I had been thinking these days that it would be useful to have
> format-patch generate those. How about adding a --signed-off-by-cc to
> format-patch?
That seems like a reasonable feature. Probably it should be
--cc-from-trailer=signed-off-by, and then you could do the same with
other trailers.
It feels like you could _almost_ do it with the existing
--format='%(trailers)' functionality, but there's no way to say "do the
regular --format=email output, but also stick this extra format in the
headers section". Plus there are probably some niceties you'd get from
Git knowing that you're adding headers (like de-duping addresses).
That feature might end up somewhat hairy, though, as then you get into
questions of parsing address lists, etc. We do all that now in perl with
send-email, where we can lean on some parsing libraries. So I dunno.
> > If you're sending a long series, it's helpful to pre-populate various
> > headers in the format-patch command with "--to", etc. I usually do so by
> > sending the cover letter directly via mutt, and then using some perl
> > hackery to convert those headers into format-patch args. The script I
>
> Indeed, that hackery is what send-email already does, so how about
> moving those features a bit upstream so that format-patch can do them
> too?
Yeah, if they existed in format-patch I might be able to reuse them. I
am hesitant, though, just because handling all the corner cases on
parsing is going to be a bit of new C code.
> Although then, maybe it's simpler to teach send-email to learn to use
> mutt(1) under the hood for the actual send.
I think you will find some corner cases in trying to make mutt act just
like an mta accepting delivery. Two I can think of:
1. It will take a body on stdin, but not a whole message. We can hack
around that with some postponed-folder magic, though.
2. Bcc headers are stripped before sendmail sees the message (but
those addresses appear on the command-line). Converting that back
to bcc so that mutt can then re-strip them would be annoying but
possible. If you don't use bcc, it probably makes sense to just
punt on this.
So maybe a script like this:
-- >8 --
#!/bin/sh
# ignore arguments; mutt will parse them itself
# from to/cc headers. Note that we'll miss bcc this
# way, but handling that would probably be kind of
# tricky; we'd need to re-add those recipients as actual
# bcc headers so that mutt knows how to handle them.
# spool the message to a fake mbox; we need to add
# a "From" line to make it look legit
trap 'rm -f to-send' 0 &&
{
echo "From whatever Mon Sep 17 00:00:00 2001" &&
cat
} >to-send &&
# and then have mutt "resume" it. We have to redirect
# stdin back from the terminal, since ours is a pipe
# with the message contents.
mutt -p \
-e 'set postponed=to-send' \
-e 'set edit_headers=yes' \
</dev/tty
-- 8< --
and then in your git config:
[sendemail]
sendmailcmd = /path/to/mutt-as-mta.sh
There are mutt-specific bits there that I don't think send-email should
have to know about. Perhaps there are generic options that send-email
could learn, but it really feels like you'd do better teaching mutt to
be more ready to handle this (like taking a whole message on stdin,
headers and all, rather than just a body).
-Peff
^ permalink raw reply
* Re: [PATCH 7/9] ref-filter.c: filter & format refs in the same callback
From: Victoria Dye @ 2023-11-07 19:45 UTC (permalink / raw)
To: Patrick Steinhardt, Victoria Dye via GitGitGadget; +Cc: git
In-Reply-To: <ZUoWVPSE1GcJdHFE@tanuki>
Patrick Steinhardt wrote:
>> diff --git a/ref-filter.c b/ref-filter.c
>> index ff00ab4b8d8..384cf1595ff 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -2863,6 +2863,44 @@ static void free_array_item(struct ref_array_item *item)
>> free(item);
>> }
>>
>> +struct ref_filter_and_format_cbdata {
>> + struct ref_filter *filter;
>> + struct ref_format *format;
>> +
>> + struct ref_filter_and_format_internal {
>> + int count;
>> + } internal;
>> +};
>> +
>> +static int filter_and_format_one(const char *refname, const struct object_id *oid, int flag, void *cb_data)
>> +{
>> + struct ref_filter_and_format_cbdata *ref_cbdata = cb_data;
>> + struct ref_array_item *ref;
>> + struct strbuf output = STRBUF_INIT, err = STRBUF_INIT;
>> +
>> + ref = apply_ref_filter(refname, oid, flag, ref_cbdata->filter);
>> + if (!ref)
>> + return 0;
>> +
>> + if (format_ref_array_item(ref, ref_cbdata->format, &output, &err))
>> + die("%s", err.buf);
>> +
>> + if (output.len || !ref_cbdata->format->array_opts.omit_empty) {
>> + fwrite(output.buf, 1, output.len, stdout);
>> + putchar('\n');
>> + }
>> +
>> + strbuf_release(&output);
>> + strbuf_release(&err);
>> + free_array_item(ref);
>> +
>> + if (ref_cbdata->format->array_opts.max_count &&
>> + ++ref_cbdata->internal.count >= ref_cbdata->format->array_opts.max_count)
>> + return -1;
>
> It feels a bit weird to return a negative value here, which usually
> indicates that an error has happened whereas we only use it here to
> abort the iteration. But we ignore the return value of
> `do_iterate_refs()` anyway, so it doesn't make much of a difference.
I'll update it to 1, and also add a comment that the non-zero return value
stops iteration since it's not immediately clear from other 'each_ref_fn's
what that means. For reference, there appears to only be one other
'each_ref_fn' that even has the potential to return a nonzero return value
('ref_present()' in 'refs/files-backend.c).
>
>> + return 0;
>> +}
>> +
>> /* Free all memory allocated for ref_array */
>> void ref_array_clear(struct ref_array *array)
>> {
>> @@ -3046,16 +3084,46 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
>> return ret;
>> }
>>
>> +static inline int can_do_iterative_format(struct ref_filter *filter,
>> + struct ref_sorting *sorting,
>> + struct ref_format *format)
>> +{
>> + /*
>> + * Refs can be filtered and formatted in the same iteration as long
>> + * as we aren't filtering on reachability, sorting the results, or
>> + * including ahead-behind information in the formatted output.
>> + */
>
> Do we want to format this as a bulleted list so that it's more readily
> extensible if we ever need to pay attention to new options here? Also, I
> noted that this commit doesn't add any new tests -- do we already
> exercise all of these conditions?
Sure, I'll convert it to a bulleted list. I don't really expect it to change
much, though; to have any effect on this condition, the new filter/format
would need to act on the pre-filtered ref_array, which isn't particularly
common.
And yes, the existing tests cover scenarios where this function returns true
(e.g. 'git for-each-ref --no-sort') & where it returns false (essentially
anything else).
>
> More generally, I worry a bit about maintainability of this code snippet
> as we need to remember to always update this condition whenever we add a
> new option, and this can be quite easy to miss. The performance benefit
> might be worth the effort though.
I'll add more detailed comments to clarify what's going on here.
In practice, though, I don't think this would be all that easy to miss. As I
noted above, the only filters/formats that affect this are ones that need to
loop over an entire filtered ref_array after the initial
'for_each_fullref_in()'. To have it actually apply to commands that use
'filter_and_format_refs()', they'll need to add that behavior here (like
'filter_ahead_behind()'), where it should be apparent that
'can_do_iterative_format()' is relevant to their change.
>
> Patrick
^ permalink raw reply
* Re: [PATCH 2/9] for-each-ref: clarify interaction of --omit-empty & --count
From: Victoria Dye @ 2023-11-07 19:30 UTC (permalink / raw)
To: Øystein Walle, gitgitgadget; +Cc: git
In-Reply-To: <20231107192326.48296-1-oystwa@gmail.com>
Øystein Walle wrote:
> Hi Victoria,
>
> Victoria Dye <vdye@github.com> writes:
>
>> Update the 'for-each-ref' builtin documentation to clarify that refs
>> "omitted" by --omit-empty are still counted toward the limit specified
>> by --count. The use of the term "omit" would otherwise be somewhat
>> ambiguous and could incorrectly be construed as excluding empty refs
>> entirely (i.e. not counting them towards the total ref count).
>
> I implemented --omit-empty and I completely overlooked --count!
>
> (If I were to do it all over again I probably would have implemented it
> so that so-called omitted refs did not count towards the total. It makes
> sense to me since e.g. `git log -3 -- git.c` prints the three most
> recent commits that touch git.c regardless of how many commits were
> walked in the process.)
Since the interaction isn't clearly defined at the moment, we could probably
still update it to work like you're describing here. I'm happy to drop this
patch and implement your recommendation in a follow-up series. Let me know
what you think!
>
> This is a good and welcome clarification.
>
> Acked-by: Øystein Walle <oystwa@gmail.com>
^ permalink raw reply
* Re: [PATCH 2/9] for-each-ref: clarify interaction of --omit-empty & --count
From: Øystein Walle @ 2023-11-07 19:23 UTC (permalink / raw)
To: gitgitgadget; +Cc: git, vdye, Øystein Walle
In-Reply-To: <88eba4146cd250fcabfb9ffa9b410ce912a82ce7.1699320362.git.gitgitgadget@gmail.com>
Hi Victoria,
Victoria Dye <vdye@github.com> writes:
> Update the 'for-each-ref' builtin documentation to clarify that refs
> "omitted" by --omit-empty are still counted toward the limit specified
> by --count. The use of the term "omit" would otherwise be somewhat
> ambiguous and could incorrectly be construed as excluding empty refs
> entirely (i.e. not counting them towards the total ref count).
I implemented --omit-empty and I completely overlooked --count!
(If I were to do it all over again I probably would have implemented it
so that so-called omitted refs did not count towards the total. It makes
sense to me since e.g. `git log -3 -- git.c` prints the three most
recent commits that touch git.c regardless of how many commits were
walked in the process.)
This is a good and welcome clarification.
Acked-by: Øystein Walle <oystwa@gmail.com>
^ permalink raw reply
* Re: [PATCH 6/9] ref-filter.c: refactor to create common helper functions
From: Victoria Dye @ 2023-11-07 18:41 UTC (permalink / raw)
To: Patrick Steinhardt, Victoria Dye via GitGitGadget; +Cc: git
In-Reply-To: <ZUoWT8GyrZlvH_Go@tanuki>
Patrick Steinhardt wrote:
> On Tue, Nov 07, 2023 at 01:25:58AM +0000, Victoria Dye via GitGitGadget wrote:
>> From: Victoria Dye <vdye@github.com>
>>
>> Factor out parts of 'ref_array_push()', 'ref_filter_handler()', and
>> 'filter_refs()' into new helper functions ('ref_array_append()',
>> 'apply_ref_filter()', and 'do_filter_refs()' respectively), as well as
>> rename 'ref_filter_handler()' to 'filter_one()'. In this and later
>> patches, these helpers will be used by new ref-filter API functions. This
>> patch does not result in any user-facing behavior changes or changes to
>> callers outside of 'ref-filter.c'.
>>
>> The changes are as follows:
>>
>> * The logic to grow a 'struct ref_array' and append a given 'struct
>> ref_array_item *' to it is extracted from 'ref_array_push()' into
>> 'ref_array_append()'.
>> * 'ref_filter_handler()' is renamed to 'filter_one()' to more clearly
>> distinguish it from other ref filtering callbacks that will be added in
>> later patches. The "*_one()" naming convention is common throughout the
>> codebase for iteration callbacks.
>> * The code to filter a given ref by refname & object ID then create a new
>> 'struct ref_array_item' is moved out of 'filter_one()' and into
>> 'apply_ref_filter()'. 'apply_ref_filter()' returns either NULL (if the ref
>> does not match the given filter) or a 'struct ref_array_item *' created
>> with 'new_ref_array_item()'; 'filter_one()' appends that item to
>> its ref array with 'ref_array_append()'.
>> * The filter pre-processing, contains cache creation, and ref iteration of
>> 'filter_refs()' is extracted into 'do_filter_refs()'. 'do_filter_refs()'
>> takes its ref iterator function & callback data as an input from the
>> caller, setting it up to be used with additional filtering callbacks in
>> later patches.
>
> To me, a bulleted list spelling out the different changes I'm doing
> often indicates that I might want to split up the commit into one for
> each of the items. I don't feel strongly about this, but think that it
> might help the reviewer in this case.
While that's a good guideline to keep in mind, it's not universally
applicable. In this case, (almost) all of the changes are done the same way,
focused on the same goal: extract bits of 'filter_refs()' into generic,
internal helpers so we can use those bits elsewhere in later patches.
Splitting those extractions into multiple patches would essentially lead to
a handful of very small patches that more-or-less have the same commit
message. As I mentioned in [1], I think there's value to having the
immediate context of related changes in a single patch (as long as that
single patch doesn't become unwieldy), so I'm not inclined to split this up.
That said, I did say "(almost) all" of the changes are conceptually similar.
Looking at this now, the rename of 'ref_filter_handler()' => 'filter_one()'
doesn't really fit the "extract into helper functions" theme of the rest of
the patch, I'll pull that out into its own.
[1] https://lore.kernel.org/git/a833b5a7-0201-4c2e-8821-f2a1930cb403@github.com/
^ permalink raw reply
* Re: git-send-email: Send with mutt(1)
From: Alejandro Colomar @ 2023-11-07 18:36 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20231107174803.GA507007@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 3961 bytes --]
Hi Jeff,
On Tue, Nov 07, 2023 at 12:48:03PM -0500, Jeff King wrote:
> I think there's a lot of overlap between what git-send-email does and
> what mutt does, to the point that you probably don't need to use
> send-email at all.
>
> I assume what you want out of send-email here is the actual generation
> of patch emails. But under the hood that is all done by git-format-patch
> anyway. So for example if you do:
Yeah, most of it is done by format-patch. There are few things I
actually need from send-email. One of them is generating the Cc from
the sign-offs and other tags found in the patch.
I had been thinking these days that it would be useful to have
format-patch generate those. How about adding a --signed-off-by-cc to
format-patch?
>
> git format-patch --stdout origin..HEAD >patches
> mutt -f patches
>
> And then you can use mutt's "resend-message" function to send each one.
> I use config like this:
>
> macro index,pager b ":set edit_headers=yes<enter><resend-message>:set edit_headers=no<enter>"
>
> If you're sending a long series, it's helpful to pre-populate various
> headers in the format-patch command with "--to", etc. I usually do so by
> sending the cover letter directly via mutt, and then using some perl
> hackery to convert those headers into format-patch args. The script I
Indeed, that hackery is what send-email already does, so how about
moving those features a bit upstream so that format-patch can do them
too?
Although then, maybe it's simpler to teach send-email to learn to use
mutt(1) under the hood for the actual send.
> use is below (it will also, when run without a terminal, generate the
> patch summary for the cover letter; I use it with "r!my-script" while
> writing the cover letter in vim).
>
> (This script is what I use every day, so it should be fairly robust. But
> it is also over 15 years old, so I don't promise there isn't a simpler
> way to do some of what it does ;) ).
>
> -- >8 --
> #!/bin/sh
> upstream_branch() {
> current=`git symbolic-ref HEAD`
> upstream=`git for-each-ref --format='%(upstream)' "$current"`
> if test -n "$upstream"; then
> echo $upstream
> else
> echo origin
> fi
> }
>
> get_reply_headers() {
> perl -ne '
> if (defined $opt) {
> if (/^\s+(.*)/) {
> $val .= " $1";
> next;
> }
> print "--$opt=", quotemeta($val), " ";
> $opt = $val = undef;
> }
> if (/^(cc|to):\s*(.*)/i) {
> $opt = lc($1);
> $val = $2;
> }
> elsif (/^message-id:\s*(.*)/i) {
> $opt = "in-reply-to";
> $val = $1;
> }
> elsif (/^subject:\s*\[PATCH v(\d+)/i) {
> print "-v$1 ";
> }
> elsif (/^$/) {
> last;
> }
> '
> }
>
> format_patch() {
> git format-patch -s --stdout --from "$@"
> }
>
> has_nonoption=
> for i in "$@"; do
> case "$i" in
> -[0-9]*) has_nonoption=yes ;;
> -*) ;;
> *) has_nonoption=yes
> esac
> done
>
> : ${REPLY:=$HOME/patch}
> test -e "$REPLY" && eval "set -- `get_reply_headers <\"$REPLY\"` \"\$@\""
> test "$has_nonoption" = "yes" || set -- "$@" `upstream_branch`
>
> if test -t 1; then
> format_patch "$@" >.mbox
> mutt -e 'set sort=mailbox-order' -f .mbox
> rm -f .mbox
> else
> format_patch "$@" |
> perl -lne '
> if (/^Subject: (.*)/) {
> $subject = $1;
> }
> elsif ($subject && /^\s+(.*)/) {
> $subject .= " $1";
> }
> elsif ($subject) {
> print $subject;
> $subject = undef;
> }
> ' |
> sed -e 's/\[PATCH /[/' \
> -e 's/]/]:/' \
> -e 's/^/ /'
> echo
> format_patch --cover-letter "$@" |
> sed -ne '/|/,/^$/p; /^-- /q'
> fi
Thanks! I'll try it. Although I don't know perl, so I hope I don't
need to tweak it much. :)
Cheers,
Alex
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [RFC PATCH 3/3] builtin/replay.c: introduce `--write-pack`
From: Taylor Blau @ 2023-11-07 18:23 UTC (permalink / raw)
To: git
Cc: Jeff King, Patrick Steinhardt, Elijah Newren, Junio C Hamano,
Johannes Schindelin
In-Reply-To: <cover.1699381371.git.me@ttaylorr.com>
Now that the prerequisites are in place, we can implement a
`--write-pack` option for `git replay`, corresponding to the existing
one in `git merge-tree`.
The changes are mostly limited to:
- introducing a new option in the builtin
- replacing the main object store with the temporary one
- then repacking and migrating the temporary object store back into
the main object store after the replay has completed
Along with tests and documentation to ensure that the new behavior
matches our expectations.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/git-replay.txt | 4 ++++
builtin/replay.c | 18 ++++++++++++++++++
t/t3650-replay-basics.sh | 37 ++++++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+)
diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index e7551aec54..f424c1a676 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -42,6 +42,10 @@ When `--advance` is specified, the update-ref command(s) in the output
will update the branch passed as an argument to `--advance` to point at
the new commits (in other words, this mimics a cherry-pick operation).
+--write-pack::
+ Write any new objects into a separate packfile instead of as
+ individual loose objects.
+
<revision-range>::
Range of commits to replay. More than one <revision-range> can
be passed, but in `--advance <branch>` mode, they should have
diff --git a/builtin/replay.c b/builtin/replay.c
index c3d53ff0cd..72b7b7f43a 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -17,6 +17,7 @@
#include "strmap.h"
#include <oidset.h>
#include <tree.h>
+#include "tmp-objdir.h"
static const char *short_commit_name(struct commit *commit)
{
@@ -272,6 +273,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
struct commit *onto = NULL;
const char *onto_name = NULL;
int contained = 0;
+ int write_pack = 0;
struct rev_info revs;
struct commit *last_commit = NULL;
@@ -279,6 +281,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
struct merge_options merge_opt;
struct merge_result result;
struct strset *update_refs = NULL;
+ struct tmp_objdir *tmp_objdir = NULL;
kh_oid_map_t *replayed_commits;
int i, ret = 0;
@@ -296,6 +299,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
N_("replay onto given commit")),
OPT_BOOL(0, "contained", &contained,
N_("advance all branches contained in revision-range")),
+ OPT_BOOL(0, "write-pack", &write_pack,
+ N_("write new objects to a pack instead of as loose")),
OPT_END()
};
@@ -352,8 +357,15 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
init_merge_options(&merge_opt, the_repository);
memset(&result, 0, sizeof(result));
merge_opt.show_rename_progress = 0;
+ merge_opt.write_pack = write_pack;
last_commit = onto;
replayed_commits = kh_init_oid_map();
+
+ if (merge_opt.write_pack) {
+ tmp_objdir = tmp_objdir_create("replay");
+ tmp_objdir_replace_primary_odb(tmp_objdir, 0);
+ }
+
while ((commit = get_revision(&revs))) {
const struct name_decoration *decoration;
khint_t pos;
@@ -417,5 +429,11 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
/* Return */
if (ret < 0)
exit(128);
+ if (ret && tmp_objdir) {
+ if (tmp_objdir_repack(tmp_objdir) < 0)
+ ret = 0;
+ else if (tmp_objdir_migrate(tmp_objdir) < 0)
+ ret = 0;
+ }
return ret ? 0 : 1;
}
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 389670262e..e7048748c2 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -67,6 +67,43 @@ test_expect_success 'using replay to rebase two branches, one on top of other' '
test_cmp expect result
'
+packdir=.git/objects/pack
+
+test_expect_success 'using replay to rebase two branches, with --write-pack' '
+ # Remove the results of the previous rebase, ensuring that they
+ # are pruned from the object store.
+ git gc --prune=now &&
+ test_must_fail git cat-file -t "$(cut -d " " -f 3 expect)" &&
+
+ # Create an extra packfile to ensure that the tmp-objdir repack
+ # takes place outside of the main object store.
+ git checkout --detach &&
+ test_commit unreachable &&
+ git repack -d &&
+ git checkout main &&
+
+ find $packdir -type f -name "*.idx" | sort >packs.before &&
+ git replay --write-pack --onto main topic1..topic2 >result &&
+ find $packdir -type f -name "*.idx" | sort >packs.after &&
+
+ comm -13 packs.before packs.after >packs.new &&
+
+ # Ensure that we got a single new pack.
+ test_line_count = 1 result &&
+ test_line_count = 1 packs.new &&
+
+ # ... and that the rest of the results match our expeectations.
+ git log --format=%s $(cut -f 3 -d " " result) >actual &&
+ test_write_lines E D M L B A >expect &&
+ test_cmp expect actual &&
+
+ printf "update refs/heads/topic2 " >expect &&
+ printf "%s " $(cut -f 3 -d " " result) >>expect &&
+ git rev-parse topic2 >>expect &&
+
+ test_cmp expect result
+'
+
test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' '
git -C bare replay --onto main topic1..topic2 >result-bare &&
test_cmp expect result-bare
--
2.42.0.446.g0b9ef90488
^ permalink raw reply related
* [RFC PATCH 2/3] tmp-objdir: introduce `tmp_objdir_repack()`
From: Taylor Blau @ 2023-11-07 18:22 UTC (permalink / raw)
To: git
Cc: Jeff King, Patrick Steinhardt, Elijah Newren, Junio C Hamano,
Johannes Schindelin
In-Reply-To: <cover.1699381371.git.me@ttaylorr.com>
In the following commit, we will teach `git replay` how to write a pack
containing the set of new objects created as a result of the `replay`
operation.
Since `replay` needs to be able to see the object(s) written
from previous steps in order to replay each commit, the ODB transaction
may have multiple pending packs. Migrating multiple packs back into the
main object store has a couple of downsides:
- It is error-prone to do so: each pack must be migrated in the
correct order (with the ".idx" file staged last), and the set of
packs themselves must be moved over in the correct order to avoid
racy behavior.
- It is a (potentially significant) performance degradation to migrate
a large number of packs back into the main object store.
Introduce a new function that combines the set of all packs in the
temporary object store to produce a single pack which is the logical
concatenation of all packs created during that level of the ODB
transaction.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
tmp-objdir.c | 13 +++++++++++++
tmp-objdir.h | 6 ++++++
2 files changed, 19 insertions(+)
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 5f9074ad1c..ef53180b47 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -12,6 +12,7 @@
#include "strvec.h"
#include "quote.h"
#include "object-store-ll.h"
+#include "run-command.h"
struct tmp_objdir {
struct strbuf path;
@@ -277,6 +278,18 @@ int tmp_objdir_migrate(struct tmp_objdir *t)
return ret;
}
+int tmp_objdir_repack(struct tmp_objdir *t)
+{
+ struct child_process cmd = CHILD_PROCESS_INIT;
+
+ cmd.git_cmd = 1;
+
+ strvec_pushl(&cmd.args, "repack", "-a", "-d", "-k", "-l", NULL);
+ strvec_pushv(&cmd.env, tmp_objdir_env(t));
+
+ return run_command(&cmd);
+}
+
const char **tmp_objdir_env(const struct tmp_objdir *t)
{
if (!t)
diff --git a/tmp-objdir.h b/tmp-objdir.h
index 237d96b660..d00e3b3e27 100644
--- a/tmp-objdir.h
+++ b/tmp-objdir.h
@@ -36,6 +36,12 @@ struct tmp_objdir *tmp_objdir_create(const char *prefix);
*/
const char **tmp_objdir_env(const struct tmp_objdir *);
+/*
+ * Combines all packs in the tmp_objdir into a single pack before migrating.
+ * Removes original pack(s) after installing the combined pack into place.
+ */
+int tmp_objdir_repack(struct tmp_objdir *);
+
/*
* Finalize a temporary object directory by migrating its objects into the main
* object database, removing the temporary directory, and freeing any
--
2.42.0.446.g0b9ef90488
^ permalink raw reply related
* [RFC PATCH 1/3] merge-ort.c: finalize ODB transactions after each step
From: Taylor Blau @ 2023-11-07 18:22 UTC (permalink / raw)
To: git
Cc: Jeff King, Patrick Steinhardt, Elijah Newren, Junio C Hamano,
Johannes Schindelin
In-Reply-To: <cover.1699381371.git.me@ttaylorr.com>
In a previous commit, the ORT merge backend learned how to use the
bulk-checkin mechanism to emit a single pack containing any new objects
created during the merge. This functionality was implemented by setting
up a new ODB transaction, and finalizing it at the end of the merge via
`process_entries()`.
In a future commit, we will extend this functionality to the new `git
replay` command, which needs to see objects from previous steps in order
to replay each commit.
As a step towards implementing this, teach the ORT backend to flush the
ODB transaction at the end of each step in `process_entries()`, and then
finalize the result with `end_odb_transaction()` when calling
`merge_finalize()`.
For normal `merge-tree --write-pack` invocations, this produces no
functional change: the pack is written out at the end of
`process_entries()`, and then the `end_odb_transaction()` call is a
noop.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
merge-ort.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/merge-ort.c b/merge-ort.c
index 523577d71e..7b352451cc 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -4354,7 +4354,7 @@ static int process_entries(struct merge_options *opt,
ret = -1;
if (opt->write_pack)
- end_odb_transaction();
+ flush_odb_transaction();
cleanup:
string_list_clear(&plist, 0);
@@ -4726,6 +4726,9 @@ void merge_switch_to_result(struct merge_options *opt,
void merge_finalize(struct merge_options *opt,
struct merge_result *result)
{
+ if (opt->write_pack)
+ end_odb_transaction();
+
if (opt->renormalize)
git_attr_set_direction(GIT_ATTR_CHECKIN);
assert(opt->priv == NULL);
--
2.42.0.446.g0b9ef90488
^ 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