Conversation
| if test -e docs/man5/tinyproxy.conf.5 ; then | ||
| touch docs/man5/tinyproxy.conf.5 | ||
| fi | ||
| if test -e docs/man8/tinyproxy.8 ; then |
There was a problem hiding this comment.
the intent of these lines was to ship pre-built manpages so the build doesn't break when pod2man isn't installed. the touching is necessary else make thinks the target needs to be rebuilt because of file timestamps.
There was a problem hiding this comment.
because of this, maybe the best option is to do the path seds on the final manpage (or an intermediate thereof, after pod2man is applied). in that case we could ship the generated intermediate manpages and touch those.
There was a problem hiding this comment.
I misunderstood the comment above them and thought they might not be needed for man8 since it was moved out of AC_CONFIG_FILES, but I tried now and it's required. I reverted to the earlier commit which is working fine for me when building from the dist with and without pod2man.
There was a problem hiding this comment.
Clarified the comments a bit and removed one line which is not needed.
e58617f to
0acbdcc
Compare
configure.ac
Outdated
|
|
||
| if test -e docs/man8/tinyproxy.8 ; then | ||
| touch docs/man8/tinyproxy.8 | ||
| # this isn't shipped, so touch it to avoid a rebuild of the manpage |
There was a problem hiding this comment.
this comment seems confusing. at least i dont get it. also why is the .8 input text touched but not the .5 input ?
the change of indentation is gratuitous.
There was a problem hiding this comment.
this comment seems confusing. at least i dont get it. also why is the .8 input text touched but not the .5 input
The .5 input is freshly generated already as part of the configure, so touching it doesn't do anything. We only need to touch the generated manpage to be newer than the input.
For .8 it's different, that file is not shipped at all in the release tarball, so make can't tell if the man page is updated or not. So we need to touch it and then touch the man page so make ignores it.
the change of indentation is gratuitous.
Do you mean the whitespace? I wanted to match the indentation above, but I can revert it.
There was a problem hiding this comment.
Do you mean the whitespace?
yes. i'm a big fan of minimal diffs, trumping consistent indendation.
For .8 it's different
i suppose that's because the path transformation is only done on one of those? imo it would be preferable if both manpages get the exact same treatment, even if there are currently no paths to be replaced, but that might change in the future. having consistency here would avoid a lot of confusion.
There was a problem hiding this comment.
yes. i'm a big fan of minimal diffs, trumping consistent indendation.
Fair, I'll simplify the commit.
i suppose that's because the path transformation is only done on one of those?
The path transformation is done on both, but at different stages. For tinyproxy.conf.txt it shouldn't really be done in AC_CONFIG_FILES, because that's meant for Makefiles and doesn't do a full expansion, it only happens to work because there's only one variable being replaced in that file, TINYPROXY_STATHOST which is a simple value so this isn't a problem.
having consistency here would avoid a lot of confusion.
I agree. I'll do that change in this PR? Or I'll just keep the fix only here?
There was a problem hiding this comment.
I reverted back to the ultra simple fix. I can do all the other changes in another PR since it will be more involved.
There was a problem hiding this comment.
I can do all the other changes in another PR since it will be more involved.
thanks, waiting for it to happen.
No description provided.