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

Doesn't support target-specific export environment variable #134

Open
JayXon opened this issue Feb 23, 2018 · 2 comments
Open

Doesn't support target-specific export environment variable #134

JayXon opened this issue Feb 23, 2018 · 2 comments

Comments

@JayXon
Copy link

JayXon commented Feb 23, 2018

export foo0=bar0
a: foo1=bar1
a: export foo2=bar2
a:
	echo ${foo0} ${foo1} ${foo2}
	sh -c 'echo $${foo0} $${foo1} $${foo2}'
$ make
echo bar0 bar1 bar2
bar0 bar1 bar2
sh -c 'echo ${foo0} ${foo1} ${foo2}'
bar0 bar2
$ ckati
echo bar0 bar1 
bar0 bar1
sh -c 'echo ${foo0} ${foo1} ${foo2}'
bar0

https://www.gnu.org/software/make/manual/html_node/Target_002dspecific.html

@danw
Copy link
Collaborator

danw commented Feb 23, 2018

Interesting, I hadn't seen that before. I'd be rather hesitant about using this however, since target-specific variables can infect their dependencies. In most cases, this isn't a problem if you only use what you define, but it can be a source of bugs too.

With exported variables, we'd need to change the command line for all of the dependencies, and we can't reproduce the exact same behavior as make (at least in Android, where we don't regenerate the ninja file when changing which targets to build):

export foo0=bar0
a: foo1=bar1
a: export foo2=bar2
a: b
        echo A: ${foo0} ${foo1} ${foo2}
        sh -c 'echo A: $${foo0} $${foo1} $${foo2}'

b:
        echo B: ${foo0} ${foo1} ${foo2}
        sh -c 'echo B: $${foo0} $${foo1} $${foo2}'
$ make b
echo B: bar0
B: bar0
sh -c 'echo B: ${foo0} ${foo1} ${foo2}'
B: bar0
$ make a
echo B: bar0 bar1 bar2
B: bar0 bar1 bar2
sh -c 'echo B: ${foo0} ${foo1} ${foo2}'
B: bar0 bar2
echo A: bar0 bar1 bar2
A: bar0 bar1 bar2
sh -c 'echo A: ${foo0} ${foo1} ${foo2}'
A: bar0 bar2

I'll likely be restricting the use of export in Android makefiles in the future, as part of an effort to increase the reliability of builds when build steps use environment variables. We'll need to track environment variable changes per-build-step, and re-run the build step when the environment variables change. It'll be important to limit the global environment variables that actions can see in that case.

I'm assuming you've got a build rule that contains multiple lines that you'd like to apply the environment variable to all lines? If it's just a line or two, adding it at the beginning of the line isn't too bad, and is what I've been suggesting here.

@JayXon
Copy link
Author

JayXon commented Feb 23, 2018

Thanks for the quick reply, yes I need to change the PATH for three lines in a rule, and I was just trying to reduce the duplication and was surprised that it didn't work.
The dependency issue you mentioned is interesting, but isn't that applies to non-exported variables (foo1) as well?
I'm fine if kati chose to not support this, but I think it should show an error/warning in this case.

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

No branches or pull requests

2 participants