search

containers / buildah

A tool that facilitates building OCI images.
https://buildah.io
Apache License 2.0
7.3k stars 771 forks source link

Excess backslashes in man page markdown; guidance requested re making a fix. #4986

Closed PeterWhittaker closed 9 months ago

PeterWhittaker commented 1 year ago

NOTE FROM AUTHOR I'm opening this to get guidance re how best to tackle this. This seems to be a common problem, but in some cases the apparently extraneous backslashes are ignored by troff, while in other cases they are displayed to the user. I'm quite happy to craft a sed script to fix the problem, but I don't want to introduce new problems, so I'm seeking guidance as to why these extra backslashes are there (escaping troff codes, perhaps?) and how best to tackle this.

Thoughts?

@edsantiago Tagging you as an active user who might know best whom to ping. Thanks!

Description

Several of the man page files contain unnecessary backslashes that are displayed when the page is output via man. Cf, for example, the output of the following command run in the docs folder:

$ git grep RUNTIME | grep '\\'
buildah-build.1.md:Note: You can also override the default runtime by setting the BUILDAH\_RUNTIME
buildah-commit.1.md:Path of the authentication file. Default is ${XDG_\RUNTIME\_DIR}/containers/auth.json. If XDG_RUNTIME_DIR is not set, the default is /run/containers/$UID/auth.json. This file is created using `buildah login`.
buildah-from.1.md:  An image in a registry implementing the "Docker Registry HTTP API V2". By default, uses the authorization state in `$XDG\_RUNTIME\_DIR/containers/auth.json`, which is set using `(buildah login)`.  If XDG_RUNTIME_DIR is not set, the default is /run/containers/$UID/auth.json. If the authorization state is not found there, `$HOME/.docker/config.json` is checked, which is set using `(docker login)`.
buildah-from.1.md:Path of the authentication file. Default is ${XDG_\RUNTIME\_DIR}/containers/auth.json. If XDG_RUNTIME_DIR is not set, the default is /run/containers/$UID/auth.json. This file is created using `buildah login`.
buildah-login.1.md:flag. The default path used is **${XDG\_RUNTIME_DIR}/containers/auth.json**. If XDG_RUNTIME_DIR
buildah-login.1.md:Path of the authentication file. Default is ${XDG_\RUNTIME\_DIR}/containers/auth.json. If XDG_RUNTIME_DIR is not set, the default is /run/user/$UID/containers/auth.json. This file is created using `buildah login`.
buildah-logout.1.md:The default path used is **${XDG\_RUNTIME_DIR}/containers/auth.json**.  If XDG_RUNTIME_DIR is not set, the default is /run/containers/$UID/auth.json.
buildah-logout.1.md:Path of the authentication file. Default is ${XDG_\RUNTIME\_DIR}/containers/auth.json.  If XDG_RUNTIME_DIR is not set, the default is /run/user/$UID/containers/auth.json.
buildah-manifest-add.1.md:Path of the authentication file. Default is ${XDG_\RUNTIME\_DIR}/containers/auth.json. If XDG_RUNTIME_DIR is not set, the default is /run/containers/$UID/auth.json. This file is created using `buildah login`.
buildah-manifest-inspect.1.md:Path of the authentication file. Default is ${XDG\_RUNTIME\_DIR}/containers/auth.json, which is set using `buildah login`.
buildah-manifest-push.1.md:Path of the authentication file. Default is ${XDG\_RUNTIME\_DIR}/containers/auth.json, which is set using `buildah login`.
buildah-pull.1.md:Path of the authentication file. Default is ${XDG_\RUNTIME\_DIR}/containers/auth.json. If XDG_RUNTIME_DIR is not set, the default is /run/containers/$UID/auth.json. This file is created using `buildah login`.
buildah-push.1.md:Path of the authentication file. Default is ${XDG_\RUNTIME\_DIR}/containers/auth.json. If XDG_RUNTIME_DIR is not set, the default is /run/containers/$UID/auth.json. This file is created using `buildah login`.
buildah-run.1.md:Note: You can also override the default runtime by setting the BUILDAH\_RUNTIME

