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

Activity

1. danw commented on Feb 23, 2018

   

   danw

   on Feb 23, 2018

   Collaborator

   More actions

   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.
2. JayXon commented on Feb 23, 2018

   

   JayXon

   on Feb 23, 2018

   Author

   More actions

   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.