From: Kai Lüke Date: Fri, 3 Jul 2026 13:39:22 +0000 (+0900) Subject: sysupdate: Evaluate all patterns before descending into a subdir X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c8a2e035ce0e8b22bedc6727559101808ce456b3;p=thirdparty%2Fsystemd.git sysupdate: Evaluate all patterns before descending into a subdir When we have two match patterns, e.g., because the repository layout changed and we look for old and new style files, we can have the case that one pattern suggests a descend into a subdir but the other pattern would be a direct match. This was missed and only the descent was done. Yet the other way round also has to be covered: We can't just use the direct match because it might be that it's rejected if the type doesn't fit (directory vs. regular file). In this case we should still descent. Remember that we want to descend but continue checking the other pattern first to maybe get a direct match. Introduce a new combined return code YES_AND_RETRY to give the full information to the caller which now can decide to fallback to a descent. While at it, use the stat info for the directory check. Also add tests to ensure that the order of patterns doesn't matter and we handle the above corner cases. --- diff --git a/src/sysupdate/sysupdate-cleanup.c b/src/sysupdate/sysupdate-cleanup.c index 2d0229423d5..f45ac37e2c9 100644 --- a/src/sysupdate/sysupdate-cleanup.c +++ b/src/sysupdate/sysupdate-cleanup.c @@ -184,7 +184,7 @@ static int context_is_path_currently_owned( return log_error_errno(r, "Failed to match patterns '%s' against '%s': %m", cl, relpath); } - if (IN_SET(r, PATTERN_MATCH_YES, PATTERN_MATCH_RETRY)) /* Yay, this path is pinned by this transfer file */ + if (IN_SET(r, PATTERN_MATCH_YES, PATTERN_MATCH_RETRY, PATTERN_MATCH_YES_AND_RETRY)) /* Yay, this path is pinned by this transfer file */ return true; assert(r == PATTERN_MATCH_NO); diff --git a/src/sysupdate/sysupdate-pattern.c b/src/sysupdate/sysupdate-pattern.c index ddc9adb881e..459c8fda29d 100644 --- a/src/sysupdate/sysupdate-pattern.c +++ b/src/sysupdate/sysupdate-pattern.c @@ -454,12 +454,20 @@ retry: } int pattern_match_many(char **patterns, const char *s, InstanceMetadata *ret) { - _cleanup_(instance_metadata_destroy) InstanceMetadata found = INSTANCE_METADATA_NULL; + _cleanup_(instance_metadata_destroy) InstanceMetadata matched = INSTANCE_METADATA_NULL; + bool have_match = false, retry_descent = false; int r; + /* Evaluate all patterns and report the combined result: a direct match wins and provides the + * extracted fields, but when another pattern would descend into a subdirectory this is reported + * too, so the caller can descend as fallback when the direct match is no valid candidate. */ STRV_FOREACH(p, patterns) { + _cleanup_(instance_metadata_destroy) InstanceMetadata found = INSTANCE_METADATA_NULL; const char *pat = *p, *input_path; + if (have_match && retry_descent) + break; /* nothing more to learn */ + /* A glob directory prefix on a pattern means to match the rest of the pattern against the * last path component only (the basename, "find this file anywhere under the source tree") */ if (pattern_skip_glob_directory_prefix(&pat)) @@ -470,20 +478,20 @@ int pattern_match_many(char **patterns, const char *s, InstanceMetadata *ret) { r = pattern_match(pat, input_path, &found); if (r < 0) return r; - if (r != PATTERN_MATCH_NO) { - if (ret) { - *ret = found; - found = (InstanceMetadata) INSTANCE_METADATA_NULL; - } - - return r; + if (r == PATTERN_MATCH_YES && !have_match) { + matched = TAKE_GENERIC(found, InstanceMetadata, INSTANCE_METADATA_NULL); + have_match = true; } + if (r == PATTERN_MATCH_RETRY) + retry_descent = true; } if (ret) - *ret = (InstanceMetadata) INSTANCE_METADATA_NULL; + *ret = TAKE_GENERIC(matched, InstanceMetadata, INSTANCE_METADATA_NULL); - return PATTERN_MATCH_NO; + if (have_match) + return retry_descent ? PATTERN_MATCH_YES_AND_RETRY : PATTERN_MATCH_YES; + return retry_descent ? PATTERN_MATCH_RETRY : PATTERN_MATCH_NO; } bool pattern_skip_glob_directory_prefix(const char **pattern) { diff --git a/src/sysupdate/sysupdate-pattern.h b/src/sysupdate/sysupdate-pattern.h index 8bc9f09e861..6bc9b243c13 100644 --- a/src/sysupdate/sysupdate-pattern.h +++ b/src/sysupdate/sysupdate-pattern.h @@ -7,6 +7,7 @@ enum { PATTERN_MATCH_NO, PATTERN_MATCH_YES, PATTERN_MATCH_RETRY, + PATTERN_MATCH_YES_AND_RETRY, }; int pattern_match(const char *pattern, const char *s, InstanceMetadata *ret); diff --git a/src/sysupdate/sysupdate-resource.c b/src/sysupdate/sysupdate-resource.c index ab98f649eb5..08a3f9ef7ca 100644 --- a/src/sysupdate/sysupdate-resource.c +++ b/src/sysupdate/sysupdate-resource.c @@ -185,7 +185,10 @@ static int resource_load_from_directory_recursive( if (!S_ISDIR(st.st_mode)) /* The pattern continues with '/' but this is no directory */ continue; descend = true; - } + } else if (r == PATTERN_MATCH_YES_AND_RETRY && S_ISDIR(st.st_mode) && m != S_IFDIR) + /* The direct match is unusable since a directory can't be an instance here, + * but another pattern wants to descend, so do that instead */ + descend = true; } if (descend) { @@ -205,7 +208,7 @@ static int resource_load_from_directory_recursive( continue; } - if (de->d_type == DT_DIR && m != S_IFDIR) + if (S_ISDIR(st.st_mode) && m != S_IFDIR) continue; joined = path_join(rr->path, rel_joined); @@ -610,7 +613,7 @@ static int resource_load_from_web( r = pattern_match_many(rr->patterns, fn, &extracted_fields); if (r < 0) return log_error_errno(r, "Failed to match pattern: %m"); - if (r == PATTERN_MATCH_YES) { + if (IN_SET(r, PATTERN_MATCH_YES, PATTERN_MATCH_YES_AND_RETRY)) { _cleanup_free_ char *path = NULL; r = import_url_append_component(rr->path, fn, &path); diff --git a/test/units/TEST-72-SYSUPDATE.sh b/test/units/TEST-72-SYSUPDATE.sh index 5c812420d02..d3a8a4bc61f 100755 --- a/test/units/TEST-72-SYSUPDATE.sh +++ b/test/units/TEST-72-SYSUPDATE.sh @@ -1326,4 +1326,104 @@ EOF cmp "$WORKDIR/source/depth-v3/contents/image.raw" "$WORKDIR/blobs/depth-v3.raw" rm -rf "$CONFIGDIR/01-depth.transfer" "$WORKDIR/source/depth-v3" +# Pattern matching should not depend on the pattern order. A transfer can list one pattern per repository +# layout, e.g., when a source switches from one directory per version (bundle-v1/image.raw) to flat files +# (bundle-v2.raw) and both layouts coexist during a migration. +# Check that after a subdirectory pattern which descends we still can match a top-level file through a +# '**/' pattern. +# Also check that after a non-matching '**/' pattern we still can match a plain top-level file. +# Then check that after a subdirectory pattern we can still match a plain top-level file. +# The first and last work because the wildcard field also captures the '.raw' suffix, so the top-level file +# matches the directory component of the subdirectory pattern and triggers the descend-retry logic. +# The fourth transfer checks the vice versa migration where one directory per version is the new layout and +# the old versions are flat files without a suffix: the directory name fully matches the flat pattern but +# must still be descended into for the subdirectory pattern. +rm -rf "$CONFIGDIR" "$WORKDIR/blobs" "$WORKDIR/source/bundle-v1" "$WORKDIR/source/pack-v2" +mkdir -p "$CONFIGDIR" "$WORKDIR/blobs"/{glob-last,glob-first,subdir-first,yes-and-retry} \ + "$WORKDIR/source/bundle-v1" "$WORKDIR/source/pack-v2" +echo 'version-one' >"$WORKDIR/source/bundle-v1/image.raw" +echo 'version-two' >"$WORKDIR/source/bundle-v2.raw" +echo 'version-v1' >"$WORKDIR/source/pack-v1" +echo 'version-v2' >"$WORKDIR/source/pack-v2/image.raw" +cat >"$CONFIGDIR/01-glob-last.transfer" <"$CONFIGDIR/02-glob-first.transfer" <"$CONFIGDIR/03-subdir-first.transfer" <"$CONFIGDIR/04-yes-and-retry.transfer" <"$WORKDIR/source/pack-v1/image.raw" +echo 'version-v2' >"$WORKDIR/source/pack-v2" +(cd "$WORKDIR/source" && sha256sum pack-v1/image.raw pack-v2 >SHA256SUMS) +cat >"$CONFIGDIR/01-manifest-yes-and-retry.transfer" <