Steps to reproduce the issue:

  1. man builds-build
  2. search for RUNTIME, e.g., /RUNTIME
  3. Under --authfile, there is a baskslash in front of the first instance of RUN: ...Default is ${XDG_\RUN...

Describe the results you received:

Cf above.

Describe the results you expected:

No extraneous backslashes

Output of rpm -q buildah or apt list buildah:

buildah-1.27.3-1.el9_1.x86_64

Output of buildah version:

Version:         1.27.3
Go Version:      go1.18.4
Image Spec:      1.0.2-dev
Runtime Spec:    1.0.2-dev
CNI Spec:        1.0.0
libcni Version:  v1.1.2
image Version:   5.22.1
Git Commit:      
Built:           Mon Jan 23 10:38:35 2023
OS/Arch:         linux/amd64
BuildPlatform:   linux/amd64

Output of podman version if reporting a podman build issue:

(paste your output here)

*Output of `cat /etc/release`:**

AlmaLinux release 9.1 (Lime Lynx)
NAME="AlmaLinux"
VERSION="9.1 (Lime Lynx)"
ID="almalinux"
ID_LIKE="rhel centos fedora"
VERSION_ID="9.1"
PLATFORM_ID="platform:el9"
PRETTY_NAME="AlmaLinux 9.1 (Lime Lynx)"
ANSI_COLOR="0;34"
LOGO="fedora-logo-icon"
CPE_NAME="cpe:/o:almalinux:almalinux:9::baseos"
HOME_URL="https://almalinux.org/"
DOCUMENTATION_URL="https://wiki.almalinux.org/"
BUG_REPORT_URL="https://bugs.almalinux.org/"

ALMALINUX_MANTISBT_PROJECT="AlmaLinux-9"
ALMALINUX_MANTISBT_PROJECT_VERSION="9.1"
REDHAT_SUPPORT_PRODUCT="AlmaLinux"
REDHAT_SUPPORT_PRODUCT_VERSION="9.1"
AlmaLinux release 9.1 (Lime Lynx)
AlmaLinux release 9.1 (Lime Lynx)

Output of uname -a:

Linux lapa.sphyrna.ca 5.14.0-162.23.1.el9_1.x86_64 #1 SMP PREEMPT_DYNAMIC Tue Apr 11 10:43:28 EDT 2023 x86_64 x86_64 x86_64 GNU/Linux

Output of cat /etc/containers/storage.conf:

[storage]
driver = "overlay"
runroot = "/run/containers/storage"
graphroot = "/var/lib/containers/storage"
[storage.options]
additionalimagestores = [
]
pull_options = {enable_partial_images = "false", use_hard_links = "false", ostree_repos=""}
[storage.options.overlay]
mountopt = "nodev,metacopy=on"
[storage.options.thinpool]
edsantiago commented 1 year ago

Thank you for reporting. It looks like a lot of the XDG strings are broken. Would you like to open a PR to fix them? A quick oneliner could be:

$ sed -i -e 's/XDG_\\RUNTIME\\_DIR/XDG_RUNTIME_DIR/' docs/*.md

That leaves one XDG unfixed (in buildah-from), but that's easy to fix by hand. And there's the broken BUILDAH_FORMAT in buildah-commit, also fixable by hand.

A quick oneliner to find them all is:

$ for i in docs/*\.[1-9];do x=$(man -l $i | grep '\\');if [[ -n "$x" ]]; then printf "\n%s\n%s\n" "$i" "$x";fi;done

Once that comes up empty, profit! I mean, do some manual double-checking, and submit a PR! (If you'd like to).

Thanks again.

PeterWhittaker commented 1 year ago

My first cut was to do a very similar sed script, but then I noticed a few other stray \, hence the issue. For example, if you run git grep '\\' docs/*.md, you'll see an awful lot, e.g., in buildah\-rm in buildah-rm.1.md, BUILD\_REGISTRY\_SOURCES in buildah-build.1.md. They all look like attempts to escape - and _, neither of which are necessary AFAICT.

But only AFAICT. Hence the issue. :->

I'm quite happy to fix only the various XDG_RUNTIME_DIR ones, and leave the others, since they appear to be harmless, but I wanted to confirm that as a direction.

As I see it, there are three alternatives:

  1. Fix only XDG_RUNTIME_DIR and leave the others.
  2. Fix XDG_RUNTIME_DIR and open an issue re the others (I doubt I'll get to those); that will be work for someone else somewhere down the line.
  3. Do all of them (probably won't be me...).

I think that 1 or 2 are the reasonable ones right now (given I don't have time for 3).

Let me know your preference, and, if it's 1 or 2, I'll get it done soon.

(While we are here: I've noticed that some of the md files, e.g. buildah-build, have odd formatting that causes the text of the note to not reflow properly. E.g., man buildah-build in a wide terminal with COLUMN_WIDTH set to 72 (alias man='MANWIDTH=$((COLUMNS > 72 ? 72 : COLUMNS)) man' will do it): The note re not combining O and Z will not be wrapped at 72 because the troff code is .nf. I have no idea how to fix this one. Do you want to handle in herein, or open a separate issue for it?)

edsantiago commented 1 year ago

I'm not seeing a strong reason to fix all backslashes: that would make a bigger PR, harder to review, harder to confirm in all cases (some of the backslashes are--or were thought to be--necessary to avoid italicizing the HTML results). Focusing on the clearly-bad *roff output is a nice smaller step, with an easily-seen good result (no confusing backslashes in user-visible man pages). So, a definite 1 for me!

Wrap: oof. Another nice catch, thank you for reporting that. I see the problem, and it shouldn't be hard to fix but it'll be tedious. If you don't mind opening an issue and tagging me, I would appreciate that very much.

github-actions[bot] commented 1 year ago

A friendly reminder that this issue had no activity for 30 days.

rhatdan commented 11 months ago

@PeterWhittaker Still working on the PR?

PeterWhittaker commented 11 months ago

@PeterWhittaker Still working on the PR?

@rhatdan Planning on getting back to it, hopefully early October. Work then life went redline....

github-actions[bot] commented 10 months ago

A friendly reminder that this issue had no activity for 30 days.

rhatdan commented 10 months ago

@PeterWhittaker reminder

github-actions[bot] commented 9 months ago

A friendly reminder that this issue had no activity for 30 days.

rhatdan commented 9 months ago

@PeterWhittaker Friendly ping