Skip to content
GitLab
    • GitLab: the DevOps platform
    • Explore GitLab
    • Install GitLab
    • How GitLab compares
    • Get started
    • GitLab docs
    • GitLab Learn
  • Pricing
  • Talk to an expert
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
    • Switch to GitLab Next
    Projects Groups Snippets
  • Sign up now
  • Login
  • Sign in / Register
  • gitlab-runner gitlab-runner
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Issues 972
    • Issues 972
    • List
    • Boards
    • Service Desk
    • Milestones
    • Iterations
    • Requirements
  • Jira
    • Jira
  • Merge requests 88
    • Merge requests 88
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Artifacts
    • Schedules
    • Test Cases
  • Deployments
    • Deployments
    • Environments
  • Packages and registries
    • Packages and registries
    • Container Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • Code review
    • Insights
    • Issue
  • Activity
  • Create a new issue
  • Jobs
  • Issue Boards
Collapse sidebar

Do not update/delete: Banner broadcast message test data

Do not update/delete: Notification broadcast message test data

  • GitLab.orgGitLab.org
  • gitlab-runnergitlab-runner
  • Issues
  • #1550
Closed
Open
Issue created Aug 02, 2016 by username-removed-134837@onlyjob

bashism; meaningless/incorrect shell detection; no quotes

shells/bash.go contains very silly code that fails in various ways.

First of all that type of shell detection is wrong an unnecessary because one could just rely on PATH and safely reduce the whole thing to bash $@.
In container PATH is most likely set to something like PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin (as per /etc/profile) so bash $@ just works.


Secondly when container is set with ENTRYPOINT ["/init.sh"] and init.sh that contains exec /dumb-init -- $@
then execution fails as follows because shell detection fragment is injected without line breaks:

+ exec /dumb-init sh -c if [ -x /usr/local/bin/bash ]; then exec /usr/local/bin/bash elif [ -x /usr/bin/bash ]; then exec /usr/bin/bash elif [ -x /bin/bash ]; then exec /bin/bash elif [ -x /usr/local/bin/sh ]; then exec /usr/local/bin/sh elif [ -x /usr/bin/sh ]; then exec /usr/bin/sh elif [ -x /bin/sh ]; then exec /bin/sh else echo shell not found exit 1 fi
[: 1: [: Syntax error: end of file unexpected
ERROR: Build failed: exit code 2

Arguably it could be fixed in container's init.sh by quoting $@ as follows: exec /dumb-init -- "$@" however it should not be necessary if shell detection is corrected to be more resilient. One should be able to re-write it as one-liner with ; separators and quoting. It is a bad style to use echo shell not found rather than printf "E: shell not found.\n" as echo may fail too depending on type of shell, let alone problematic absence of quotes.


Finally shell detection fails with any shell other than bash because of bashism "-o pipefail":

sh: 1: set: Illegal option -o pipefail
sh: 1: set: Illegal option -o pipefail
ERROR: Build failed: exit code 2

I suppose shell detection code has never been reviewed or justified. It would be great to remove it or at least improve.

Assignee
Assign to
Time tracking