git/list-objects-filter.c, branch v2.30.2

list-objects-filter: treat NULL filter_options as "disabled"

2020-05-05T04:57:58Z

In most callers, we have an actual list_objects_filter_options struct, and if no filtering is desired its "choice" element will be LOFC_DISABLED. However, some code may have only a pointer to such a struct which may be NULL (because _their_ callers didn't care about filtering, either). Rather than forcing them to handle this explicitly like: if (filter_options) traverse_commit_list_filtered(filter_options, revs, show_commit, show_object, show_data, NULL); else traverse_commit_list(revs, show_commit, show_object, show_data); let's just treat a NULL filter_options the same as LOFC_DISABLED. We only need a small change, since that option struct is converted into a real filter only in the "init" function. Signed-off-by: Jeff King Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano

Merge branch 'jk/partial-clone-sparse-blob'

2019-10-07T02:32:54Z

The name of the blob object that stores the filter specification for sparse cloning/fetching was interpreted in a wrong place in the code, causing Git to abort. * jk/partial-clone-sparse-blob: list-objects-filter: use empty string instead of NULL for sparse "base" list-objects-filter: give a more specific error sparse parsing error list-objects-filter: delay parsing of sparse oid t5616: test cloning/fetching with sparse:oid= filter

list-objects-filter: use empty string instead of NULL for sparse "base"

2019-09-16T19:47:51Z

We use add_excludes_from_blob_to_list() to parse a sparse blob. Since we don't have a base path, we pass NULL and 0 for the base and baselen, respectively. But the rest of the exclude code passes a literal empty string instead of NULL for this case. And indeed, we eventually end up with match_pathname() calling fspathncmp(), which then calls the system strncmp(path, base, baselen). This works on many platforms, which notice that baselen is 0 and do not look at the bytes of "base" at all. But it does violate the C standard, and building with SANITIZE=undefined will complain. You can also see it by instrumenting fspathncmp like this: diff --git a/dir.c b/dir.c index d021c908e5..4bb3d3ec96 100644 --- a/dir.c +++ b/dir.c @@ -71,6 +71,8 @@ int fspathcmp(const char *a, const char *b) int fspathncmp(const char *a, const char *b, size_t count) { + if (!a || !b) + BUG("null fspathncmp arguments"); return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count); } We could perhaps be more defensive in match_pathname(), but even if we did so, it makes sense for this code to match the rest of the exclude callers. Signed-off-by: Jeff King Acked-by: Jeff Hostetler Signed-off-by: Junio C Hamano

list-objects-filter: give a more specific error sparse parsing error

2019-09-16T19:47:45Z

The sparse:oid filter has two error modes: we might fail to resolve the name to an OID, or we might fail to parse the contents of that OID. In the latter case, let's give a less generic error message, and mention the OID we did find. While we're here, let's also mark both messages as translatable. Signed-off-by: Jeff King Acked-by: Jeff Hostetler Signed-off-by: Junio C Hamano

list-objects-filter: delay parsing of sparse oid

2019-09-16T19:47:37Z

The list-objects-filter code has two steps to its initialization: 1. parse_list_objects_filter() makes sure the spec is a filter we know about and is syntactically correct. This step is done by "rev-list" or "upload-pack" that is going to apply a filter, but also by "git clone" or "git fetch" before they send the spec across the wire. 2. list_objects_filter__init() runs the type-specific initialization (using function pointers established in step 1). This happens at the start of traverse_commit_list_filtered(), when we're about to actually use the filter. It's a good idea to parse as much as we can in step 1, in order to catch problems early (e.g., a blob size limit that isn't a number). But one thing we _shouldn't_ do is resolve any oids at that step (e.g., for sparse-file contents specified by oid). In the case of a fetch, the oid has to be resolved on the remote side. The current code does resolve the oid during the parse phase, but ignores any error (which we must do, because we might just be sending the spec across the wire). This leads to two bugs: - if we're not in a repository (e.g., because it's git-clone parsing the spec), then we trigger a BUG() trying to resolve the name - if we did hit the error case, we still have to notice that later and bail. The code path in rev-list handles this, but the one in upload-pack does not, leading to a segfault. We can fix both by moving the oid resolution into the sparse-oid init function. At that point we know we have a repository (because we're about to traverse), and handling the error there fixes the segfault. As a bonus, we can drop the NULL sparse_oid_value check in rev-list, since this is now handled in the sparse-oid-filter init function. Signed-off-by: Jeff King Acked-by: Jeff Hostetler Signed-off-by: Junio C Hamano

unpack-trees: rename 'is_excluded_from_list()'

2019-09-05T21:05:12Z

The first consumer of pattern-matching filenames was the .gitignore feature. In that context, storing a list of patterns as a 'struct exclude_list' makes sense. However, the sparse-checkout feature then adopted these structures and methods, but with the opposite meaning: these patterns match the files that should be included! Now that this library is renamed to use 'struct pattern_list' and 'struct pattern', we can now rename the method used by the sparse-checkout feature to determine which paths should appear in the working directory. The method is_excluded_from_list() is only used by the sparse-checkout logic in unpack-trees and list-objects-filter. The confusing part is that it returned 1 for "excluded" (i.e. it matches the list of exclusions) but that really manes that the path matched the list of patterns for _inclusion_ in the working directory. Rename the method to be path_matches_pattern_list() and have it return an explicit 'enum pattern_match_result'. Here, the values MATCHED = 1, UNMATCHED = 0, and UNDECIDED = -1 agree with the previous integer values. This shift allows future consumers to better understand what the retur values mean, and provides more type checking for handling those values. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano

treewide: rename 'exclude' methods to 'pattern'

2019-09-05T21:05:12Z

The first consumer of pattern-matching filenames was the .gitignore feature. In that context, storing a list of patterns as a 'struct exclude_list' makes sense. However, the sparse-checkout feature then adopted these structures and methods, but with the opposite meaning: these patterns match the files that should be included! It would be clearer to rename this entire library as a "pattern matching" library, and the callers apply exclusion/inclusion logic accordingly based on their needs. This commit renames several methods defined in dir.h to make more sense with the renamed 'struct exclude_list' to 'struct pattern_list' and 'struct exclude' to 'struct path_pattern': * last_exclude_matching() -> last_matching_pattern() * parse_exclude() -> parse_path_pattern() In addition, the word 'exclude' was replaced with 'pattern' in the methods below: * add_exclude_list() * add_excludes_from_file_to_list() * add_excludes_from_file() * add_excludes_from_blob_to_list() * add_exclude() * clear_exclude_list() A few methods with the word "exclude" remain. These will be handled seperately. In particular, the method "is_excluded()" is concretely about the .gitignore file relative to a specific directory. This is the important boundary between library and consumer: is_excluded() cares about .gitignore, but is_excluded() calls last_matching_pattern() to make that decision. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano

treewide: rename 'struct exclude_list' to 'struct pattern_list'

2019-09-05T21:05:11Z

list-objects-filter: implement composite filters

2019-06-28T15:41:53Z

Allow combining filters such that only objects accepted by all filters are shown. The motivation for this is to allow getting directory listings without also fetching blobs. This can be done by combining blob:none with tree:. There are massive repositories that have larger-than-expected trees - even if you include only a single commit. A combined filter supports any number of subfilters, and is written in the following form: combine:++ Certain non-alphanumeric characters in each filter must be URL-encoded. For now, combined filters must be specified in this form. In a subsequent commit, rev-list will support multiple --filter arguments which will have the same effect as specifying one filter argument starting with "combine:". The documentation will be updated in that commit, as the URL-encoding scheme is in general not meant to be used directly by the user, and it is better to describe the URL-encoding feature in terms of the repeated flag. Helped-by: Emily Shaffer Helped-by: Jeff Hostetler Helped-by: Johannes Schindelin Helped-by: Jonathan Tan Helped-by: Junio C Hamano Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano

list-objects-filter: put omits set in filter struct

2019-06-28T15:41:53Z

The oidset *omits pointer must be accessed by the combine filter in a type-agnostic way once the graph traversal is over. Store that pointer in the general `filter` struct. This will be used in a follow-up patch to implement the combine filter. Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano