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

WIP: [CI:DOCS] Refactor common options in man pages #14931

Closed
wants to merge 1 commit into from

Conversation

edsantiago
Copy link
Member

Many options are common to multiple commands and man pages,
e.g., podman run and create have countless identical options.
To date, those have been documented via copy/pasting, and
(shudder) maintaining, across multiple .md files.

Solution: add an 'include' mechanism such that multiple
md source files can fetch from a common file documenting
an option.

See discussion #13435

Signed-off-by: Ed Santiago [email protected]

Consolidated descriptions of common options across man pages

@edsantiago edsantiago added the kind/documentation Categorizes issue or PR as related to documentation. label Jul 13, 2022
@openshift-ci openshift-ci bot added release-note do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jul 13, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 13, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 13, 2022
@edsantiago
Copy link
Member Author

@eriksjolund PTAL. This is a proof-of-concept only. I tried to consolidate --volumes, which is kind-of-shared across build, create, run, but it would take me hours to deduplicate that and clean up. I don't want to waste effort, so am submitting this as a shoot-me-down-early. If you or anyone finds a reason why this won't work, please nix it soon.

@eriksjolund
Copy link
Contributor

eriksjolund commented Jul 14, 2022

Cool, it would be great to have include machinery! I don't know make and sed well enough to really have any opinion on the implementation.

I assume you mean --volume (not --volumes). Isn't podman build vendored in from https://github.com/containers/buildah/blob/main/docs/buildah-build.1.md ?
This would mean that the include machinery needs to be enabled in buildah too.

I ran

 egrep -r 'source/markdown/.*md'

and noticed that there is for instance a link to podman-create.1.md that would stop working after renaming the file to podman-create.1.md.in

| `docker create` | [`podman create`](./docs/source/markdown/podman-create.1.md) |

I guess that could be solved by linking to Podman.io instead.

@edsantiago
Copy link
Member Author

Yes, --volume, singular, sorry. No, podman build doc has nothing to do with buildah build except as Inspired By. podman build doc is hand-maintained. And, the link, no, it's fine, because .md.in.md.

@edsantiago
Copy link
Member Author

Hey everyone, I'm starting to really like this approach. Look at all I've been able to deduplicate, and I've barely gotten started!

This push is the result of me writing a couple of scripts to automate the work: one to find options common across lots of man pages, one to compare the option descriptions and, if 100% identical, write an options/foo.md file and edit the man pages to add an @@option link. This is the result of mostly just doing the low-hanging fruit, the super-easy identical or almost-identical stuff.

There are still a TON of almost-identical ones where the diff is something like using backticks in one page, stars in another. Those are trivial to fix.

There are also a bunch where the difference is in wording: 'pod' vs 'container'. Those are more difficult, and I don't plan to address them now. Even without that, I think we can make a huge difference in man-page maintenance by using this.

Once again, please shoot down early. I've spent about two more hours on it today, not a huge investment. Doing the manual difference reconciliation will take lots more hours, so I'd rather not do that if this approach is doomed.

@edsantiago
Copy link
Member Author

For posterity, here's the current result of find-dup-options. Obviously not all of these can be deduplicated. Format is: option, followed by a list of two or more man pages.

--annotation: build create kube-play manifest-add manifest-annotate run
--arch: build create import manifest-add manifest-annotate pull run
--attach: create run start
--authfile: auto-update build container-runlabel create image-sign kube-play login logout manifest-add manifest-push pull push run search
--blkio-weight: create run
--blkio-weight-device: create run
--cap-add: build create run
--cap-drop: build create run
--cert-dir: build container-runlabel image-sign kube-play login manifest-add manifest-push pull push
--cgroup-parent: build create pod-clone pod-create run
--cgroupns: build create run
--cgroups: create run
--change: commit import
--cidfile: create kill rm run stop
--color: logs pod-logs
--compress: build container-checkpoint push save
--conmon-pidfile: create run
--connection: podman remote
--cpu-period: build container-clone create run
--cpu-quota: build container-clone create run
--cpu-rt-period: container-clone create run
--cpu-rt-runtime: container-clone create run
--cpu-shares: build container-clone create run
--cpus: container-clone create machine-init machine-set pod-clone pod-create run
--cpuset-cpus: build container-clone create pod-clone pod-create run
--cpuset-mems: build container-clone create run
--creds: build container-runlabel kube-play manifest-add manifest-push pull push
--destroy: container-clone pod-clone
--detach: exec run
--detach-keys: attach exec run start
--device: build create pod-clone pod-create run
--device-cgroup-rule: create run
--device-read-bps: create pod-clone pod-create run
--device-read-iops: create run
--device-write-bps: create run
--device-write-iops: create run
--digestfile: manifest-push push
--disable-content-trust: build create pull push run
--disk-size: machine-init machine-set
--dns: build create pod-create run
--dns-opt: create pod-create run
--dns-search: build create pod-create run
--driver: network-create secret-create volume-create
--env: build create exec run secret-create
--env-file: create exec run
--external: container-exists image-prune ps
--features: manifest-add manifest-annotate
--file: build completion
--file-locks: container-checkpoint container-restore
--filter: container-prune events image-prune images network-ls network-prune pod-ps ps search secret-ls start stop system-prune volume-ls volume-prune
--follow: logs pod-logs
--force: container-clone container-prune image-prune image-unmount machine-rm network-disconnect network-prune network-rm pod-prune pod-rm rm rmi system-prune system-reset unmount volume-prune volume-rm
--format: auto-update build commit container-diff container-inspect diff events generate-systemd history image-diff image-inspect image-mount images info inspect machine-info machine-inspect machine-list manifest-push mount network-inspect network-ls pod-inspect pod-ps pod-stats ps push save search secret-inspect secret-ls stats system-connection-list system-df version volume-inspect volume-ls
--gidmap: create pod-clone pod-create run
--health-cmd: create run
--health-interval: create run
--health-retries: create run
--health-start-period: create run
--health-timeout: create run
--help: build container-runlabel create events export healthcheck-run history image-exists image-prune image-scp image-sign image-tree image-trust import kube-play load login logout machine-info machine-init machine-inspect machine-list machine-rm machine-set machine-ssh machine-start machine-stop manifest-exists network-exists pod-clone pod-create pod-ps pod-top podman ps pull remote run save search secret-create secret-inspect secret-rm system-prune system-reset system-service tag top unshare untag volume-create volume-exists volume-export volume-inspect volume-ls volume-prune volume-rm wait
--hostname: create pod-clone pod-create run
--http-proxy: build create run
--identity: podman remote system-connection-add
--ignore: pod-rm pod-stop rm rmi stop
--ignore-rootfs: container-checkpoint container-restore
--ignore-volumes: container-checkpoint container-restore
--iidfile: build commit
--infra-command: pod-clone pod-create
--infra-conmon-pidfile: pod-clone pod-create
--infra-name: pod-clone pod-create
--interactive: create exec run start
--interval: stats wait
--ip: create kube-play network-connect pod-create run
--ip6: create network-connect pod-create run
--ipc: build create run
--keep: container-checkpoint container-restore
--label: build create network-create pod-clone pod-create run volume-create
--label-file: create pod-clone pod-create run
--link-local-ip: create run
--log-driver: create kube-play run
--log-level: podman remote
--log-opt: create kube-play run
--mac-address: create kube-play network-connect pod-create run
--memory: build container-clone create machine-init machine-set pod-clone pod-create run
--memory-reservation: container-clone create run
--memory-swap: build container-clone create run
--memory-swappiness: container-clone create run
--message: commit import
--name: container-clone container-restore container-runlabel create generate-systemd pod-clone pod-create run
--names: logs pod-logs
--namespace: podman ps
--network: build create kube-play pod-create run
--network-alias: create pod-create run
--no-hosts: build create kube-play pod-create run
--no-reset: pod-stats stats
--no-stream: pod-stats stats
--no-trunc: events history images mount network-ls pod-ps ps search
--noheading: image-trust images machine-list network-ls pod-ps ps secret-ls volume-ls
--oom-score-adj: create run
--opt: network-create volume-create
--os: build create import manifest-add manifest-annotate pull run
--os-version: build manifest-add manifest-annotate
--output: build export save volume-export
--pid: build create pod-clone pod-create run
--platform: build create pull run
--pod: container-clone container-restore create ps run
--pod-id-file: create pod-create pod-rm pod-start pod-stop run
--print-stats: container-checkpoint container-restore
--publish: container-restore create pod-create run
--publish-all: create run
--pull: build create run
--read-only-tmpfs: create run
--remove-signatures: manifest-push push
--replace: container-runlabel create kube-play pod-create run
--requires: create generate-systemd run
--restart: create run
--rm: build container-cleanup create manifest-push run
--rmi: container-cleanup run
--rootful: machine-init machine-set
--runtime: build podman
--security-opt: build create pod-clone pod-create run
--shm-size: build create pod-clone pod-create run
--sig-proxy: attach run start
--sign-by: build image-sign manifest-push push
--signal: kill pod-kill
--since: events logs pod-logs
--size: container-inspect inspect ps
--sort: images pod-ps ps
--squash: build commit
--start: kube-play pod-clone
--stop-timeout: create generate-systemd run
--subgidname: create pod-clone pod-create run
--subuidname: create pod-clone pod-create run
--sysctl: create pod-clone pod-create run
--tail: logs pod-logs
--tcp-established: container-checkpoint container-restore
--time: network-rm pod-rm pod-stop restart rm stop system-service volume-rm
--timestamps: logs pod-logs
--tls-verify: build container-runlabel create kube-play login manifest-add manifest-push pull push run search
--tmpfs: create run
--tty: create exec run
--type: image-trust inspect
--uidmap: create pod-clone pod-create run
--until: events logs pod-logs
--url: podman remote
--user: create exec run
--username: login machine-ssh
--userns: build create kube-play pod-clone pod-create run
--uts: build create pod-clone pod-create run
--variant: build create import manifest-add manifest-annotate pull run
--verbose: login system-df
--version: podman remote
--volume: build create machine-init pod-clone pod-create run
--volumes: rm system-prune
--volumes-from: create pod-clone pod-create run
--workdir: create exec run

@rhatdan
Copy link
Member

rhatdan commented Jul 15, 2022

I might be mistaken, but with this change there is no longer a fully assembled man page on github for each man page correct? If this is correct, this would be a problem since these are key for read the docs and people perusing this website.

@edsantiago
Copy link
Member Author

That's true: some man pages would need to be assembled. If this is a showstopper, I'll close this.

@rhatdan
Copy link
Member

rhatdan commented Jul 15, 2022

Well would it be worth it to have the man pages updated and committed. Basically have

podman-create.md created from podman-create.md.in and both committed to the repo.
If podman-create.md is updated without an update of the lower level code, then this is a bug.

@Luap99
Copy link
Member

Luap99 commented Jul 16, 2022

We should only point people at https://docs.podman.io and not github directly. As long as the build process makes sure readthedocs can still build the page after this change we should be fine.

Is there any reason that we have to keep using markdown? This feels like a workaround of missing features.
We could also directly use restructuredtext which allows us to include external files, https://docutils.sourceforge.io/docs/ref/rst/directives.html#including-an-external-document-fragment.
docs.podman.io(readthedocs) uses restructuredtext with sphinx so the build process for the page already converts markdown to restructuredtext.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 23, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 1, 2022
Many options are common to multiple commands and man pages,
e.g., podman run and create have countless identical options.
To date, those have been documented via copy/pasting, then
not-quite-maintaining, across multiple .md files.

Solution: add an 'include' mechanism such that multiple
md source files can fetch from a common file documenting
an option.

See discussion containers#13435

Signed-off-by: Ed Santiago <[email protected]>
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 7, 2022
@openshift-merge-robot
Copy link
Collaborator

@edsantiago: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@edsantiago
Copy link
Member Author

This was solved in #15174. I kept this PR open because of Paul's question:

Is there any reason that we have to keep using markdown? This feels like a workaround of missing features. ... [use rst instead]

It's a good question, and I looked into rst back then but migrating is way, way beyond my abilities. If such is desired, it deserves a separate issue or discussion (or even PR). Given the traction on the @@option scheme, which seems to be working better than I'd dreamed, I will close this obsolete PR.

@edsantiago edsantiago closed this Aug 24, 2022
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/documentation Categorizes issue or PR as related to documentation. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants