[v4,04/19] convert: add conv_attrs classification

Commit Message

Matheus Tavares Nov. 4, 2020, 8:33 p.m. UTC

From: Jeff Hostetler <[e-mail]>

Create `enum conv_attrs_classification` to express the different ways
that attributes are handled for a blob during checkout.

This will be used in a later commit when deciding whether to add a file
to the parallel or delayed queue during checkout. For now, we can also
use it in get_stream_filter_ca() to simplify the function (as the
classifying logic is the same).

Signed-off-by: Jeff Hostetler <[e-mail]>
[matheus.bernardino: use classification in get_stream_filter_ca()]
Signed-off-by: Matheus Tavares <[e-mail]>
---
 convert.c | 26 +++++++++++++++++++-------
 convert.h | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 7 deletions(-)

Comments

Christian Couder Dec. 5, 2020, 12:07 p.m. UTC | #1

On Wed, Nov 4, 2020 at 9:33 PM Matheus Tavares
<[e-mail]> wrote:
>
> From: Jeff Hostetler <[e-mail]>
>
> Create `enum conv_attrs_classification` to express the different ways
> that attributes are handled for a blob during checkout.

Micronit: the subject of the patch might want to be a bit more
explicit like "convert: add conv_attrs_classification enum". Otherwise
it could make one wonder if it is missing an underscore between
"conv_attrs" and "classification".

> This will be used in a later commit when deciding whether to add a file
> to the parallel or delayed queue during checkout. For now, we can also
> use it in get_stream_filter_ca() to simplify the function (as the
> classifying logic is the same).
>
> Signed-off-by: Jeff Hostetler <[e-mail]>
> [matheus.bernardino: use classification in get_stream_filter_ca()]

Maybe "Co-authored-by: Matheus Tavares <[e-mail]>" instead?

> Signed-off-by: Matheus Tavares <[e-mail]>

Matheus Tavares Dec. 5, 2020, 10:08 p.m. UTC | #2

On Sat, Dec 5, 2020 at 9:07 AM Christian Couder
<[e-mail]> wrote:
>
> On Wed, Nov 4, 2020 at 9:33 PM Matheus Tavares
> <[e-mail]> wrote:
> >
> > From: Jeff Hostetler <[e-mail]>
> >
> > Create `enum conv_attrs_classification` to express the different ways
> > that attributes are handled for a blob during checkout.
>
> Micronit: the subject of the patch might want to be a bit more
> explicit like "convert: add conv_attrs_classification enum". Otherwise
> it could make one wonder if it is missing an underscore between
> "conv_attrs" and "classification".

Makes sense, thanks!

Patch

diff --git a/convert.c b/convert.c
index 0a61e4e9bf..3b2d626268 100644
--- a/convert.c
+++ b/convert.c
@@ -1951,13 +1951,7 @@  struct stream_filter *get_stream_filter_ca(const struct conv_attrs *ca,
 {
 	struct stream_filter *filter = NULL;
 
-	if (ca->drv && (ca->drv->process || ca->drv->smudge || ca->drv->clean))
-		return NULL;
-
-	if (ca->working_tree_encoding)
-		return NULL;
-
-	if (ca->crlf_action == CRLF_AUTO || ca->crlf_action == CRLF_AUTO_CRLF)
+	if (classify_conv_attrs(ca) != CA_CLASS_STREAMABLE)
 		return NULL;
 
 	if (ca->ident)
@@ -2013,3 +2007,21 @@  void clone_checkout_metadata(struct checkout_metadata *dst,
 	if (blob)
 		oidcpy(&dst->blob, blob);
 }
+
+enum conv_attrs_classification classify_conv_attrs(const struct conv_attrs *ca)
+{
+	if (ca->drv) {
+		if (ca->drv->process)
+			return CA_CLASS_INCORE_PROCESS;
+		if (ca->drv->smudge || ca->drv->clean)
+			return CA_CLASS_INCORE_FILTER;
+	}
+
+	if (ca->working_tree_encoding)
+		return CA_CLASS_INCORE;
+
+	if (ca->crlf_action == CRLF_AUTO || ca->crlf_action == CRLF_AUTO_CRLF)
+		return CA_CLASS_INCORE;
+
+	return CA_CLASS_STREAMABLE;
+}
diff --git a/convert.h b/convert.h
index 484b50965d..43e567a59b 100644
--- a/convert.h
+++ b/convert.h
@@ -200,4 +200,37 @@  int stream_filter(struct stream_filter *,
 		  const char *input, size_t *isize_p,
 		  char *output, size_t *osize_p);
 
+enum conv_attrs_classification {
+	/*
+	 * The blob must be loaded into a buffer before it can be
+	 * smudged. All smudging is done in-proc.
+	 */
+	CA_CLASS_INCORE,
+
+	/*
+	 * The blob must be loaded into a buffer, but uses a
+	 * single-file driver filter, such as rot13.
+	 */
+	CA_CLASS_INCORE_FILTER,
+
+	/*
+	 * The blob must be loaded into a buffer, but uses a
+	 * long-running driver process, such as LFS. This might or
+	 * might not use delayed operations. (The important thing is
+	 * that there is a single subordinate long-running process
+	 * handling all associated blobs and in case of delayed
+	 * operations, may hold per-blob state.)
+	 */
+	CA_CLASS_INCORE_PROCESS,
+
+	/*
+	 * The blob can be streamed and smudged without needing to
+	 * completely read it into a buffer.
+	 */
+	CA_CLASS_STREAMABLE,
+};
+
+enum conv_attrs_classification classify_conv_attrs(
+	const struct conv_attrs *ca);
+
 #endif /* CONVERT_H */