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

The "-c" option does not exist. #107

Closed
kevin-appelt opened this issue Mar 12, 2020 · 37 comments
Closed

The "-c" option does not exist. #107

kevin-appelt opened this issue Mar 12, 2020 · 37 comments

Comments

@kevin-appelt
Copy link

Using the image "composer:1.9.3" is broken since last night.

How does it work (yesterday evening)

Using Docker executor with image composer:1.9.3 ...
 Pulling docker image composer:1.9.3 ...
 Using docker image sha256:d95b3f498002c38d16668671783ce3b0ed76e24badab25c1ff9ab829fd70982d for composer:1.9.3 ...

How does the error occur (today morning)

Using Docker executor with image composer:1.9.3 ...
 Pulling docker image composer:1.9.3 ...
 Using docker image sha256:0813bfddf00c429ecfc9600af7c9c336e0920639debc5cc3ad93f4b2420b73d9 for composer:1.9.3 ...

I guess the related change is 96bfff8

With the newer image we see the following error within GitLab CI on the same commit without any changes in "our" code.

   [Symfony\Component\Console\Exception\RuntimeException]  
   The "-c" option does not exist.  
@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

I'm missing some critical and useful information, like what command you are running to begin with. It would be nice if you could provide this too.

@kevin-appelt
Copy link
Author

failing_job:
  image: composer:1.9.3
  before_script:
    - apk add bash --no-cache
    - apk add git --update
  script:
    - composer install --no-dev --ignore-platform-reqs --no-suggest
another_failing_job:
  image: composer:1.9.3
  script:
    - composer install --ignore-platform-reqs --no-suggest
    - ./vendor/bin/typoscript-lint --config=typoscript-lint.yml --fail-on-warnings
    - ./vendor/bin/php-cs-fixer fix --dry-run --diff --config ./ci/linter/.php_cs.php
    - ./vendor/bin/rector process --dry-run

Hope this can help. If you need more information just let me know.

Updating to image composer:1.10.0 does not solve the problem.

Workaround
For now I've downgraded the image to composer:1.8.6 which works well.

@mmazurowski
Copy link

Hello @alcohol,

This error comes just after pulling docker image, without running any command. (first in chain is echo which does not run)
I am using it with gitlab CI and therefore I expect the issue is related to latest changes as yesterday everything worked properly.

For the test I've pulled previous version by hash name and everything works perfectly.

Kind regards,
Martin

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

We changed something in the entrypoint, but I'm not quite sure why it causes these failures. Will investigate.

@mmazurowski
Copy link

I guess as this is quite popular image maybe you could revert to the previous iteration of the repository and after some more testing merge last changes again?
I am sure many companies are trying to figure this problem out right now.

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

I think I have a hunch...

My guess is, that your CI provider is calling the container with sh -c 'command to run'. We changed the entrypoint to be more powerful in supporting short aliases for composer commands. sh is short for the composer show command. If you want to run sh as the entrypoint, you have to override the Container default entrypoint script.

@mmazurowski
Copy link

That could be it I guess

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

I find the circleci documentation rather awful. I think you can specify your own entrypoint using the entrypoint property, but since I do not use circleci I have no way to verify this. But basically, you want to set it to an empty value.

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

Using the default entrypoint, sh is interpreted as the shorthand for show, and show has no -c flag/option:

[2020-03-12 10:00:20 +0100] rob@galaga /tmp/tmp.xakk1stcfX $ docker run composer sh -c 'echo hello'

                                                          
  [Symfony\Component\Console\Exception\RuntimeException]  
  The "-c" option does not exist.                         
                                                          

show [--all] [-i|--installed] [-p|--platform] [-a|--available] [-s|--self] [-N|--name-only] [-P|--path] [-t|--tree] [-l|--latest] [-o|--outdated] [--ignore IGNORE] [-m|--minor-only] [-D|--direct] [--strict] [-f|--format FORMAT] [--] [<package>] [<version>]

With an empty entrypoint, sh becomes the command to run.

[2020-03-12 10:03:57 +0100] rob@galaga /tmp/tmp.xakk1stcfX $ docker run --entrypoint "" composer sh -c 'echo hello'
hello

