-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add support for using the CLJ_CONFIG env var. #34
base: master
Are you sure you want to change the base?
Conversation
This is very helpful for e.g. certain monorepo workflows.
I'm not sure about merging this. I know some people are (ab)using CLJ_CONFIG for monorepo setups, but I haven't actually found it necessary at all with Edge. If someone actually were using CLJ_CONFIG (eg for development purposes) then this would cause dev dependencies created on their machine to differ from other people's. I expect @AlexMiller will create an alternative solution to solve the monorepo problems more generally in the future. |
I think there are a few things here:
As part of this discussion I feel like the first point is the most objective one - inconsistency is never a good thing. I personally then feel the other 2 points relate to the subjective view of allowing users to shoot themselves in the foot and make their own risk/return decisions. Even if we had some other magical feature for better monorepo support I'd argue it makes sense for consistent use of CLJ_CONFIG to avoid surprises. I'm sure there will be better support for monorepos, but I don't think that makes this PR void. |
I believe that packs behaviour is consistent with -Srepro, which is the intention. |
It is - but without the user having passed -Srepro asking for it. That choice was taken away from from the user. My patch should be updated to suppress CLJ_CONFIG when -Srepro is indeed passed. Would that be a good trade-off? Or if you really hate using the same semantics leading to it being on by default, making it off by default but adding a pack flag to enable it? |
The thought process is that release artefacts are a sensitive output. Reproducibility is an important property. So factoring in the user deps.edn file is a no no. I don't have a good idea yet of what I think should happen here. Maybe supporting -Sdeps would resolve this? Alternatively, maybe this is an opportunity to expose more of an API interface from pack? Then users can layer on as many deps.edn files as they like. |
I agree reproducibility is an important feature. I enforce mine through a pack script (e.g. I use no pack alias or anything like that at all, it's all in the pack script), which will indeed make my builds reproducible. In my setup the CLJ_CONFIG is not a user file, it's a monorepo file under control of the monorepo. -Sdeps is in principle an escape hatch, but it wont be smooth as it'd require e.g. reading in a deps.edn file and pass via -Sdeps (with potential quoting issues etc). Some similar in spirit flag to indeed add in "any" deps.edn file would solve my situation. In general, for clj itself as well, if I had a flag to merge in another arbitrary deps.edn file the monorepo case would "work". I use CLJ_CONFIG as my "extra" deps.edn, and I'd gladly use a different way to get the extra deps.edn into the mix without "borrowing" CLJ_CONFIG to be a monorepo setting. |
Fwiw, quoting isn't too difficult here because bash takes care of that. Generally, |
To make matters worse I need both linux and windows support :/ I still haven't worked out the cmd/powershell stuff properly. But that's encouraging. A flag to add a deps.edn file feels like it could be a decent way forward. I'll mention it to Alex and see if it's something considered or worthy of consideration for clj itself. |
What was the response from Alex regarding a flag for providing a file to clj? |
https://clojure.atlassian.net/projects/TDEPS/issues/TDEPS-115?filter=allopenissues&orderby=priority%20DESC&keyword=file someone else might have reported this. |
This is very helpful for e.g. certain monorepo workflows.