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

move env var handling into profiles.clj #30

Conversation

Projects
None yet
3 participants
@martinklepsch
Copy link

commented Oct 16, 2014

I've just noticed a problem trying to set some custom environment variables in a profiles.clj in the project dir. As soon as you specify an :env key in there the :is-dev env var is dropped. I.e. the contents of the :env key in my profiles.clj replace the contents of the :env key in the project.clj.

There are two things that could be done:

  1. Put the :is-dev key into profiles.clj, commit once, add to .gitignore
  2. Setup two profiles :project/dev and :profiles/dev that are then aliased to :dev (see more below)

I'm in favour of the first option, as it's much simpler and intuitive to newcomers, PR attached.

Some IRC conversations I had about this as I was surprised myself:

[12:12:15]  <clgv>  martinklepsch: profiles are merged by leiningen
[12:12:38]  <clgv>  martinklepsch: you can check with "lein pprint"
[12:14:38]  <martinklepsch> the documentation said it'd merge recursively, that's why I was expecting both keys to be in the :env map
[12:17:18]  <weavejester>   Lein profiles are merged recursively, though if you have the same profile declared in two locations, only one version is used.
[12:21:53]  <clgv>  weavejester: really? that's unexpected when everything else is merged and ^:replace metadata can be used as well
[12:22:16]  <weavejester>   clgv: Yeah, it’s a bit of a gotcha
[12:22:34]  <weavejester>   clgv: For example, if you have a :dev profile in your project.clj and a local profiles.clj
[12:22:58]  <clgv>  weavejester: indeed
[12:22:59]  <weavejester>   So I tend to have :local/dev and :repo/dev profiles
[12:23:21]  <clgv>  weavejester: and then aliases in the project.clj?
[12:23:21]  <weavejester>   Though maybe I should call them :profiles/dev and :project/dev
[12:23:35]  <weavejester>   Yeah, :dev [:repo/dev :local/dev]
[12:36:52]  <martinklepsch> weavejester: in a project.clj with :env {:is-dev true} it wouldn't make much sense to go the :profiles/dev :project/dev route, right?
[12:37:23]  <martinklepsch> you could just commit profiles.clj with that single entry and add it to .gitignore afterwards
[12:39:23]  <weavejester>   martinklepsch: Well, you really only need to go down the :profiles/dev route if (a) you want to have local overrides to your project that aren’t in source control, and (b) you already have a :dev profile you don’t want to overwrite, but merge into
[12:41:32]  <martinklepsch> ok, thanks

@martinklepsch martinklepsch force-pushed the martinklepsch:profile-env-var-improvements branch from 64ba986 to 210d4f3 Oct 16, 2014

@plexus

This comment has been minimized.

Copy link
Owner

commented Oct 16, 2014

Not sure I like pulling out just that part to profiles.clj. If we're gonna introduce profiles.clj then better move all the profile definitions there.

This is a very specific issue though. A beginner who wants to add stuff to the env will more likely do so directly in project.clj.

@martinklepsch

This comment has been minimized.

Copy link
Author

commented Oct 16, 2014

If we move all the definitions there we'd have to check it into version control as well right? With just using profiles.clj for env vars there would be a clear distinction: profiles.clj for local dev stuff, project.clj for project-wide things.

This is a very specific issue though. A beginner who wants to add stuff to the env will more likely do so directly in project.clj.

Probably you're right about this. Eventually though I think people want to avoid dirty files in their working directory.

@plexus

This comment has been minimized.

Copy link
Owner

commented Oct 16, 2014

oh right, you would generate profiles.clj but not check it in... Is that common? I feel that if used it's very much part of the project. There's ~/.lein/profiles.clj obviously although I know it's not the same, you might want stuff local to a project. But if you want environment stuff that's only for your dev setup then you can just define environment variables. Environ will read those just like stuff in the :env map.

How I see it is that how leiningen profile merging works is inherently confusing if you're not used to it. So far all profiles were kept in project.clj because it meant having to generate one file less, which is one file less for a beginner to figure out why it's there. OTOH if moving profiles to profiles.clj prevents people from making mistakes then that's a good argument. I just wonder if then we will get issues from people adding profiles to project.clj again and being surprised again by how merging works.

@martinklepsch

This comment has been minimized.

Copy link
Author

commented Oct 16, 2014

It's not that all the profiles have to be moved to profiles.clj, it's just important that keys are not duplicate in those profiles' maps (i.e. an :env key for the :dev profile). That said the :dev profile containing cljsbuild instructions etc. could still be completely defined in the project.clj (where I think it belongs).

@weavejester

This comment has been minimized.

Copy link

commented Oct 16, 2014

For what it's worth, my approach is here:

I have profiles for :profiles/dev and :project/dev, then make :dev into a composite profile, with the profiles file taking priority. I then include a sample profiles file for those that want to use it.

@martinklepsch

This comment has been minimized.

Copy link
Author

commented Oct 16, 2014

So actually my suggestion of using profiles.clj to specify the :env key in there was misled.

Lein profiles are merged recursively, though if you have the same profile declared in two locations, only one version is used.

With this in mind the only setup that seems to be possible to separate local :dev settings and project :dev settings seems to be the one @weavejester is using in the referenced files.

@martinklepsch

This comment has been minimized.

Copy link
Author

commented Oct 18, 2014

I tested the :dev/profiles :dev/project approach yesterday and it's working great. I will definitely continue using this method for myself. I'm not sure though if it's the right thing for Chestnut. Definitely could confuse beginners. Might be worth it though.

@plexus plexus closed this Dec 11, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.