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

podman images: not newline-terminated #2388

Closed
edsantiago opened this issue Feb 20, 2019 · 8 comments

Comments

Projects
None yet
3 participants
@edsantiago
Copy link
Collaborator

commented Feb 20, 2019

podman images, with a non-json output format, omits the final newline:

$ podman images | cat
REPOSITORY                 TAG      IMAGE ID       CREATED       SIZE
docker.io/library/alpine   latest   caf27325b298   2 weeks ago   5.8 MB$ <<<-- this is my prompt

This would seem like a minor nit, except it might greatly surprise anyone who does:

$ podman images --all --format '{{.Repository}}:{{.Tag}}\t{{.ID}}' | while read name id;do echo "got here <$name> <$id>";done

If we have one image, nothing is output. Even more confusingly, if there's more than one, $id (the last element of the line) includes a spurious newline: (never mind: that was an artifact of some debugging I was trying)

(Note to reviewer: I thought I had filed an issue about this months ago but can't find it now).

podman-1.0.0-1.git82e8011.fc29.x86_64

@mheon mheon added the kind/bug label Feb 20, 2019

@rhatdan

This comment has been minimized.

Copy link
Member

commented Feb 20, 2019

The code specifically does this now. It checks if output is a tty and if it is executes a printf("\n") otherwise it does not. And since you are piping to cat, it does not do it.

baude added a commit to baude/libpod that referenced this issue Feb 20, 2019

add newline to images output
ensure a final newline is always added to images output.

fixes containers#2388

Signed-off-by: baude <bbaude@redhat.com>
@rhatdan

This comment has been minimized.

Copy link
Member

commented Feb 20, 2019

//Out method for Go templates
func (t StdoutTemplate) Out() error {
	tmpl, err := template.New("image").Parse(t.Template)
	if err != nil {
		return errors.Wrapf(err, "template parsing error")
	}
	err = tmpl.Execute(os.Stdout, t.Output)
	if err != nil {
		return err
	}
	humanNewLine()
	return nil
}
@rhatdan

This comment has been minimized.

Copy link
Member

commented Feb 20, 2019

@edsantiago Do you think we should not check for Humans on Stdout Templates and always print the newline?
@baude Do you agree?

@mheon

This comment has been minimized.

Copy link
Collaborator

commented Feb 20, 2019

Did we add this in response to another bug? I can't think of another reason it would be there

@edsantiago

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 20, 2019

@rhatdan I strongly think output should be consistently newline-terminated irrespective of whether stdout is a tty. See my first comment about piping through a while loop: the last line of output will be silently ignored.

FWIW I seem to have found in my records a note about reporting this in ps back in October; it got fixed, but I can't find a PR for it. (Note that this means that ps behaves correctly right now; images does not)

@rhatdan

This comment has been minimized.

Copy link
Member

commented Feb 20, 2019

Ps has this line
fmt.Fprint(w, "\n")

I have no problem adding it.

@mheon

This comment has been minimized.

Copy link
Collaborator

commented Feb 20, 2019

I did a quick search, and I can't find any issues that relate to this, so I have no idea why we added it in the first place.

Given that, I'm fine with removing the TTY check.

@baude @rhatdan We really need to start forcing bugfix commits to add comments with links to the issue number so we don't start reverting bugfixes accidentally

@edsantiago

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 20, 2019

Further info: it's a POSIX thing:

3.206 Line

A sequence of zero or more non- <newline> characters plus a terminating <newline> character.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.