Matheus Tavares <matheus.bernardino@xxxxxx> writes:
> +/* Note: ca is used (and required) iff the entry refers to a regular file. */
This reflects how the current code happens to work, and it is
unlikely to change (in other words, I offhand do not think of a
reason why attributes may affect checking out a symlink or a
submodule), so that's probably OK. I mention this specifically
because ...
> +static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca,
> + const struct checkout *state, int to_tempfile)
> {
> unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT;
> struct delayed_checkout *dco = state->delayed_checkout;
> @@ -281,8 +282,7 @@ static int write_entry(struct cache_entry *ce,
> clone_checkout_metadata(&meta, &state->meta, &ce->oid);
>
> if (ce_mode_s_ifmt == S_IFREG) {
> - struct stream_filter *filter = get_stream_filter(state->istate, ce->name,
> - &ce->oid);
> + struct stream_filter *filter = get_stream_filter_ca(ca, &ce->oid);
> if (filter &&
> !streaming_write_entry(ce, path, filter,
> state, to_tempfile,
> @@ -329,14 +329,17 @@ static int write_entry(struct cache_entry *ce,
> * Convert from git internal format to working tree format
> */
> if (dco && dco->state != CE_NO_DELAY) {
> - ret = async_convert_to_working_tree(state->istate, ce->name, new_blob,
> - size, &buf, &meta, dco);
> + ret = async_convert_to_working_tree_ca(ca, ce->name,
> + new_blob, size,
> + &buf, &meta, dco);
> if (ret && string_list_has_string(&dco->paths, ce->name)) {
> free(new_blob);
> goto delayed;
> }
> - } else
> - ret = convert_to_working_tree(state->istate, ce->name, new_blob, size, &buf, &meta);
> + } else {
> + ret = convert_to_working_tree_ca(ca, ce->name, new_blob,
> + size, &buf, &meta);
> + }
>
> if (ret) {
> free(new_blob);
> @@ -442,6 +445,7 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state,
> {
> static struct strbuf path = STRBUF_INIT;
> struct stat st;
> + struct conv_attrs ca;
>
> if (ce->ce_flags & CE_WT_REMOVE) {
> if (topath)
> @@ -454,8 +458,13 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state,
> return 0;
> }
>
> - if (topath)
> - return write_entry(ce, topath, state, 1);
> + if (topath) {
> + if (S_ISREG(ce->ce_mode)) {
> + convert_attrs(state->istate, &ca, ce->name);
> + return write_entry(ce, topath, &ca, state, 1);
> + }
> + return write_entry(ce, topath, NULL, state, 1);
> + }
... it looked somewhat upside-down at the first glance that we
decide if lower level routines are allowed to use the ca at this
high level in the callchain. But it is the point of this change
to lift the point to make the decision to use attributes higher in
the callchain, so it would be OK (or "unavoidable").
I wonder if it is worth to avoid early return from the inner block,
like this:
struct conv_attrs *use_ca = NULL;
...
if (topath) {
struct conv_attrs ca;
if (S_ISREG(...)) {
convert_attrs(... &ca ...);
use_ca = &ca;
}
return write_entry(ce, topath, use_ca, state, 1);
}
which would make it easier to further add code that is common to
both regular file and other things before we call write_entry().
The same comment applies to the codepath where a new file gets
created in the next hunk.
> @@ -517,9 +526,16 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state,
> return 0;
>
> create_directories(path.buf, path.len, state);
> +
> if (nr_checkouts)
> (*nr_checkouts)++;
> - return write_entry(ce, path.buf, state, 0);
> +
> + if (S_ISREG(ce->ce_mode)) {
> + convert_attrs(state->istate, &ca, ce->name);
> + return write_entry(ce, path.buf, &ca, state, 0);
> + }
> +
> + return write_entry(ce, path.buf, NULL, state, 0);
> }
>
> void unlink_entry(const struct cache_entry *ce)
