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

composer: always use --no-interaction option #2348

Merged

Conversation

gangelop
Copy link
Contributor

@gangelop gangelop commented Apr 25, 2021

SUMMARY

Since the composer command provides a no-interaction option, and since
we always call it non-interactively, we should always use it.

The lack of this option caused composer to not exit at least in one
instance when called as 'php /usr/local/bin/composer help install'.
The '-n' added with this patch resolved that issue.

ISSUE TYPE
  • Bugfix Pull Request
    As far as I can tell from the open issues, I'm the first one to report this.
COMPONENT NAME
  • composer
ADDITIONAL INFORMATION

Versions and stuff.

On host:

$ uname -a
Linux pi 5.4.0-1034-raspi #37-Ubuntu SMP PREEMPT Mon Apr 12 23:14:49 UTC 2021 aarch64 aarch64 aarch64 GNU/Linux

$ php7.4 -v
PHP 7.4.16 (cli) (built: Mar  5 2021 07:54:38) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.16, Copyright (c), by Zend Technologies

$ php7.4 /usr/local/bin/composer -V
Composer version 2.0.12 2021-04-01 10:14:59

On controller:

$ ansible --version
ansible 2.10.8
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/george/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.9/site-packages/ansible
  executable location = /bin/ansible
  python version = 3.9.3 (default, Apr  8 2021, 23:35:02) [GCC 10.2.0]

Output of composer showing the -n option:

php7.4 /usr/local/bin/composer help 
Usage:
  help [options] [--] [<command_name>]

Arguments:
  command                        The command to execute
  command_name                   The command name [default: "help"]

Options:
      --xml                      To output help as XML
      --format=FORMAT            The output format (txt, xml, json, or md) [default: "txt"]
      --raw                      To output raw command help
  -h, --help                     Display this help message
  -q, --quiet                    Do not output any message
  -V, --version                  Display this application version
      --ansi                     Force ANSI output
      --no-ansi                  Disable ANSI output
  -n, --no-interaction           Do not ask any interactive question
      --profile                  Display timing and memory usage information
      --no-plugins               Whether to disable plugins.
  -d, --working-dir=WORKING-DIR  If specified, use the given directory as working directory.
      --no-cache                 Prevent use of the cache
  -v|vv|vvv, --verbose           Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug

Help:
  The help command displays help for a given command:
  
    php /usr/local/bin/composer help list
  
  You can also output the help in other formats by using the --format option:
  
    php /usr/local/bin/composer help --format=xml list
  
  To display the list of available commands, please use the list command.

@gangelop gangelop force-pushed the gangelop-composer-fix branch from a30e77a to 4ffb4f7 Compare April 25, 2021 21:58
@felixfontein
Copy link
Collaborator

Thanks for your contribution!

Is -n supported by all versions, or has it only been added recently?

Also, you need to add a changelog fragment.

@felixfontein
Copy link
Collaborator

CC @dmtrs @resmo @geerlingguy

@gangelop
Copy link
Contributor Author

Hi, thanks for checking.

Is -n supported by all versions, or has it only been added recently?

This did cross my mind and I haven't checked yet.

Also, you need to add a changelog fragment.

Wasn't aware.

Will check both, maybe in the afternoon. Lots of deployments to do today. :)

@resmo
Copy link
Member

resmo commented Apr 26, 2021

not using composer (anymore)

@ansibullbot ansibullbot added affects_2.10 bug This issue/PR relates to a bug community_review language module module new_contributor Help guide this first time contributor packaging plugins plugin (any type) python3 small_patch Hopefully easy to review labels Apr 26, 2021
@gangelop
Copy link
Contributor Author

gangelop commented Apr 26, 2021

So...

In upstream composer, it looks like this is the commit where -n was introduced.

git log --oneline -G 'no-interaction' | tail -n1
3f665e8bb  * method to gather if this session is 'interactive' or 'non interactive'

composer/composer@3f665e8

% git describe --contains 3f665e8bbbf6d4dec9e2eb1d59724c519bbdbda8
1.0.0-alpha2~48^2~48

It looks like -n has been around for a while. But looking at the current composer source, I can't figure out where in the code the argument parsing is done and that bothers me... 🤔

@aminvakil
Copy link
Contributor

FWIW I've found this PR from 2012: composer/composer#739 which is about no-interaction option...

@felixfontein
Copy link
Collaborator

Looks like it has been around since the beginning (initial release of composer was on March 1st, 2012) then. Good :)

@felixfontein
Copy link
Collaborator

When looking at the code, it looks like it already specifies --no-interaction: https://github.com/ansible-collections/community.general/blob/4ffb4f7f8e26514f98726d5a6e8f2d5748adb00a/plugins/modules/packaging/language/composer.py#L254

