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

Passthrough additional environment variables #214

Closed
wants to merge 4 commits into from
Closed

Passthrough additional environment variables #214

wants to merge 4 commits into from

Conversation

wherget
Copy link
Contributor

@wherget wherget commented May 12, 2015

This PR allows all Mojos to pass through additional environment variables to the build, accessible there through process.env.
This is a relatively quick and convenient way to transport (e.g.) Maven properties into the Node world, which is what I would be using it for.

<configuration>
    <environmentVariables>
        <OUTPUT_DIR>${project.build.directory}</OUTPUT_DIR>
    </environmentVariables> 
</configuration>

It would also fix #158.

@dantran
Copy link

dantran commented Jun 8, 2015

+1

ogolberg pushed a commit to vecnatechnologies/frontend-maven-plugin that referenced this pull request Jul 31, 2015
@achingbrain
Copy link

+1 This would be very useful.

@wherget
Copy link
Contributor Author

wherget commented Oct 6, 2015

I've ported this forward to current master. I'd really appreciate feedback.

@eirslett
Copy link
Owner

eirslett commented Oct 6, 2015

The advice I have given so far (in other issues buried deeeeeep down in the github issues list) is to pass on variables as flags:

<arguments>build --foo=${foo} --bar=${bar}</arguments>

and that might seem like a stupid solution at first, but it's actually pretty nice, since you can then run your frontend build independent of whatever you do in Maven. That's a huge win for frontend developers. You don't have to wait 3 minutes for the Java classes to compile and the unit tests to be run, in order to debug the gulp build.

@wherget
Copy link
Contributor Author

wherget commented Oct 6, 2015

I will (again) hold against that recommendation that it requires extreme care with quoting arguments. From what I read in NodeTaskExecutor.getArguments you're splitting at spaces. If I'm not severely mistaken, this breaks as soon as one of the ${foo} variables contains something with a space.

My use case would put an output directory there, which very well could contain spaces.

Secondly, relying on environment variables doesn't restrict independence of the build either:

FOO="/path/to directory/with space/" gulp build

works just the same from the command line.

@pope4e
Copy link

pope4e commented Dec 5, 2015

+1 I would appreciate this to be included also. I opted to the maven-exec-plugin for a current use case i am working on because of unavailability to tweak a bit the runtime environment variables here.

@pwielgolaski
Copy link
Contributor

+1 I see real cases when I need to tweak som npm/boer configuration, not everything can be done by parameters

@eirslett
Copy link
Owner

Ok, let's get this merged in! :-)
@l337r007 I'm sorry, I've been so slow to look at this that it appears it has a merge conflict with master already... is it a big one?

eirslett added a commit that referenced this pull request Jan 24, 2016
Passthrough additional environment variables (by l337r007) merge on top of master
@eirslett
Copy link
Owner

This was fixed in #346 :)

@eirslett eirslett closed this Jan 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to configure ENV
6 participants