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

Use Bash as shell in Makefile #345

Merged
merged 1 commit into from
Nov 26, 2018
Merged

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Nov 25, 2018

This issue has been bugging me for a while and I was finally triggered enough to do something about it. If one runs make on macOS then echo ignores the -n flag which suppresses newline output. Then instead of a nice output:

one sees the following monstrosity:

This is caused by multitude of factors. First of all, GNU make uses /bin/sh by default to execute shell commands. This can be overridden by setting the SHELL make variable (environment variable is ignored).

Usually on UNIX systems /bin/sh is a symlink or a drop-in replacement to some 'maximally portable POSIX shell' which resembles the original Bourne shell. On macOS /bin/sh is renamed Bash binary. I believe some other UNIX systems do so as well. When Bash is invoked as "sh" it enables Bourne shell compatibility mode:

If bash is invoked with the name sh, it tries to mimic the startup behavior of historical versions of sh as closely as possible, while conforming to the POSIX standard as well.

— man 1 bash

echo is normally a shell built-in (so that the shell does not execute actual /bin/echo all the time), so its options are determined by the shell and its compatibility mode. The -n option is supported by Bash and by most of echo binaries, but not by Bourne shell's echo builtin.

So, set SHELL to Bash in order to have nice behavior of the "echo" builtin. A more correct way would be to disable the builtin and make sure that the shell runs /bin/echo, but that's much harder to do. This can be offensive towards POSIX compatibility purists, but if they run Themis builds on a system where Bash is not available they can comment out that line and get their /bin/sh back. While we're gonna enjoy nice progress reports with correct newlines.

This issue has been bugging me for a while and I was finally triggered
enough to do something about it. If one runs make on macOS then "echo"
ignores the "-n" flag which suppresses newline output. Then instead
of a nice output:

    link soter_static                   [OK]
    link themis_static                  [OK]
    link soter_shared                   [OK]
    link themis_shared                  [OK]
    0.10.0

one sees the following monstrosity:

    -n link
    soter_static                   [OK]
    -n link
    themis_static                  [OK]
    -n link
    soter_shared                   [OK]
    -n link
    themis_shared                  [OK]
    0.10.0

This is caused by multitude of factors. First of all, GNU make uses
/bin/sh by default to execute shell commands. This can be overridden
by setting the SHELL make variable (*environment* variable is ignored).

Usually on UNIX systems /bin/sh is a symlink or a drop-in replacement
to some 'maximally portable POSIX shell' which resembles the original
Bourne shell. On macOS /bin/sh is renamed Bash binary. I believe
some other UNIX systems do so as well. When Bash is invoked as "sh"
it enables Bourne shell compatibility mode:

    If bash is invoked with the name sh, it tries to mimic the startup
    behavior of historical versions of sh as closely as possible, while
    conforming to the POSIX standard as well.
                                                       -- man 1 bash

"echo" is normally a shell built-in (so that the shell does not execute
actual /bin/echo all the time), so its options are determined by the
shell and its compatibility mode. The "-n" option is supported by Bash
and by most of echo binaries, but not by Bourne shell's echo builtin.

So, set SHELL to Bash in order to have nice behavior of the "echo"
builtin. A more correct way would be to disable the builtin and make
sure that the shell runs /bin/echo, but that's much harder to do.
This can be offensive towards POSIX compatibility purists, but if they
run Themis builds on a system where Bash is not available they can
comment out that line and get their /bin/sh back. While we're gonna
enjoy nice progress reports with correct newlines.
@vixentael vixentael added infrastructure Automated building and packaging O-macOS 💻 Operating system: macOS labels Nov 26, 2018
@vixentael
Copy link
Contributor

Thank you @ilammy! I never thought that -n logs could be changed :D

I've tested on MacOS, works perfectly fine. If @shadinua doesn't have any considerations, let's merge it.

Copy link
Contributor

@shadinua shadinua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR, @ilammy!

Actually, you're right — most of desktop Linux-based systems (such as Debian, RHEL, Centos, Ubuntu, Gentoo, Suse) have pre-installed bash and sh is just a symbolic link to bash. Surely, on that systems suggested variant will work well.

But we shouldn't forget that themis — is a multiplatform library and we tend to keep it compatible to wide range of platforms. For example, FreeBSD 11 has /bin/sh as a separate binary and has no pre-installed bash. Moreover, bash in FreeBSD can be installed from package system and placed to /usr/local/bin/bash, which is standard path for FreeBSD:

[shad@freebsd-test ~]$ uname -a
FreeBSD freebsd-test 11.1-RELEASE-p9 FreeBSD 11.1-RELEASE-p9

[shad@freebsd-test ~]$ for f in sh bash; do ls -al $(which $f); done
-r-xr-xr-x  1 root  wheel  161936 Jul 21  2017 /bin/sh
-rwxr-xr-x  1 root  wheel  1131024 Mar 15  2018 /usr/local/bin/bash

Besides, there are a lot of shrinked system on tiny devices without bash, themis may be compiled and installed there as well.

In any of that cases suggested Makefile will not work.

Unfortunately, our Makefile is somewhat messy and requires refactoring. We plan to do this in the nearest future, also adding support for multiple operating systems.

In any case, I am very grateful to you for participating in the project! Thank you!

@vixentael
Copy link
Contributor

Thank you @shadinua!

So we will merge this change now, and update it during Makefile refactoring (planned, waiting in task queue) to add FreeBSD support.

@vixentael vixentael merged commit 0761eb4 into cossacklabs:master Nov 26, 2018
@ilammy ilammy deleted the makefile-echo branch January 17, 2019 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Automated building and packaging O-macOS 💻 Operating system: macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants