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

Add crun.1 file to dist tar ball #425

Closed
wants to merge 1 commit into from
Closed

Add crun.1 file to dist tar ball #425

wants to merge 1 commit into from

Conversation

kloczek
Copy link

@kloczek kloczek commented Jul 2, 2020

I think that forcing to have to go based tool only to generate man page
on build stage is kind of overkill. Usually, roff man pages are part of
the dist tar ball. So this really does this.
Another small correction is that automake automatically can choose
right man directory based on the extension so all man pages
can be put in one variable man_MANS.

I think that forcing to have to go based tool only to generate man page
on build stage is kind of overkill. Usually, roff man pages are part of
the dist tar ball. So this really does this.
Another small correction is that automake automatically can choose
right man<num> directory based on the extension so all man pages
can be put in one variable man_MANS.
@giuseppe
Copy link
Member

giuseppe commented Jul 3, 2020

I am fine with the change, just should we add crun.1 to the git repo as well?

Otherwise we will break the build for systems where go-md2man is not available

@kloczek
Copy link
Author

kloczek commented Jul 3, 2020

No, no .. that change will cause that in dist tar ball will be included generated man page.
No other changes are needed.because man_MANS files are automatically added to dist tar ball :)
Looks like you we've been doing make distcheck with disabled use md2man and that caused that this conditional part was not active.
Now with moved man1_MANS (and now man_MANS) that file will be always included :)

@giuseppe
Copy link
Member

giuseppe commented Jul 3, 2020

but how will the crun.1 file be generated without go-md2man?

It is currently guarded with if HAVE_MD2MAN and you'd like to move it outside of this block?

@giuseppe
Copy link
Member

giuseppe commented Jul 3, 2020

with your patch, if I try to build without go-md2man now I get:

make[2]: *** No rule to make target 'crun.1', needed by 'all-am'.  Stop.

@giuseppe
Copy link
Member

giuseppe commented Jul 3, 2020

I agree it is painful to require go just to generate the man page. That is why I suggest we add it to the git repo. It will significantly simplify the nix builder as well

@kloczek
Copy link
Author

kloczek commented Jul 3, 2020

You need to have installed go-md2man to allow autoconf to be able generate crun.1 man page.

To be honest I think tht you shoud switch crun source file from md to docbook because I see first ever project which is using md as refference source form of the documentation.
If you want I can try to transform md to xml/docbook which is quite often used and than to generate roff file will be possible to use xsltproc.

If you want to stay with md you need to have installed md2man on your system where you are generating dist tar ball.
If you decide switch to xml/docbook I can prepara patch for such transition.

@giuseppe
Copy link
Member

giuseppe commented Jul 3, 2020

You need to have installed go-md2man to allow autoconf to be able generate crun.1 man page.

if we add the crun.1 file to the git repo, only the developer who changes the .md file requires go-md2man installed. The crun.1 file would be like any other file in the repository and we can drop the go-md2man dependency altogether.

I think the markdown format is nicer to write than xml and renders directly on github

@kloczek
Copy link
Author

kloczek commented Jul 3, 2020

Sometimes it is not about niceness but keep use some standards :)

Again I can offer my time to transform that to xml.
You can peak on one of my projects to see how such docbook man page can look like https://github.com/kloczek/zapish

@giuseppe
Copy link
Member

giuseppe commented Jul 3, 2020

thanks for the offer but doesn't really change my idea about xml :-) most of the documentation in the containers ecosystem is written in markdown, and that is the main reason why I've picked it. An additional point is that it renders directly in github (https://github.com/containers/crun/blob/master/crun.1.md). If it wasn't for these two reasons, I'd have probably picked texinfo.

@kloczek
Copy link
Author

kloczek commented Jul 3, 2020

Using xslt is possible to generate md from docbook :P

@giuseppe
Copy link
Member

giuseppe commented Jul 4, 2020

Using xslt is possible to generate md from docbook :P

we'd still need to add the .md file to the git repository in order to display it in github

@giuseppe
Copy link
Member

@kloczek are you still working on this?

@kloczek
Copy link
Author

kloczek commented Jul 24, 2020

Sorry I was busy.

@giuseppe
Copy link
Member

let's just add crun.1 to the git repository and make sure it is part of EXTRA_DIST.

Other users are hitting the issue: #441 (comment)

@kloczek
Copy link
Author

kloczek commented Jul 30, 2020

let's just add crun.1 to the git repository and make sure it is part of EXTRA_DIST.

Exerything which is *_DATA or *_MANS is automatically added to dist tar ball so use here EXTRA_DIST is redundant, and to install it in correct place you needd to place it in <some>_MANS (where exactly exact man page is installed depends on file extension).

@giuseppe
Copy link
Member

Exerything which is *_DATA or *_MANS is automatically added to dist tar ball so use here EXTRA_DIST is redundant, and to install it in correct place you needd to place it in <some>_MANS (where exactly exact man page is installed depends on file extension).

thanks, that works for me.

@rhatdan
Copy link
Member

rhatdan commented Aug 6, 2020

I don't think that this still needed now that the other PR merged. Reopen if I am mistaken.

@rhatdan rhatdan closed this Aug 6, 2020
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 this pull request may close these issues.

3 participants