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

make.bash: drop bash as a build dependency #41698

Closed
mcandre opened this issue Sep 29, 2020 · 17 comments
Closed

make.bash: drop bash as a build dependency #41698

mcandre opened this issue Sep 29, 2020 · 17 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@mcandre
Copy link

mcandre commented Sep 29, 2020

The bash build scripts can be rewritten in terms of pure POSIX sh, simplifying the bootstrap development environment.

@ALTree ALTree changed the title Drop bash as a build dependency make.bash: drop bash as a build dependency Sep 29, 2020
@ALTree
Copy link
Member

ALTree commented Sep 29, 2020

Would this solve any actual problem? I don't think I've ever seen anyone complaining about make.bash being a bash script in the past 6 years.

@ianlancetaylor
Copy link
Contributor

In practice we can only do this if there is some way to write a test to ensure that we never regress back to using bash-isms. I don't know how to write such a test but perhaps there is some way.

(I also agree that I don't see why this is a problem in practice.)

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 29, 2020
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Sep 29, 2020
@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 29, 2020
@beoran
Copy link

beoran commented Sep 30, 2020

This can be checked easily using shellcheck https://github.com/koalaman/shellcheck, or chechbashisms https://linux.die.net/man/1/checkbashisms. The former has the additional benefit of detecting common errors in the shell script.

Of course, it then means you have a dependency on one of those tools in stead of bash, which may not be an improvement.

@davecheney
Copy link
Contributor

What’s the driver for this? The resource usage of various shells compared to the 1.3gb of ram needed to compile some of the internal packages in the compiler.

@mvdan
Copy link
Member

mvdan commented Sep 30, 2020

While I generally agree that it's nice to be portable, I also don't see why Bash would be a problem today if it hasn't been a problem for the past decade.

There are also a dozen other bash scripts scattered across the repository, such as src/all.bash and src/iostest.bash. Translating all of them would be quite a bit of work and surely introduce a number of bugs, even if we're careful.

@mvdan
Copy link
Member

mvdan commented Oct 11, 2020

A heads up for others wanting to contribute to this thread; it's very unlikely that the author will actually provide any more information here, see #41739 (comment). Unless you support this change, it's probably not worth your time to reply.

@beoran
Copy link

beoran commented Oct 12, 2020

I actually support this change together with introducing shellcheck, but seeing that this has been working as is for years, and that there are more urgent priorities, I don't mind if this issue is postponed or closed either.

@timkuijsten
Copy link

Just had my first experience with Go, which was nice. The one thing that surprised me a little bit was that bash was required to build. On OpenBSD the standard shell is a hardened ksh. bash is avoided everywhere in base so I had to install that, no biggie, but the question I would phrase in the spirit of portability and reducing dependencies is "why require more if technically all you need is POSIX shell"? I'm wondering if such a change would be desirable, aside from the question who's going to make it happen.

@ianlancetaylor
Copy link
Contributor

I don't think the suggested change is very important but I think it would be fine if someone wants to work on it and has some way to prevent backsliding. (I don't think the packages mentioned above are going to be useful in that regard--we certainly don't want the Go build to depend on them: that would be worse than depending on bash. But if someone can come up with a workable approach, great.)

@illiliti
Copy link

Patch that allows building go without bash - https://github.com/kisslinux/community/blob/d99595e575a471698032dbad0cd2c9977d6b0c79/community/go/patches/posix-build.patch . As you see, there is no huge dependency on bash and everything can be implemented in POSIX shell

@ianlancetaylor
Copy link
Contributor

Thanks. Now, how do we prevent backsliding?

@beoran
Copy link

beoran commented Oct 14, 2020 via email

@feld
Copy link

feld commented Oct 14, 2020

Thanks. Now, how do we prevent backsliding?

One CI/CD host that isn't Linux

@periish
Copy link

periish commented Oct 14, 2020

Note that some shells implement common bashisms; dash implements == in test(iirc), for instance.

The most reliable method would be checkbashisms or shellcheck.

@timkuijsten
Copy link

FWIW, I've started a separate discussion to get some feedback from a community I'm familiar with, in order to prevent being a loud and chatty newbie overhere before getting to the point. ;-) For anyone interested: https://lobste.rs/s/lzc7ac/make_bash_drop_bash_as_build_dependency

@khm
Copy link

khm commented Oct 15, 2020

Duplicate of #3235

@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@golang golang locked and limited conversation to collaborators Oct 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

13 participants