Skip to content

Commit be0f73c

Browse files
claudeclaude
committed
Refactor dockerignore logic into IgnoreSpec type
Extract the .dockerignore parsing and pattern matching logic from BuildFSSync into a dedicated IgnoreSpec type. This improves code organization, testability, and reusability. Changes: - Create new IgnoreSpec.swift with IgnoreSpec type that takes Data in its initializer and provides shouldIgnore(relPath:isDirectory:) method - Update BuildFSSync to load .dockerignore file data and create an IgnoreSpec instance - Rename DockerignoreTests to IgnoreSpecTests - Add unit tests for IgnoreSpec type This addresses feedback from jglogan in PR apple#879.
1 parent 2289c34 commit be0f73c

File tree

3 files changed

+145
-61
lines changed

3 files changed

+145
-61
lines changed

‎Sources/ContainerBuild/BuildFSSync.swift‎

Lines changed: 12 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,17 @@ actor BuildFSSync: BuildPipelineHandler {
136136
var entries: [String: Set<DirEntry>] = [:]
137137
let followPaths: [String] = packet.followPaths() ?? []
138138

139-
// Parse .dockerignore if present
140-
let ignorePatterns = try parseDockerignore()
139+
// Load .dockerignore if present
140+
let dockerignorePath = contextDir.appendingPathComponent(".dockerignore")
141+
let ignoreSpec: IgnoreSpec? = {
142+
guard FileManager.default.fileExists(atPath: dockerignorePath.path) else {
143+
return nil
144+
}
145+
guard let data = try? Data(contentsOf: dockerignorePath) else {
146+
return nil
147+
}
148+
return IgnoreSpec(data)
149+
}()
141150

142151
let followPathsWalked = try walk(root: self.contextDir, includePatterns: followPaths)
143152
for url in followPathsWalked {
@@ -151,7 +160,7 @@ actor BuildFSSync: BuildPipelineHandler {
151160
let relPath = try url.relativeChildPath(to: contextDir)
152161

153162
// Check if the file should be ignored
154-
if try shouldIgnore(relPath, patterns: ignorePatterns, isDirectory: url.hasDirectoryPath) {
163+
if let ignoreSpec = ignoreSpec, try ignoreSpec.shouldIgnore(relPath: relPath, isDirectory: url.hasDirectoryPath) {
155164
continue
156165
}
157166

@@ -287,62 +296,6 @@ actor BuildFSSync: BuildPipelineHandler {
287296
return Array(globber.results)
288297
}
289298

290-
/// Parse .dockerignore file and return list of ignore patterns
291-
private func parseDockerignore() throws -> [String] {
292-
let dockerignorePath = contextDir.appendingPathComponent(".dockerignore")
293-
294-
guard FileManager.default.fileExists(atPath: dockerignorePath.path) else {
295-
return []
296-
}
297-
298-
let contents = try String(contentsOf: dockerignorePath, encoding: .utf8)
299-
300-
return
301-
contents
302-
.split(separator: "\n")
303-
.map { line in line.trimmingCharacters(in: .whitespaces) }
304-
.filter { line in !line.isEmpty && !line.hasPrefix("#") }
305-
.map { String($0) }
306-
}
307-
308-
/// Check if a file path should be ignored based on .dockerignore patterns
309-
private func shouldIgnore(_ path: String, patterns: [String], isDirectory: Bool) throws -> Bool {
310-
guard !patterns.isEmpty else {
311-
return false
312-
}
313-
314-
let globber = Globber(URL(fileURLWithPath: "/"))
315-
316-
for pattern in patterns {
317-
// Try to match the pattern against the path
318-
let pathToMatch = isDirectory ? path + "/" : path
319-
320-
let matchesWithSlash = try globber.glob(pathToMatch, pattern)
321-
if matchesWithSlash {
322-
return true
323-
}
324-
325-
// Also try without the trailing slash for directories
326-
if isDirectory {
327-
let matchesWithoutSlash = try globber.glob(path, pattern)
328-
if matchesWithoutSlash {
329-
return true
330-
}
331-
}
332-
333-
// Check if pattern matches with ** prefix for nested paths
334-
let shouldAddPrefix = !pattern.hasPrefix("**/") && !pattern.hasPrefix("/")
335-
if shouldAddPrefix {
336-
let matchesWithPrefix = try globber.glob(pathToMatch, "**/" + pattern)
337-
if matchesWithPrefix {
338-
return true
339-
}
340-
}
341-
}
342-
343-
return false
344-
}
345-
346299
private func processDirectory(
347300
_ currentDir: String,
348301
inputEntries: [String: Set<DirEntry>],

‎Sources/ContainerBuild/IgnoreSpec.swift‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
//===----------------------------------------------------------------------===//
2+
// Copyright © 2025 Apple Inc. and the container project authors.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// https://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
//===----------------------------------------------------------------------===//
16+
17+
import Foundation
18+
19+
/// A type that handles .dockerignore pattern matching
20+
struct IgnoreSpec {
21+
private let patterns: [String]
22+
23+
/// Initialize an IgnoreSpec from .dockerignore file data
24+
/// - Parameter data: The contents of a .dockerignore file
25+
init(_ data: Data) {
26+
guard let contents = String(data: data, encoding: .utf8) else {
27+
self.patterns = []
28+
return
29+
}
30+
31+
self.patterns = contents
32+
.split(separator: "\n")
33+
.map { line in line.trimmingCharacters(in: .whitespaces) }
34+
.filter { line in !line.isEmpty && !line.hasPrefix("#") }
35+
.map { String($0) }
36+
}
37+
38+
/// Check if a file path should be ignored based on .dockerignore patterns
39+
/// - Parameters:
40+
/// - relPath: The relative path to check
41+
/// - isDirectory: Whether the path is a directory
42+
/// - Returns: true if the path should be ignored, false otherwise
43+
func shouldIgnore(relPath: String, isDirectory: Bool) throws -> Bool {
44+
guard !patterns.isEmpty else {
45+
return false
46+
}
47+
48+
let globber = Globber(URL(fileURLWithPath: "/"))
49+
50+
for pattern in patterns {
51+
// Try to match the pattern against the path
52+
let pathToMatch = isDirectory ? relPath + "/" : relPath
53+
54+
let matchesWithSlash = try globber.glob(pathToMatch, pattern)
55+
if matchesWithSlash {
56+
return true
57+
}
58+
59+
// Also try without the trailing slash for directories
60+
if isDirectory {
61+
let matchesWithoutSlash = try globber.glob(relPath, pattern)
62+
if matchesWithoutSlash {
63+
return true
64+
}
65+
}
66+
67+
// Check if pattern matches with ** prefix for nested paths
68+
let shouldAddPrefix = !pattern.hasPrefix("**/") && !pattern.hasPrefix("/")
69+
if shouldAddPrefix {
70+
let matchesWithPrefix = try globber.glob(pathToMatch, "**/" + pattern)
71+
if matchesWithPrefix {
72+
return true
73+
}
74+
}
75+
}
76+
77+
return false
78+
}
79+
}

‎Tests/ContainerBuildTests/DockerignoreTests.swift‎ renamed to ‎Tests/ContainerBuildTests/IgnoreSpecTests.swift‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ enum TestError: Error {
2323
case missingBuildTransfer
2424
}
2525

26-
@Suite class DockerignoreTests {
26+
@Suite class IgnoreSpecTests {
2727
private var baseTempURL: URL
2828
private let fileManager = FileManager.default
2929

3030
init() throws {
3131
self.baseTempURL = URL.temporaryDirectory
32-
.appendingPathComponent("DockerignoreTests-\(UUID().uuidString)")
32+
.appendingPathComponent("IgnoreSpecTests-\(UUID().uuidString)")
3333
try fileManager.createDirectory(at: baseTempURL, withIntermediateDirectories: true, attributes: nil)
3434
}
3535

@@ -51,6 +51,58 @@ enum TestError: Error {
5151
try #require(created)
5252
}
5353

54+
// MARK: - Unit tests for IgnoreSpec
55+
56+
@Test func testIgnoreSpecParsesPatterns() throws {
57+
let content = """
58+
# This is a comment
59+
*.log
60+
61+
node_modules
62+
# Another comment
63+
temp/
64+
"""
65+
let data = content.data(using: .utf8)!
66+
let spec = IgnoreSpec(data)
67+
68+
// Test that it correctly ignores files matching patterns
69+
#expect(try spec.shouldIgnore(relPath: "debug.log", isDirectory: false))
70+
#expect(try spec.shouldIgnore(relPath: "node_modules", isDirectory: true))
71+
#expect(try spec.shouldIgnore(relPath: "temp/", isDirectory: true))
72+
#expect(!try spec.shouldIgnore(relPath: "src/app.swift", isDirectory: false))
73+
}
74+
75+
@Test func testIgnoreSpecHandlesEmptyData() throws {
76+
let data = Data()
77+
let spec = IgnoreSpec(data)
78+
79+
// Empty spec should not ignore anything
80+
#expect(!try spec.shouldIgnore(relPath: "anyfile.txt", isDirectory: false))
81+
}
82+
83+
@Test func testIgnoreSpecHandlesNestedPaths() throws {
84+
let content = "*.log"
85+
let data = content.data(using: .utf8)!
86+
let spec = IgnoreSpec(data)
87+
88+
// Should match .log files in nested directories
89+
#expect(try spec.shouldIgnore(relPath: "logs/debug.log", isDirectory: false))
90+
#expect(try spec.shouldIgnore(relPath: "app/logs/error.log", isDirectory: false))
91+
#expect(!try spec.shouldIgnore(relPath: "logs/debug.txt", isDirectory: false))
92+
}
93+
94+
@Test func testIgnoreSpecHandlesDirectories() throws {
95+
let content = "node_modules"
96+
let data = content.data(using: .utf8)!
97+
let spec = IgnoreSpec(data)
98+
99+
// Should match directories
100+
#expect(try spec.shouldIgnore(relPath: "node_modules", isDirectory: true))
101+
#expect(try spec.shouldIgnore(relPath: "src/node_modules", isDirectory: true))
102+
}
103+
104+
// MARK: - Integration tests with BuildFSSync
105+
54106
@Test func testDockerignoreExcludesMatchingFiles() async throws {
55107
// Setup: Create a build context with files and .dockerignore
56108
let contextDir = baseTempURL.appendingPathComponent("build-context")

0 commit comments

Comments
 (0)