search

pfalcon / esp-open-sdk

Free and open (as much as possible) integrated SDK for ESP8266/ESP8285 chips
1.97k stars 623 forks source link

Use env shell cmd in Makefile for portability #374

Closed vyorkin closed 4 years ago

vyorkin commented 4 years ago

I suggest using the

/usr/bin/env bash

instead of just

/bin/bash

in shell scripts, for portability.

For example, this fixes the build for NixOS users (like me).

pfalcon commented 4 years ago

Thanks. I'd suggest to read Make manual and use make SHELL=anything_you_want_without_affecting_all_other_people instead.

vyorkin commented 4 years ago

Gotcha, yeah, I'm aware of the SHELL env var. My point is that it would just be convenient if it worked for everyone by default. Is it possible that the change introduced by this PR might break the build for any GNU/POSIX system?

pfalcon commented 4 years ago

I'm aware of the SHELL env var.

Here, SHELL is not env var.

My point is that it would just be convenient if it worked for everyone by default.

It does. Unless you don't have a shell in the standard location, in which case, you will need to do extra leg work.

Is it possible that the change introduced by this PR might break the build for any GNU/POSIX system?

Changes like that are potential debuggability and even security issue. Like, some have a random "bash" executable/script lying on their $PATH, and now that executable/script starts to be executed, instead of the well-defined system shell.

Overall, this project follows "ain't broken, ain't fix" it approach. If you want to override things on your side - yes, you can.

vyorkin commented 4 years ago

Thanks for a detailed explanation @pfalcon. Makes sense!

Here, SHELL is not env var.

I should definitely read the Make manual :)