Kjetil Barvik <barvik@xxxxxxxxxxxx> writes:
> OK, maybe I instead should have tried to merge the function
> create_directories() with the safe_create_leading_directories() and
> *_const() functions? What do pepople think?
Strictly speaking, the safe_create_leading_* functions are meant to work
on paths inside $GIT_DIR and are not meant for paths inside the work tree,
which is this function is about. Their semantics are different with
respect to adjust_shared_perm().
Which means that you would either have two variants (one for work tree
paths, and another for paths inside $GIT_DIR), or a unified function that
has an argument to specify whether to run adjust_shared_perm().
HOWEVER.
That is only "strictly speaking".
A non-bare repository that is shared feels like an oximoron, but perhaps
there is a valid "shared work area" workflow that may benefit from such a
setup.
I see existing (mis)uses of the safe_create_leading_* in builtin-apply.c,
builtin-clone.c (the one that creates the work_tree, the other one is Ok),
merge-recursive.c (both call sites) that are used to touch the work tree,
but all places that create regular files in the work tree do not run
adjust_shared_perm() but simply honor the user's umask.
If we _were_ to support a "shared work area" workflow, having a unified
"create leading directory" function that always calls adjust_shared_perm()
may make sense (note that adjust_shared_perm() is a no-op in a non-shared
repository). We then need to also call adjust_shared_perm() for codepaths
that create regular files as well, though (e.g. write_entry() in entry.c,
but there are many others).
> diff --git a/entry.c b/entry.c
> index 05aa58d..c2404ea 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -2,15 +2,19 @@
> #include "blob.h"
> #include "dir.h"
>
> -static void create_directories(const char *path, const struct checkout *state)
> +static void
> +create_directories(int path_len, const char *path, const struct checkout *state)
Please do not split the line like this.
The existing sources are not laid out to allow "grep ^funcname(", nor are
we going to reformat all the files to support such a use case.
When we pass <string, length> pair to functions, I think we pass them in
the order I just wrote in all the other functions.
The micro-optimization itself makes sense to me, though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
