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

Error at the end of the build #473

Closed
andrei-mihaila opened this issue Oct 6, 2023 · 3 comments · Fixed by #474
Closed

Error at the end of the build #473

andrei-mihaila opened this issue Oct 6, 2023 · 3 comments · Fixed by #474

Comments

@andrei-mihaila
Copy link

The bug

Getting an error at the end of the build:

error: improper list

Usage:
 ps [options]

 Try 'ps --help <simple|list|output|threads|misc|all>'
  or 'ps --help <s|l|o|t|m|a>'
 for additional help text.

For more details see ps(1)

This probably just affects the message displayed at the end of the build ('You can activate this installation running the following command:') - not displayed.

Software versions

A list of software versions where the bug is apparent, as detailed as possible:

How to replicate

An ordered list of steps to replicate the bug, e.g.:

  1. run kerl build ...
  2. search for error: improper list in the error output

Expected behaviour

The error should not be happening and the right message should be displayed at the end of the build.

Additional context

I think the problem is on this line: 1462.
The code is running ps to get the parent PID (used later to determine if the current terminal is bash or zsh or other). The result of that line can contain leading spaces, for example:

$ ps -p $$ -o ppid=
 2970

On the next line ps expects a list of comma separated PIDs (no spaces). For example:

$ ps -p " 2970" -o ucomm | tail -n 1
error: improper list

Usage:
 ps [options]

 Try 'ps --help <simple|list|output|threads|misc|all>'
  or 'ps --help <s|l|o|t|m|a>'
 for additional help text.

For more details see ps(1).
# it works with the spaces removed
$ ps -p " 2970" -o ucomm | tail -n 1
bash

Thanks!

@paulo-ferraz-oliveira
Copy link
Contributor

paulo-ferraz-oliveira commented Oct 8, 2023

Thanks for reporting. I can fix this based on your input, and using a Fedora image. I can't replicate it locally, but probably because my process id is already so high that no space is output.

Edit: the error was introduced by me, here.

@paulo-ferraz-oliveira
Copy link
Contributor

@andrei-mihaila, I've pull requested to fix this. Would you care to check the proposed changes and let us know if they're effective for your use case: https://github.com/kerl/kerl/pull/474/files?

Thanks.

Ah, as a side note: I could just go back to the "old" code and add the shellcheck disable, but this way it's clearer as to what our intentions are. (the lack of comment there was what led me to believe we we're "just avoiding" ShellCheck).

@andrei-mihaila
Copy link
Author

Tested and it works. Thank you!

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

Successfully merging a pull request may close this issue.

2 participants