* Re: [PATCH] ci: do not die on deprecated-declarations warning
From: Jeff King @ 2023-01-15 20:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ramsay Jones
In-Reply-To: <xmqqzgakgu0n.fsf@gitster.g>
On Sat, Jan 14, 2023 at 11:02:32PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > But anyway, let's use CURL_DISABLE_DEPRECATION first to see how it
> > goes.
>
> The "DEVELOPER_CFLAGS += -Wno-error=deprecated-declarations" version
> was merged to 'next', only because I wanted to see the commit
> cleanly pass the tests (and it does), but I do think in the longer
> term (like, before the topic hits 'master'), it probably is better
> to do this for everybody, not just for those who use DEVELOPER=Yes.
>
> So, further patches on top are very much welcomed.
So I took a look at just dropping the deprecated bits, and it wasn't
_too_ bad. Here's that series. The first two I hope are obviously good.
The third one is _ugly_, but at least it punts on the whole "how should
we silence this" argument, and it takes us in the direction we'd
ultimately want to go.
[1/3]: http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT
[2/3]: http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION
[3/3]: http: support CURLOPT_PROTOCOLS_STR
git-curl-compat.h | 8 +++++++
http-push.c | 6 ++---
http.c | 61 +++++++++++++++++++++++++++++++++--------------
http.h | 2 +-
remote-curl.c | 28 ++++++++++------------
5 files changed, 68 insertions(+), 37 deletions(-)
^ permalink raw reply
* Re: [PATCH] ci: do not die on deprecated-declarations warning
From: Jeff King @ 2023-01-15 20:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ramsay Jones, git
In-Reply-To: <xmqq4jssi8qh.fsf@gitster.g>
On Sat, Jan 14, 2023 at 10:59:18PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On the other hand, I don't know how useful those deprecations are going
> > to be, as it depends on the timeframe. If the deprecation is added for
> > the same version of libcurl that implements the alternative (which is
> > roughly the case here), then we'd always be stuck supporting the old and
> > new forms (old for backwards compatibility, and new to silence the
> > deprecation warning).
>
> ... or just keep the warning without promoting, with "-Wno-error=...".
I'm pretty strongly against that solution for anything long-term. Having
to see those warnings is not only ugly, but it's confusing and trains
people to ignore them (and many times I've noticed ugly warnings
floating by and realized that oops, I had temporarily disabled -Werror
because I had been working with some older code).
> > We care a lot more about the deprecation once the
> > alternative has been around for a while, and/or the old way of doing
> > things is about to be removed. And if we just wait until that removal,
> > then we do not have to rely on deprecation warnings. The build will
> > break just fine on its own. :)
>
> Yes and no. It is not always like "this symbol is now known under
> this different name", which is trivial to adjust. I briefly tried
> to see how IOCTL -> SEEK change should look like until I realized
> that the new way was invented too recently and stopped looking, but
> it would involve changes to the function logic in the callback
> functions, as the function signature---both parameters and its
> return values---of the callback changes. I do not want to see us
> scrambling to make such adjustments to the code at the last minute,
> so some sort of advance warning is a good thing to have.
True.
I do think the IOCTL/SEEK one is old enough that we can do, though. The
deprecation is newer, but the SEEK interface was added in an old enough
version.
-Peff
^ permalink raw reply
* Re: ctrl-z ignored by git; creates blobs from non-existent repos
From: Jeff King @ 2023-01-15 18:36 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Crls, git
In-Reply-To: <Y8Qfj32h89hq5UD6@mit.edu>
On Sun, Jan 15, 2023 at 10:45:19AM -0500, Theodore Ts'o wrote:
> > Expected: The same issue does not happen with other non-existent repos e.g.,
> > git clone git.zx2c4/ it returns the message of fatal repo not found
>
> So what's going on is that github.com is not returning a non-existent
> repo error; it's prompting for a username/password, as _if_ the
> repository exists. That's presumably to prevent disclosing
> information as to whether or not a private repository exists or not.
I can confirm that this is exactly the reason.
-Peff
^ permalink raw reply
* Re: Segmentation fault within git read-tree
From: Jeff King @ 2023-01-15 18:35 UTC (permalink / raw)
To: Christian Couder; +Cc: Junio C Hamano, Frédéric Fort, git
In-Reply-To: <CAP8UFD0pKZT57jpUOg7gckcr4stoq24YDB2Fu0-AwbGPrEweqg@mail.gmail.com>
On Sun, Jan 15, 2023 at 12:53:25PM +0100, Christian Couder wrote:
> From a very quick look, it seems that in setup_traverse_info() in
> tree-walk.c we do:
>
> static struct traverse_info dummy;
> ...
> if (pathlen)
> info->prev = &dummy;
>
> but then later in debug_path() in unpack-trees.c we check
> *info->prev->name which segfaults.
>
> I am not very familiar with this code and don't have much time right
> now, so I think I will leave it to others to investigate this further.
I'm not sure if Frédéric is seeing another segfault in practice (when
not using --debug-unpack), but yeah, it is very easy to trigger this
segfault. It does not even have to do with sparse checkouts, etc. Here's
an even more minimal example:
git init repo
cd repo
touch file
git add file
git commit -m added
git read-tree --debug-unpack --prefix=subtree HEAD
I was going to bisect, but it looks like it was broken all the way back
to Junio's ba655da537 (read-tree --debug-unpack, 2009-09-14).
-Peff
^ permalink raw reply
* Re: ctrl-z ignored by git; creates blobs from non-existent repos
From: Theodore Ts'o @ 2023-01-15 15:45 UTC (permalink / raw)
To: Crls; +Cc: git
In-Reply-To: <632d051b-d81b-b35d-0641-c2488a124810@gmail.com>
On Fri, Jan 13, 2023 at 05:01:01PM -0500, Crls wrote:
> Ctrl-Z is ignored by git; Git-clone injects blobs even with non-existent
> repos
>
> Steps to reproduce 1- git clone github whateverrepo/whatevernonexistentrepo
> or 1- git clone gitlab whateverrepo/whatevernonexistentrepo 2= Git prompts
> for a username
% git clone github whateverrepo/whatevernonexistentrepo
fatal: repository 'github' does not exist
I think what you meant was:
% git clone https://github.com/whateverrepo/whatevernonexistentrepo
Cloning into 'whatevernonexistentrepo'...
Username for 'https://github.com':
> 3- Press Ctrl-Z to stop *git* from running either on the virtual console/tty
> *git* automatically creates blobs with directories and disregards
So it's not that Control-Z is being ignored. It's that by the time
you see the prompt for "Username for 'https://github.com': ", the
directories already exist. Try looking at
whatevernonexistentrepo/.git as soon as the prompt shows up. You'll
see that the .git directory has been greated.
Now, when you type ^Z, the git processes are stopped --- but the
objects are created already.
Username for 'https://github.com': ^Z
[1]+ Stopped git clone https://github.com/whateverrepo/whatevernonexistentrepo
% ps aux | grep git
tytso 5097 0.0 0.0 9736 4480 pts/0 T 10:41 0:00 git clone https://github.com/wha
tytso 5098 0.0 0.0 9736 3992 pts/0 T 10:41 0:00 /usr/lib/git-core/git remote-htt
tytso 5099 0.0 0.1 102332 16104 pts/0 T 10:41 0:00 /usr/lib/git-core/git-remote-htt
tytso 5140 0.0 0.0 6332 2072 pts/0 S+ 10:43 0:00 grep git
The 'T' means that the processes are stopped.
> Expected: The same issue does not happen with other non-existent repos e.g.,
> git clone git.zx2c4/ it returns the message of fatal repo not found
So what's going on is that github.com is not returning a non-existent
repo error; it's prompting for a username/password, as _if_ the
repository exists. That's presumably to prevent disclosing
information as to whether or not a private repository exists or not.
Once the authentication fails, git will remove the partially created
repro, so it's really not a problem in practice.
Cheers,
- Ted
^ permalink raw reply
* Re: Gitorious should use CRC128 / 256 / 512 instead of SHA-1
From: Michal Suchánek @ 2023-01-15 13:53 UTC (permalink / raw)
To: Hans Petter Selasky; +Cc: git
In-Reply-To: <9c0fda42-67ab-f406-489b-38a2d9bbcfc2@selasky.org>
Hello,
On Fri, Jan 13, 2023 at 02:23:59PM +0100, Hans Petter Selasky wrote:
> Hi,
>
> Currently GIT only supports cryptographic hashes for its commit tags.
>
> That means:
>
> 1) It's very difficult to edit the history without also recomputing the hash
> tags for all commits after the needed change-point, which then means
> references to a repository is broken.
That also makes it difficult to alter the repository intentionally
without anyone noticing. With SHA1 being somewhat weak it may be
possible to alter repository content although I am not aware of any
practical attacks shown so far. For that reason using stronger hashes is
planned in the future.
> 2) Only a single bit error in the main repository can break everything!
>
> 3) Illicit contents may be present in binary blobs, which in the future may
> be need to be removed without warrant and the only way to do that is by
> rebasing and force pushing, which will break "everything". It can be
> everything from child-porn to expired distribution licenses.
It's good to avoid spam getting into your repository. If you really need
to alter it long into the past you still can. Everyone will notice that
you did, and that's an intentional feature. In some situations it is
understandably an annoyance but there's so much you can do. At least
tags should remain stable.
> Many people think that bit errors cannot happen because the memory uses ECC
> and the file system uses cryptographic hashes to verify the integrity of the
> data. But what many people forget about is that when copying data from
> memory to disk, typically using a DMA channel data is copied w/o any kind of
> integrity protection, because the integrity protection is not end-to-end.
> The integrity protection is only per-link.
So long as all links have integrity protection it's end-to-end.
Integrity checks for CPU chaches, buses, and IO protocols do exist.
It's not that errors cannot happen, they are very unlikely.
In the very rare case that such error happens so long as non-corrupted
version of the object can be supplied by anyone who has a copy of the
repository it is recoverable.
For old objects this should be your backup system.
For new objects the worst case is that the history is rolled back so the
missing object is not needed.
Thanks
Michal
^ permalink raw reply
* Re: Segmentation fault within git read-tree
From: Christian Couder @ 2023-01-15 11:53 UTC (permalink / raw)
To: Frédéric Fort; +Cc: git
In-Reply-To: <d0f9520d-9ccc-a899-609e-fbbb4529e005@free.fr>
Hi,
On Sat, Jan 14, 2023 at 11:22 PM Frédéric Fort <fortfrederic@free.fr> wrote:
>
> Hello,
>
> I am doing some tests trying to do a sparse checkout of a partial clone
> within a subtree.
> However, I get a segfault when trying to run git read-tree as is done by
> git subtree internally.
>
> Maybe what I am doing isn't supposed to work at all, but I suppose it
> shouldn't at least cause git read-tree to segfault.
Yeah, it shouldn't segfault. Thanks for the report!
> Here should be a MWE to reproduce my error:
I reproduced the issue using your steps with a few changes.
> # Run this to create the repo that will become your subtree
> git init subtree.git
> cd subtree.git
> touch x y
> git add .
> git commit -m "first commit"
>
> # Run this to create the repo that has a sparse checkout of a partial
> clone within a subtree
> git init repo
> cd repo
> git commit --allow-empty "first commit"
I think the above command is missing '-m' before "first commit".
> git sparse-checkout set "subtree/x"
> git remote add origin-subtree git@my.server:/with/the/subtree.git
I reproduced using a local remote like:
git remote add origin-subtree /path/to/subtree.git
> git fetch --filter=blob:none origin-subtree
> git rev-parse --verify "FETCH_HEAD^{commit}"
> git read-tree --debug-unpack --prefix="subtree" FETCH_HEAD
Yeah, I get the following under gdb:
Program received signal SIGSEGV, Segmentation fault.
debug_path (info=0x7fffffffc880) at unpack-trees.c:1395
1395 if (*info->prev->name)
(gdb) bt
#0 debug_path (info=0x7fffffffc880) at unpack-trees.c:1395
#1 0x000055555587d917 in debug_unpack_callback (n=1, mask=1,
dirmask=0, names=0x7fffffffc3b0, info=0x7fffffffc880) at
unpack-trees.c:1417
#2 0x000055555587dc48 in unpack_callback (n=1, mask=1, dirmask=0,
names=0x7fffffffc3b0, info=0x7fffffffc880) at unpack-trees.c:1491
#3 0x00005555558779c2 in traverse_trees (istate=0x5555559f3b00
<the_index>, n=1, t=0x7fffffffcb50, info=0x7fffffffc880) at
tree-walk.c:536
#4 0x000055555587ef85 in unpack_trees (len=1, t=0x7fffffffcb50,
o=0x7fffffffcd90) at unpack-trees.c:1979
#5 0x000055555562ab45 in cmd_read_tree (argc=1, argv=0x7fffffffdc90,
cmd_prefix=0x0) at builtin/read-tree.c:263
#6 0x0000555555575a4d in run_builtin (p=0x5555559bf8f0
<commands+2256>, argc=4, argv=0x7fffffffdc90) at git.c:445
#7 0x0000555555575ed2 in handle_builtin (argc=4, argv=0x7fffffffdc90)
at git.c:700
#8 0x0000555555576179 in run_argv (argcp=0x7fffffffdaec,
argv=0x7fffffffdae0) at git.c:764
#9 0x0000555555576764 in cmd_main (argc=4, argv=0x7fffffffdc90) at git.c:899
#10 0x0000555555682dfe in main (argc=5, argv=0x7fffffffdc88) at common-main.c:57
From a very quick look, it seems that in setup_traverse_info() in
tree-walk.c we do:
static struct traverse_info dummy;
...
if (pathlen)
info->prev = &dummy;
but then later in debug_path() in unpack-trees.c we check
*info->prev->name which segfaults.
I am not very familiar with this code and don't have much time right
now, so I think I will leave it to others to investigate this further.
Best,
Christian.
^ permalink raw reply
* Re: Gitorious should use CRC128 / 256 / 512 instead of SHA-1
From: demerphq @ 2023-01-15 10:09 UTC (permalink / raw)
To: brian m. carlson, Hans Petter Selasky, git
In-Reply-To: <Y8NB21PExmifhyeQ@tapette.crustytoothpaste.net>
On Sun, 15 Jan 2023 at 01:05, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> This is a problem in every Merkle tree-like system. Most repositories
> have some sort of code review or access control that prevents people
> from generally pushing inappropriate content. For example, if somebody
> proposed to push any sort of pornography or other inappropriate content
> (e.g., a racist screed) to one of my repositories or one of my
> employer's, I'd refuse to approve or merge such a change, because
> that wouldn't be appropriate for the repository.
>
> I don't feel this is enough of a problem that using a Merkle tree-like
> construction is a bad idea, given the benefits it offers.
[resend in plain text]
It isn't clear to me why this needs to be a problem at all. If the
Merkele tree contains data later in its chain that says "replace
Object X with Y", provided the replacement mechanism doesn't touch
commit objects, only blobs, then you can replace files in the history
with other files without altering the commit history.
Provided the toolchain validates that it has found a proper
"replacement instruction" in the history, it should be possible to
safely replace blobs without a full history rewrite.
The replacement mechanism could be structured so that you can only
"nuke" a file, eg, replace it with a zero byte blob, making it
somewhat less open to abuse, or it could allow arbitrary blobs to be
mapped to each other. So long as the mapping data is in the commit
history it should be as secure as the original mapping no? Git could
be taught to warn the user "Checking out a rewritten blob X as Y, see
012deadbeef for the rewrite instruction." when it happened.
Again, provided this does not touch the *commit* tree, just raw blobs,
I dont see why you can't have an object replacement facility. Am I
missing something?
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ permalink raw reply
* Re: [PATCH v4 0/5] submodule: parallelize diff
From: Junio C Hamano @ 2023-01-15 9:31 UTC (permalink / raw)
To: Calvin Wan; +Cc: git, emilyshaffer, avarab, phillip.wood123, myriamanis
In-Reply-To: <20221108184200.2813458-1-calvinwan@google.com>
Calvin Wan <calvinwan@google.com> writes:
> Original cover letter for context:
> https://lore.kernel.org/git/20221011232604.839941-1-calvinwan@google.com/
> ...
> Calvin Wan (5):
> run-command: add duplicate_output_fn to run_processes_parallel_opts
> submodule: strbuf variable rename
> submodule: move status parsing into function
> diff-lib: refactor match_stat_with_submodule
> diff-lib: parallelize run_diff_files for submodules
>
> Documentation/config/submodule.txt | 12 ++
> diff-lib.c | 103 +++++++++++--
> run-command.c | 13 +-
> run-command.h | 24 +++
> submodule.c | 229 +++++++++++++++++++++++++----
> submodule.h | 9 ++
> t/helper/test-run-command.c | 21 +++
> t/t0061-run-command.sh | 39 +++++
> t/t4027-diff-submodule.sh | 19 +++
> t/t7506-status-submodule.sh | 19 +++
> 10 files changed, 441 insertions(+), 47 deletions(-)
While the topic is marked as "Needs review" in the recent "What's
cooking" reports, merging this topic also breaks the "linux-leaks"
job by causing many tests fail:
t3040-subprojects-basic.sh
t4010-diff-pathspec.sh
t4015-diff-whitespace.sh
t4027-diff-submodule.sh
t7403-submodule-sync.sh
t7409-submodule-detached-work-tree.sh
t7416-submodule-dash-url.sh
t7450-bad-git-dotfiles.sh
t7506-status-submodule.sh
Two of the test scripts are touched by this topic, and their
breakage could be caused by newly using other git subcommands that
were known to be leaking (iow, not because this series introduced
new leaks). It also is possible that they fail because this series
added new leaks to the commands these two test scripts use. In
either case, other tests that haven't been touched by this topic
were definitely broken by new leaks introduced by the changes made
by this series.
Anybody interested should be able to see the breakage themselves by
checking out 'seen' and running
SANTIZE=leak GIT_TEST_PASSING_SANITIZE_LEAK=true \
make test
to see the tree with all in-flight topics are clean, and then by
running the same test after merging this topic to 'seen'.
Thanks.
^ permalink raw reply
* Re: [PATCH v5 0/4] Optionally skip hashing index on write
From: Junio C Hamano @ 2023-01-15 9:31 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, vdye, avarab, newren, Jacob Keller, Derrick Stolee
In-Reply-To: <pull.1439.v5.git.1673022717.gitgitgadget@gmail.com>
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Writing the index is a critical action that takes place in multiple Git
> commands. The recent performance improvements available with the sparse
> index show how often the I/O costs around the index can affect different Git
> commands, although reading the index takes place more often than a write.
https://github.com/git/git/actions/runs/3922482355/jobs/6705454550#step:4:2006
shows that all other platforms passed but osx-gcc failed one of the
tests this series added. Could it be that there is something racy
about the test (1006.6 if I am not mistaken)?
^ permalink raw reply
* [PATCH] format-patch: unleak "-v <num>"
From: Junio C Hamano @ 2023-01-15 8:03 UTC (permalink / raw)
To: git
The "subject_prefix" member of "struct revision" usually is always
set to a borrowed string (either a string literal like "PATCH" that
appear in the program text as a hardcoded default, or the value of
"format.subjectprefix") and is never freed when the containing
revision structure is released. The "-v <num>" codepath however
violates this rule and stores a pointer to an allocated string to
this member, relinquishing the responsibility to free it when it is
done using the revision structure, leading to a small one-time leak.
Instead, keep track of the string it allocates to let the revision
structure borrow, and clean it up when it is done.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/log.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index e1fe7bbe71..3dddaadc1e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1879,6 +1879,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
struct strbuf rdiff1 = STRBUF_INIT;
struct strbuf rdiff2 = STRBUF_INIT;
struct strbuf rdiff_title = STRBUF_INIT;
+ struct strbuf sprefix = STRBUF_INIT;
int creation_factor = -1;
const struct option builtin_format_patch_options[] = {
@@ -2019,12 +2020,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
if (reroll_count) {
- struct strbuf sprefix = STRBUF_INIT;
-
strbuf_addf(&sprefix, "%s v%s",
rev.subject_prefix, reroll_count);
rev.reroll_count = reroll_count;
- rev.subject_prefix = strbuf_detach(&sprefix, NULL);
+ rev.subject_prefix = sprefix.buf;
}
for (i = 0; i < extra_hdr.nr; i++) {
@@ -2388,6 +2387,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
strbuf_release(&rdiff1);
strbuf_release(&rdiff2);
strbuf_release(&rdiff_title);
+ strbuf_release(&sprefix);
free(to_free);
if (rev.ref_message_ids)
string_list_clear(rev.ref_message_ids, 0);
--
2.39.0-198-ga38d39a4c5
^ permalink raw reply related
* Re: [PATCH] ci: do not die on deprecated-declarations warning
From: Junio C Hamano @ 2023-01-15 7:02 UTC (permalink / raw)
To: git; +Cc: Ramsay Jones, Jeff King
In-Reply-To: <xmqqilh9kqdy.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> But anyway, let's use CURL_DISABLE_DEPRECATION first to see how it
> goes.
The "DEVELOPER_CFLAGS += -Wno-error=deprecated-declarations" version
was merged to 'next', only because I wanted to see the commit
cleanly pass the tests (and it does), but I do think in the longer
term (like, before the topic hits 'master'), it probably is better
to do this for everybody, not just for those who use DEVELOPER=Yes.
So, further patches on top are very much welcomed.
Thanks.
^ permalink raw reply
* Re: [PATCH] ci: do not die on deprecated-declarations warning
From: Junio C Hamano @ 2023-01-15 6:59 UTC (permalink / raw)
To: Jeff King; +Cc: Ramsay Jones, git
In-Reply-To: <Y8LjTYhTycp/tTBn@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On the other hand, I don't know how useful those deprecations are going
> to be, as it depends on the timeframe. If the deprecation is added for
> the same version of libcurl that implements the alternative (which is
> roughly the case here), then we'd always be stuck supporting the old and
> new forms (old for backwards compatibility, and new to silence the
> deprecation warning).
... or just keep the warning without promoting, with "-Wno-error=...".
> We care a lot more about the deprecation once the
> alternative has been around for a while, and/or the old way of doing
> things is about to be removed. And if we just wait until that removal,
> then we do not have to rely on deprecation warnings. The build will
> break just fine on its own. :)
Yes and no. It is not always like "this symbol is now known under
this different name", which is trivial to adjust. I briefly tried
to see how IOCTL -> SEEK change should look like until I realized
that the new way was invented too recently and stopped looking, but
it would involve changes to the function logic in the callback
functions, as the function signature---both parameters and its
return values---of the callback changes. I do not want to see us
scrambling to make such adjustments to the code at the last minute,
so some sort of advance warning is a good thing to have.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 1/5] notes.c: cleanup 'strbuf_grow' call in 'append_edit'
From: Eric Sunshine @ 2023-01-15 4:53 UTC (permalink / raw)
To: Teng Long; +Cc: avarab, git, tenglong.tl
In-Reply-To: <f00a7596587bbf2d055dbf77a19506be1a6350fd.1673490953.git.dyroneteng@gmail.com>
On Wed, Jan 11, 2023 at 9:48 PM Teng Long <dyroneteng@gmail.com> wrote:
> Let's cleanup the unnecessary 'strbuf_grow' call in 'append_edit'. This
> "strbuf_grow(&d.buf, size + 1);" is prepared for insert a blank line if
> needed, but actually when inserting, "strbuf_insertstr(&d.buf, 0,
> "\n");" will do the "grow" for us.
>
> Signed-off-by: Teng Long <dyroneteng@gmail.com>
> ---
> diff --git a/builtin/notes.c b/builtin/notes.c
> @@ -618,7 +618,6 @@ static int append_edit(int argc, const char **argv, const char *prefix)
> char *prev_buf = read_object_file(note, &type, &size);
>
> - strbuf_grow(&d.buf, size + 1);
> if (d.buf.len && prev_buf && size)
> strbuf_insertstr(&d.buf, 0, "\n");
Indeed, it's not clear why that was there in the first place. Digging
through history doesn't shed any light on it. It was introduced by
2347fae50b (builtin-notes: Add "append" subcommand for appending to
note objects, 2010-02-13)[1], but there's no explanation as to why it
was coded that way. Best guess may be that the author originally
inserted "\n" manually by direct manipulation of the strbuf rather
than employing a strbuf function, but then switched to strbuf_insert()
before submitting the series and forgot to remove the now-unnecessary
strbuf_grow().
[1]: https://lore.kernel.org/git/1266096518-2104-26-git-send-email-johan@herland.net/
^ permalink raw reply
* Re: [PATCH v8 3/4] worktree add: add --orphan flag
From: Junio C Hamano @ 2023-01-15 3:49 UTC (permalink / raw)
To: rsbecker
Cc: 'Jacob Abel', phillip.wood, git,
'Ævar Arnfjörð Bjarmason',
'Eric Sunshine', 'Phillip Wood',
'Rubén Justo', 'Taylor Blau'
In-Reply-To: <013701d92893$3fad5d10$bf081730$@nexbridge.com>
<rsbecker@nexbridge.com> writes:
> I am wondering whether --detached is a more semantically consistent option. While --orphan has meaning in checkout (not one I ever liked), detached makes more sense as a description of what is intended here - as in not connected.
An orphan is not even detached, if I understand correctly.
The state is what is called "being on an unborn branch", where your
HEAD does not even point at any commit. HEAD only knows a name of a
branch that is not yet created but will be when you make a commit.
While "(HEAD) being detached" means that you are on an existing
commit---it is just that future history you extend by making a
commit from that state will not be on any branch.
So if we wanted to fix the misnomer, s/orphan/unborn/ would be how I
would go about it.
^ permalink raw reply
* RE: [PATCH v8 3/4] worktree add: add --orphan flag
From: rsbecker @ 2023-01-15 3:41 UTC (permalink / raw)
To: 'Junio C Hamano', 'Jacob Abel'
Cc: phillip.wood, git,
'Ævar Arnfjörð Bjarmason',
'Eric Sunshine', 'Phillip Wood',
'Rubén Justo', 'Taylor Blau'
In-Reply-To: <xmqqo7r0ijdv.fsf@gitster.g>
On January 14, 2023 10:09 PM, Junio C Hamano wrote:
>Jacob Abel <jacobabel@nullpo.dev> writes:
>
>>> git worktree add --orphan -b topic main
>>> git worktree add --orphan -B topic main
>>
>> I am hesitant to add these as they break away from the syntax used in
>> `git switch` and `git checkout`.
>
>Not that I care too deeply, but doesn't it introduce end-user confusion if we try to
>be compatible with "git checkout --orphan <branch>", while allowing this to be
>compatible with the default choice of the branch name done by "git worktree
>add"? "--orphan" in "git checkout" behaves similar to "-b|-B" in that it always
>wants a name, but "git worktree add" wants to make it optional.
>
>By the way "--orphan" in checkout|switch wants to take a name for itself, e.g.
>
> git checkout --orphan $name [$commit]
> git checkout -b $name [$commit]
> git checkout -B $name [$commit]
>
>so it is impossible to force their "--orphan" to rename an existing branch, which is
>probalby a design mistake we may want to fix.
>
>In any case, as I said, I do not care too deeply which way you guys decide to go,
>because I think the whole "orphan" UI is a design mistake that instills a broken
>mental model to its users [*].
>
>But let's wait a bit more to see which among
>
>(1) git worktree add [[--orphan] -b $branch] $path
> This allows --orphan to act as a modifier to existing -b,
>
>(2) git worktree add [(--orphan|-b) $branch] $path
> This allows --orphan to be another mode of -b, or
>
>(3) git worktree add [--orphan [$branch]|(-b $branch)] $path
> This allows --orphan to default to $(basename $path)
>
>people prefer.
>
>
>[Footnote]
>
>* I am not saying that it is wrong or useless to keep an unrelated
> history, especially one that records trees that have no relevance
> to the main history like created with "switch --orphan", in the
> same repository. Allowing "git switch --orphan" to create such a
> separate history in the same repository blurs the distinction. It
> would help newbies to form the right mental model if they start a
> separate repository that the separate history originates in, and
> pull from it to bootstrap the unrelated history in the local
> repository.
I am wondering whether --detached is a more semantically consistent option. While --orphan has meaning in checkout (not one I ever liked), detached makes more sense as a description of what is intended here - as in not connected.
--Randall
^ permalink raw reply
* Re: [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Junio C Hamano @ 2023-01-15 3:34 UTC (permalink / raw)
To: Strawbridge, Michael; +Cc: git@vger.kernel.org, Tuikov, Luben
In-Reply-To: <xmqqa62lm76t.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> "Strawbridge, Michael" <Michael.Strawbridge@amd.com> writes:
>>
>>> +test_expect_success $PREREQ "--validate hook supports header argument" '
>>> + test_when_finished "rm my-hooks.ran" &&
>>> + write_script my-hooks/sendemail-validate <<-\EOF &&
>>> + filesize=$(stat -c%s "$2")
>>
>> That "stat -c" is a GNU-ism, I think. macOS CI jobs at GitHub do
>> not seem to like it.
>>
>>> + if [ "$filesize" != "0" ]; then
>>
>> Also, please see Documentation/CodingGuidelines to learn the subset
>> of shell script syntax and style we adopted for this project.
I'll tentatively queue this as a fix-up on top of the topic, but is
this testing the right thing? Should we inspect "$2" and verify
that it gives us what we expect, not just it being non-empty?
t/t9001-send-email.sh | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git c/t/t9001-send-email.sh w/t/t9001-send-email.sh
index f02b1eba16..b19997cbbc 100755
--- c/t/t9001-send-email.sh
+++ w/t/t9001-send-email.sh
@@ -568,9 +568,10 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
test_expect_success $PREREQ "--validate hook supports header argument" '
test_when_finished "rm my-hooks.ran" &&
write_script my-hooks/sendemail-validate <<-\EOF &&
- filesize=$(stat -c%s "$2")
- if [ "$filesize" != "0" ]; then
- >my-hooks.ran
+ rm -f my-hooks.ran
+ if test -s "$2"
+ then
+ >my-hooks.ran
fi
exit 1
EOF
^ permalink raw reply related
* Re: Gitorious should use CRC128 / 256 / 512 instead of SHA-1
From: Junio C Hamano @ 2023-01-15 3:14 UTC (permalink / raw)
To: brian m. carlson; +Cc: Hans Petter Selasky, git
In-Reply-To: <Y8NB21PExmifhyeQ@tapette.crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> 3) Illicit contents may be present in binary blobs, which in the future may
>> be need to be removed without warrant and the only way to do that is by
>> rebasing and force pushing, which will break "everything". It can be
>> everything from child-porn to expired distribution licenses.
>
> This is a problem in every Merkle tree-like system. Most repositories
> have some sort of code review or access control that prevents people
> from generally pushing inappropriate content. For example, if somebody
> proposed to push any sort of pornography or other inappropriate content
> (e.g., a racist screed) to one of my repositories or one of my
> employer's, I'd refuse to approve or merge such a change, because
> that wouldn't be appropriate for the repository.
>
> I don't feel this is enough of a problem that using a Merkle tree-like
> construction is a bad idea, given the benefits it offers.
While I agree with the primary thrust of your argument, this one is
a bit tricky to reason about. External rules change and can declare
what has been accepted as appropriate inappropriate on a whim, long
after you reviewed the material coming into your history and decided
it was perfectly fine, under the then-prevailing definition of what
is and isn't appropriate.
^ permalink raw reply
* Re: [PATCH v8 3/4] worktree add: add --orphan flag
From: Junio C Hamano @ 2023-01-15 3:09 UTC (permalink / raw)
To: Jacob Abel
Cc: phillip.wood, git, Ævar Arnfjörð Bjarmason,
Eric Sunshine, Phillip Wood, Rubén Justo, Taylor Blau,
rsbecker
In-Reply-To: <20230114224715.ewec6sz5h3q3iijs@phi>
Jacob Abel <jacobabel@nullpo.dev> writes:
>> git worktree add --orphan -b topic main
>> git worktree add --orphan -B topic main
>
> I am hesitant to add these as they break away from the syntax used in
> `git switch` and `git checkout`.
Not that I care too deeply, but doesn't it introduce end-user
confusion if we try to be compatible with "git checkout --orphan
<branch>", while allowing this to be compatible with the default
choice of the branch name done by "git worktree add"? "--orphan" in
"git checkout" behaves similar to "-b|-B" in that it always wants a
name, but "git worktree add" wants to make it optional.
By the way "--orphan" in checkout|switch wants to take a name for
itself, e.g.
git checkout --orphan $name [$commit]
git checkout -b $name [$commit]
git checkout -B $name [$commit]
so it is impossible to force their "--orphan" to rename an existing
branch, which is probalby a design mistake we may want to fix.
In any case, as I said, I do not care too deeply which way you guys
decide to go, because I think the whole "orphan" UI is a design
mistake that instills a broken mental model to its users [*].
But let's wait a bit more to see which among
(1) git worktree add [[--orphan] -b $branch] $path
This allows --orphan to act as a modifier to existing -b,
(2) git worktree add [(--orphan|-b) $branch] $path
This allows --orphan to be another mode of -b, or
(3) git worktree add [--orphan [$branch]|(-b $branch)] $path
This allows --orphan to default to $(basename $path)
people prefer.
[Footnote]
* I am not saying that it is wrong or useless to keep an unrelated
history, especially one that records trees that have no relevance
to the main history like created with "switch --orphan", in the
same repository. Allowing "git switch --orphan" to create such a
separate history in the same repository blurs the distinction. It
would help newbies to form the right mental model if they start a
separate repository that the separate history originates in, and
pull from it to bootstrap the unrelated history in the local
repository.
^ permalink raw reply
* Re: ja/worktree-orphan (was What's cooking in git.git (Jan 2023, #04; Sat, 14))
From: Junio C Hamano @ 2023-01-15 2:07 UTC (permalink / raw)
To: Phillip Wood; +Cc: git, Jacob Abel, Ævar Arnfjörð Bjarmason
In-Reply-To: <11be1b0e-ee38-119f-1d80-cb818946116b@dunelm.org.uk>
Phillip Wood <phillip.wood123@gmail.com> writes:
> On 14/01/2023 08:36, Junio C Hamano wrote:
>> * ja/worktree-orphan (2023-01-13) 4 commits
>> - worktree add: add hint to direct users towards --orphan
>> - worktree add: add --orphan flag
>> - worktree add: refactor opt exclusion tests
>> - worktree add: include -B in usage docs
>> 'git worktree add' learned how to create a worktree based on an
>> orphaned branch with `--orphan`.
>> Will merge to 'next'.
>> source: <20230109173227.29264-1-jacobabel@nullpo.dev>
>
> If possible it would be nice to clean up the ui before merging this, I
> think it would be quite a small change to the implementation. cf
> <e5aadd5d-9b85-4dc9-e9f7-117892b4b283@dunelm.org.uk>
Thanks. Let's wait for and queue an updated one.
^ permalink raw reply
* Re: [PATCH] env-helper: move this built-in to to "test-tool env-helper"
From: Junio C Hamano @ 2023-01-15 2:06 UTC (permalink / raw)
To: Andrei Rybak; +Cc: Ævar Arnfjörð Bjarmason, git, Jeff King
In-Reply-To: <744c8abe-6065-872d-8c9f-a1b1ab682d52@gmail.com>
Andrei Rybak <rybak.a.v@gmail.com> writes:
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -799,6 +799,7 @@ TEST_BUILTINS_OBJS += test-dump-fsmonitor.o
>> TEST_BUILTINS_OBJS += test-dump-split-index.o
>> TEST_BUILTINS_OBJS += test-dump-untracked-cache.o
>> TEST_BUILTINS_OBJS += test-example-decorate.o
>> +TEST_BUILTINS_OBJS += test-env-helper.o
>
> The .o files are sorted alphabetically in TEST_BUILTINS_OBJS, so
> test-env-helper.o should be above test-example-decorate.o
I'll locally tweak to correct this, to prevent this patch from
making it worse, but the ordering of other entries are already
broken at other places (e.g. around test-rot13-filter.o). Somebody
may want to send in a clean-up patch, or alternatively we may not
care too deeply and doing nothing about it ;-).
Thanks.
^ permalink raw reply
* Re: Gitorious should use CRC128 / 256 / 512 instead of SHA-1
From: brian m. carlson @ 2023-01-14 23:59 UTC (permalink / raw)
To: Hans Petter Selasky; +Cc: git
In-Reply-To: <9c0fda42-67ab-f406-489b-38a2d9bbcfc2@selasky.org>
[-- Attachment #1: Type: text/plain, Size: 3907 bytes --]
On 2023-01-13 at 13:23:59, Hans Petter Selasky wrote:
> Hi,
>
> Currently GIT only supports cryptographic hashes for its commit tags.
>
> That means:
>
> 1) It's very difficult to edit the history without also recomputing the hash
> tags for all commits after the needed change-point, which then means
> references to a repository is broken.
This is intentional. Commit and tag signing requires an unbroken Merkle
tree-like construction that prevents the history from being modified by
signing a single commit or tag.
> 2) Only a single bit error in the main repository can break everything!
git fsck is designed to detect this, and by default it's run every time
the repository is repacked (such as by git gc). But yes, this is a
problem, and changing to an algorithm which isn't cryptographically
secure won't change that. Prudent users back up data to prevent data
loss.
> 3) Illicit contents may be present in binary blobs, which in the future may
> be need to be removed without warrant and the only way to do that is by
> rebasing and force pushing, which will break "everything". It can be
> everything from child-porn to expired distribution licenses.
This is a problem in every Merkle tree-like system. Most repositories
have some sort of code review or access control that prevents people
from generally pushing inappropriate content. For example, if somebody
proposed to push any sort of pornography or other inappropriate content
(e.g., a racist screed) to one of my repositories or one of my
employer's, I'd refuse to approve or merge such a change, because
that wouldn't be appropriate for the repository.
I don't feel this is enough of a problem that using a Merkle tree-like
construction is a bad idea, given the benefits it offers.
> Therefore I propose the following changes to GIT.
>
> 1) Use a CRC128 / 256 or 512 non-cryptographic based hashing algorithm as
> default.
As the person who wrote the SHA-256 support, I'm pleased to report that
adding a new hash algorithm isn't very difficult anymore. The largest
part of the work is updating all the tests. I've tried very hard to
make this substantially easier for everyone.
However, Git is moving in the direction of stronger cryptographic
algorithms, rather than insecure hashing algorithms. I don't think your
proposal is a good idea, nor do I think it's likely to be adopted.
If it were adopted, the signing of commits and tags would be
meaningless, and because it would be trivial to create collisions[0], there
would clearly be some pairs of objects which could not be stored. This
would make Git much less useful, and it might allow users to attempt to
forge or replace content without being detected.
That being said, you are free to create your own fork of the code which
does so, provided you comply with the terms of the license.
> 2) Add support for a CRC fixup field, which usually is zero, but when merges
> are needed, it can be non-zero, to allow the hash-tag-value to remain the
> same! This also allows for easy conversion of existing GIT repositories to
> the new scheme.
For the same reason as above, I don't think this is a good idea.
> 3) All git objects should be uncompressed.
This would dramatically increase the size of most repositories. I've
easily seen repositories where the uncompressed contents exceed 1 TB in
size yet the repository is only double-digit gigabytes, if that. Most
people will find the increase in disk usage unacceptable, and I'm
certain that includes Git hosterse.
[0] CRC is linear and the following relations apply, which makes forgery
trivial (see https://en.wikipedia.org/wiki/Cyclic_redundancy_check):
CRC(x XOR y) = CRC(x) XOR CRC(y) XOR c for some c
CRC(x XOR y XOR z) = CRC(x) XOR CRC(y) XOR CRC(z)
--
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] worktree add: introduce basic DWYM for --orphan
From: Jacob Abel @ 2023-01-14 22:58 UTC (permalink / raw)
To: Jacob Abel
Cc: Ævar Arnfjörð Bjarmason, Eric Sunshine,
Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
rsbecker, git
In-Reply-To: <20230114224956.24801-1-jacobabel@nullpo.dev>
On 23/01/14 10:50PM, Jacob Abel wrote:
>
> [...]
Oops for some reason this didn't show up as a reply on the lore. This is
supposed to be a reply to the following email chain:
https://lore.kernel.org/git/20230114224715.ewec6sz5h3q3iijs@phi/
^ permalink raw reply
* [PATCH] worktree add: introduce basic DWYM for --orphan
From: Jacob Abel @ 2023-01-14 22:50 UTC (permalink / raw)
To: git
Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
rsbecker
Introduces a DWYM shorthand of --orphan for when the worktree directory
and the to-be-created branch share the same name.
Current Behavior:
% git worktree list
/path/to/git/repo a38d39a4c5 [main]
% git worktree add --orphan new_branch ../new_branch/
Preparing worktree (new branch 'new_branch')
% git worktree add --orphan ../new_branch2/
usage: git worktree add [<options>] <path> [<commit-ish>]
or: git worktree list [<options>]
[...]
%
New Behavior:
% git worktree list
/path/to/git/repo a38d39a4c5 [main]
% git worktree add --orphan new_branch ../new_branch/
Preparing worktree (new branch 'new_branch')
% git worktree list
/path/to/git/repo a38d39a4c5 [main]
/path/to/git/new_branch a38d39a4c5 [new_branch]
% git worktree add --orphan ../new_branch2/
Preparing worktree (new branch 'new_branch2')
% git worktree list
/path/to/git/repo a38d39a4c5 [main]
/path/to/git/new_branch a38d39a4c5 [new_branch]
/path/to/git/new_branch2 a38d39a4c5 [new_branch2]
%
Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
Documentation/git-worktree.txt | 13 +++++++++----
builtin/worktree.c | 21 +++++++++++++++------
t/t2400-worktree-add.sh | 10 ++++++++++
3 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index d78460c29c..a56ddb0185 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -12,7 +12,7 @@ SYNOPSIS
'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
[(-b | -B) <new-branch>] <path> [<commit-ish>]
'git worktree add' [-f] [--lock [--reason <string>]]
- --orphan <new-branch> <path>
+ --orphan [<new-branch>] <path>
'git worktree list' [-v | --porcelain [-z]]
'git worktree lock' [--reason <string>] <worktree>
'git worktree move' <worktree> <new-path>
@@ -99,13 +99,16 @@ in the new worktree, if it's not checked out anywhere else, otherwise the
command will refuse to create the worktree (unless `--force` is used).
+
------------
-$ git worktree add --orphan <branch> <path>
+$ git worktree add --orphan [<branch>] <path>
------------
+
Create a worktree containing no files, with an empty index, and associated
with a new orphan branch named `<branch>`. The first commit made on this new
branch will have no parents and will be the root of a new history disconnected
from any other branches.
++
+If a branch name `<branch>` is not supplied, the name is derived from the
+supplied path `<path>`.
list::
@@ -233,9 +236,11 @@ This can also be set up as the default behaviour by using the
With `prune`, do not remove anything; just report what it would
remove.
---orphan <new-branch>::
+--orphan [<new-branch>]::
With `add`, make the new worktree and index empty, associating
- the worktree with a new orphan branch named `<new-branch>`.
+ the worktree with a new orphan branch named `<new-branch>`. If
+ `<new-branch>` is not supplied, the new branch name is derived
+ from `<path>`.
--porcelain::
With `list`, output in an easy-to-parse format for scripts.
diff --git a/builtin/worktree.c b/builtin/worktree.c
index d975628353..481f895075 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -19,7 +19,7 @@
N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
" [(-b | -B) <new-branch>] <path> [<commit-ish>]"), \
N_("git worktree add [-f] [--lock [--reason <string>]]\n" \
- " --orphan <new-branch> <path>")
+ " --orphan [<new-branch>] <path>")
#define BUILTIN_WORKTREE_LIST_USAGE \
N_("git worktree list [-v | --porcelain [-z]]")
@@ -681,10 +681,13 @@ static int add(int ac, const char **av, const char *prefix)
else if (keep_locked)
opts.keep_locked = _("added with --lock");
- if (ac < 1 || ac > 2)
+ if (ac < 1 && opts.orphan) {
+ path = prefix_filename(prefix, orphan_branch);
+ } else if (ac >= 1 && ac <= 2) {
+ path = prefix_filename(prefix, av[0]);
+ } else {
usage_with_options(git_worktree_add_usage, options);
-
- path = prefix_filename(prefix, av[0]);
+ }
branch = ac < 2 ? "HEAD" : av[1];
if (!strcmp(branch, "-"))
@@ -702,14 +705,20 @@ static int add(int ac, const char **av, const char *prefix)
strbuf_release(&symref);
}
- if (opts.orphan) {
- new_branch = orphan_branch;
+ if (ac < 1 && opts.orphan) {
+ const char *s = dwim_branch(path, &orphan_branch);
+ if (s)
+ orphan_branch = s;
} else if (ac < 2 && !new_branch && !opts.detach) {
const char *s = dwim_branch(path, &new_branch);
if (s)
branch = s;
}
+ if (opts.orphan) {
+ new_branch = orphan_branch;
+ }
+
if (ac == 2 && !new_branch && !opts.detach) {
struct object_id oid;
struct commit *commit;
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 1bf8d619e2..c3de277738 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -354,6 +354,16 @@ test_expect_success '"add --orphan" fails if the branch already exists' '
test_path_is_missing orphandir2
'
+test_expect_success '"add --orphan" with basic DWYM' '
+ test_when_finished "rm -rf empty_repo" &&
+ echo refs/heads/worktreedir >expected &&
+ GIT_DIR="empty_repo" git init --bare &&
+ # Use non-trivial path to verify it DWYMs properly.
+ git -C empty_repo worktree add --orphan ../empty_repo/worktreedir &&
+ git -C empty_repo/worktreedir symbolic-ref HEAD >actual &&
+ test_cmp expected actual
+'
+
test_expect_success '"add --orphan" with empty repository' '
test_when_finished "rm -rf empty_repo" &&
echo refs/heads/newbranch >expected &&
--
2.38.2
^ permalink raw reply related
* Re: [PATCH v8 3/4] worktree add: add --orphan flag
From: Jacob Abel @ 2023-01-14 22:47 UTC (permalink / raw)
To: phillip.wood
Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
rsbecker
In-Reply-To: <e5aadd5d-9b85-4dc9-e9f7-117892b4b283@dunelm.org.uk>
On 23/01/13 10:20AM, Phillip Wood wrote:
> Hi Jacob
>
> On 09/01/2023 17:33, Jacob Abel wrote:
> > [...]
>
> It's perhaps a bit late to bring this up but I've only just realized
> that it is unfortunate that --orphan takes a branch name rather than
> working in conjunction with -b/-B. It means that in the common case
> where the branch name is the same as the worktree the user has to repeat
> it on the command line as shown above. It also means there is no way to
> force an orphan branch (that's admittedly quite niche). If instead
> --orphan did not take an argument we could have
>
> git worktree add --orphan main
> git worktree add --orphan -b topic main
> git worktree add --orphan -B topic main
>
> Best Wishes
>
> Phillip
>
> > [...]
I think this is a good idea and something similar was brought up previously
however I originally wanted to handle this and a common --orphan DWYM in a later
patch.
> git worktree add --orphan main
I am OK implementing this option and have been workshopping it prior to
responding. I think I have it worked out now as an additional patch which can be
be applied on top of the v8 patchset.
I'll reply to this message with the one-off patch to get feedback. Since this is
essentially a discrete change on top of v8, I can either keep it as a separate
patch or reroll depending on how much needs to be changed (and what would be
easier for everyone).
> git worktree add --orphan -b topic main
> git worktree add --orphan -B topic main
I am hesitant to add these as they break away from the syntax used in
`git switch` and `git checkout`.
Also apologies for the tangent but while researching this path, I noticed that
--orphan behaves unexpectedly on both `git switch` and `git checkout` when mixed
with `-c` and `-b` respectively.
% git switch --orphan -c foobar
fatal: invalid reference: foobar
% git switch -c --orphan foobar
fatal: invalid reference: foobar
% git checkout -b --orphan foobar
fatal: 'foobar' is not a commit and a branch '--orphan' cannot be created from it
% git checkout --orphan -b foobar
fatal: 'foobar' is not a commit and a branch '-b' cannot be created from it
I tried this on my system install as well as from a fresh fetch of next FWIW.
[Info: fresh build from next]
git version 2.39.0.287.g8cbeef4abd
cpu: x86_64
built from commit: 8cbeef4abda4907dd68ea144d9dcb85f0b49c3e6
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
[Info: system install]
git version 2.38.2
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
If this bug is something that needs to be addressed, I can dig a bit deeper and
put together a patch for it in the next few days.
VR,
Abel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox