chore: detect build systems

[jobala]

🎉 Thanks for submitting a pull request! 🎉

Summary

Fixes #928

---

For us to review and ship your PR efficiently, please perform the following steps:

• Open a bug/issue before writing your code 🧑‍💻. This ensures
  we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
  something that`s on fire 🔥 (e.g. incident related), you can skip this step.
• Read the contribution guidelines 📖. This ensures your code follows our style guide and
  passes our tests.
• Update or add tests (if any source code was changed or added) 🧪
• Update or add documentation (if features were changed or added) 📝
• Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

[github-actions]

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files. Consider converting them to TypeScript.
Thank you for converting JavaScript files to TypeScript 🎉"

[lukasholzer]

Maybe you can add this one for the detection as well? https://moonrepo.dev/docs/setup-workspace

[lukasholzer]

Can you maybe add some test cases that would reflect a monorepo setup like:

const cwd = mockFileSystem({
  'frontend/WORKSPACE': '',
  'frontend/BUILD.bazel': '',
  'frontend/apps/my-app/BUILD.bazel': '',
})

and the baseDirectory is set to frontend/apps/my-app

like in monorepos (where build systems are used) it's mostly a baseDirectory and you don't search in the repo root. This should test the look up functionality.

Currently you only test the happy path from the workspace root. But often we don't know the workspace root as the repo root does not need to be the workspace root.

The repo root can contain no reference to JS projects at all and is somewhere in the baseDirectory.

[lukasholzer]

I think the values that are passed here are absolute paths 🤔

[jobala]

What we pass as arguments here makes no difference sincegetContext resolves the absolute paths.

build/packages/build-info/src/context.ts

Lines 36 to 44 in 34b80c6

	const { projectDir = cwd(), rootDir = '' } = config
	// Get the absolute dirs for both project and root
	// We resolve the projectDir from the rootDir
	const absoluteProjectDir = resolve(rootDir, projectDir)
	// If a relative absolute path is given we rely on cwd
	const absoluteRootDir = rootDir ? resolve(cwd(), rootDir) : undefined
	// We only pass through the root dir if it was provided and is actually different
	// from the project dir
	const validRootDir = absoluteRootDir && absoluteRootDir !== absoluteProjectDir ? absoluteRootDir : undefined

[lukasholzer]

But inside get-build-info.ts we are passing different values (so the tests are run with wrong data that is not used in production) Therefore we should change it to reflect the reality.

await detectBuildSystems(join(cwd, 'packages/website'), cwd)

[lukasholzer]

But inside get-build-info.ts we are passing different values (so the tests are run with wrong data that is not used in production) Therefore we should change it to reflect the reality.

await detectBuildSystems(join(cwd, 'packages/website'), cwd)

[jobala]

so the tests are run with wrong data that is not used in production

I don't understand what you mean by wrong data here. getContext will resolve the relative baseDir to an absolute path. When you say wrong data does that mean reading a non-existent or wrong file?

[lukasholzer]

so the tests are run with wrong data that is not used in production

I don't understand what you mean by wrong data here. getContext will resolve the relative baseDir to an absolute path. When you say wrong data does that mean reading a non-existent or wrong file?

In your test the function getting called with the following parameters:

const buildSystems = await detectBuildSystems('packages/website', cwd)

1. Function parameter: 'package/website'
2. Function parameter: '/some/absolute/path/to/theproject/dir'

BUT in production it get's called with different data because there it get's called with absolute paths:

1. Function parameter: '/some/absolute/path/to/theproject/dir/package/website'
2. Function parameter: '/some/absolute/path/to/theproject/dir'

So your testcase does not reflect production data that's what I'm saying here

[lukasholzer]

Looks good to me!