Skip to content

Conversation

@invidian
Copy link
Contributor

What this PR does / why we need it:

There are at least 2 bugs in the current implementation of IgnoreCache function:

  1. bufio.NewReader.ReadString as stated in the docs:

    If ReadString encounters an error before finding a delimiter, it returns the data read before the error and the error itself (often io.EOF).

    This means when there is only a single filter specified, it will be returned together with io.EOF, meaning no cache ignoring will actually happen with existing code.

  2. bufio.NewReader.ReadString as stated in the docs:

    ReadString reads until the first occurrence of delim in the input, returning a string containing the data up to and including the delimiter.

    As we register keys without a delimiter but compare them with one, ignoring cache will actually never work.

This patch fixes both issues by using strings.SplitSeq to split the filters.

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Closes #949

@invidian invidian marked this pull request as ready for review January 30, 2026 08:14
Copilot AI review requested due to automatic review settings January 30, 2026 08:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes two critical bugs in the IgnoreCache function that prevented no-cache filters from working correctly:

  1. EOF handling bug: When a single filter was specified, bufio.ReadString would return the data along with io.EOF, causing the function to exit early without processing the filter.
  2. Delimiter inclusion bug: bufio.ReadString includes the delimiter (comma) in the returned string, but the comparison was done against keys without delimiters, causing matches to always fail.

The fix replaces the bufio.ReadString loop with strings.SplitSeq, which properly splits on commas without including delimiters and doesn't return EOF errors.

Changes:

  • Replaced buggy bufio.ReadString parsing logic with strings.SplitSeq in the IgnoreCache function
  • Removed unused bufio import from frontend/request.go
  • Added integration test respects_container_cache_key to verify the fix works correctly

Reviewed changes

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

File Description
frontend/request.go Fixed IgnoreCache function by replacing bufio.ReadString with strings.SplitSeq for parsing comma-separated cache filter values
test/linux_target_test.go Added integration test to verify no-cache filters work correctly for container builds, including import of targets package

@invidian invidian force-pushed the invidian/fix-no-cache-filter branch from b6e94cb to e586163 Compare January 30, 2026 08:59
There are at least 2 bugs in the current implementation of IgnoreCache
function:

1. bufio.NewReader.ReadString as stated in the docs:

   If ReadString encounters an error before finding a delimiter, it
   returns the data read before the error and the error itself (often io.EOF).

   This means when there is only a single filter specified, it will be
   returned together with io.EOF, meaning no cache ignoring will actually
   happen with existing code.

2. bufio.NewReader.ReadString as stated in the docs:

   ReadString reads until the first occurrence of delim in the input,
   returning a string containing the data up to and including the delimiter.

   As we register keys without a delimiter but compare them with one, ignoring
   cache will actually never work.

This patch fixes both issues by using strings.SplitSeq to split the filters.

Closes project-dalec#949

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian invidian force-pushed the invidian/fix-no-cache-filter branch from e586163 to 6c4f7cd Compare January 30, 2026 09:02
@cpuguy83 cpuguy83 merged commit 64821d9 into project-dalec:main Jan 31, 2026
40 of 43 checks passed
@invidian invidian deleted the invidian/fix-no-cache-filter branch January 31, 2026 07:04
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.

[BUG] No cache filter is broken

2 participants