Support paths relative to the root directory

[pepicrft]

Short description 📝

This PR adds support for defining paths relative to the root directory. The paths can be initialized with .relativeToRoot("file.swift") or just prefixing the string with two slashes: //file.swift.

Implementation 👩‍💻👨‍💻

• Add the new case to Path.
• Extend GeneratorPaths.
• Add unit & acceptance tests.

[github-actions]

@pepibumur your pull request is missing a changelog!

[pepicrft]

I think you'll love this one @andreacipriani

[kwridan]

Nice work 👍

[kwridan]

In this instance, performing a root look up per resolve call seems unnecessary - the root path is unlikely to change during the generation process especially for an individual project which is the scope of the GeneratorPaths struct usage. As such we can pass in the rootPath in the initialiser and do the lookup once at the model loader.

[pepicrft]

the root path is unlikely to change during the generation

The path depends on the project and different projects might have different root directories because we support multiple Tuist directories. For that reason I added the look up using the RootDirectoryLocator, which optimizes the lookups using the cache.

[kwridan]

I did a terrible job explaining this 😛 - it's ok we can improve separately if needed

GeneratorModelLoader.swift#L44 creates the GeneratorPaths struct per project load as such it could be tweaked:

 func loadProject(at path: AbsolutePath) throws -> TuistCore.Project {
      let manifest = try manifestLoader.loadProject(at: path)
      let rootPath = RootDirectoryLocator.shared.locate(from: path)
      let generatorPaths = GeneratorPaths(manifestDirectory: path, rootPath: rootPath)
     // ...

That way inside we don't need to do lookups per resolve (true it's cached under the hood, however not needed).

Was a minor comment - no worries

[kwridan]

[minor/personal opinion here - please take my comments with a pinch of salt 🧂]

I appreciate we need to maintain a single instance to leverage and benefit from the caching the RootDirectoryLocator performs internally.

Sadly having shared instances makes it really easy to access those components which may have side effects from anywhere in the codebase. E.g. like in the generator paths struct earlier ... I am perhaps more accustomed to assembly style DI practices like the one suggested by @marciniwanicki in #351 to solve these kind of problems - but I digress.

Seeing we have adopted shared instances elsewhere to solve this, having this defined as one keeps things consistent - we should use them judiciously and with care.

[pepicrft]

[minor/personal opinion here - please take my comments with a pinch of salt 🧂]

Thanks for keep sharing those because it's always better to have always different points of view.

I think dependency injection is another valid approach, but I had to make a trade-off here (like most of the times in programming). The risk of having shared instances that hold state is that they might have side effects that might get out of control. I think it's not the case of RootDirectoryLocator where the state is private and changes in the state are not relevant to the rest of the code base.

Dependency injection comes with its downsides too. Swift doesn't make things easier and that results in some boilerplate code that is hard to reason about. In my experience, I always find very hard o reason about injected dependencies (unless they were injected through the constructor). When we build abstractions for easing dependency injection, or taking it to a whole new level, reasoning about the code becomes really hard: what am I getting here? who is injecting the dependency?, from where is the dependency injected?

I'm happy to reconsider the approach if we see the current one resulting in side effects that make the code execution go through unexpected paths. Given the amount of global dependencies that we have at the moment, and the fact that they are not seeing big issues with it, I'd not focus on this at the moment.

[andreacipriani]

@pepibumur I'll try it as soon as it's merged