On Wed, Jan 24, 2024 at 7:21 AM Patrick Steinhardt <ps@xxxxxx> wrote:
>
> diff --git a/reftable/stack_test.c b/reftable/stack_test.c
> index 289e902146..2e7d1768b7 100644
> --- a/reftable/stack_test.c
> +++ b/reftable/stack_test.c
> @@ -443,15 +443,16 @@ static void test_reftable_stack_add(void)
> int err = 0;
> struct reftable_write_options cfg = {
> .exact_log_message = 1,
> + .default_permissions = 0660,
> };
> struct reftable_stack *st = NULL;
> char *dir = get_tmp_dir(__LINE__);
> -
> struct reftable_ref_record refs[2] = { { NULL } };
> struct reftable_log_record logs[2] = { { NULL } };
> + struct strbuf scratch = STRBUF_INIT;
The variable name `scratch` seems rather vague to me as opposed to something
like `path`. After a quick search though, `scratch` appears to be a fairly
common name used in similar scenarios. So probably not a big deal, but
something
I thought I'd mention.
> + struct stat stat_result;
> int N = ARRAY_SIZE(refs);
>
> -
> err = reftable_new_stack(&st, dir, cfg);
> EXPECT_ERR(err);
> st->disable_auto_compact = 1;
> @@ -509,12 +510,32 @@ static void test_reftable_stack_add(void)
> reftable_log_record_release(&dest);
> }
>
> +#ifndef GIT_WINDOWS_NATIVE
> + strbuf_addstr(&scratch, dir);
> + strbuf_addstr(&scratch, "/tables.list");
> + err = stat(scratch.buf, &stat_result);
> + EXPECT(!err);
> + EXPECT((stat_result.st_mode & 0777) == cfg.default_permissions);
> +
> + strbuf_reset(&scratch);
> + strbuf_addstr(&scratch, dir);
> + strbuf_addstr(&scratch, "/");
> + /* do not try at home; not an external API for reftable. */
> + strbuf_addstr(&scratch, st->readers[0]->name);
> + err = stat(scratch.buf, &stat_result);
> + EXPECT(!err);
> + EXPECT((stat_result.st_mode & 0777) == cfg.default_permissions);
> +#else
> + (void) stat_result;
> +#endif
Why do we ignore Windows here? And would it warrant explaining in the commit
message?
-Justin
