"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
>
> Create files with mode 0666, so umask works as intended. Provides an override,
> which is useful to support shared repos (test t1301-shared-repo.sh).
> + /* Default mode for creating files. If unset, use 0666 (+umask) */
> + unsigned int default_permissions;
> +
Presumably this is primed by a call to get_shared_repository(),
possibly followed by calc_shared_perm(), but having it in the
interface is a good idea, as it allows us to avoid making these
git-core specific calls from reftable/ library proper?
> diff --git a/reftable/stack.c b/reftable/stack.c
> index df5021ebf08..56bf5f2d84a 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -469,7 +469,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
> strbuf_addstr(&add->lock_file_name, ".lock");
>
> add->lock_file_fd = open(add->lock_file_name.buf,
> - O_EXCL | O_CREAT | O_WRONLY, 0644);
> + O_EXCL | O_CREAT | O_WRONLY, 0666);
This change makes sense.
> if (add->lock_file_fd < 0) {
> if (errno == EEXIST) {
> err = REFTABLE_LOCK_ERROR;
> @@ -478,6 +478,13 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
> }
> goto done;
> }
> + if (st->config.default_permissions) {
> + if (chmod(add->lock_file_name.buf, st->config.default_permissions) < 0) {
> + err = REFTABLE_IO_ERROR;
> + goto done;
This part does not exactly make sense, though.
If this were a library code meant to link with ONLY with Git, I
would have recommended to make a adjust_shared_perm() call from
here, without having to have "unsigned int default_permissions" to
reftable_write_options structure.
I wonder if it is a better design to make the new member in the
structure a pointer to a generic and opaque helper function that is
called from here, i.e.
if (st->config.adjust_perm &&
st->config.adjust_perm(add->lock_file_name.buf) < 0) {
err = REFTABLE_IO_ERROR;
goto done;
}
so that when linking with and calling from git, we can just stuff
the pointer to adjust_shared_perm function to the member?
> + }
> + }
> +
