Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inheriting :test-paths and other paths #68

Closed
robhanlon22 opened this issue Aug 30, 2020 · 7 comments · Fixed by #75
Closed

Inheriting :test-paths and other paths #68

robhanlon22 opened this issue Aug 30, 2020 · 7 comments · Fixed by #75

Comments

@robhanlon22
Copy link
Contributor

robhanlon22 commented Aug 30, 2020

It appears that Leiningen absolutizes all paths at project load time. This breaks monolith/inheriting of paths, as the inherited paths will always be absolute and in the root monolith project. This manifests as build errors when monolith attempts to re-absolutize an already absolute path. Would it be sound to strip the root from the paths before adding them to the inherited profile so that paths can be specified in the root monolith project?

I can provide a repro project when I'm not on my phone.

@greglook
Copy link
Collaborator

It sounds like the desire here is wanting to share a set of relative paths among all projects; I'm definitely curious what the concrete use-case is. Making this change seems like it would break a different capability, which is sharing a single directory (with an absolute location) among all subprojects. I don't know if anyone is relying on that behavior right now, but this would be a breaking change.

@robhanlon22
Copy link
Contributor Author

Yeah, it's just so subprojects can inherit the relative directories, so that they don't all have to specify them—for instance, if you know that the project is going to have multiple test suites under different directories, or if you wanted to apply a different target-path to all subprojects.

@greglook
Copy link
Collaborator

greglook commented Sep 1, 2020

🤔 Interesting - would it work to use Leiningen's update-in? For example, something like:

lein monolith each update-in :source-paths conj "cljs-test-runner-out/gen" do ...

(based on a snippet from an alias we use to run Clojurescript tests)

Admittedly, that only works on the specific command, rather than by default, but maybe that's enough for your needs. If we were to add the path relativization, I think it would need to be opt-in somehow.

@robhanlon22
Copy link
Contributor Author

robhanlon22 commented Sep 1, 2020

This would be a great situation for metadata…except you can't attach meta to Strings. :( Perhaps the [:monolith :inherit] map could be expanded to take maps of literal inherited values to be merged into the inherited profile:

:monolith {:inherit [{:test-paths ["test/unit" "test/integration"]}]}

This would be forward and backward compatible. Alternatively, there could be another key like :inherit-raw:

:monolith {:inherit-raw [:test-paths]}

Which could then read the project in its raw state and pass the values in unaltered.

@robhanlon22
Copy link
Contributor Author

I put up a PR that implements the first pattern—what do you think?

@greglook
Copy link
Collaborator

Hey, sorry for the radio silence. Thanks for the PR - I think the alternate version you suggested above would be less confusing overall. We already have precedent for different behavior with different keys, using :inherit and :inherit-leaky. Adding :inherit-raw would fit that better than mixing maps into the :inherit setting.

One thought I have is how we would allow some relative and some absolute paths, but since there's no support for relative paths yet at all I think that can wait for a future change. 🤔

@robhanlon22
Copy link
Contributor Author

robhanlon22 commented Sep 26, 2020

Opened a proof-of-concept here: #75

EDIT: it is done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants