Skip to content

add extra_paths input to support caching additional paths#40

Closed
HiGarfield wants to merge 0 commit into
stupidloud:mainfrom
HiGarfield:main
Closed

add extra_paths input to support caching additional paths#40
HiGarfield wants to merge 0 commit into
stupidloud:mainfrom
HiGarfield:main

Conversation

@HiGarfield

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings April 13, 2026 16:21

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new extra_paths input, allowing users to specify additional directories for caching. The changes include updates to the documentation, action configuration, and path processing logic. A review comment correctly identifies a potential issue where changes to extra_paths would not trigger a cache key update, potentially leading to cache collisions or stale data; a suggestion was provided to incorporate a hash of these paths into the cache key to ensure uniqueness.

Comment thread utils.js Outdated
Comment on lines +30 to +33
const otherPaths = core.getMultilineInput("extra_paths").filter(Boolean);
if (otherPaths.length > 0) {
paths.push(...otherPaths);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The extra_paths input is added to the list of paths to be cached, but it is not included in the keyString calculation. This means that if a user changes the extra_paths in their workflow, the cache key will remain the same. If a cache for the previous paths already exists, the action will report a cache hit and skip saving the new paths, resulting in the additional directories never being cached. To ensure cache integrity, the keyString should incorporate a representation of the extra_paths.

    const otherPaths = core.getMultilineInput("extra_paths").filter(Boolean);
    if (otherPaths.length > 0) {
        paths.push(...otherPaths);
        // Ensure cache uniqueness when extra_paths change
        const pathsHash = Math.abs(otherPaths.join("\n").split("").reduce((a, b) => ((a << 5) - a) + b.charCodeAt(0), 0)).toString(36);
        keyString += `-${pathsHash}`;
    }

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new extra_paths input to the GitHub Action so users can include additional directories/patterns in the cache set (beyond toolchain/ccache defaults), and documents the new input.

Changes:

  • Add extra_paths input in action.yml.
  • Read extra_paths in buildBaseConfig() and append to the cache paths.
  • Document extra_paths usage and example in README.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
utils.js Reads extra_paths and includes them in the cache path list.
action.yml Declares the new extra_paths action input.
README.md Documents extra_paths and provides an example configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread utils.js Outdated
Comment on lines +30 to +33
const otherPaths = core.getMultilineInput("extra_paths").filter(Boolean);
if (otherPaths.length > 0) {
paths.push(...otherPaths);
}

Copilot AI Apr 13, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra_paths values are appended without validation. Because @actions/cache will tar/extract entries relative to GITHUB_WORKSPACE, allowing absolute paths or paths containing .. can cache/restore files outside the workspace (and potentially write outside the repo on restore). Consider rejecting absolute paths and any path that resolves outside GITHUB_WORKSPACE (or outside the prefix directory), and fail fast with a clear error message when such a value is provided.

Copilot uses AI. Check for mistakes.
Comment thread README.md Outdated
### `extra_paths`

Additional cache directories to include (one per line, can specify multiple). Default `''`.
Relative paths are resolved against the directory set by `prefix` (i.e. the openwrt build root); absolute paths are also supported.

Copilot AI Apr 13, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs state that absolute paths are supported, but caching absolute/out-of-workspace paths via @actions/cache can result in tar entries with .. segments and restore writing outside GITHUB_WORKSPACE. If absolute paths aren’t intended/safe, please document that extra_paths must stay within the workspace (or within prefix) and that path traversal (..) is rejected.

Suggested change
Relative paths are resolved against the directory set by `prefix` (i.e. the openwrt build root); absolute paths are also supported.
Paths must be relative to the directory set by `prefix` (i.e. the OpenWrt build root) and must stay within that directory.
Absolute paths are not supported. Path traversal such as `..` is not allowed.

Copilot uses AI. Check for mistakes.
@stupidloud

Copy link
Copy Markdown
Owner

ai决战吗,我去

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants