* Re: Unable to install latest git version. Claims git process pid running
From: David Brown @ 2018-12-21 20:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <nycvar.QRO.7.76.6.1812211652150.41@tvgsbejvaqbjf.bet>
Howdy Johannes, thanks for the speedy reply.
Unfortunately I have hit refresh many times in previous attempts.
Notwithstanding, the dialog maintains the error condition claiming pid
10128 must be closed prior to install.
I'm a big fan of Git but not a Windows fan in the least.
I have downloaded the latest Windows git and killing as many apps and
pids possible after Windows restart the installation goes south
nonetheless.
Please advise.
On 2018-12-21 09:55, Johannes Schindelin wrote:
> Hi,
>
> On Thu, 20 Dec 2018, David Brown wrote:
>
>> - [ ] I was not able to find an
>> [open](https://github.com/git-for-windows/git/issues?q=is%3Aopen) or
>> [closed](https://github.com/git-for-windows/git/issues?q=is%3Aclosed)
>> issue
>> matching what I'm seeing
>
> That's a new one. I saw many bug reporters simply deleting the issue
> reporting template at https://github.com/git-for-windows/git/issues/new
> (and usually then filing only very little information, definitely not
> enough to make sense of the bug report), but I never saw this bug
> reporting template being used to report a Git for Windows bug to the
> Git
> mailing list yet ;-)
>
>> ### Setup
>>
>> - Which version of Git for Windows are you using? Is it 32-bit or
>> 64-bit?
>>
>> $ git --version --build-options
>>
>> git version 2.20.0.windows.1 cpu: x86_64 built from commit:
>> 95155834166f64fe9666f2c0a4909f076080893a
>> sizeof-long: 4
>> sizeof-size_t: 8 **
>>
>> - Which version of Windows are you running? Vista, 7, 8, 10? Is it
>> 32-bit or
>> 64-bit?
>>
>> $ cmd.exe /c ver
>>
>> ** Microsoft Windows [Version 6.1.7601] **
>>
>> - What options did you set as part of the installation? Or did you
>> choose the
>> defaults?
>>
>> # One of the following:
>>
>> $ cat /etc/install-options.txt Editor Option: VIM Custom Editor Path:
>> Path Option: BashOnly
>> SSH Option: OpenSSH
>> CURL Option: WinSSL
>> CRLF Option: CRLFAlways
>> Bash Terminal Option: MinTTY
>> Performance Tweaks
>> FSCache: Enabled
>> Use Credential Manager: Enabled
>> Enable Symlinks: Disabled
>>
>> - Any other interesting things about your environment that might be
>> related
>> to the issue you're seeing?
>>
>> ** A so-called VDI running on a Dell WYSE **
>>
>> ### Details
>>
>> - Which terminal/shell are you running Git from? e.g
>> Bash/CMD/PowerShell/other
>>
>> ** Bash **
>>
>> - What commands did you run to trigger this issue? If you can provide
>> a
>> [Minimal, Complete, and Verifiable
>> example](http://stackoverflow.com/help/mcve)
>> this will help us understand the issue.
>>
>> ** Windows git upgrade dialog **
>>
>> - What did you expect to occur after running these commands?
>>
>> ** latest version of git installed **
>>
>> - What actually happened instead?
>>
>> ** No such pid 10128 to close therefore cancel install **
>
> This typically means that that process existed *at the time the
> installer
> checked*. If it was a short-lived process, you can check again by
> clicking
> that "Refresh" button on the lower left.
>
> Ciao,
> Johannes
>
>>
>> - If the probleminsert URL herey, can you provide the
>> URL to that repository to help us with testing?
>>
>> ** Internal company url. Windows git install dialog is issue **
>>
^ permalink raw reply
* [PATCH v2 4/7] midx: refactor permutation logic
From: Derrick Stolee via GitGitGadget @ 2018-12-21 16:28 UTC (permalink / raw)
To: git; +Cc: sbeller, peff, jrnieder, avarab, Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.92.v2.git.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
When writing a multi-pack-index, we keep track of an integer
permutation, tracking the list of pack-files that we know about
(both from the existing multi-pack-index and the new pack-files
being introduced) and converting them into a sorted order for
the new multi-pack-index.
In anticipation of dropping pack-files from the existing multi-
pack-index, refactor the logic around how we track this permutation.
First, insert the permutation into the pack_list structure. This
allows us to grow the permutation dynamically as we add packs.
Second, fill the permutation with values corresponding to their
position in the list of pack-files, sorted as follows:
1. The pack-files in the existing multi-pack-index,
sorted lexicographically.
2. The pack-files not in the existing multi-pack-index,
sorted as discovered from the filesystem.
There is a subtle thing in how we initialize this permutation,
specifically how we use 'i' for the initial value. This will
matter more when we implement the logic for dropping existing
packs, as we will create holes in the ordering.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
midx.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/midx.c b/midx.c
index bb825ef816..3bd7183a53 100644
--- a/midx.c
+++ b/midx.c
@@ -380,9 +380,11 @@ static size_t write_midx_header(struct hashfile *f,
struct pack_list {
struct packed_git **list;
char **names;
+ uint32_t *perm;
uint32_t nr;
uint32_t alloc_list;
uint32_t alloc_names;
+ uint32_t alloc_perm;
size_t pack_name_concat_len;
struct multi_pack_index *m;
};
@@ -398,6 +400,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,
ALLOC_GROW(packs->list, packs->nr + 1, packs->alloc_list);
ALLOC_GROW(packs->names, packs->nr + 1, packs->alloc_names);
+ ALLOC_GROW(packs->perm, packs->nr + 1, packs->alloc_perm);
packs->list[packs->nr] = add_packed_git(full_path,
full_path_len,
@@ -417,6 +420,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,
return;
}
+ packs->perm[packs->nr] = packs->nr;
packs->names[packs->nr] = xstrdup(file_name);
packs->pack_name_concat_len += strlen(file_name) + 1;
packs->nr++;
@@ -443,7 +447,7 @@ static void sort_packs_by_name(char **pack_names, uint32_t nr_packs, uint32_t *p
ALLOC_ARRAY(pairs, nr_packs);
for (i = 0; i < nr_packs; i++) {
- pairs[i].pack_int_id = i;
+ pairs[i].pack_int_id = perm[i];
pairs[i].pack_name = pack_names[i];
}
@@ -755,7 +759,6 @@ int write_midx_file(const char *object_dir)
struct hashfile *f = NULL;
struct lock_file lk;
struct pack_list packs;
- uint32_t *pack_perm = NULL;
uint64_t written = 0;
uint32_t chunk_ids[MIDX_MAX_CHUNKS + 1];
uint64_t chunk_offsets[MIDX_MAX_CHUNKS + 1];
@@ -774,18 +777,22 @@ int write_midx_file(const char *object_dir)
packs.nr = 0;
packs.alloc_list = packs.m ? packs.m->num_packs : 16;
- packs.alloc_names = packs.alloc_list;
+ packs.alloc_perm = packs.alloc_names = packs.alloc_list;
packs.list = NULL;
packs.names = NULL;
+ packs.perm = NULL;
packs.pack_name_concat_len = 0;
ALLOC_ARRAY(packs.list, packs.alloc_list);
ALLOC_ARRAY(packs.names, packs.alloc_names);
+ ALLOC_ARRAY(packs.perm, packs.alloc_perm);
if (packs.m) {
for (i = 0; i < packs.m->num_packs; i++) {
ALLOC_GROW(packs.list, packs.nr + 1, packs.alloc_list);
ALLOC_GROW(packs.names, packs.nr + 1, packs.alloc_names);
+ ALLOC_GROW(packs.perm, packs.nr + 1, packs.alloc_perm);
+ packs.perm[packs.nr] = i;
packs.list[packs.nr] = NULL;
packs.names[packs.nr] = xstrdup(packs.m->pack_names[i]);
packs.pack_name_concat_len += strlen(packs.names[packs.nr]) + 1;
@@ -802,10 +809,9 @@ int write_midx_file(const char *object_dir)
packs.pack_name_concat_len += MIDX_CHUNK_ALIGNMENT -
(packs.pack_name_concat_len % MIDX_CHUNK_ALIGNMENT);
- ALLOC_ARRAY(pack_perm, packs.nr);
- sort_packs_by_name(packs.names, packs.nr, pack_perm);
+ sort_packs_by_name(packs.names, packs.nr, packs.perm);
- entries = get_sorted_entries(packs.m, packs.list, pack_perm, packs.nr, &nr_entries);
+ entries = get_sorted_entries(packs.m, packs.list, packs.perm, packs.nr, &nr_entries);
for (i = 0; i < nr_entries; i++) {
if (entries[i].offset > 0x7fffffff)
@@ -923,8 +929,8 @@ int write_midx_file(const char *object_dir)
free(packs.list);
free(packs.names);
+ free(packs.perm);
free(entries);
- free(pack_perm);
free(midx_name);
return 0;
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 7/7] midx: implement midx_repack()
From: Derrick Stolee via GitGitGadget @ 2018-12-21 16:28 UTC (permalink / raw)
To: git; +Cc: sbeller, peff, jrnieder, avarab, Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.92.v2.git.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
To repack using a multi-pack-index, first sort all pack-files by
their modified time. Second, walk those pack-files from oldest
to newest, adding the packs to a list if they are smaller than the
given pack-size. Finally, collect the objects from the multi-pack-
index that are in those packs and send them to 'git pack-objects'.
While first designing a 'git multi-pack-index repack' operation, I
started by collecting the batches based on the size of the objects
instead of the size of the pack-files. This allows repacking a
large pack-file that has very few referencd objects. However, this
came at a significant cost of parsing pack-files instead of simply
reading the multi-pack-index and getting the file information for
the pack-files. This object-size idea could be a direction for
future expansion in this area.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
midx.c | 109 +++++++++++++++++++++++++++++++++++-
t/t5319-multi-pack-index.sh | 25 +++++++++
2 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/midx.c b/midx.c
index 127c43f7b0..f6bc111438 100644
--- a/midx.c
+++ b/midx.c
@@ -8,6 +8,7 @@
#include "sha1-lookup.h"
#include "midx.h"
#include "progress.h"
+#include "run-command.h"
#define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
#define MIDX_VERSION 1
@@ -1111,7 +1112,113 @@ int expire_midx_packs(const char *object_dir)
return result;
}
-int midx_repack(const char *object_dir, size_t batch_size)
+struct time_and_id {
+ timestamp_t mtime;
+ uint32_t pack_int_id;
+};
+
+static int compare_by_mtime(const void *a_, const void *b_)
{
+ const struct time_and_id *a, *b;
+
+ a = (const struct time_and_id *)a_;
+ b = (const struct time_and_id *)b_;
+
+ if (a->mtime < b->mtime)
+ return -1;
+ if (a->mtime > b->mtime)
+ return 1;
return 0;
}
+
+int midx_repack(const char *object_dir, size_t batch_size)
+{
+ int result = 0;
+ uint32_t i, packs_to_repack;
+ size_t total_size;
+ struct time_and_id *pack_ti;
+ unsigned char *include_pack;
+ struct child_process cmd = CHILD_PROCESS_INIT;
+ struct strbuf base_name = STRBUF_INIT;
+ struct multi_pack_index *m = load_multi_pack_index(object_dir, 1);
+
+ if (!m)
+ return 0;
+
+ include_pack = xcalloc(m->num_packs, sizeof(unsigned char));
+ pack_ti = xcalloc(m->num_packs, sizeof(struct time_and_id));
+
+ for (i = 0; i < m->num_packs; i++) {
+ pack_ti[i].pack_int_id = i;
+
+ if (prepare_midx_pack(m, i))
+ continue;
+
+ pack_ti[i].mtime = m->packs[i]->mtime;
+ }
+ QSORT(pack_ti, m->num_packs, compare_by_mtime);
+
+ total_size = 0;
+ packs_to_repack = 0;
+ for (i = 0; total_size < batch_size && i < m->num_packs; i++) {
+ int pack_int_id = pack_ti[i].pack_int_id;
+ struct packed_git *p = m->packs[pack_int_id];
+
+ if (!p)
+ continue;
+ if (p->pack_size >= batch_size)
+ continue;
+
+ packs_to_repack++;
+ total_size += p->pack_size;
+ include_pack[pack_int_id] = 1;
+ }
+
+ if (total_size < batch_size || packs_to_repack < 2)
+ goto cleanup;
+
+ argv_array_push(&cmd.args, "pack-objects");
+
+ strbuf_addstr(&base_name, object_dir);
+ strbuf_addstr(&base_name, "/pack/pack");
+ argv_array_push(&cmd.args, base_name.buf);
+ strbuf_release(&base_name);
+
+ cmd.git_cmd = 1;
+ cmd.in = cmd.out = -1;
+
+ if (start_command(&cmd)) {
+ error(_("could not start pack-objects"));
+ result = 1;
+ goto cleanup;
+ }
+
+ for (i = 0; i < m->num_objects; i++) {
+ struct object_id oid;
+ uint32_t pack_int_id = nth_midxed_pack_int_id(m, i);
+
+ if (!include_pack[pack_int_id])
+ continue;
+
+ nth_midxed_object_oid(&oid, m, i);
+ xwrite(cmd.in, oid_to_hex(&oid), the_hash_algo->hexsz);
+ xwrite(cmd.in, "\n", 1);
+ }
+ close(cmd.in);
+
+ if (finish_command(&cmd)) {
+ error(_("could not finish pack-objects"));
+ result = 1;
+ goto cleanup;
+ }
+
+ result = write_midx_internal(object_dir, m, NULL);
+ m = NULL;
+
+cleanup:
+ if (m)
+ close_midx(m);
+ free(include_pack);
+ free(pack_ti);
+ return result;
+}
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index f675621080..3f5e9ea653 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -421,4 +421,29 @@ test_expect_success 'repack with minimum size does not alter existing packs' '
)
'
+test_expect_success 'repack creates a new pack' '
+ (
+ cd dup &&
+ SECOND_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 2 | tail -n 1) &&
+ BATCH_SIZE=$(($SECOND_SMALLEST_SIZE + 1)) &&
+ git multi-pack-index repack --batch-size=$BATCH_SIZE &&
+ ls .git/objects/pack/*idx >idx-list &&
+ test_line_count = 5 idx-list &&
+ test-tool read-midx .git/objects | grep idx >midx-list &&
+ test_line_count = 5 midx-list
+ )
+'
+
+test_expect_success 'expire removes repacked packs' '
+ (
+ cd dup &&
+ ls -S .git/objects/pack/*pack | head -n 3 >expect &&
+ git multi-pack-index expire &&
+ ls -S .git/objects/pack/*pack >actual &&
+ test_cmp expect actual &&
+ test-tool read-midx .git/objects | grep idx >midx-list &&
+ test_line_count = 3 midx-list
+ )
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 5/7] multi-pack-index: implement 'expire' verb
From: Derrick Stolee via GitGitGadget @ 2018-12-21 16:28 UTC (permalink / raw)
To: git; +Cc: sbeller, peff, jrnieder, avarab, Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.92.v2.git.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
The 'git multi-pack-index expire' command looks at the existing
mult-pack-index, counts the number of objects referenced in each
pack-file, deletes the pack-fils with no referenced objects, and
rewrites the multi-pack-index to no longer reference those packs.
Refactor the write_midx_file() method to call write_midx_internal()
which now takes an existing 'struct multi_pack_index' and a list
of pack-files to drop (as specified by the names of their pack-
indexes). As we write the new multi-pack-index, we drop those
file names from the list of known pack-files.
The expire_midx_packs() method removes the unreferenced pack-files
after carefully closing the packs to avoid open handles.
Test that a new pack-file that covers the contents of two other
pack-files leads to those pack-files being deleted during the
expire command. Be sure to read the multi-pack-index to ensure
it no longer references those packs.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
midx.c | 82 +++++++++++++++++++++++++++++++++++--
t/t5319-multi-pack-index.sh | 15 +++++++
2 files changed, 93 insertions(+), 4 deletions(-)
diff --git a/midx.c b/midx.c
index 3bd7183a53..043cd1fd97 100644
--- a/midx.c
+++ b/midx.c
@@ -751,7 +751,8 @@ static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_off
return written;
}
-int write_midx_file(const char *object_dir)
+static int write_midx_internal(const char *object_dir, struct multi_pack_index *m,
+ struct string_list *packs_to_drop)
{
unsigned char cur_chunk, num_chunks = 0;
char *midx_name;
@@ -765,6 +766,7 @@ int write_midx_file(const char *object_dir)
uint32_t nr_entries, num_large_offsets = 0;
struct pack_midx_entry *entries = NULL;
int large_offsets_needed = 0;
+ int result = 0;
midx_name = get_midx_filename(object_dir);
if (safe_create_leading_directories(midx_name)) {
@@ -773,7 +775,10 @@ int write_midx_file(const char *object_dir)
midx_name);
}
- packs.m = load_multi_pack_index(object_dir, 1);
+ if (m)
+ packs.m = m;
+ else
+ packs.m = load_multi_pack_index(object_dir, 1);
packs.nr = 0;
packs.alloc_list = packs.m ? packs.m->num_packs : 16;
@@ -787,7 +792,25 @@ int write_midx_file(const char *object_dir)
ALLOC_ARRAY(packs.perm, packs.alloc_perm);
if (packs.m) {
+ int drop_index = 0, missing_drops = 0;
for (i = 0; i < packs.m->num_packs; i++) {
+ if (packs_to_drop && drop_index < packs_to_drop->nr) {
+ int cmp = strcmp(packs.m->pack_names[i],
+ packs_to_drop->items[drop_index].string);
+
+ if (!cmp) {
+ drop_index++;
+ continue;
+ } else if (cmp > 0) {
+ error(_("did not see pack-file %s to drop"),
+ packs_to_drop->items[drop_index].string);
+ drop_index++;
+ i--;
+ missing_drops++;
+ continue;
+ }
+ }
+
ALLOC_GROW(packs.list, packs.nr + 1, packs.alloc_list);
ALLOC_GROW(packs.names, packs.nr + 1, packs.alloc_names);
ALLOC_GROW(packs.perm, packs.nr + 1, packs.alloc_perm);
@@ -798,6 +821,12 @@ int write_midx_file(const char *object_dir)
packs.pack_name_concat_len += strlen(packs.names[packs.nr]) + 1;
packs.nr++;
}
+
+ if (packs_to_drop && (drop_index < packs_to_drop->nr || missing_drops)) {
+ error(_("did not see all pack-files to drop"));
+ result = 1;
+ goto cleanup;
+ }
}
for_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs);
@@ -932,7 +961,12 @@ int write_midx_file(const char *object_dir)
free(packs.perm);
free(entries);
free(midx_name);
- return 0;
+ return result;
+}
+
+int write_midx_file(const char *object_dir)
+{
+ return write_midx_internal(object_dir, NULL, NULL);
}
void clear_midx_file(struct repository *r)
@@ -1034,5 +1068,45 @@ int verify_midx_file(const char *object_dir)
int expire_midx_packs(const char *object_dir)
{
- return 0;
+ uint32_t i, *count, result = 0;
+ struct string_list packs_to_drop = STRING_LIST_INIT_DUP;
+ struct multi_pack_index *m = load_multi_pack_index(object_dir, 1);
+
+ if (!m)
+ return 0;
+
+ count = xcalloc(m->num_packs, sizeof(uint32_t));
+ for (i = 0; i < m->num_objects; i++) {
+ int pack_int_id = nth_midxed_pack_int_id(m, i);
+ count[pack_int_id]++;
+ }
+
+ for (i = 0; i < m->num_packs; i++) {
+ char *pack_name;
+
+ if (count[i])
+ continue;
+
+ if (prepare_midx_pack(m, i))
+ continue;
+
+ if (m->packs[i]->pack_keep)
+ continue;
+
+ pack_name = xstrdup(m->packs[i]->pack_name);
+ close_pack(m->packs[i]);
+ FREE_AND_NULL(m->packs[i]);
+
+ string_list_insert(&packs_to_drop, m->pack_names[i]);
+ unlink_pack_path(pack_name, 0);
+ free(pack_name);
+ }
+
+ free(count);
+
+ if (packs_to_drop.nr)
+ result = write_midx_internal(object_dir, m, &packs_to_drop);
+
+ string_list_clear(&packs_to_drop, 0);
+ return result;
}
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 948effc1ee..210279a3cf 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -395,4 +395,19 @@ test_expect_success 'expire does not remove any packs' '
)
'
+test_expect_success 'expire removes unreferenced packs' '
+ (
+ cd dup &&
+ git pack-objects --revs .git/objects/pack/pack-combined <<-EOF &&
+ refs/heads/A
+ ^refs/heads/C
+ EOF
+ git multi-pack-index write &&
+ ls .git/objects/pack | grep -v -e pack-[AB] >expect &&
+ git multi-pack-index expire &&
+ ls .git/objects/pack >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 6/7] multi-pack-index: prepare 'repack' subcommand
From: Derrick Stolee via GitGitGadget @ 2018-12-21 16:28 UTC (permalink / raw)
To: git; +Cc: sbeller, peff, jrnieder, avarab, Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.92.v2.git.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
In an environment where the multi-pack-index is useful, it is due
to many pack-files and an inability to repack the object store
into a single pack-file. However, it is likely that many of these
pack-files are rather small, and could be repacked into a slightly
larger pack-file without too much effort. It may also be important
to ensure the object store is highly available and the repack
operation does not interrupt concurrent git commands.
Introduce a 'repack' subcommand to 'git multi-pack-index' that
takes a '--batch-size' option. The verb will inspect the
multi-pack-index for referenced pack-files whose size is smaller
than the batch size, until collecting a list of pack-files whose
sizes sum to larger than the batch size. Then, a new pack-file
will be created containing the objects from those pack-files that
are referenced by the multi-pack-index. The resulting pack is
likely to actually be smaller than the batch size due to
compression and the fact that there may be objects in the pack-
files that have duplicate copies in other pack-files.
The current change introduces the command-line arguments, and we
add a test that ensures we parse these options properly. Since
we specify a small batch size, we will guarantee that future
implementations do not change the list of pack-files.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-multi-pack-index.txt | 11 +++++++++++
builtin/multi-pack-index.c | 10 +++++++++-
midx.c | 5 +++++
midx.h | 1 +
t/t5319-multi-pack-index.sh | 11 +++++++++++
5 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt
index 6186c4c936..cc63531cc0 100644
--- a/Documentation/git-multi-pack-index.txt
+++ b/Documentation/git-multi-pack-index.txt
@@ -36,6 +36,17 @@ expire::
have no objects referenced by the MIDX. Rewrite the MIDX file
afterward to remove all references to these pack-files.
+repack::
+ Collect a batch of pack-files whose size are all at most the
+ size given by --batch-size, but whose sizes sum to larger
+ than --batch-size. The batch is selected by greedily adding
+ small pack-files starting with the oldest pack-files that fit
+ the size. Create a new pack-file containing the objects the
+ multi-pack-index indexes into those pack-files, and rewrite
+ the multi-pack-index to contain that pack-file. A later run
+ of 'git multi-pack-index expire' will delete the pack-files
+ that were part of this batch.
+
EXAMPLES
--------
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 145de3a46c..d87a2235e3 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -5,12 +5,13 @@
#include "midx.h"
static char const * const builtin_multi_pack_index_usage[] = {
- N_("git multi-pack-index [--object-dir=<dir>] (write|verify|expire)"),
+ N_("git multi-pack-index [--object-dir=<dir>] (write|verify|expire|repack --batch-size=<size>)"),
NULL
};
static struct opts_multi_pack_index {
const char *object_dir;
+ unsigned long batch_size;
} opts;
int cmd_multi_pack_index(int argc, const char **argv,
@@ -19,6 +20,8 @@ int cmd_multi_pack_index(int argc, const char **argv,
static struct option builtin_multi_pack_index_options[] = {
OPT_FILENAME(0, "object-dir", &opts.object_dir,
N_("object directory containing set of packfile and pack-index pairs")),
+ OPT_MAGNITUDE(0, "batch-size", &opts.batch_size,
+ N_("during repack, collect pack-files of smaller size into a batch that is larger than this size")),
OPT_END(),
};
@@ -40,6 +43,11 @@ int cmd_multi_pack_index(int argc, const char **argv,
return 1;
}
+ if (!strcmp(argv[0], "repack"))
+ return midx_repack(opts.object_dir, (size_t)opts.batch_size);
+ if (opts.batch_size)
+ die(_("--batch-size option is only for 'repack' verb"));
+
if (!strcmp(argv[0], "write"))
return write_midx_file(opts.object_dir);
if (!strcmp(argv[0], "verify"))
diff --git a/midx.c b/midx.c
index 043cd1fd97..127c43f7b0 100644
--- a/midx.c
+++ b/midx.c
@@ -1110,3 +1110,8 @@ int expire_midx_packs(const char *object_dir)
string_list_clear(&packs_to_drop, 0);
return result;
}
+
+int midx_repack(const char *object_dir, size_t batch_size)
+{
+ return 0;
+}
diff --git a/midx.h b/midx.h
index e3a2b740b5..394a21ee96 100644
--- a/midx.h
+++ b/midx.h
@@ -50,6 +50,7 @@ int write_midx_file(const char *object_dir);
void clear_midx_file(struct repository *r);
int verify_midx_file(const char *object_dir);
int expire_midx_packs(const char *object_dir);
+int midx_repack(const char *object_dir, size_t batch_size);
void close_midx(struct multi_pack_index *m);
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 210279a3cf..f675621080 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -410,4 +410,15 @@ test_expect_success 'expire removes unreferenced packs' '
)
'
+test_expect_success 'repack with minimum size does not alter existing packs' '
+ (
+ cd dup &&
+ ls .git/objects/pack >expect &&
+ MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) &&
+ git multi-pack-index repack --batch-size=$MINSIZE &&
+ ls .git/objects/pack >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 3/7] multi-pack-index: prepare for 'expire' subcommand
From: Derrick Stolee via GitGitGadget @ 2018-12-21 16:28 UTC (permalink / raw)
To: git; +Cc: sbeller, peff, jrnieder, avarab, Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.92.v2.git.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
The multi-pack-index tracks objects in a collection of pack-files.
Only one copy of each object is indexed, using the modified time
of the pack-files to determine tie-breakers. It is possible to
have a pack-file with no referenced objects because all objects
have a duplicate in a newer pack-file.
Introduce a new 'expire' subcommand to the multi-pack-index builtin.
This subcommand will delete these unused pack-files and rewrite the
multi-pack-index to no longer refer to those files. More details
about the specifics will follow as the method is implemented.
Add a test that verifies the 'expire' subcommand is correctly wired,
but will still be valid when the verb is implemented. Specifically,
create a set of packs that should all have referenced objects and
should not be removed during an 'expire' operation.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-multi-pack-index.txt | 5 +++
builtin/multi-pack-index.c | 4 ++-
midx.c | 5 +++
midx.h | 1 +
t/t5319-multi-pack-index.sh | 47 ++++++++++++++++++++++++++
5 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt
index 1af406aca2..6186c4c936 100644
--- a/Documentation/git-multi-pack-index.txt
+++ b/Documentation/git-multi-pack-index.txt
@@ -31,6 +31,11 @@ write::
verify::
Verify the contents of the MIDX file.
+expire::
+ Delete the pack-files that are tracked by the MIDX file, but
+ have no objects referenced by the MIDX. Rewrite the MIDX file
+ afterward to remove all references to these pack-files.
+
EXAMPLES
--------
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index fca70f8e4f..145de3a46c 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -5,7 +5,7 @@
#include "midx.h"
static char const * const builtin_multi_pack_index_usage[] = {
- N_("git multi-pack-index [--object-dir=<dir>] (write|verify)"),
+ N_("git multi-pack-index [--object-dir=<dir>] (write|verify|expire)"),
NULL
};
@@ -44,6 +44,8 @@ int cmd_multi_pack_index(int argc, const char **argv,
return write_midx_file(opts.object_dir);
if (!strcmp(argv[0], "verify"))
return verify_midx_file(opts.object_dir);
+ if (!strcmp(argv[0], "expire"))
+ return expire_midx_packs(opts.object_dir);
die(_("unrecognized verb: %s"), argv[0]);
}
diff --git a/midx.c b/midx.c
index 730ff84dff..bb825ef816 100644
--- a/midx.c
+++ b/midx.c
@@ -1025,3 +1025,8 @@ int verify_midx_file(const char *object_dir)
return verify_midx_error;
}
+
+int expire_midx_packs(const char *object_dir)
+{
+ return 0;
+}
diff --git a/midx.h b/midx.h
index 774f652530..e3a2b740b5 100644
--- a/midx.h
+++ b/midx.h
@@ -49,6 +49,7 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i
int write_midx_file(const char *object_dir);
void clear_midx_file(struct repository *r);
int verify_midx_file(const char *object_dir);
+int expire_midx_packs(const char *object_dir);
void close_midx(struct multi_pack_index *m);
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 70926b5bc0..948effc1ee 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -348,4 +348,51 @@ test_expect_success 'verify incorrect 64-bit offset' '
"incorrect object offset"
'
+test_expect_success 'setup expire tests' '
+ mkdir dup &&
+ (
+ cd dup &&
+ git init &&
+ for i in $(test_seq 1 20)
+ do
+ test_commit $i
+ done &&
+ git branch A HEAD &&
+ git branch B HEAD~8 &&
+ git branch C HEAD~13 &&
+ git branch D HEAD~16 &&
+ git branch E HEAD~18 &&
+ git pack-objects --revs .git/objects/pack/pack-E <<-EOF &&
+ refs/heads/E
+ EOF
+ git pack-objects --revs .git/objects/pack/pack-D <<-EOF &&
+ refs/heads/D
+ ^refs/heads/E
+ EOF
+ git pack-objects --revs .git/objects/pack/pack-C <<-EOF &&
+ refs/heads/C
+ ^refs/heads/D
+ EOF
+ git pack-objects --revs .git/objects/pack/pack-B <<-EOF &&
+ refs/heads/B
+ ^refs/heads/C
+ EOF
+ git pack-objects --revs .git/objects/pack/pack-A <<-EOF &&
+ refs/heads/A
+ ^refs/heads/B
+ EOF
+ git multi-pack-index write
+ )
+'
+
+test_expect_success 'expire does not remove any packs' '
+ (
+ cd dup &&
+ ls .git/objects/pack >expect &&
+ git multi-pack-index expire &&
+ ls .git/objects/pack >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/7] Docs: rearrange subcommands for multi-pack-index
From: Derrick Stolee via GitGitGadget @ 2018-12-21 16:28 UTC (permalink / raw)
To: git; +Cc: sbeller, peff, jrnieder, avarab, Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.92.v2.git.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
We will add new subcommands to the multi-pack-index, and that will
make the documentation a bit messier. Clean up the 'verb'
descriptions by renaming the concept to 'subcommand' and removing
the reference to the object directory.
Helped-by: Stefan Beller <sbeller@google.com>
Helped-by: Szeder Gábor <szeder.dev@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-multi-pack-index.txt | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt
index f7778a2c85..1af406aca2 100644
--- a/Documentation/git-multi-pack-index.txt
+++ b/Documentation/git-multi-pack-index.txt
@@ -9,7 +9,7 @@ git-multi-pack-index - Write and verify multi-pack-indexes
SYNOPSIS
--------
[verse]
-'git multi-pack-index' [--object-dir=<dir>] <verb>
+'git multi-pack-index' [--object-dir=<dir>] <subcommand>
DESCRIPTION
-----------
@@ -23,13 +23,13 @@ OPTIONS
`<dir>/packs/multi-pack-index` for the current MIDX file, and
`<dir>/packs` for the pack-files to index.
+The following subcommands are available:
+
write::
- When given as the verb, write a new MIDX file to
- `<dir>/packs/multi-pack-index`.
+ Write a new MIDX file.
verify::
- When given as the verb, verify the contents of the MIDX file
- at `<dir>/packs/multi-pack-index`.
+ Verify the contents of the MIDX file.
EXAMPLES
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/7] Create 'expire' and 'repack' verbs for git-multi-pack-index
From: Derrick Stolee via GitGitGadget @ 2018-12-21 16:28 UTC (permalink / raw)
To: git; +Cc: sbeller, peff, jrnieder, avarab, Junio C Hamano
In-Reply-To: <pull.92.git.gitgitgadget@gmail.com>
The multi-pack-index provides a fast way to find an object among a large
list of pack-files. It stores a single pack-reference for each object id, so
duplicate objects are ignored. Among a list of pack-files storing the same
object, the most-recently modified one is used.
Create new subcommands for the multi-pack-index builtin.
* 'git multi-pack-index expire': If we have a pack-file indexed by the
multi-pack-index, but all objects in that pack are duplicated in
more-recently modified packs, then delete that pack (and any others like
it). Delete the reference to that pack in the multi-pack-index.
* 'git multi-pack-index repack --batch-size=': Starting from the oldest
pack-files covered by the multi-pack-index, find those whose on-disk size
is below the batch size until we have a collection of packs whose sizes
add up to the batch size. Create a new pack containing all objects that
the multi-pack-index references to those packs.
This allows us to create a new pattern for repacking objects: run 'repack'.
After enough time has passed that all Git commands that started before the
last 'repack' are finished, run 'expire' again. This approach has some
advantages over the existing "repack everything" model:
1. Incremental. We can repack a small batch of objects at a time, instead
of repacking all reachable objects. We can also limit ourselves to the
objects that do not appear in newer pack-files.
2. Highly Available. By adding a new pack-file (and not deleting the old
pack-files) we do not interrupt concurrent Git commands, and do not
suffer performance degradation. By expiring only pack-files that have no
referenced objects, we know that Git commands that are doing normal
object lookups* will not be interrupted.
3. Note: if someone concurrently runs a Git command that uses
get_all_packs(), then that command could try to read the pack-files and
pack-indexes that we are deleting during an expire command. Such
commands are usually related to object maintenance (i.e. fsck, gc,
pack-objects) or are related to less-often-used features (i.e.
fast-import, http-backend, server-info).
We plan to use this approach in VFS for Git to do background maintenance of
the "shared object cache" which is a Git alternate directory filled with
packfiles containing commits and trees. We currently download pack-files on
an hourly basis to keep up-to-date with the central server. The cache
servers supply packs on an hourly and daily basis, so most of the hourly
packs become useless after a new daily pack is downloaded. The 'expire'
command would clear out most of those packs, but many will still remain with
fewer than 100 objects remaining. The 'repack' command (with a batch size of
1-3gb, probably) can condense the remaining packs in commands that run for
1-3 min at a time. Since the daily packs range from 100-250mb, we will also
combine and condense those packs.
Updates in V2:
* Added a method, unlink_pack_path() to remove packfiles, but with the
additional check for a .keep file. This borrows logic from
builtin/repack.c.
* Modified documentation and commit messages to replace 'verb' with
'subcommand'. Simplified the documentation. (I left 'verbs' in the title
of the cover letter for consistency.)
Thanks, -Stolee
Derrick Stolee (7):
repack: refactor pack deletion for future use
Docs: rearrange subcommands for multi-pack-index
multi-pack-index: prepare for 'expire' subcommand
midx: refactor permutation logic
multi-pack-index: implement 'expire' verb
multi-pack-index: prepare 'repack' subcommand
midx: implement midx_repack()
Documentation/git-multi-pack-index.txt | 26 ++-
builtin/multi-pack-index.c | 12 +-
builtin/repack.c | 14 +-
midx.c | 217 +++++++++++++++++++++++--
midx.h | 2 +
packfile.c | 28 ++++
packfile.h | 7 +
t/t5319-multi-pack-index.sh | 98 +++++++++++
8 files changed, 376 insertions(+), 28 deletions(-)
base-commit: 26aa9fc81d4c7f6c3b456a29da0b7ec72e5c6595
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-92%2Fderrickstolee%2Fmidx-expire%2Fupstream-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-92/derrickstolee/midx-expire/upstream-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/92
Range-diff vs v1:
-: ---------- > 1: a697df120c repack: refactor pack deletion for future use
-: ---------- > 2: 55df6b20ff Docs: rearrange subcommands for multi-pack-index
1: 1e34b48a20 ! 3: 2529afe89e multi-pack-index: prepare for 'expire' verb
@@ -1,6 +1,6 @@
Author: Derrick Stolee <dstolee@microsoft.com>
- multi-pack-index: prepare for 'expire' verb
+ multi-pack-index: prepare for 'expire' subcommand
The multi-pack-index tracks objects in a collection of pack-files.
Only one copy of each object is indexed, using the modified time
@@ -8,12 +8,12 @@
have a pack-file with no referenced objects because all objects
have a duplicate in a newer pack-file.
- Introduce a new 'expire' verb to the multi-pack-index builtin.
- This verb will delete these unused pack-files and rewrite the
+ Introduce a new 'expire' subcommand to the multi-pack-index builtin.
+ This subcommand will delete these unused pack-files and rewrite the
multi-pack-index to no longer refer to those files. More details
about the specifics will follow as the method is implemented.
- Add a test that verifies the 'expire' verb is correctly wired,
+ Add a test that verifies the 'expire' subcommand is correctly wired,
but will still be valid when the verb is implemented. Specifically,
create a set of packs that should all have referenced objects and
should not be removed during an 'expire' operation.
@@ -24,16 +24,13 @@
--- a/Documentation/git-multi-pack-index.txt
+++ b/Documentation/git-multi-pack-index.txt
@@
- When given as the verb, verify the contents of the MIDX file
- at `<dir>/packs/multi-pack-index`.
+ verify::
+ Verify the contents of the MIDX file.
+expire::
-+ When given as the verb, delete the pack-files that are tracked
-+ by the MIDX file at `<dir>/packs/multi-pack-index` but have
-+ no objects referenced by the MIDX. All objects in these pack-
-+ files have another copy in a more-recently modified pack-file.
-+ Rewrite the MIDX file afterward to remove all references to
-+ these pack-files.
++ Delete the pack-files that are tracked by the MIDX file, but
++ have no objects referenced by the MIDX. Rewrite the MIDX file
++ afterward to remove all references to these pack-files.
+
EXAMPLES
2: 8f496ccb46 = 4: 0c29a242fe midx: refactor permutation logic
3: 244bdf2a6f ! 5: 1c4af93f5e multi-pack-index: implement 'expire' verb
@@ -75,6 +75,7 @@
+ drop_index++;
+ i--;
+ missing_drops++;
++ continue;
+ }
+ }
+
@@ -114,8 +115,6 @@
{
- return 0;
+ uint32_t i, *count, result = 0;
-+ size_t dirlen;
-+ struct strbuf buf = STRBUF_INIT;
+ struct string_list packs_to_drop = STRING_LIST_INIT_DUP;
+ struct multi_pack_index *m = load_multi_pack_index(object_dir, 1);
+
@@ -128,31 +127,27 @@
+ count[pack_int_id]++;
+ }
+
-+ strbuf_addstr(&buf, object_dir);
-+ strbuf_addstr(&buf, "/pack/");
-+ dirlen = buf.len;
-+
+ for (i = 0; i < m->num_packs; i++) {
++ char *pack_name;
++
+ if (count[i])
+ continue;
+
-+ if (m->packs[i]) {
-+ close_pack(m->packs[i]);
-+ m->packs[i] = NULL;
-+ }
++ if (prepare_midx_pack(m, i))
++ continue;
+
-+ string_list_insert(&packs_to_drop, m->pack_names[i]);
++ if (m->packs[i]->pack_keep)
++ continue;
+
-+ strbuf_setlen(&buf, dirlen);
-+ strbuf_addstr(&buf, m->pack_names[i]);
-+ unlink(buf.buf);
++ pack_name = xstrdup(m->packs[i]->pack_name);
++ close_pack(m->packs[i]);
++ FREE_AND_NULL(m->packs[i]);
+
-+ strip_suffix_mem(buf.buf, &buf.len, "idx");
-+ strbuf_addstr(&buf, "pack");
-+ unlink(buf.buf);
++ string_list_insert(&packs_to_drop, m->pack_names[i]);
++ unlink_pack_path(pack_name, 0);
++ free(pack_name);
+ }
+
-+ strbuf_release(&buf);
+ free(count);
+
+ if (packs_to_drop.nr)
4: 72b2139591 ! 6: af08e21c97 multi-pack-index: prepare 'repack' verb
@@ -1,6 +1,6 @@
Author: Derrick Stolee <dstolee@microsoft.com>
- multi-pack-index: prepare 'repack' verb
+ multi-pack-index: prepare 'repack' subcommand
In an environment where the multi-pack-index is useful, it is due
to many pack-files and an inability to repack the object store
@@ -10,16 +10,16 @@
to ensure the object store is highly available and the repack
operation does not interrupt concurrent git commands.
- Introduce a 'repack' verb to 'git multi-pack-index' that takes a
- '--batch-size' option. The verb will inspect the multi-pack-index
- for referenced pack-files whose size is smaller than the batch
- size, until collecting a list of pack-files whose sizes sum to
- larger than the batch size. Then, a new pack-file will be created
- containing the objects from those pack-files that are referenced
- by the multi-pack-index. The resulting pack is likely to actually
- be smaller than the batch size due to compression and the fact
- that there may be objects in the pack-files that have duplicate
- copies in other pack-files.
+ Introduce a 'repack' subcommand to 'git multi-pack-index' that
+ takes a '--batch-size' option. The verb will inspect the
+ multi-pack-index for referenced pack-files whose size is smaller
+ than the batch size, until collecting a list of pack-files whose
+ sizes sum to larger than the batch size. Then, a new pack-file
+ will be created containing the objects from those pack-files that
+ are referenced by the multi-pack-index. The resulting pack is
+ likely to actually be smaller than the batch size due to
+ compression and the fact that there may be objects in the pack-
+ files that have duplicate copies in other pack-files.
The current change introduces the command-line arguments, and we
add a test that ensures we parse these options properly. Since
@@ -32,20 +32,19 @@
--- a/Documentation/git-multi-pack-index.txt
+++ b/Documentation/git-multi-pack-index.txt
@@
- Rewrite the MIDX file afterward to remove all references to
- these pack-files.
+ have no objects referenced by the MIDX. Rewrite the MIDX file
+ afterward to remove all references to these pack-files.
+repack::
-+ When given as the verb, collect a batch of pack-files whose
-+ size are all at most the size given by --batch-size, but
-+ whose sizes sum to larger than --batch-size. The batch is
-+ selected by greedily adding small pack-files starting with
-+ the oldest pack-files that fit the size. Create a new pack-
-+ file containing the objects the multi-pack-index indexes
-+ into thos pack-files, and rewrite the multi-pack-index to
-+ contain that pack-file. A later run of 'git multi-pack-index
-+ expire' will delete the pack-files that were part of this
-+ batch.
++ Collect a batch of pack-files whose size are all at most the
++ size given by --batch-size, but whose sizes sum to larger
++ than --batch-size. The batch is selected by greedily adding
++ small pack-files starting with the oldest pack-files that fit
++ the size. Create a new pack-file containing the objects the
++ multi-pack-index indexes into those pack-files, and rewrite
++ the multi-pack-index to contain that pack-file. A later run
++ of 'git multi-pack-index expire' will delete the pack-files
++ that were part of this batch.
+
EXAMPLES
@@ -123,7 +122,7 @@
)
'
-+test_expect_success 'repack does not create any packs' '
++test_expect_success 'repack with minimum size does not alter existing packs' '
+ (
+ cd dup &&
+ ls .git/objects/pack >expect &&
5: 41ef671ec8 = 7: bef7aa007c midx: implement midx_repack()
--
gitgitgadget
^ permalink raw reply
* [PATCH v2 1/7] repack: refactor pack deletion for future use
From: Derrick Stolee via GitGitGadget @ 2018-12-21 16:28 UTC (permalink / raw)
To: git; +Cc: sbeller, peff, jrnieder, avarab, Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.92.v2.git.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
The repack builtin deletes redundant pack-files and their
associated .idx, .promisor, .bitmap, and .keep files. We will want
to re-use this logic in the future for other types of repack, so
pull the logic into 'unlink_pack_path()' in packfile.c.
The 'ignore_keep' parameter is enabled for the use in repack, but
will be important for a future caller.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
builtin/repack.c | 14 ++------------
packfile.c | 28 ++++++++++++++++++++++++++++
packfile.h | 7 +++++++
3 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 45583683ee..3d445b34b4 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -129,19 +129,9 @@ static void get_non_kept_pack_filenames(struct string_list *fname_list,
static void remove_redundant_pack(const char *dir_name, const char *base_name)
{
- const char *exts[] = {".pack", ".idx", ".keep", ".bitmap", ".promisor"};
- int i;
struct strbuf buf = STRBUF_INIT;
- size_t plen;
-
- strbuf_addf(&buf, "%s/%s", dir_name, base_name);
- plen = buf.len;
-
- for (i = 0; i < ARRAY_SIZE(exts); i++) {
- strbuf_setlen(&buf, plen);
- strbuf_addstr(&buf, exts[i]);
- unlink(buf.buf);
- }
+ strbuf_addf(&buf, "%s/%s.pack", dir_name, base_name);
+ unlink_pack_path(buf.buf, 1);
strbuf_release(&buf);
}
diff --git a/packfile.c b/packfile.c
index d1e6683ffe..bacecb4d0d 100644
--- a/packfile.c
+++ b/packfile.c
@@ -352,6 +352,34 @@ void close_all_packs(struct raw_object_store *o)
}
}
+void unlink_pack_path(const char *pack_name, int force_delete)
+{
+ static const char *exts[] = {".pack", ".idx", ".keep", ".bitmap", ".promisor"};
+ int i;
+ struct strbuf buf = STRBUF_INIT;
+ size_t plen;
+
+ strbuf_addstr(&buf, pack_name);
+ strip_suffix_mem(buf.buf, &buf.len, ".pack");
+ plen = buf.len;
+
+ if (!force_delete) {
+ strbuf_addstr(&buf, ".keep");
+ if (!access(buf.buf, F_OK)) {
+ strbuf_release(&buf);
+ return;
+ }
+ }
+
+ for (i = 0; i < ARRAY_SIZE(exts); i++) {
+ strbuf_setlen(&buf, plen);
+ strbuf_addstr(&buf, exts[i]);
+ unlink(buf.buf);
+ }
+
+ strbuf_release(&buf);
+}
+
/*
* The LRU pack is the one with the oldest MRU window, preferring packs
* with no used windows, or the oldest mtime if it has no windows allocated.
diff --git a/packfile.h b/packfile.h
index 6c4037605d..5b7bcdb1dd 100644
--- a/packfile.h
+++ b/packfile.h
@@ -86,6 +86,13 @@ extern void unuse_pack(struct pack_window **);
extern void clear_delta_base_cache(void);
extern struct packed_git *add_packed_git(const char *path, size_t path_len, int local);
+/*
+ * Unlink the .pack and associated extension files.
+ * Does not unlink if 'force_delete' is false and the pack-file is
+ * marked as ".keep".
+ */
+extern void unlink_pack_path(const char *pack_name, int force_delete);
+
/*
* Make sure that a pointer access into an mmap'd index file is within bounds,
* and can provide at least 8 bytes of data.
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 2/2] Rebase: Run post-checkout hook on checkout
From: Johannes Schindelin @ 2018-12-21 16:12 UTC (permalink / raw)
To: Orgad Shaneh; +Cc: git
In-Reply-To: <20181220214823.21378-2-orgads@gmail.com>
Hi Orgad,
On Thu, 20 Dec 2018, orgads@gmail.com wrote:
> From: Orgad Shaneh <orgads@gmail.com>
>
> Signed-off-by: Orgad Shaneh <orgads@gmail.com>
Feel free to steal the PR description I added to your PR at
https://github.com/git-for-windows/git/pull/1992
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index b5c99ec10c..78a09dcda2 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
>
> #define RESET_HEAD_DETACH (1<<0)
> #define RESET_HEAD_HARD (1<<1)
> +#define RESET_HEAD_RUN_HOOK (1<<2)
>
> static int reset_head(struct object_id *oid, const char *action,
> const char *switch_to_branch, unsigned flags,
> @@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
> {
> unsigned detach_head = flags & RESET_HEAD_DETACH;
> unsigned reset_hard = flags & RESET_HEAD_HARD;
> + unsigned run_hook = flags & RESET_HEAD_RUN_HOOK;
> struct object_id head_oid;
> struct tree_desc desc[2] = { { NULL }, { NULL } };
> struct lock_file lock = LOCK_INIT;
> @@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
> ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
> UPDATE_REFS_MSG_ON_ERR);
> }
> + if (run_hook)
> + run_hook_le(NULL, "post-checkout",
> + oid_to_hex(orig ? orig : &null_oid),
> + oid_to_hex(oid), "1", NULL);
IIRC there was one `git checkout` in the scripted version that ran the
hook, and one did not. The latter did not run it because it did not
actually change the HEAD, but just switched branches.
We could imitate that here by extending the `if (run_hook)` to `if
(run_hook && !oideq(&head_oid, oid))`, I think. Do you agree?
The rest of the patch makes sense to me (and I merged the above-mentioned
PR already).
Thank you!
Johannes
>
> leave_reset_head:
> strbuf_release(&msg);
> @@ -1539,7 +1545,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> strbuf_addf(&msg, "%s: checkout %s",
> getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
> if (reset_head(&options.onto->object.oid, "checkout", NULL,
> - RESET_HEAD_DETACH, NULL, msg.buf))
> + RESET_HEAD_DETACH | RESET_HEAD_RUN_HOOK, NULL, msg.buf))
> die(_("Could not detach HEAD"));
> strbuf_release(&msg);
>
> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> index 7e941537f9..de9c7fb871 100755
> --- a/t/t5403-post-checkout-hook.sh
> +++ b/t/t5403-post-checkout-hook.sh
> @@ -9,6 +9,8 @@ test_description='Test the post-checkout hook.'
> test_expect_success setup '
> test_commit one &&
> test_commit two &&
> + test_commit rebase-on-me &&
> + git reset --hard HEAD^ &&
> test_commit three three &&
> mv .git/hooks-disabled .git/hooks
> '
> @@ -52,6 +54,22 @@ test_expect_success 'post-checkout receives the right args when not switching br
> test $old = $new && test $flag = 0
> '
>
> +test_expect_success 'post-checkout is triggered on rebase' '
> + git checkout -b rebase-test master &&
> + rm -f .git/post-checkout.args &&
> + git rebase rebase-on-me &&
> + read old new flag < .git/post-checkout.args &&
> + test $old != $new && test $flag = 1
> +'
> +
> +test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
> + git checkout -b ff-rebase-test rebase-on-me^ &&
> + rm -f .git/post-checkout.args &&
> + git rebase rebase-on-me &&
> + read old new flag < .git/post-checkout.args &&
> + test $old != $new && test $flag = 1
> +'
> +
> if test "$(git config --bool core.filemode)" = true; then
> mkdir -p templates/hooks
> cat >templates/hooks/post-checkout <<'EOF'
> --
> 2.20.1
>
>
^ permalink raw reply
* Re: [PATCH 1/2] t5403: Refactor
From: Johannes Schindelin @ 2018-12-21 16:06 UTC (permalink / raw)
To: Orgad Shaneh; +Cc: git
In-Reply-To: <20181220214823.21378-1-orgads@gmail.com>
Hi Orgad,
On Thu, 20 Dec 2018, orgads@gmail.com wrote:
> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> index fc898c9eac..7e941537f9 100755
> --- a/t/t5403-post-checkout-hook.sh
> +++ b/t/t5403-post-checkout-hook.sh
> @@ -7,67 +7,48 @@ test_description='Test the post-checkout hook.'
> . ./test-lib.sh
>
> test_expect_success setup '
> - echo Data for commit0. >a &&
> - echo Data for commit0. >b &&
> - git update-index --add a &&
> - git update-index --add b &&
> - tree0=$(git write-tree) &&
> - commit0=$(echo setup | git commit-tree $tree0) &&
> - git update-ref refs/heads/master $commit0 &&
> - git clone ./. clone1 &&
> - git clone ./. clone2 &&
> - GIT_DIR=clone2/.git git branch new2 &&
> - echo Data for commit1. >clone2/b &&
> - GIT_DIR=clone2/.git git add clone2/b &&
> - GIT_DIR=clone2/.git git commit -m new2
> + test_commit one &&
> + test_commit two &&
> + test_commit three three &&
A very nice simplification (but please use tabs to indent).
> + mv .git/hooks-disabled .git/hooks
> '
>
> -for clone in 1 2; do
> - cat >clone${clone}/.git/hooks/post-checkout <<'EOF'
> +cat >.git/hooks/post-checkout <<'EOF'
> #!/bin/sh
> -echo $@ > $GIT_DIR/post-checkout.args
> +echo $@ > .git/post-checkout.args
While at it, you could lose the space after the redirector that we seem to
no longer prefer:
> +echo $@ >.git/post-checkout.args
And since we are already cleaning up, we could easily move use
write_script instead, *and* move it into the `setup` test case (which
makes it easier to use something like
sh t5403-post-checkout-hook.sh --run=1,13
The rest looks good (modulo indentation issues). I would have preferred
the separate concerns to be addressed in individual commits (one commit to
replace the `awk` calls, one to avoid the clones, one to simplify by using
`test_commit`, etc), as that would have been easier to review. But others
might disagree (Junio recently made the case of smooshing separate
concerns into single commits, even squashing two of my patches into one
against my wish), so... I guess you don't have to change this.
Thank you,
Johannes
> EOF
> - chmod u+x clone${clone}/.git/hooks/post-checkout
> -done
> +chmod u+x .git/hooks/post-checkout
>
> test_expect_success 'post-checkout runs as expected ' '
> - GIT_DIR=clone1/.git git checkout master &&
> - test -e clone1/.git/post-checkout.args
> + git checkout master &&
> + test -e .git/post-checkout.args
> '
>
> test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
> - old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
> - new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
> - flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
> + read old new flag < .git/post-checkout.args &&
> test $old = $new && test $flag = 1
> '
>
> test_expect_success 'post-checkout runs as expected ' '
> - GIT_DIR=clone1/.git git checkout master &&
> - test -e clone1/.git/post-checkout.args
> + git checkout master &&
> + test -e .git/post-checkout.args
> '
>
> test_expect_success 'post-checkout args are correct with git checkout -b ' '
> - GIT_DIR=clone1/.git git checkout -b new1 &&
> - old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
> - new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
> - flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
> + git checkout -b new1 &&
> + read old new flag < .git/post-checkout.args &&
> test $old = $new && test $flag = 1
> '
>
> test_expect_success 'post-checkout receives the right args with HEAD changed ' '
> - GIT_DIR=clone2/.git git checkout new2 &&
> - old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
> - new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
> - flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
> + git checkout two &&
> + read old new flag < .git/post-checkout.args &&
> test $old != $new && test $flag = 1
> '
>
> test_expect_success 'post-checkout receives the right args when not switching branches ' '
> - GIT_DIR=clone2/.git git checkout master b &&
> - old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
> - new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
> - flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
> + git checkout master -- three &&
> + read old new flag < .git/post-checkout.args &&
> test $old = $new && test $flag = 0
> '
>
> --
> 2.20.1
>
>
^ permalink raw reply
* Re: Unable to install latest git version. Claims git process pid running
From: Johannes Schindelin @ 2018-12-21 15:55 UTC (permalink / raw)
To: David Brown; +Cc: git
In-Reply-To: <d2bb51d06b263f5a4d60256e5c2ff33c@davidwbrown.name>
Hi,
On Thu, 20 Dec 2018, David Brown wrote:
> - [ ] I was not able to find an
> [open](https://github.com/git-for-windows/git/issues?q=is%3Aopen) or
> [closed](https://github.com/git-for-windows/git/issues?q=is%3Aclosed) issue
> matching what I'm seeing
That's a new one. I saw many bug reporters simply deleting the issue
reporting template at https://github.com/git-for-windows/git/issues/new
(and usually then filing only very little information, definitely not
enough to make sense of the bug report), but I never saw this bug
reporting template being used to report a Git for Windows bug to the Git
mailing list yet ;-)
> ### Setup
>
> - Which version of Git for Windows are you using? Is it 32-bit or 64-bit?
>
> $ git --version --build-options
>
> git version 2.20.0.windows.1 cpu: x86_64 built from commit:
> 95155834166f64fe9666f2c0a4909f076080893a
> sizeof-long: 4
> sizeof-size_t: 8 **
>
> - Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or
> 64-bit?
>
> $ cmd.exe /c ver
>
> ** Microsoft Windows [Version 6.1.7601] **
>
> - What options did you set as part of the installation? Or did you choose the
> defaults?
>
> # One of the following:
>
> $ cat /etc/install-options.txt Editor Option: VIM Custom Editor Path:
> Path Option: BashOnly
> SSH Option: OpenSSH
> CURL Option: WinSSL
> CRLF Option: CRLFAlways
> Bash Terminal Option: MinTTY
> Performance Tweaks
> FSCache: Enabled
> Use Credential Manager: Enabled
> Enable Symlinks: Disabled
>
> - Any other interesting things about your environment that might be related
> to the issue you're seeing?
>
> ** A so-called VDI running on a Dell WYSE **
>
> ### Details
>
> - Which terminal/shell are you running Git from? e.g
> Bash/CMD/PowerShell/other
>
> ** Bash **
>
> - What commands did you run to trigger this issue? If you can provide a
> [Minimal, Complete, and Verifiable
> example](http://stackoverflow.com/help/mcve)
> this will help us understand the issue.
>
> ** Windows git upgrade dialog **
>
> - What did you expect to occur after running these commands?
>
> ** latest version of git installed **
>
> - What actually happened instead?
>
> ** No such pid 10128 to close therefore cancel install **
This typically means that that process existed *at the time the installer
checked*. If it was a short-lived process, you can check again by clicking
that "Refresh" button on the lower left.
Ciao,
Johannes
>
> - If the probleminsert URL herey, can you provide the
> URL to that repository to help us with testing?
>
> ** Internal company url. Windows git install dialog is issue **
>
^ permalink raw reply
* Re: [PATCH] t5570: drop racy test
From: Johannes Schindelin @ 2018-12-21 15:48 UTC (permalink / raw)
To: Jeff King
Cc: Thomas Gummerer, Torsten Bögershausen, Git Mailing List,
szeder.dev, Jan Palus
In-Reply-To: <20181220171438.GA6684@sigill.intra.peff.net>
Hi Thomas & Peff,
On Thu, 20 Dec 2018, Jeff King wrote:
> On Thu, Dec 20, 2018 at 04:41:50PM +0000, Thomas Gummerer wrote:
>
> > > That doesn't really fix it, but just broadens the race window. I dunno.
> > > Maybe that is enough in practice. We could do something like:
> > >
> > > repeat_with_timeout () {
> > > local i=0
> > > while test $i -lt 10
> > > do
> > > "$@" && return 0
> > > sleep 1
> > > done
> > > # no success even after 10 seconds
> > > return 1
> > > }
> > >
> > > repeat_with_timeout grep -i extended.attribute daemon.log
> > >
> > > to make the pattern a bit more obvious (and make it easy to extend the
> > > window arbitrarily; surely 10s is enough?).
> >
> > I gave this a try, with below patch to lib-git-daemon.sh that you
> > proposed in the previous thread about this racyness. That shows
> > another problem though, namely when truncating 'daemon.log' before
> > running 'git ls-remote' in this test, we're not sure all 'git deamon'
> > has flushed everything from previous invocations. That may be an even
> > rarer problem in practice, but still something to keep in mind.
>
> Right, that makes sense. Making this race-proof really does require a
> separate log stream for each test. I guess we'd need to be able to send
> git-daemon a signal to re-open the log (which actually is not as
> unreasonable as it may seem; lots of daemons have this for log
> rotation).
>
> I think getting rid of the "cat" would also help a lot here.
> Unfortunately I think we use it not just for its "tee" effect, but also
> to avoid startup races by checking the "Ready to rumble" line. So again,
> we'd need some cooperation from git-daemon to tell us out-of-band that
> it has completed its startup (e.g., by touching another file).
>
> > Dscho also mentioned on #git-devel a while ago that he may have a look
> > at actually making this test race-proof, but I guess he's been busy
> > with the 2.20 release.
And GitGitGadget. And working on the Azure Pipelines support. And
mentoring two interns.
This is what I still have in my internal ticket:
Try to work around occasional t5570 failures in Git's test suite
Seems that there is a race condition in
https://github.com/git/git/blob/master/t/lib-git-daemon.sh#L48-L69
that could possibly be solved by writing to the daemon.log
directly, and showing the output only via `tail -f` (and only when
running in verbose mode, as it simply won't make sense otherwise).
However, if the preferred route is to go ahead and just remove that test
altogether, I'm fine with that, too.
The only reason, in my mind, why we still have `git-daemon` is that it
allows for easy standing up your own Git server, e.g. as an ad-hoc way to
collaborate in a small ad-hoc team. If we ever get to the point where we
can stand up a minimal HTTP/HTTPS server with an internal Git command (not
requiring sysadmin privileges), from my point of view `git-daemon` can
even go the way of the Kale Island (but for much better reasons [*1*]).
> > I'm also not sure it's worth spending a lot of time trying to fix this
> > test, but I'd definitely be happy if someone proposes a different
> > solution.
>
> Yeah. I'm sure it's fixable with enough effort, but I just think there
> are more interesting and important things to work on.
>
> > --- >8 ---
> > Subject: [PATCH] t5570: drop racy test
>
> So yeah, I'm still fine with this. But...
>
> > ---
> > t/t5570-git-daemon.sh | 13 -------------
> > 1 file changed, 13 deletions(-)
>
> This is the only user of daemon.log, so we could drop those bits from
> lib-git-daemon.sh, too. That would also prevent people from adding new
> tests, thinking that this was somehow not horribly racy). I.e.,
> reverting 314a73d658 (t/lib-git-daemon: record daemon log, 2018-01-25).
Indeed, that would be good.
The only reason to keep daemon.log that I can think of is to make
debugging easier, but then, if it should become necessary, it is probably
easier to freopen() stdout or stderr into a file in `git daemon`, anyway.
Ciao,
Dscho
Footnote *1*: Kale Island, along with Rapita, Rehana, Kakatina and Zollies
is prominently featured in a scientific article at
http://iopscience.iop.org/article/10.1088/1748-9326/11/5/054011 that is on
my "important papers I read in 2018" list.
^ permalink raw reply
* Show-branch without commits
From: Brian Johnson @ 2018-12-21 14:56 UTC (permalink / raw)
To: git
Is it possible (or could a flag be added) to have show-branch only
show the branch hierarchy at the top and not print out the commit
list?
--Brian
^ permalink raw reply
* Re: "git show --usage" no "usage" displayed
From: Duy Nguyen @ 2018-12-21 14:49 UTC (permalink / raw)
To: Nicholas Hsiang; +Cc: Git Mailing List
In-Reply-To: <CAEcaDLk_vid3Y2z-kX17d5_LWEQTF4CYiztqrUFmOpN-YW-3yQ@mail.gmail.com>
On Fri, Dec 21, 2018 at 9:40 AM Nicholas Hsiang <xianghongai@gmail.com> wrote:
>
> Dear git:
>
> When I type `git remote --usage`, The command panel will prompt
> 'error: unknown option `usage', Then there will be `git remote`
> related instructions.
> But I type `git show --usage`, just display 'fatal: unrecognized
> argument: --usage'.
>
> 
This is a known issue because "git log" and friends do not use the
same option parser as the rest. I have something in the queue to fix
this, but it will take some time to get reviewed and merged because
it's a big big change.
> Sorry, the previou email forgot to write the title. The previou
> previou email did not remove the HTML format.
>
> Yours sincerely,
> Xiang HongAi.
> --
> Live Long and Prosper.
--
Duy
^ permalink raw reply
* Re: [PATCH 0/4] Expose gpgsig in pretty-print
From: Michał Górny @ 2018-12-21 13:52 UTC (permalink / raw)
To: John Passaro, Jeff King; +Cc: git, Junio C Hamano, Alexey Shumkin
In-Reply-To: <CAJdN7KixEG+VKJAZz281RFEiVPRpRz_fBy6J2dBJiJMYT1mpBg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2800 bytes --]
On Wed, 2018-12-19 at 00:59 -0500, John Passaro wrote:
> On Fri, Dec 14, 2018 at 6:10 PM John Passaro wrote:
> > All seems to work fine when I treat %Gs as a detached signature.
>
> In light of this, my best guess as to why the cleartext PGP message
> didn't verify properly is that the commit data normally doesn't end
> with \n, but as far as I can tell there's no way to express that in
> the cleartext format. I don't see a way around this.
You are most likely right. I've just skimmed through RFC 4880
and indeed it seems to rely on the newline encoding being quite
normalized in the message.
> However, as long
> as the following works, I think we have proof-of-concept that this
> enhancement allows you to play with signature data however you please
> without leaving it to git under the hood:
>
> gpg --verify <(git show -s --format=format:%Gs commit) <(git show -s
> --format=format:%Gp commit)
That's a nice trick. Thanks for the effort you're putting into this!
>
> On Mon, Dec 17, 2018 at 3:24 PM Jeff King <peff@peff.net> wrote:
> >
> > On Fri, Dec 14, 2018 at 11:07:03AM -0500, John Passaro wrote:
> >
> > > Then I might rename the other new placeholders too:
> > >
> > > %Gs: signed commit signature (blank when unsigned)
> > > %Gp: signed commit payload (i.e. in practice minus the gpgsig header;
> > > also blank when unsigned as well)
> >
> > One complication: the pretty-printing code sees the commit data in the
> > i18n.logOutputEncoding charset (utf8 by default). But the signature will
> > be over the raw commit data. That's also utf8 by default, but there may
> > be an encoding header indicating that it's something else. In that case,
> > you couldn't actually verify the signature from the "%Gs%Gp" pair.
> >
> > I don't think that's insurmountable in the code. You'll have to jump
> > through a few hoops to make sure you have the _original_ payload, but we
> > obviously do have that data. However, it does feel a little weird to
> > include content from a different encoding in the middle of the log
> > output stream which claims to be i18n.logOutputEncoding.
> >
>
> Thanks for the feedback! This is an interesting conflict. If the user
> requests %Gp, the payload for the signature, they almost certainly do
> want it in the original encoding; if i18n.logOutputEncoding is
> something incompatible, whether explicitly or by default, that seems
> like an error. Not much we can do to reconcile the two requests
> (commit encoding vs output encoding) so seems reasonable to treat it
> as fatal.
>
> Updated patch coming as soon as I work out Peff's aforementioned "few
> hoops" to get properly encoded data -- and also how to test success
> and failure!
--
Best regards,
Michał Górny
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]
^ permalink raw reply
* [PATCH 4/4] built-in rebase: call `git am` directly
From: Johannes Schindelin via GitGitGadget @ 2018-12-21 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <pull.24.git.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
While the scripted `git rebase` still has to rely on the
`git-rebase--am.sh` script to implement the glue between the `rebase`
and the `am` commands, we can go a more direct route in the built-in
rebase and avoid using a shell script altogether.
This patch represents a straight-forward port of `git-rebase--am.sh` to
C, along with the glue code to call it directly from within
`builtin/rebase.c`.
This reduces the chances of Git for Windows running into trouble due to
problems with the POSIX emulation layer (known as "MSYS2 runtime",
itself a derivative of the Cygwin runtime): when no shell script is
called, the POSIX emulation layer is avoided altogether.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/rebase.c | 183 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 183 insertions(+)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 303175fdf1..e327c30b6b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -246,6 +246,37 @@ static int read_basic_state(struct rebase_options *opts)
return 0;
}
+static int write_basic_state(struct rebase_options *opts)
+{
+ write_file(state_dir_path("head-name", opts), "%s",
+ opts->head_name ? opts->head_name : "detached HEAD");
+ write_file(state_dir_path("onto", opts), "%s",
+ opts->onto ? oid_to_hex(&opts->onto->object.oid) : "");
+ write_file(state_dir_path("orig-head", opts), "%s",
+ oid_to_hex(&opts->orig_head));
+ write_file(state_dir_path("quiet", opts), "%s",
+ opts->flags & REBASE_NO_QUIET ? "" : "t");
+ if (opts->flags & REBASE_VERBOSE)
+ write_file(state_dir_path("verbose", opts), "%s", "");
+ if (opts->strategy)
+ write_file(state_dir_path("strategy", opts), "%s",
+ opts->strategy);
+ if (opts->strategy_opts)
+ write_file(state_dir_path("strategy_opts", opts), "%s",
+ opts->strategy_opts);
+ if (opts->allow_rerere_autoupdate >= 0)
+ write_file(state_dir_path("allow_rerere_autoupdate", opts),
+ "-%s-rerere-autoupdate",
+ opts->allow_rerere_autoupdate ? "" : "-no");
+ if (opts->gpg_sign_opt)
+ write_file(state_dir_path("gpg_sign_opt", opts), "%s",
+ opts->gpg_sign_opt);
+ if (opts->signoff)
+ write_file(state_dir_path("strategy", opts), "--signoff");
+
+ return 0;
+}
+
static int apply_autostash(struct rebase_options *opts)
{
const char *path = state_dir_path("autostash", opts);
@@ -459,6 +490,30 @@ static int reset_head(struct object_id *oid, const char *action,
return ret;
}
+static int move_to_original_branch(struct rebase_options *opts)
+{
+ struct strbuf orig_head_reflog = STRBUF_INIT, head_reflog = STRBUF_INIT;
+ int ret;
+
+ if (!opts->head_name)
+ return 0; /* nothing to move back to */
+
+ if (!opts->onto)
+ BUG("move_to_original_branch without onto");
+
+ strbuf_addf(&orig_head_reflog, "rebase finished: %s onto %s",
+ opts->head_name, oid_to_hex(&opts->onto->object.oid));
+ strbuf_addf(&head_reflog, "rebase finished: returning to %s",
+ opts->head_name);
+ ret = reset_head(NULL, "checkout", opts->head_name,
+ RESET_HEAD_REFS_ONLY,
+ orig_head_reflog.buf, head_reflog.buf);
+
+ strbuf_release(&orig_head_reflog);
+ strbuf_release(&head_reflog);
+ return ret;
+}
+
static const char *resolvemsg =
N_("Resolve all conflicts manually, mark them as resolved with\n"
"\"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n"
@@ -466,6 +521,129 @@ N_("Resolve all conflicts manually, mark them as resolved with\n"
"To abort and get back to the state before \"git rebase\", run "
"\"git rebase --abort\".");
+static int run_am(struct rebase_options *opts)
+{
+ struct child_process am = CHILD_PROCESS_INIT;
+ struct child_process format_patch = CHILD_PROCESS_INIT;
+ struct strbuf revisions = STRBUF_INIT;
+ int status;
+ char *rebased_patches;
+
+ am.git_cmd = 1;
+ argv_array_push(&am.args, "am");
+
+ if (opts->action && !strcmp("continue", opts->action)) {
+ argv_array_push(&am.args, "--resolved");
+ argv_array_pushf(&am.args, "--resolvemsg=%s", resolvemsg);
+ if (opts->gpg_sign_opt)
+ argv_array_push(&am.args, opts->gpg_sign_opt);
+ status = run_command(&am);
+ if (status)
+ return status;
+
+ discard_cache();
+ return move_to_original_branch(opts);
+ }
+ if (opts->action && !strcmp("skip", opts->action)) {
+ argv_array_push(&am.args, "--skip");
+ argv_array_pushf(&am.args, "--resolvemsg=%s", resolvemsg);
+ status = run_command(&am);
+ if (status)
+ return status;
+
+ discard_cache();
+ return move_to_original_branch(opts);
+ }
+ if (opts->action && !strcmp("show-current-patch", opts->action)) {
+ argv_array_push(&am.args, "--show-current-patch");
+ return run_command(&am);
+ }
+
+ strbuf_addf(&revisions, "%s...%s",
+ oid_to_hex(opts->root ?
+ /* this is now equivalent to ! -z "$upstream" */
+ &opts->onto->object.oid :
+ &opts->upstream->object.oid),
+ oid_to_hex(&opts->orig_head));
+
+ rebased_patches = xstrdup(git_path("rebased-patches"));
+ format_patch.out = open(rebased_patches,
+ O_WRONLY | O_CREAT | O_TRUNC, 0666);
+ if (format_patch.out < 0) {
+ status = error_errno(_("could not write '%s'"),
+ rebased_patches);
+ free(rebased_patches);
+ argv_array_clear(&am.args);
+ return status;
+ }
+
+ format_patch.git_cmd = 1;
+ argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout",
+ "--full-index", "--cherry-pick", "--right-only",
+ "--src-prefix=a/", "--dst-prefix=b/", "--no-renames",
+ "--no-cover-letter", "--pretty=mboxrd", NULL);
+ if (opts->git_format_patch_opt.len)
+ argv_array_split(&format_patch.args,
+ opts->git_format_patch_opt.buf);
+ argv_array_push(&format_patch.args, revisions.buf);
+ if (opts->restrict_revision)
+ argv_array_pushf(&format_patch.args, "^%s",
+ oid_to_hex(&opts->restrict_revision->object.oid));
+
+ status = run_command(&format_patch);
+ if (status) {
+ unlink(rebased_patches);
+ free(rebased_patches);
+ argv_array_clear(&am.args);
+
+ reset_head(&opts->orig_head, "checkout", opts->head_name, 0,
+ "HEAD", NULL);
+ error(_("\ngit encountered an error while preparing the "
+ "patches to replay\n"
+ "these revisions:\n"
+ "\n %s\n\n"
+ "As a result, git cannot rebase them."),
+ opts->revisions);
+
+ strbuf_release(&revisions);
+ return status;
+ }
+ strbuf_release(&revisions);
+
+ am.in = open(rebased_patches, O_RDONLY);
+ if (am.in < 0) {
+ status = error_errno(_("could not read '%s'"),
+ rebased_patches);
+ free(rebased_patches);
+ argv_array_clear(&am.args);
+ return status;
+ }
+
+ argv_array_pushv(&am.args, opts->git_am_opts.argv);
+ argv_array_push(&am.args, "--rebasing");
+ argv_array_pushf(&am.args, "--resolvemsg=%s", resolvemsg);
+ argv_array_push(&am.args, "--patch-format=mboxrd");
+ if (opts->allow_rerere_autoupdate > 0)
+ argv_array_push(&am.args, "--rerere-autoupdate");
+ else if (opts->allow_rerere_autoupdate == 0)
+ argv_array_push(&am.args, "--no-rerere-autoupdate");
+ if (opts->gpg_sign_opt)
+ argv_array_push(&am.args, opts->gpg_sign_opt);
+ status = run_command(&am);
+ unlink(rebased_patches);
+ free(rebased_patches);
+
+ if (!status) {
+ discard_cache();
+ return move_to_original_branch(opts);
+ }
+
+ if (is_directory(opts->state_dir))
+ write_basic_state(opts);
+
+ return status;
+}
+
static int run_specific_rebase(struct rebase_options *opts)
{
const char *argv[] = { NULL, NULL };
@@ -546,6 +724,11 @@ static int run_specific_rebase(struct rebase_options *opts)
goto finished_rebase;
}
+ if (opts->type == REBASE_AM) {
+ status = run_am(opts);
+ goto finished_rebase;
+ }
+
add_var(&script_snippet, "GIT_DIR", absolute_path(get_git_dir()));
add_var(&script_snippet, "state_dir", opts->state_dir);
--
gitgitgadget
^ permalink raw reply related
* [PATCH 3/4] rebase: teach `reset_head()` to optionally skip the worktree
From: Johannes Schindelin via GitGitGadget @ 2018-12-21 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <pull.24.git.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This is what the legacy (scripted) rebase does in
`move_to_original_branch`, and we will need this functionality in the
next commit.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/rebase.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 768bea0da8..303175fdf1 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -337,6 +337,7 @@ static void add_var(struct strbuf *buf, const char *name, const char *value)
#define RESET_HEAD_DETACH (1<<0)
#define RESET_HEAD_HARD (1<<1)
+#define RESET_HEAD_REFS_ONLY (1<<2)
static int reset_head(struct object_id *oid, const char *action,
const char *switch_to_branch, unsigned flags,
@@ -344,6 +345,7 @@ static int reset_head(struct object_id *oid, const char *action,
{
unsigned detach_head = flags & RESET_HEAD_DETACH;
unsigned reset_hard = flags & RESET_HEAD_HARD;
+ unsigned refs_only = flags & RESET_HEAD_REFS_ONLY;
struct object_id head_oid;
struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
@@ -359,7 +361,7 @@ static int reset_head(struct object_id *oid, const char *action,
if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
BUG("Not a fully qualified branch: '%s'", switch_to_branch);
- if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) {
+ if (!refs_only && hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) {
ret = -1;
goto leave_reset_head;
}
@@ -372,6 +374,9 @@ static int reset_head(struct object_id *oid, const char *action,
if (!oid)
oid = &head_oid;
+ if (flags & RESET_HEAD_REFS_ONLY)
+ goto reset_head_refs;
+
memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
setup_unpack_trees_porcelain(&unpack_tree_opts, action);
unpack_tree_opts.head_idx = 1;
@@ -412,6 +417,7 @@ static int reset_head(struct object_id *oid, const char *action,
goto leave_reset_head;
}
+reset_head_refs:
reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : "rebase");
prefix_len = msg.len;
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/4] rebase: move `reset_head()` into a better spot
From: Johannes Schindelin via GitGitGadget @ 2018-12-21 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <pull.24.git.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Over the next commits, we want to make use of it in `run_am()` (i.e.
running the `--am` backend directly, without detouring to Unix shell
script code) which in turn will be called from `run_specific_rebase()`.
So let's move it before that latter function.
This commit is best viewed using --color-moved.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/rebase.c | 238 +++++++++++++++++++++++------------------------
1 file changed, 119 insertions(+), 119 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index b5c99ec10c..e1dfa74ca8 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -333,6 +333,125 @@ static void add_var(struct strbuf *buf, const char *name, const char *value)
}
}
+#define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
+
+#define RESET_HEAD_DETACH (1<<0)
+#define RESET_HEAD_HARD (1<<1)
+
+static int reset_head(struct object_id *oid, const char *action,
+ const char *switch_to_branch, unsigned flags,
+ const char *reflog_orig_head, const char *reflog_head)
+{
+ unsigned detach_head = flags & RESET_HEAD_DETACH;
+ unsigned reset_hard = flags & RESET_HEAD_HARD;
+ struct object_id head_oid;
+ struct tree_desc desc[2] = { { NULL }, { NULL } };
+ struct lock_file lock = LOCK_INIT;
+ struct unpack_trees_options unpack_tree_opts;
+ struct tree *tree;
+ const char *reflog_action;
+ struct strbuf msg = STRBUF_INIT;
+ size_t prefix_len;
+ struct object_id *orig = NULL, oid_orig,
+ *old_orig = NULL, oid_old_orig;
+ int ret = 0, nr = 0;
+
+ if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
+ BUG("Not a fully qualified branch: '%s'", switch_to_branch);
+
+ if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) {
+ ret = -1;
+ goto leave_reset_head;
+ }
+
+ if ((!oid || !reset_hard) && get_oid("HEAD", &head_oid)) {
+ ret = error(_("could not determine HEAD revision"));
+ goto leave_reset_head;
+ }
+
+ if (!oid)
+ oid = &head_oid;
+
+ memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
+ setup_unpack_trees_porcelain(&unpack_tree_opts, action);
+ unpack_tree_opts.head_idx = 1;
+ unpack_tree_opts.src_index = the_repository->index;
+ unpack_tree_opts.dst_index = the_repository->index;
+ unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
+ unpack_tree_opts.update = 1;
+ unpack_tree_opts.merge = 1;
+ if (!detach_head)
+ unpack_tree_opts.reset = 1;
+
+ if (read_index_unmerged(the_repository->index) < 0) {
+ ret = error(_("could not read index"));
+ goto leave_reset_head;
+ }
+
+ if (!reset_hard && !fill_tree_descriptor(&desc[nr++], &head_oid)) {
+ ret = error(_("failed to find tree of %s"),
+ oid_to_hex(&head_oid));
+ goto leave_reset_head;
+ }
+
+ if (!fill_tree_descriptor(&desc[nr++], oid)) {
+ ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
+ goto leave_reset_head;
+ }
+
+ if (unpack_trees(nr, desc, &unpack_tree_opts)) {
+ ret = -1;
+ goto leave_reset_head;
+ }
+
+ tree = parse_tree_indirect(oid);
+ prime_cache_tree(the_repository->index, tree);
+
+ if (write_locked_index(the_repository->index, &lock, COMMIT_LOCK) < 0) {
+ ret = error(_("could not write index"));
+ goto leave_reset_head;
+ }
+
+ reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
+ strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : "rebase");
+ prefix_len = msg.len;
+
+ if (!get_oid("ORIG_HEAD", &oid_old_orig))
+ old_orig = &oid_old_orig;
+ if (!get_oid("HEAD", &oid_orig)) {
+ orig = &oid_orig;
+ if (!reflog_orig_head) {
+ strbuf_addstr(&msg, "updating ORIG_HEAD");
+ reflog_orig_head = msg.buf;
+ }
+ update_ref(reflog_orig_head, "ORIG_HEAD", orig, old_orig, 0,
+ UPDATE_REFS_MSG_ON_ERR);
+ } else if (old_orig)
+ delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
+ if (!reflog_head) {
+ strbuf_setlen(&msg, prefix_len);
+ strbuf_addstr(&msg, "updating HEAD");
+ reflog_head = msg.buf;
+ }
+ if (!switch_to_branch)
+ ret = update_ref(reflog_head, "HEAD", oid, orig,
+ detach_head ? REF_NO_DEREF : 0,
+ UPDATE_REFS_MSG_ON_ERR);
+ else {
+ ret = create_symref("HEAD", switch_to_branch, msg.buf);
+ if (!ret)
+ ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
+ UPDATE_REFS_MSG_ON_ERR);
+ }
+
+leave_reset_head:
+ strbuf_release(&msg);
+ rollback_lock_file(&lock);
+ while (nr)
+ free((void *)desc[--nr].buffer);
+ return ret;
+}
+
static const char *resolvemsg =
N_("Resolve all conflicts manually, mark them as resolved with\n"
"\"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n"
@@ -526,125 +645,6 @@ static int run_specific_rebase(struct rebase_options *opts)
return status ? -1 : 0;
}
-#define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
-
-#define RESET_HEAD_DETACH (1<<0)
-#define RESET_HEAD_HARD (1<<1)
-
-static int reset_head(struct object_id *oid, const char *action,
- const char *switch_to_branch, unsigned flags,
- const char *reflog_orig_head, const char *reflog_head)
-{
- unsigned detach_head = flags & RESET_HEAD_DETACH;
- unsigned reset_hard = flags & RESET_HEAD_HARD;
- struct object_id head_oid;
- struct tree_desc desc[2] = { { NULL }, { NULL } };
- struct lock_file lock = LOCK_INIT;
- struct unpack_trees_options unpack_tree_opts;
- struct tree *tree;
- const char *reflog_action;
- struct strbuf msg = STRBUF_INIT;
- size_t prefix_len;
- struct object_id *orig = NULL, oid_orig,
- *old_orig = NULL, oid_old_orig;
- int ret = 0, nr = 0;
-
- if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
- BUG("Not a fully qualified branch: '%s'", switch_to_branch);
-
- if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) {
- ret = -1;
- goto leave_reset_head;
- }
-
- if ((!oid || !reset_hard) && get_oid("HEAD", &head_oid)) {
- ret = error(_("could not determine HEAD revision"));
- goto leave_reset_head;
- }
-
- if (!oid)
- oid = &head_oid;
-
- memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
- setup_unpack_trees_porcelain(&unpack_tree_opts, action);
- unpack_tree_opts.head_idx = 1;
- unpack_tree_opts.src_index = the_repository->index;
- unpack_tree_opts.dst_index = the_repository->index;
- unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
- unpack_tree_opts.update = 1;
- unpack_tree_opts.merge = 1;
- if (!detach_head)
- unpack_tree_opts.reset = 1;
-
- if (read_index_unmerged(the_repository->index) < 0) {
- ret = error(_("could not read index"));
- goto leave_reset_head;
- }
-
- if (!reset_hard && !fill_tree_descriptor(&desc[nr++], &head_oid)) {
- ret = error(_("failed to find tree of %s"),
- oid_to_hex(&head_oid));
- goto leave_reset_head;
- }
-
- if (!fill_tree_descriptor(&desc[nr++], oid)) {
- ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
- goto leave_reset_head;
- }
-
- if (unpack_trees(nr, desc, &unpack_tree_opts)) {
- ret = -1;
- goto leave_reset_head;
- }
-
- tree = parse_tree_indirect(oid);
- prime_cache_tree(the_repository->index, tree);
-
- if (write_locked_index(the_repository->index, &lock, COMMIT_LOCK) < 0) {
- ret = error(_("could not write index"));
- goto leave_reset_head;
- }
-
- reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
- strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : "rebase");
- prefix_len = msg.len;
-
- if (!get_oid("ORIG_HEAD", &oid_old_orig))
- old_orig = &oid_old_orig;
- if (!get_oid("HEAD", &oid_orig)) {
- orig = &oid_orig;
- if (!reflog_orig_head) {
- strbuf_addstr(&msg, "updating ORIG_HEAD");
- reflog_orig_head = msg.buf;
- }
- update_ref(reflog_orig_head, "ORIG_HEAD", orig, old_orig, 0,
- UPDATE_REFS_MSG_ON_ERR);
- } else if (old_orig)
- delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
- if (!reflog_head) {
- strbuf_setlen(&msg, prefix_len);
- strbuf_addstr(&msg, "updating HEAD");
- reflog_head = msg.buf;
- }
- if (!switch_to_branch)
- ret = update_ref(reflog_head, "HEAD", oid, orig,
- detach_head ? REF_NO_DEREF : 0,
- UPDATE_REFS_MSG_ON_ERR);
- else {
- ret = create_symref("HEAD", switch_to_branch, msg.buf);
- if (!ret)
- ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
- UPDATE_REFS_MSG_ON_ERR);
- }
-
-leave_reset_head:
- strbuf_release(&msg);
- rollback_lock_file(&lock);
- while (nr)
- free((void *)desc[--nr].buffer);
- return ret;
-}
-
static int rebase_config(const char *var, const char *value, void *data)
{
struct rebase_options *opts = data;
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/4] rebase: avoid double reflog entry when switching branches
From: Johannes Schindelin via GitGitGadget @ 2018-12-21 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <pull.24.git.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When switching a branch *and* updating said branch to a different
revision, let's avoid a double entry by first updating the branch and
then adjusting the symbolic ref HEAD.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/rebase.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index e1dfa74ca8..768bea0da8 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -438,10 +438,11 @@ static int reset_head(struct object_id *oid, const char *action,
detach_head ? REF_NO_DEREF : 0,
UPDATE_REFS_MSG_ON_ERR);
else {
- ret = create_symref("HEAD", switch_to_branch, msg.buf);
+ ret = update_ref(reflog_orig_head, switch_to_branch, oid,
+ NULL, 0, UPDATE_REFS_MSG_ON_ERR);
if (!ret)
- ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
- UPDATE_REFS_MSG_ON_ERR);
+ ret = create_symref("HEAD", switch_to_branch,
+ reflog_head);
}
leave_reset_head:
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/4] Let the builtin rebase call the git am command directly
From: Johannes Schindelin via GitGitGadget @ 2018-12-21 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Especially on Windows, where Unix shell scripting is a foreign endeavor, and
an expensive one at that, we really want to avoid running through the Bash.
This not only makes everything faster, but also more robust, as the Bash we
use on Windows relies on a derivative of the Cygwin runtime, which in turn
has to jump through a couple of hoops that are sometimes a little too tricky
to make things work. Read: the less we rely on Unix shell scripting, the
more likely Windows users will be able to enjoy our software.
Johannes Schindelin (4):
rebase: move `reset_head()` into a better spot
rebase: avoid double reflog entry when switching branches
rebase: teach `reset_head()` to optionally skip the worktree
built-in rebase: call `git am` directly
builtin/rebase.c | 428 ++++++++++++++++++++++++++++++++++-------------
1 file changed, 309 insertions(+), 119 deletions(-)
base-commit: b21ebb671bb7dea8d342225f0d66c41f4e54d5ca
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-24%2Fdscho%2Fbuiltin-rebase--am-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-24/dscho/builtin-rebase--am-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/24
--
gitgitgadget
^ permalink raw reply
* Does "git push" open a pack for read before closing it?
From: git-mailinglist @ 2018-12-21 12:46 UTC (permalink / raw)
To: git
[Major ignorance alert]
I'm writing software to implement a FUSE mount for a decentralised file
system and during testing with git I see some strange behaviour which
I'd like to investigate. It might be a bug in my code, or even the FUSE
lib I'm using, or it might be intended behaviour by git.
So one thing I'd like to do is check if this is expected in git.
SYSTEM
OS: Ubuntu 18.10
git version 2.19.1
Decentralised storage mounted at ~/SAFE
What I'm doing
I'm testing my FUSE implementation for SAFE Network while exploring the
use of git with decentralised storage, so not necessarily in a sensible
arrangement (comments on that also welcome).
I have a folder at ~/SAFE/_public/tests/data1/ and want to create a bare
repo there to use as a remote from my local drive for an existing git
repo at ~/src/safe/sjs.git
Anyway, I do the following sequence of commands which are all fine up
until the last one which eventually fails:
cd ~/SAFE/_public/tests/data1
git init --bare blah
cd ~/src/safe/sjs.git
git remote remove origin
git remote add origin ~/SAFE/_public/tests/data1/blah
git push origin master
Here's the output from the last command above:
Enumerating objects: 373, done.
Counting objects: 100% (373/373), done.
Delta compression using up to 8 threads
Compressing objects: 100% (371/371), done.
Writing objects: 100% (373/373), 187.96 KiB | 33.00 KiB/s, done.
Total 373 (delta 254), reused 0 (delta 0)
remote: fatal: unable to open
/home/mrh/SAFE/_public/tests/data1/blah/./objects/incoming-73lbb6/pack/tmp_pack_pL28kQ:
Remote I/O error
error: remote unpack failed: index-pack abnormal exit
To /home/mrh/SAFE/_public/tests/data1/blah
! [remote rejected] master -> master (unpacker error)
error: failed to push some refs to '/home/mrh/SAFE/_public/tests/data1/blah'
Inspecting the logs from my FUSE implementation I see that there's a
problem related to this file on the mounted storage:
/_public/tests/data1/blah/objects/incoming-73lbb6/pack/tmp_pack_pL28kQ
Prior to the error the file is written to multiple times by git - all
good (about 200kB in all). Then, before the file is closed I see an
attempt to open it for read, which fails. The failure is because I don't
support read on a file that is open for write yet, and I'm not sure if
that is sensible or what git might be expecting to do given the file has
not even been flushed to disk at this point.
So I'd like to know if this is expected behaviour by git (or where to
look to find out), and if it is expected, then what might git expect to
do if the file were opened successfully?
N.B. After the failure, the file is closed and then deleted!
Also note that it is possible the behaviour I'm seeing is not really git
but another issue, such as a bug in the sync/async aspect of my code.
Thanks
Mark
--
Secure Access For Everyone:
- SAFE Network
- First Autonomous Decentralised Internet
https://safenetwork.tech
^ permalink raw reply
* Re: [PATCH 1/1] Add author and committer configuration settings
From: Johannes Schindelin @ 2018-12-21 12:15 UTC (permalink / raw)
To: William Hubbs; +Cc: git, chutzpah
In-Reply-To: <20181219183939.16358-2-williamh@gentoo.org>
Hi William,
thank you for putting this together.
On Wed, 19 Dec 2018, William Hubbs wrote:
> -extern const char *fmt_name(const char *name, const char *email);
> +extern const char *fmt_committer_name(void);
If it would not be too much trouble for you, could I ask you to split this
change out into a separate commit (which would be the first of now two
patches)? It could have a commit message like this:
ident: rename fmt_name() to fmt_committer_name()
Ever since 4c28e4ada03f (commit: die before asking to edit the log
message, 2010-12-20), all remaining callers of that function want
to format the committer name. To simplify the code, therefore, we
rename the function and move the getenv() call into it.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2] git: add -P as a short option for --no-pager
From: Johannes Schindelin @ 2018-12-21 12:04 UTC (permalink / raw)
To: Johannes Sixt
Cc: Git Mailing List, Junio C Hamano, Eric Sunshine, Kaartic Sivaraam
In-Reply-To: <23493ba1-1704-1e8c-f448-95540a36f886@kdbg.org>
Hi Hannes,
On Thu, 3 May 2018, Johannes Sixt wrote:
> It is possible to configure 'less', the pager, to use an alternate
> screen to show the content, for example, by setting LESS=RS in the
> environment. When it is closed in this configuration, it switches
> back to the original screen, and all content is gone.
>
> It is not uncommon to request that the output remains visible in
> the terminal. For this, the option --no-pager can be used. But
> it is a bit cumbersome to type, even when command completion is
> available. Provide a short option, -P, to make the option easier
> accessible.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> Given the positive feedback, I resurrect the patch.
>
> Changes since v1:
> - Use -P instead of -N
> - Commit message changed as proposed by Kaartic
Just a quick note, as mine was a vocal voice against this patch: I find
myself using this more and more. So I was wrong to object, and I
apologize.
Thank you,
Dscho
^ permalink raw reply
* Re: [PATCH] diff: add support for reading files literally with --no-index
From: Johannes Schindelin @ 2018-12-21 11:52 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Jeff King, Duy Nguyen
In-Reply-To: <20181220002610.43832-1-sandals@crustytoothpaste.net>
Hi Brian,
On Thu, 20 Dec 2018, brian m. carlson wrote:
> In some shells, such as bash and zsh, it's possible to use a command
> substitution to provide the output of a command as a file argument to
> another process, like so:
>
> diff -u <(printf "a\nb\n") <(printf "a\nc\n")
>
> However, this syntax does not produce useful results with git diff
> --no-index.
>
> On macOS, the arguments to the command are named pipes under /dev/fd,
> and git diff doesn't know how to handle a named pipe. On Linux, the
> arguments are symlinks to pipes, so git diff "helpfully" diffs these
> symlinks, comparing their targets like "pipe:[1234]" and "pipe:[5678]".
... and in Git for Windows' Bash, it would result in something like:
$ git -P diff --no-index <(printf "a\nb\n") <(printf "a\nc\n")
error: Could not access '/proc/24012/fd/63'
... because the Bash is "MSYS2-aware" and knows about `/proc/` while
`git.exe` is a pure Win32 executable that has no idea what Bash is talking
about.
Sadly, your patch does not change the situation one bit (because it does
not change the fact that the MSYS2 Bash passes a path to `git.exe` that is
not a valid Windows path, and neither could it, but that's not the problem
of your patch).
I reviewed your patch and it looks good to me!
Thanks,
Dscho
> Because this behavior is not very helpful, and because git diff has many
> features that people would like to use even on non-Git files, add an
> option to git diff --no-index to read files literally, dereferencing
> symlinks and reading them as a normal file.
>
> Note that this behavior requires that the files be read entirely into
> memory, just as we do when reading from standard input.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> This is a long-standing annoyance of mine, and it also makes some use
> cases possible (for example, diffing filtered and non-filtered objects).
>
> We don't include a test for the pipe scenario because I couldn't get
> that case to work in portable shell (although of course it works in
> bash). I have, however, tested it on both macOS and Linux. No clue how
> this works on Windows.
>
> Documentation/git-diff.txt | 5 +++++
> diff-no-index.c | 34 +++++++++++++++++++++++++++-------
> diff.c | 24 +++++++++++++-----------
> diff.h | 1 +
> diffcore.h | 1 +
> t/t4053-diff-no-index.sh | 28 ++++++++++++++++++++++++++++
> 6 files changed, 75 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
> index 030f162f30..4c4695c88d 100644
> --- a/Documentation/git-diff.txt
> +++ b/Documentation/git-diff.txt
> @@ -111,6 +111,11 @@ include::diff-options.txt[]
> "Unmerged". Can be used only when comparing the working tree
> with the index.
>
> +--literally::
> + Read the specified files literally, as `diff` would,
> + dereferencing any symlinks and reading data from pipes.
> + This option only works with `--no-index`.
> +
> <path>...::
> The <paths> parameters, when given, are used to limit
> the diff to the named paths (you can give directory
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 9414e922d1..2707206aee 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -75,7 +75,25 @@ static int populate_from_stdin(struct diff_filespec *s)
> return 0;
> }
>
> -static struct diff_filespec *noindex_filespec(const char *name, int mode)
> +static int populate_literally(struct diff_filespec *s)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + size_t size = 0;
> + int fd = xopen(s->path, O_RDONLY);
> +
> + if (strbuf_read(&buf, fd, 0) < 0)
> + return error_errno("error while reading from '%s'", s->path);
> +
> + s->should_munmap = 0;
> + s->data = strbuf_detach(&buf, &size);
> + s->size = size;
> + s->should_free = 1;
> + s->read_literally = 1;
> + return 0;
> +}
> +
> +static struct diff_filespec *noindex_filespec(const char *name, int mode,
> + struct diff_options *o)
> {
> struct diff_filespec *s;
>
> @@ -85,6 +103,8 @@ static struct diff_filespec *noindex_filespec(const char *name, int mode)
> fill_filespec(s, &null_oid, 0, mode);
> if (name == file_from_standard_input)
> populate_from_stdin(s);
> + else if (o->flags.read_literally)
> + populate_literally(s);
> return s;
> }
>
> @@ -101,14 +121,14 @@ static int queue_diff(struct diff_options *o,
>
> if (S_ISDIR(mode1)) {
> /* 2 is file that is created */
> - d1 = noindex_filespec(NULL, 0);
> - d2 = noindex_filespec(name2, mode2);
> + d1 = noindex_filespec(NULL, 0, o);
> + d2 = noindex_filespec(name2, mode2, o);
> name2 = NULL;
> mode2 = 0;
> } else {
> /* 1 is file that is deleted */
> - d1 = noindex_filespec(name1, mode1);
> - d2 = noindex_filespec(NULL, 0);
> + d1 = noindex_filespec(name1, mode1, o);
> + d2 = noindex_filespec(NULL, 0, o);
> name1 = NULL;
> mode1 = 0;
> }
> @@ -189,8 +209,8 @@ static int queue_diff(struct diff_options *o,
> SWAP(name1, name2);
> }
>
> - d1 = noindex_filespec(name1, mode1);
> - d2 = noindex_filespec(name2, mode2);
> + d1 = noindex_filespec(name1, mode1, o);
> + d2 = noindex_filespec(name2, mode2, o);
> diff_queue(&diff_queued_diff, d1, d2);
> return 0;
> }
> diff --git a/diff.c b/diff.c
> index dc9965e836..740d0087b9 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4282,18 +4282,18 @@ static void run_diff_cmd(const char *pgm,
> fprintf(o->file, "* Unmerged path %s\n", name);
> }
>
> -static void diff_fill_oid_info(struct diff_filespec *one, struct index_state *istate)
> +static void diff_fill_oid_info(struct diff_filespec *one, struct diff_options *o)
> {
> if (DIFF_FILE_VALID(one)) {
> if (!one->oid_valid) {
> struct stat st;
> - if (one->is_stdin) {
> + if (one->is_stdin || one->read_literally) {
> oidclr(&one->oid);
> return;
> }
> if (lstat(one->path, &st) < 0)
> die_errno("stat '%s'", one->path);
> - if (index_path(istate, &one->oid, one->path, &st, 0))
> + if (index_path(o->repo->index, &one->oid, one->path, &st, 0))
> die("cannot hash %s", one->path);
> }
> }
> @@ -4341,8 +4341,8 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
> return;
> }
>
> - diff_fill_oid_info(one, o->repo->index);
> - diff_fill_oid_info(two, o->repo->index);
> + diff_fill_oid_info(one, o);
> + diff_fill_oid_info(two, o);
>
> if (!pgm &&
> DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&
> @@ -4389,8 +4389,8 @@ static void run_diffstat(struct diff_filepair *p, struct diff_options *o,
> if (o->prefix_length)
> strip_prefix(o->prefix_length, &name, &other);
>
> - diff_fill_oid_info(p->one, o->repo->index);
> - diff_fill_oid_info(p->two, o->repo->index);
> + diff_fill_oid_info(p->one, o);
> + diff_fill_oid_info(p->two, o);
>
> builtin_diffstat(name, other, p->one, p->two,
> diffstat, o, p);
> @@ -4414,8 +4414,8 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
> if (o->prefix_length)
> strip_prefix(o->prefix_length, &name, &other);
>
> - diff_fill_oid_info(p->one, o->repo->index);
> - diff_fill_oid_info(p->two, o->repo->index);
> + diff_fill_oid_info(p->one, o);
> + diff_fill_oid_info(p->two, o);
>
> builtin_checkdiff(name, other, attr_path, p->one, p->two, o);
> }
> @@ -5159,6 +5159,8 @@ int diff_opt_parse(struct diff_options *options,
> options->flags.funccontext = 1;
> else if (!strcmp(arg, "--no-function-context"))
> options->flags.funccontext = 0;
> + else if (!strcmp(arg, "--literally"))
> + options->flags.read_literally = 1;
> else if ((argcount = parse_long_opt("output", av, &optarg))) {
> char *path = prefix_filename(prefix, optarg);
> options->file = xfopen(path, "w");
> @@ -5720,8 +5722,8 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid
> if (DIFF_PAIR_UNMERGED(p))
> continue;
>
> - diff_fill_oid_info(p->one, options->repo->index);
> - diff_fill_oid_info(p->two, options->repo->index);
> + diff_fill_oid_info(p->one, options);
> + diff_fill_oid_info(p->two, options);
>
> len1 = remove_space(p->one->path, strlen(p->one->path));
> len2 = remove_space(p->two->path, strlen(p->two->path));
> diff --git a/diff.h b/diff.h
> index ce5e8a8183..7dedd3bcd1 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -97,6 +97,7 @@ struct diff_flags {
> unsigned stat_with_summary:1;
> unsigned suppress_diff_headers:1;
> unsigned dual_color_diffed_diffs:1;
> + unsigned read_literally:1;
> };
>
> static inline void diff_flags_or(struct diff_flags *a,
> diff --git a/diffcore.h b/diffcore.h
> index b651061c0e..363869447a 100644
> --- a/diffcore.h
> +++ b/diffcore.h
> @@ -48,6 +48,7 @@ struct diff_filespec {
> #define DIRTY_SUBMODULE_UNTRACKED 1
> #define DIRTY_SUBMODULE_MODIFIED 2
> unsigned is_stdin : 1;
> + unsigned read_literally : 1;
> unsigned has_more_entries : 1; /* only appear in combined diff */
> /* data should be considered "binary"; -1 means "don't know yet" */
> signed int is_binary : 2;
> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
> index 6e0dd6f9e5..53e6bcdc19 100755
> --- a/t/t4053-diff-no-index.sh
> +++ b/t/t4053-diff-no-index.sh
> @@ -137,4 +137,32 @@ test_expect_success 'diff --no-index from repo subdir with absolute paths' '
> test_cmp expect actual
> '
>
> +test_expect_success 'diff --no-index --literally' '
> + echo "diff --git a/../../non/git/a b/../../non/git/b" >expect &&
> + test_expect_code 1 \
> + git -C repo/sub \
> + diff --literally ../../non/git/a ../../non/git/b >actual &&
> + head -n 1 <actual >actual.head &&
> + test_cmp expect actual.head
> +'
> +
> +test_expect_success SYMLINKS 'diff --no-index --literally with symlinks' '
> + test_write_lines a b c >f1 &&
> + test_write_lines a d c >f2 &&
> + ln -s f1 s1 &&
> + ln -s f2 s2 &&
> + cat >expect <<-\EOF &&
> + diff --git a/s1 b/s2
> + --- a/s1
> + +++ b/s2
> + @@ -1,3 +1,3 @@
> + a
> + -b
> + +d
> + c
> + EOF
> + test_expect_code 1 git diff --no-index --literally s1 s2 >actual &&
> + test_cmp expect actual
> +'
> +
> test_done
>
^ 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