The only exception is the call where it determines the options available, there it passes help <command> --format=json. (This also means that the patch as-is is not correct, since it specifies that option multiple times.)

Can you confirm that it hangs when run with php /usr/local/bin/composer help install --format=json, but not with php /usr/local/bin/composer help install --format=json --no-interaction? You did not mention --format=json above, and this could make a big difference (TBH I would expected that JSON output implies --no-interaction).

@gangelop
Copy link
Contributor Author

Now that you mentioned it, I'm pretty sure that the command what was stuck was indeed php /usr/local/bin/composer help install --format=json or php7.4 /usr/local/bin/composer help install --format=json and in any case, it included --format=json. I don't know what I omitted that in my initial report. That was dumb. And now I don't trust my memory so please wait a bit longer and I'll reproduce it again dig a bit deeper.

@felixfontein
Copy link
Collaborator

I really wonder why it would hang (or wait for user input?) for composer help install --format=json - I don't see why that command should wait for any user input. (I don't really know composer, but that would be strange behavior for any CLI utility...)

@gangelop
Copy link
Contributor Author

It's definitely weird. I don't get it either. It didn't wait when I ran the same command interactively. When it was stuck it was just in S state. Anyway, I'll be back with details...

@gangelop gangelop force-pushed the gangelop-composer-fix branch from 4ffb4f7 to 59b30ba Compare April 30, 2021 15:21
@gangelop
Copy link
Contributor Author

First of all, here is a reproducer.

playbook:

---
- hosts: gombuter
  gather_facts: false
  tasks:
    - name: reproducer
      composer:
        working_dir: /opt/gomboser

Expected behavior: it exits
Actual behavior: it never exits
In ps aux I see:

root      483769  1.2  0.5  65356 20488 pts/2    S+   18:00   0:00 /usr/bin/php /usr/local/bin/composer help install --format=json

Notably, the same problematic behavior can be reproduced without the ansible composer module. For example:

---
- hosts: gombuter
  gather_facts: false
  tasks:
    - name: reproducer
      command: 'php /usr/local/bin/composer help install --format=json'
      register: result

    - name: execution will never reach this task
      debug:
        var: result

Thanks @flixfontein for pointing out that actually, no-interaction is always used in the ansible module, except for the help call which discovers if it is available. I had not noticed this.

With all this in mind, I've force-pushed a new version of the patch that takes all the above into account.


Still there are the following points worth noting:

  1. We still don't know why exactly it hangs. Depending on the answer it might be something that has to also be addressed upstream. My opinion is that it should still be addressed in ansible regardless.

  2. Maybe we should use no-interaction without ever checking if it is available in the following line, considering that composer silently ignores invalid options.

    https://github.com/ansible-collections/community.general/blob/4ffb4f7f8e26514f98726d5a6e8f2d5748adb00a/plugins/modules/packaging/language/composer.py#L259

@felixfontein
Copy link
Collaborator

1. We still don't know why exactly it hangs. Depending on the answer it might be something that has to _also_ be addressed upstream. My opinion is that it should still be addressed in ansible regardless.

It definitely has to be addressed upstream. A tool should not hang when showing help. But if we can do something to help, we should definitely also do it :)

2. Maybe we should use `no-interaction` without ever checking if it is available in the following line, considering that composer silently ignores invalid options.
   https://github.com/ansible-collections/community.general/blob/4ffb4f7f8e26514f98726d5a6e8f2d5748adb00a/plugins/modules/packaging/language/composer.py#L259

I guess this is something that could change in the future, so I think it's better to keep this.

I think the current patch looks good, except that it needs a changelog fragment.

