Page MenuHomeFreeBSD

documentation/Makefile: add requirements target
ClosedPublic

Authored by sbz on May 7 2021, 2:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 13, 5:45 PM
Unknown Object (File)
Sat, Apr 13, 4:15 PM
Unknown Object (File)
Sat, Apr 13, 3:30 PM
Unknown Object (File)
Sat, Apr 13, 3:28 PM
Unknown Object (File)
Sat, Apr 13, 3:26 PM
Unknown Object (File)
Sat, Apr 13, 1:58 PM
Unknown Object (File)
Sat, Apr 13, 12:13 PM
Unknown Object (File)
Feb 18 2024, 8:13 AM
Subscribers

Details

Summary

Add the requirements target in order to prompt the user to install the
needed dependencies in order to be able to run make run successfully.

Test Plan

Deinstall/Install the required packages then launch make run

Diff Detail

Repository
R9 FreeBSD doc repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sbz requested review of this revision.May 7 2021, 2:14 PM
sbz created this revision.

After cloning the docs.git repository, I tried to run make run then it ends in lot of errors, so I thought it might be nice if the code prompt the user to install the minimal needed and missing packages to be able to use make run. Afterwards, I see it was mentioned in the fdp primer documentation in Chapter 2.1, but I like when the code prompt for what it's required to do. Alternatively, we could also just recommend to install textproc/docproj, but it's not the minimal way. Let me know what do you think.

IMO we could indeed recommend textproc/docproj. If we are going with this minimal approach, maybe try all checks and pop up the recommendations all at once, to avoid people having to run it multiple times to satisfy all dependencies? (and we can even recommend both)

I think it would be better to check for the individual component parts, as @ygy suggests, and if any of them aren't found, recommend installing ports/textproc/docproj - but only print the error message once, even if multiple dependencies aren't found.

It should be possible to do this via .if and .elif BSD make macros, as documented in make(1) and use of another phony (ie. internal) environment variable.

ygy requested changes to this revision.May 8 2021, 8:42 AM
This revision now requires changes to proceed.May 8 2021, 8:42 AM

I feel like this change also needs to incorporate https://reviews.freebsd.org/D30176 somehow - maybe check the platform first, then remove the tests that depend on absolute paths?

Rewrite using bmake(1) instead of using shell

Recommend only to install docproj package to avoid multiples runs to
satisfy dependencies as suggested by @ygy

In D30161#677216, @ygy wrote:

IMO we could indeed recommend textproc/docproj. If we are going with this minimal approach, maybe try all checks and pop up the recommendations all at once, to avoid people having to run it multiple times to satisfy all dependencies? (and we can even recommend both)

Updated, agreed it's just better to avoid multiples run here.

I think it would be better to check for the individual component parts, as @ygy suggests, and if any of them aren't found, recommend installing ports/textproc/docproj - but only print the error message once, even if multiple dependencies aren't found.

It should be possible to do this via .if and .elif BSD make macros, as documented in make(1) and use of another phony (ie. internal) environment variable.

I removed the shell part, it's just done with bmake(1) now, it's simpler like that.

In D30161#677651, @ygy wrote:

I feel like this change also needs to incorporate https://reviews.freebsd.org/D30176 somehow - maybe check the platform first, then remove the tests that depend on absolute paths?

I don't think it's related, let's not mix review here.

imp added inline comments.
documentation/Makefile
48

Is LOCALBASE defined here? That would be better than hard coding /usr/local/

Add new RUN_DEPENDS for listing the requirements, it's more readable and extensible as suggested by @debdrup

Also define LOCALBASE as suggested by @imp

sbz marked an inline comment as done.May 12 2021, 4:03 PM

It should be ready now, ping @ygy

ygy added reviewers: debdrup, imp.

Not sure if @imp & @debdrup want to take another look, but this looks good to me. Thanks again for making the changes!

This revision is now accepted and ready to land.May 12 2021, 4:25 PM

Looks good to me, let's get this commited.

This revision was automatically updated to reflect the committed changes.