@alcohol alcohol closed this as completed Mar 12, 2020
@alcohol alcohol pinned this issue Mar 12, 2020
@matt9mg
Copy link

matt9mg commented Mar 12, 2020

This also happens using gitlab ci, for now using 1.8 image bypasses the issue

@dmaphy
Copy link

dmaphy commented Mar 12, 2020

We're writing the composer commands directly into our Gitlab CI script like this:

composer:
  stage: composer
  tags:
    - dockerrunner
  image: composer:1.8
  script:
    - composer validate --no-check-all --no-check-publish
    - composer install --ignore-platform-reqs --no-scripts
    - composer show -o -l -D -m --strict

How do you suggest to override the entrypoint here?

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

Like I said, I don't know exactly how circleci works and their documentation is not very useful (admittedly I only glanced over it quickly); but my best guess would be:

composer:
  stage: composer
  tags:
    - dockerrunner
  image: composer:1.8
  entrypoint: ""
  script:
    - composer validate --no-check-all --no-check-publish
    - composer install --ignore-platform-reqs --no-scripts
    - composer show -o -l -D -m --strict

or instead of script use command maybe (again not sure, cause their documentation is confusing on this subject):

composer:
  stage: composer
  tags:
    - dockerrunner
  image: composer:1.8
  command: |
    composer validate --no-check-all --no-check-publish
    composer install --ignore-platform-reqs --no-scripts
    composer show -o -l -D -m --strict

@alcohol alcohol changed the title The "-c" option does not exist. [CircleCI] The "-c" option does not exist. Mar 12, 2020
@DanielSchwiperich
Copy link

This is not only Circle ci, it's also affecting gitlab ci @alcohol

@dmaphy
Copy link

dmaphy commented Mar 12, 2020

This one works in Gitlab CI for us:

composer:
  stage: composer
  tags:
    - dockerrunner
  image:
    name: composer:latest
    entrypoint: [""]
  script:
    - composer validate --no-check-all --no-check-publish
    - composer install --ignore-platform-reqs --no-scripts
    - composer show -o -l -D -m --strict

@alcohol alcohol changed the title [CircleCI] The "-c" option does not exist. The "-c" option does not exist. Mar 12, 2020
@mmazurowski
Copy link

@dmaphy I can confirm it works for me as well

@Avalarion
Copy link

To be honest this is a breaking change that should have not just been patched into. I now have to fix like ~100 .gitlab-ci.yml s to either use an older composer image or add an entrypoint to each file and change our internal documentation...

@drzraf
Copy link

drzraf commented Mar 12, 2020

This release just broke literally hundreds of thousands of CI jobs worldwide. And the fact it affects not only 1.10 but 1.9.3 too made it worse.

@Rohaq
Copy link

Rohaq commented Mar 12, 2020

