Re: [PATCH v3 05/19] entry: extract a header file for entry.c functions

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 30, 2020, 21:36 UTC
Message-ID: <xmqqk0v75rbt.fsf@gitster.c.googlers.com>
In-Reply-To: <2caa2c4345d524e9e3bb0c388f8dc0b99236d166.1603937110.git.matheus.bernardino@usp.br>

Matheus Tavares <matheus.bernardino@usp.br> writes:

Show 5 quoted lines

> The declarations of entry.c's public functions and structures currently
> reside in cache.h. Although not many, they contribute to the size of
> cache.h and, when changed, cause the unnecessary recompilation of
> modules that don't really use these functions. So let's move them to a
> new entry.h header.

Good idea. This is mostly moving things around, so there are only a few minor nits.

Show 26 quoted lines

> diff --git a/entry.h b/entry.h
> new file mode 100644
> index 0000000000..2d69185448
> --- /dev/null
> +++ b/entry.h
> @@ -0,0 +1,41 @@
> +#ifndef ENTRY_H
> +#define ENTRY_H
> +
> +#include "cache.h"
> +#include "convert.h"
> +
> +struct checkout {
> +	struct index_state *istate;
> +	const char *base_dir;
> +	int base_dir_len;
> +	struct delayed_checkout *delayed_checkout;
> +	struct checkout_metadata meta;
> +	unsigned force:1,
> +		 quiet:1,
> +		 not_new:1,
> +		 clone:1,
> +		 refresh_cache:1;
> +};
> +#define CHECKOUT_INIT { NULL, "" }
> +

It makes sense to have a blank here, like you did, as we just completed the definition of "struct checkout" and things directly related to it.

Show 12 quoted lines

> +#define TEMPORARY_FILENAME_LENGTH 25
> +
> +/*
> + * Write the contents from ce out to the working tree.
> + *
> + * When topath[] is not NULL, instead of writing to the working tree
> + * file named by ce, a temporary file is created by this function and
> + * its name is returned in topath[], which must be able to hold at
> + * least TEMPORARY_FILENAME_LENGTH bytes long.
> + */
> +int checkout_entry(struct cache_entry *ce, const struct checkout *state,
> +		   char *topath, int *nr_checkouts);

The comment before the above block applies to both the function and to the TEMPORARY_FILENAME_LENGTH preprocessor macro. And this is where we conclude the definition related to the function so it is a good idea to have a blank line here....

> +void enable_delayed_checkout(struct checkout *state);
> +int finish_delayed_checkout(struct checkout *state, int *nr_checkouts);

....and here, as we have finished talking about the "delayed" stuff.

Show 19 quoted lines

> +/*
> + * Unlink the last component and schedule the leading directories for
> + * removal, such that empty directories get removed.
> + */
> +void unlink_entry(const struct cache_entry *ce);
> +
> +#endif /* ENTRY_H */
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 323280dd48..a511fadd89 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -16,6 +16,7 @@
>  #include "fsmonitor.h"
>  #include "object-store.h"
>  #include "promisor-remote.h"
> +#include "entry.h"
>  
>  /*
>   * Error messages expected by scripts out of plumbing commands such as