Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,14 @@ If you know what you're doing and would like to suppress this warning, use one o
await Utils.spawn([this.argv.containerExecutable, "cp", `${fileVariablesDir}/.`, `${containerId}:${fileVariablesDir}`], argv.cwd);
this.refreshLongRunningSilentTimeout(writeStreams);
}
await Utils.spawn([this.argv.containerExecutable, "cp", `${argv.stateDir}/builds/.docker/.`, `${containerId}:${this.ciProjectDir}`], argv.cwd);
const cmd = "sed -E -e 's,^[.]?/,,' -e '/^[.]git/d' /data/src/.dockerignore >/data/ignore 2>/dev/null; rsync -avh --exclude-from /data/ignore /data/src/. /data/dest";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: copyIn applies .dockerignore filtering to cache/artifact restores, which can silently drop files. The copyIn method is used as a general-purpose copy for both initial source checkout (where .dockerignore filtering is desired) and cache/artifact restores (where it is not). When restoring caches or artifacts, if the source folder contains a .dockerignore, matching files will be excluded by rsync, breaking cache/artifact integrity. The previous container cp behavior copied all files verbatim.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/job.ts, line 734:

<comment>`copyIn` applies `.dockerignore` filtering to cache/artifact restores, which can silently drop files. The `copyIn` method is used as a general-purpose copy for both initial source checkout (where `.dockerignore` filtering is desired) and cache/artifact restores (where it is not). When restoring caches or artifacts, if the source folder contains a `.dockerignore`, matching files will be excluded by rsync, breaking cache/artifact integrity. The previous `container cp` behavior copied all files verbatim.</comment>

<file context>
@@ -731,7 +731,14 @@ If you know what you're doing and would like to suppress this warning, use one o
                 this.refreshLongRunningSilentTimeout(writeStreams);
             }
-            await Utils.spawn([this.argv.containerExecutable, "cp", `${argv.stateDir}/builds/.docker/.`, `${containerId}:${this.ciProjectDir}`], argv.cwd);
+            const cmd = "sed -E -e 's,^[.]?/,,' -e '/^[.]git/d' /data/src/.dockerignore >/data/ignore 2>/dev/null; rsync -avh --exclude-from /data/ignore /data/src/. /data/dest";
+            await Utils.spawn([
+                this.argv.containerExecutable, "run", "--rm",
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hum, another option would be to let people provide a ignore file specific to gitlab-ci-local.

My main problem is with big files in LFS. git lfs ls-files -n can output lfs-files and be used to compute a gcl-ignore file. But that would require an extra file for each such repository.

@firecow Any ideas?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Duplicate rsync/sed command logic for honoring .dockerignore increases maintenance risk

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/job.ts, line 734:

<comment>Duplicate rsync/sed command logic for honoring .dockerignore increases maintenance risk</comment>

<file context>
@@ -731,7 +731,14 @@ If you know what you're doing and would like to suppress this warning, use one o
                 this.refreshLongRunningSilentTimeout(writeStreams);
             }
-            await Utils.spawn([this.argv.containerExecutable, "cp", `${argv.stateDir}/builds/.docker/.`, `${containerId}:${this.ciProjectDir}`], argv.cwd);
+            const cmd = "sed -E -e 's,^[.]?/,,' -e '/^[.]git/d' /data/src/.dockerignore >/data/ignore 2>/dev/null; rsync -avh --exclude-from /data/ignore /data/src/. /data/dest";
+            await Utils.spawn([
+                this.argv.containerExecutable, "run", "--rm",
</file context>

await Utils.spawn([
this.argv.containerExecutable, "run", "--rm",
"-v", `${this.argv.cwd}/${this.argv.stateDir}/builds/.docker:/data/src:ro`,
"-v", `${this.buildVolumeName}:/data/dest:rw`,
this.argv.helperImage,
"bash", "-c", cmd,
]);
await Utils.spawn([this.argv.containerExecutable, "start", "--attach", containerId], argv.cwd);
await Utils.spawn([this.argv.containerExecutable, "rm", "-vf", containerId], argv.cwd);
const endTime = process.hrtime(time);
Expand Down Expand Up @@ -1336,12 +1343,19 @@ If you know what you're doing and would like to suppress this warning, use one o
writeStreams.stdout(chalk`${this.formattedJobName} {magentaBright imported artifacts} in {magenta ${prettyHrtime(endTime)}}\n`);
}

copyIn (source: string) {
async copyIn (source: string) {
const safeJobName = this.safeJobName;
if (!this.imageName(this._variables) && this.argv.shellIsolation) {
return Utils.spawn(["rsync", "-a", `${source}/.`, `${this.argv.cwd}/${this.argv.stateDir}/builds/${safeJobName}`]);
}
return Utils.spawn([this.argv.containerExecutable, "cp", `${source}/.`, `${this._containerId}:${this.ciProjectDir}`]);
const cmd = "sed -E -e 's,^[.]?/,,' -e '/^[.]git/d' /data/src/.dockerignore >/data/ignore 2>/dev/null; rsync -avh --exclude-from /data/ignore /data/src/. /data/dest";
return Utils.spawn([
this.argv.containerExecutable, "run", "--rm",
"-v", `${source}:/data/src:ro`,
"-v", `${this.buildVolumeName}:/data/dest:rw`,
this.argv.helperImage,
"bash", "-c", cmd,
]);
}

private async copyCacheOut (writeStreams: WriteStreams, expanded: {[key: string]: string}) {
Expand Down