Skip to content
Open
Show file tree
Hide file tree
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
23 changes: 14 additions & 9 deletions src/parser-includes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import path from "node:path";
import prettyHrtime from "pretty-hrtime";
import semver from "semver";
import {RE2JS} from "re2js";
import {globbySync} from "globby";

type ParserIncludesInitOptions = {
argv: Argv;
Expand Down Expand Up @@ -147,13 +148,16 @@ export class ParserIncludes {
} else if (value["project"]) {
for (const fileValue of Array.isArray(value["file"]) ? value["file"] : [value["file"]]) {
const mergedInputs = {...(value.inputs ?? {}), ...globalInputs};
const fileDoc = await Parser.loadYaml(
`${cwd}/${stateDir}/includes/${gitData.remote.host}/${value["project"]}/${value["ref"] || "HEAD"}/${fileValue}`
, {inputs: mergedInputs}
, expandVariables, writeStreams);
// Expand local includes inside a "project"-like include
fileDoc["include"] = this.expandInnerLocalIncludes(fileDoc["include"], value["project"], value["ref"], opts);
includeDatas = includeDatas.concat(await this.init(fileDoc, opts));
const includeDir = `${cwd}/${stateDir}/includes/${gitData.remote.host}/${value["project"]}/${value["ref"] || "HEAD"}`;
const normalizedFile = fileValue.replace(/^\/+/, "");
const matches = globbySync(normalizedFile, {cwd: includeDir, absolute: true}).sort((a, b) => a.localeCompare(b));

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.

P3: Switching the match sort to String#localeCompare makes the order of included project files depend on the host runtime's locale (via the Intl collator), whereas the previous default .sort() was locale-independent and deterministic. Because these matches are concatenated into includeDatas and pipeline config can be merged/overridden in that order, a developer machine with a different locale could resolve a slightly different pipeline than CI for the same inputs. If the goal was stable ordering, keep the locale-invariant default .sort() (or pass an explicit {numeric: true, sensitivity: 'base'} if case-insensitivity is intended); otherwise project includes should order consistently regardless of locale.

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

<comment>Switching the match sort to `String#localeCompare` makes the order of included project files depend on the host runtime's locale (via the Intl collator), whereas the previous default `.sort()` was locale-independent and deterministic. Because these matches are concatenated into `includeDatas` and pipeline config can be merged/overridden in that order, a developer machine with a different locale could resolve a slightly different pipeline than CI for the same inputs. If the goal was stable ordering, keep the locale-invariant default `.sort()` (or pass an explicit `{numeric: true, sensitivity: 'base'}` if case-insensitivity is intended); otherwise project includes should order consistently regardless of locale.</comment>

<file context>
@@ -150,7 +150,7 @@ export class ParserIncludes {
                     const includeDir = `${cwd}/${stateDir}/includes/${gitData.remote.host}/${value["project"]}/${value["ref"] || "HEAD"}`;
                     const normalizedFile = fileValue.replace(/^\/+/, "");
-                    const matches = globbySync(normalizedFile, {cwd: includeDir, absolute: true}).sort();
+                    const matches = globbySync(normalizedFile, {cwd: includeDir, absolute: true}).sort((a, b) => a.localeCompare(b));
                     const filePaths = matches.length > 0 ? matches : [`${includeDir}/${normalizedFile}`];
                     for (const filePath of filePaths) {
</file context>
Suggested change
const matches = globbySync(normalizedFile, {cwd: includeDir, absolute: true}).sort((a, b) => a.localeCompare(b));
const matches = globbySync(normalizedFile, {cwd: includeDir, absolute: true}).sort();

const filePaths = matches.length > 0 ? matches : [`${includeDir}/${normalizedFile}`];
for (const filePath of filePaths) {
const fileDoc = await Parser.loadYaml(filePath, {inputs: mergedInputs}, expandVariables, writeStreams);
// Expand local includes inside a "project"-like include
fileDoc["include"] = this.expandInnerLocalIncludes(fileDoc["include"], value["project"], value["ref"], opts);
includeDatas = includeDatas.concat(await this.init(fileDoc, opts));
}
}
} else if (value["component"]) {
const component = componentParseCache.get(index);
Expand Down Expand Up @@ -374,7 +378,8 @@ export class ParserIncludes {

if (remote.schema.startsWith("http")) {
const ext = "tmp-" + Math.random();
await fs.mkdirp(path.dirname(`${cwd}/${target}/${normalizedFile}`));
const destDir = path.dirname(`${cwd}/${target}/${normalizedFile}`);
await fs.mkdirp(destDir);
tmpDir = `${cwd}/${target}.${ext}`;

const isCommitSha = /^[0-9a-f]{40}$/i.test(ref);
Expand All @@ -387,7 +392,7 @@ export class ParserIncludes {
`git sparse-checkout set --no-cone ${normalizedFile}`,
isCommitSha ? "git checkout FETCH_HEAD" : "git checkout",
`cd ${cwd}/${stateDir}`,
`cp ${tmpDir}/${normalizedFile} ${cwd}/${target}/${normalizedFile}`,
`cp ${tmpDir}/${normalizedFile} ${destDir}/`,
], cwd);
} else {
await fs.mkdirp(`${cwd}/${target}`);
Expand Down
7 changes: 7 additions & 0 deletions tests/test-cases/include-project-file-wildcard/.gitlab-ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
# templates/[bt]*.yml matches both templates/build.yml and templates/test.yml,
# so the glob expands to more than one file.
include:
- project: components/go
ref: main
file: templates/[bt]*.yml
26 changes: 26 additions & 0 deletions tests/test-cases/include-project-file-wildcard/integration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import {WriteStreamsMock} from "../../../src/write-streams.js";
import {handler} from "../../../src/handler.js";
import {initSpawnSpy} from "../../mocks/utils.mock.js";
import {WhenStatics} from "../../mocks/when-statics.js";

beforeAll(() => {
initSpawnSpy([...WhenStatics.all, WhenStatics.mockGitRemoteHttp]);
});

test.concurrent("include:project with a wildcard file path includes every match", async () => {
const writeStreams = new WriteStreamsMock();

await handler({
cwd: "tests/test-cases/include-project-file-wildcard",
noColor: true,
list: true,
stateDir: ".gitlab-ci-local-include-project-file-wildcard",
}, writeStreams);

const jobNames = writeStreams.stdoutLines

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: This test (and its .gitlab-ci.yml fixture) depend on a live external GitLab project components/go and its templates/build.yml/templates/test.yml content via a real network fetch, so it will fail when offline or whenever that external project changes. Consider hosting the included project fixtures locally (e.g. a git project-include that resolves to checkout-able content) so the test is hermetic and asserts the wildcard behavior against files the repo controls.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test-cases/include-project-file-wildcard/integration.test.ts, line 20:

<comment>This test (and its .gitlab-ci.yml fixture) depend on a live external GitLab project `components/go` and its `templates/build.yml`/`templates/test.yml` content via a real network fetch, so it will fail when offline or whenever that external project changes. Consider hosting the included project fixtures locally (e.g. a git project-include that resolves to checkout-able content) so the test is hermetic and asserts the wildcard behavior against files the repo controls.</comment>

<file context>
@@ -0,0 +1,26 @@
+        stateDir: ".gitlab-ci-local-include-project-file-wildcard",
+    }, writeStreams);
+
+    const jobNames = writeStreams.stdoutLines
+        .filter(l => /^(build|test)\s/.test(l))
+        .map(l => l.split(/\s+/)[0])
</file context>

.filter(l => /^(build|test)\s/.test(l))
.map(l => l.split(/\s+/)[0])
.sort();

expect(jobNames).toEqual(["build", "test"]);
});