@gangelop gangelop force-pushed the gangelop-composer-fix branch 2 times, most recently from 10cadca to b05dd23 Compare April 30, 2021 20:27
@@ -0,0 +1,2 @@
bugfixes:
- composer - add no-interaction option when discovering available options to avoid an issue where composer hangs (https://github.com/ansible-collections/community.general/pull/2348).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- composer - add no-interaction option when discovering available options to avoid an issue where composer hangs (https://github.com/ansible-collections/community.general/pull/2348).
- composer - use ``no-interaction`` option when discovering available options to avoid an issue where composer hangs (https://github.com/ansible-collections/community.general/pull/2348).

@felixfontein felixfontein added the check-before-release PR will be looked at again shortly before release and merged if possible. label Apr 30, 2021
The composer module always uses the no-interaction option if it
discovers it _after_ calling "composer help ..." but not on the help
call itself. The lack of this option caused composer to not exit when
called through the ansible module.

The same example command when ran interactively does not prompt for user
interaction and exits immediately. It is therefore currently unknown why
the same command hangs when called through the ansible composer module
or even directly with the command module.

Example command which hangs:
php /usr/local/bin/composer help install --format=json
@gangelop gangelop force-pushed the gangelop-composer-fix branch from b05dd23 to f9e8f95 Compare April 30, 2021 20:35
@gangelop
Copy link
Contributor Author

gangelop commented Apr 30, 2021

I FOUND IT!
2021-04-30-234950_625x100_scrot

root@ubuntu:~# php /usr/local/bin/composer help install --format=json
Do not run Composer as root/super user! See https://getcomposer.org/root for details
Continue as root/super user [yes]? 

This raises yet another issue: Maybe we should also warn ansible users to not run composer as root, considering composer is almost certainly always being run as root in ansible.

@felixfontein Do you still propose this behavior be changed upstream?
Based on my command-line UX instinct, I would have chosen to fail with a warning and require a flag like --yes-root-pls to force running as root. But maybe this is just a matter of design opinion.

@gangelop
Copy link
Contributor Author

I also confirm that while this reproducer hangs:

---
- hosts: gombuter
  gather_facts: false
  tasks:
    - name: reproducer
      composer:
        working_dir: /opt/gomboser

This completes succesfully:

---
- hosts: gombuter
  gather_facts: false
  tasks:
    - name: reproducer
      become: true
      become_user: ubuntu
      composer:
        working_dir: /opt/gomboser

The root warning prompt causes the hang.

@felixfontein
Copy link
Collaborator

They seriously have an interactive warning?? Even on something like help with --format=json? That's really bad CLI UX...

@aminvakil
Copy link
Contributor

They seriously have an interactive warning?? Even on something like help with --format=json? That's really bad CLI UX...

It seems like they seriously don't like users running it as root, even on something like help with --format=json :)

@felixfontein felixfontein merged commit eb455c6 into ansible-collections:main May 1, 2021
@patchback
Copy link

patchback bot commented May 1, 2021

Backport to stable-2: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-2/eb455c69a2c7f7ec28f6162e0c5a34f0bc7932e3/pr-2348

Backported as #2402

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request May 1, 2021
The composer module always uses the no-interaction option if it
discovers it _after_ calling "composer help ..." but not on the help
call itself. The lack of this option caused composer to not exit when
called through the ansible module.

The same example command when ran interactively does not prompt for user
interaction and exits immediately. It is therefore currently unknown why
the same command hangs when called through the ansible composer module
or even directly with the command module.

Example command which hangs:
php /usr/local/bin/composer help install --format=json

(cherry picked from commit eb455c6)
@patchback
Copy link

patchback bot commented May 1, 2021

Backport to stable-3: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-3/eb455c69a2c7f7ec28f6162e0c5a34f0bc7932e3/pr-2348

Backported as #2403

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein
Copy link
Collaborator

@gangelop thanks for fixing this!
@aminvakil thanks for reviewing!

patchback bot pushed a commit that referenced this pull request May 1, 2021
The composer module always uses the no-interaction option if it
discovers it _after_ calling "composer help ..." but not on the help
call itself. The lack of this option caused composer to not exit when
called through the ansible module.

The same example command when ran interactively does not prompt for user
interaction and exits immediately. It is therefore currently unknown why
the same command hangs when called through the ansible composer module
or even directly with the command module.

Example command which hangs:
php /usr/local/bin/composer help install --format=json

(cherry picked from commit eb455c6)
felixfontein pushed a commit that referenced this pull request May 1, 2021
…#2402)

The composer module always uses the no-interaction option if it
discovers it _after_ calling "composer help ..." but not on the help
call itself. The lack of this option caused composer to not exit when
called through the ansible module.

The same example command when ran interactively does not prompt for user
interaction and exits immediately. It is therefore currently unknown why
the same command hangs when called through the ansible composer module
or even directly with the command module.

Example command which hangs:
php /usr/local/bin/composer help install --format=json

(cherry picked from commit eb455c6)

Co-authored-by: George Angelopoulos <[email protected]>
felixfontein pushed a commit that referenced this pull request May 1, 2021
…#2403)

The composer module always uses the no-interaction option if it
discovers it _after_ calling "composer help ..." but not on the help
call itself. The lack of this option caused composer to not exit when
called through the ansible module.

The same example command when ran interactively does not prompt for user
interaction and exits immediately. It is therefore currently unknown why
the same command hangs when called through the ansible composer module
or even directly with the command module.

Example command which hangs:
php /usr/local/bin/composer help install --format=json

(cherry picked from commit eb455c6)

Co-authored-by: George Angelopoulos <[email protected]>
@gangelop
Copy link
Contributor Author

gangelop commented May 1, 2021

@felixfontein @aminvakil ty!

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug community_review has_issue language module module new_contributor Help guide this first time contributor packaging plugins plugin (any type) python3 small_patch Hopefully easy to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants