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

UX bug: Show a difference between zero args and empty args #1283

Open
schneems opened this issue Sep 13, 2021 · 14 comments
Open

UX bug: Show a difference between zero args and empty args #1283

schneems opened this issue Sep 13, 2021 · 14 comments
Assignees
Labels
good first issue A good first issue to get started with. help wanted Need some extra hands to get this done. status/ready Issue ready to be worked on. type/enhancement Issue that requests a new feature or improvement.

Comments

@schneems
Copy link

Summary

I ran into an issue where (due to a buildpack version difference) one buildpack was working and one wasn't. I figured out the problem eventually, but the core issue that made debugging harder than it had to be was that lifecycle currently shows the same output for empty args [] as it does for args that contain no characters [""].

$ pack inspect cutlass_image_basic_bash | grep -B2 web
Processes:
  TYPE                 SHELL        COMMAND        ARGS
  web (default)        bash         while true; do echo 'lol'; sleep 2; done

Note that there's nothing under the "args" section, and it collapses into the command.

Here's my original notes and investigation https://github.com/Malax/libcnb.rs/issues/49

Reproduction

Steps

Make a base buildpack:

cd /tmp
mkdir my_buildpack && cd my_buildpack
mkdir -p bin

cat > buildpack.toml <<EOM

api = "0.4"
[buildpack]
id = "ultra basic invoker"
version = "0.0.0"
name = "ultra basic invoker"

[[licenses]]
type = "MIT"

[[stacks]]
id = "heroku-20"

[metadata]
EOM

cat > bin/detect <<EOM
#!/usr/bin/env bash

exit 0
EOM

Write a build script with non-empty args

cat > bin/build <<EOM
#!/usr/bin/env bash

layers_dir=\$1

cat >> "\${layers_dir}/launch.toml" <<EOL
[[processes]]
type = "web"
command = "while true; do echo 'lol'; sleep 2; done"
args = [""]  # Non-empty args
direct = false
EOL
EOM
chmod +x bin/build
chmod +x bin/detect

Build it:

pack build debug_pack_inspect_launch_args --path . -B heroku/buildpacks:20 --buildpack ./  -v

Produces:

$ pack inspect debug_pack_inspect_launch_args | grep -B2 web
Processes:
  TYPE                 SHELL        COMMAND        ARGS
  web (default)        bash         while true; do echo 'lol'; sleep 2; done

Modify to empty args in the launch:

cat > bin/build <<EOM
#!/usr/bin/env bash

layers_dir=\$1

cat >> "\${layers_dir}/launch.toml" <<EOL
[[processes]]
type = "web"
command = "while true; do echo 'lol'; sleep 2; done"
args = []  # EMPTY args
direct = false
EOL
EOM

Build it:

pack build debug_pack_inspect_launch_args --path . -B heroku/buildpacks:20 --buildpack ./  -v

Produces:

$ pack inspect debug_pack_inspect_launch_args | grep -B2 web
Processes:
  TYPE                 SHELL        COMMAND        ARGS
  web (default)        bash         while true; do echo 'lol'; sleep 2; done

So even though the image produced is not identical, the output of pack inspect is.

Current behavior

Pack inspect does shows identical output when there are no args versus when there are args with no characters.

Expected

I expected the two to produce an observable/inspectable difference in outputs because they are different (one executes, and the other does not).

I propose possibly adding arg count to the output such as:

Processes:
  TYPE                 SHELL        COMMAND                                         ARGS (count = 0)
  web (default)        bash         while true; do echo 'lol'; sleep 2; done        

Or possibly showing non-visible args:

Processes:
  TYPE                 SHELL        COMMAND                                         ARGS (count = 1)
  web (default)        bash         while true; do echo 'lol'; sleep 2; done     ""

Context

lifecycle version

Lifecycle:
Version: 0.11.4

platform version(s)
$ pack report
Pack:
  Version:  0.20.0+git-66a4f32.build-2668
  OS/Arch:  darwin/amd64

Default Lifecycle Version:  0.11.3

Supported Platform APIs:  0.3, 0.4, 0.5, 0.6

Config:
(no config file found at /Users/rschneeman/.pack/config.toml)
@natalieparellano
Copy link
Member

@schneems thanks for reporting this! I suspect the root issue is in pack, as pack is reading the processes information from the io.buildpacks.build.metadata label and on the app that I tested, I see \"args\":[\"\"] there. I could move it over to the pack repo if there are no objections.

@schneems
Copy link
Author

@natalieparellano thank you, if you can move it over that would be amazing!

@natalieparellano natalieparellano transferred this issue from buildpacks/lifecycle Sep 13, 2021
@edmorley
Copy link
Contributor

edmorley commented Sep 13, 2021

Instead of a count of the args (which seems a bit out of place in all but the zero args case), another option might be to show <NONE> or similar in the no-args case.

@schneems Also, I wonder if (as part of another issue) lifecycle should forbid empty string args given they seem to not be able to successfully launch the command, from what you found the other day?

@schneems
Copy link
Author

Also, I wonder if (as part of another issue) lifecycle should forbid empty string args given they seem to not be able to successfully launch the command, from what you found the other day?

For my case that would be great. I think there might be another case where someone writes a bash tool that needs an empty arg. If we raised/disallowed an empty arg then also including some kind of escape valve. For example allow_empty_arg: true in the launch.toml

@natalieparellano natalieparellano added type/enhancement Issue that requests a new feature or improvement. help wanted Need some extra hands to get this done. good first issue A good first issue to get started with. status/ready Issue ready to be worked on. labels Aug 21, 2023
@rizul2108
Copy link
Contributor

I want to work on this issue. @natalieparellano can you please assign this issue to me ?

@jjbustamante
Copy link
Member

Hi @rizul2108 thanks for helping us out

@jjbustamante
Copy link
Member

@rizul2108 before you try to fix this one, could you try to reproduce it with the latest main branch version? We just merged this PR, that I think it could be related but I didn't check. I do not want you to waste your time

@rizul2108
Copy link
Contributor

Ok sure I will check once

@rizul2108
Copy link
Contributor

image
Screenshot from 2024-01-27 18-12-13
yes it is still giving same output

@rizul2108
Copy link
Contributor

so i should start work on fixing this this error? @jjbustamante

@jjbustamante
Copy link
Member

Hi @rizul2108 , yes!

I like the idea suggested by Ed here, to use <NONE> with no-args case

@rizul2108
Copy link
Contributor

Okk sure I will work on it

@jjbustamante jjbustamante added this to the 0.34.0 milestone Feb 5, 2024
@jjbustamante
Copy link
Member

Hi @rizul2108!

Are you still working on this one?

@jjbustamante jjbustamante modified the milestones: 0.34.0, 0.35.0 Mar 8, 2024
@natalieparellano natalieparellano removed this from the 0.35.0 milestone Jul 1, 2024
@keshavdalmia10
Copy link

Hi @natalieparellano , @jjbustamante i am starting to work in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A good first issue to get started with. help wanted Need some extra hands to get this done. status/ready Issue ready to be worked on. type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

No branches or pull requests

6 participants