Just to clarify: Is there a plan to resolve this as an issue, or is every gitlab/circleci runner config in the world going to need to use older versions/override the entrypoint? :(

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

This is not a breaking change honestly.

But I am considering a middle ground, where we explicitly add an exception for sh as first argument to directly invoke sh. This would retain backwards compatibility at the expense of users not being able to use sh as a shorthand for show.

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

Pending docker-library/official-images#7617

@mmazurowski
Copy link

I believe that it is breaking change as the parameter interfere with commonly used syntax which caused the issue with CI tools. Probably this issue can cause more problems because of minor syntax sugar update. As an official image of a composer IMO this is not acceptable or should be addressed as separate image version.

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

I understand your opinion on the matter, but I disagree.

The whole purpose of the combination of entrypoint and cmd, is that the container can be invoked directly in a manner virtually identical to the binary if it was invoked outside of a container.

In other words, the following should behave identical:

docker run composer <args>

and

composer <args>

While I understand why these CI providers use sh -c "..." to run commands, I think they made a mistake in assuming it will always work when passing it as a cmd, rather than explicitly overriding the default entrypoint.

@mmazurowski
Copy link

I cannot disagree with you more. Yet I’d like to bring perspective of working with software which is used in production / business environment. Having that in mind I would expect at least second version of an image (with the update) which will be optional for a specific time and then migrating it to the main one. This combined with changelog in main repository and proper description of possible effects would give business and developers time to prepare needed updates to the system. If this was new image, alternative version or brand new repository this would be completely fine to change, for the commonly used module back compablity should be taken into consideration during updates.

Additionally I would like to personally thank you for your fast answers and collaboration on that issue. This behavior is highly admirable yet not commonly met. Keep on rockin

@caugner
Copy link

caugner commented Mar 12, 2020

I would suggest adding a cli tag for this behavior:

  • docker run composer:cli <args> = composer <args>

It could be tagged as latest (with a base tag for the legacy behaviour), but only if there is other popular software whose Docker images exhibit the same behaviour.

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

I'm not sure why we would need an additional tag? The proposed solution will work for existing CI pipelines. The downside of not being able to use sh as a shorthand for show is not worth maintaining an additional tag IMHO.

@Avalarion
Copy link

It will not work for mine. We call "composer install" or "mkdir" directly without calling "sh" in front of every command.

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

It will not work for mine. We call "composer install" or "mkdir" directly without calling "sh" in front of every command.

For this scenario, nothing changes in the pending solution nor has anything changed. So what is the problem?

@caugner
Copy link

caugner commented Mar 12, 2020

@Avalarion Don't worry, it will still work. The fix makes sure that an entry-point of sh will actually run the sh shell (and not composer show). From there, you can call any command.

@patlamontagne
Copy link

It worked fine yesterday but now version 1.9.3 breaks all of our gitlab-ci jobs.

Literally only calling composer install in the composer image and the error happens, seems like a pretty big breaking change to me.

Reverting one of our job to use 1.8 worked for us, we're waiting to see when if we can stop holding back our jobs or if we have to update all of our ci files at once.

@Avalarion
Copy link

Avalarion commented Mar 12, 2020

But I have no entry call sh.

Here as a quote from my .gitlab-ci.yml.

build_composer:
  image: composer
  stage: prelinting
  before_script:
    - composer global require hirak/prestissimo
  script:
    - composer install --dev
    - composer dump-autoload -o
  artifacts:
    paths:
    - .Build
    expire_in: 1 hour

And I guess that I am not alone with this kind of script and before_script usage.

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

@Avalarion the problem is that the CI provider is abstracting the docker run command away for you, and they're doing it in a way that is not compatible with our entrypoint (it merely worked up until now because we did not support shorthand aliases for composer commands).

So while you do not call sh, they do.

The pending fix implements a workaround for this scenario.

@caugner
Copy link

caugner commented Mar 12, 2020

it merely worked up until now because we did not support shorthand aliases for composer commands

@alcohol Folks relied on that behaviour, the change broke that behaviour and therefore it is a BC.

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

We can agree to disagree. 🤷‍♂️

@caugner
Copy link

caugner commented Mar 12, 2020

How do you define a breaking change?

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

A change that breaks an existing contract.

The problem is that there is no explicit contract in this scenario. There is no interface definition or API documentation. Anything resembling a contract can only be deduced if you look at the implementation of the entrypoint script and understand how it works (or is intended to work).

Just because it worked up until now, does not mean it was ever explicitly meant to work like this. But I can relate to the opinion that this in and of itself could be considered in some way a contract as well.

I propose we cease this discussion of whether or not it is a breaking change. It is petty and does not contribute to anything. I understand your frustration. The unexpected side-effects of this change have significantly impacted users and this was unforeseen and unfortunate. A solution has been implemented to remedy this.

@alcohol
Copy link
Member

alcohol commented Mar 12, 2020

The upstream PR docker-library/official-images#7617 was merged.

Any CI pipelines that do not cache (persistent) containers should be working again.

For anyone that has a CI pipeline with a persistent Docker cache, I suggest you find a way to purge/refresh it, or run docker pull composer as your very first step (include the appropriate tag if necessary).

coryzibell pushed a commit to digitalsurgeons/gitlab-ci-configs that referenced this issue Mar 12, 2020
@exussum12
Copy link
Contributor

exussum12 commented Mar 29, 2020

Worth adding in a single exception for sh in the entry point script ? Or has this been solved upstream now ?
Edit:
Never mind, this is closed, I just seen it as the pinned issue

@alcohol alcohol unpinned this issue Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests