Switch installing share/examples to FILES
ClosedPublic
Actions

Authored by brd on Apr 19 2018, 4:22 AM.

Details

Reviewers

will
bdrewery
bapt
allanjude

Commits

rS333407: Enable directory creation with FILESDIR.

Summary

This is prep work for pkgbase to create directories.

Includes work by @will to create dirs in bsd.files.mk.

Test Plan

I built a vm using:

make -C release/ vm-release WITH_VMIMAGES=yes VMFORMATS=raw

and diffed the trees.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

brd created this revision.Apr 19 2018, 4:22 AM

Herald added subscribers: emaste, imp, bdrewery. · View Herald TranscriptApr 19 2018, 4:22 AM

Harbormaster completed remote builds in B16220: Diff 41645.Apr 19 2018, 4:22 AM

brd edited the summary of this revision. (Show Details)Apr 19 2018, 4:24 AM

brd added reviewers: will, bdrewery, bapt, allanjude.

brd edited the test plan for this revision. (Show Details)

brd added a subscriber: will.

You've got 2 distinct changes here, one is shares/examples and the other is adding a DIRS thing to bsd.files.mk. They should really be separate commits.

share/examples/Makefile
10–14 ↗	(On Diff #41645)	It's already using `FILES`, so I'm confused on what's really changing here except restructuring so `DIRS` is near the `FILES` it contains. Overall it feels more complex.
253 ↗	(On Diff #41645)	It's unclear to me who would use `symlinks` but I always assume things like this may be used by someone rather than assuming otherwise, for POLA reasons. I think removing it from share/examples/Makefile is fine but I think we should keep it in include/Makefile.
share/mk/bsd.files.mk
35 ↗	(On Diff #41645)	Need to update share/mk/bsd.README for this feature.
47 ↗	(On Diff #41645)	This is probably going to break `WITH_DIRDEPS_BUILD` but maybe not as it is already running `mkdir -p` for its uses in share/mk/meta.stage.mk.
50 ↗	(On Diff #41645)	How is the ownership and mode set for this dir?

This revision now requires changes to proceed.Apr 19 2018, 9:29 PM

I am happy to break it into two separate commits, but I wanted the review to include both so that we see how they work together.

share/examples/Makefile
10–14 ↗	(On Diff #41645)	It is a bit more .. lengthy, due to each subdir being broken out, but this allows for the directory creation to happen as part of FILES. I could probably s/DIRS/SE_DIRS/ to make it clear that this is only being used on L378.
253 ↗	(On Diff #41645)	Yeah, I was thinking about adding a symlinks check and erroring out to say this is unsupported now.. So that there is a warning since I doubt people would see it in the output of installworld or similar.. Thoughts?

Address some of bdrewery's concerns

Harbormaster completed remote builds in B16277: Diff 41786.Apr 23 2018, 11:57 PM

brd marked 2 inline comments as done.Apr 23 2018, 11:58 PM

brd added inline comments.

share/examples/Makefile
10–14 ↗	(On Diff #41645)	Done, this should make it more clear.
share/mk/bsd.files.mk
35 ↗	(On Diff #41645)	Removed
50 ↗	(On Diff #41645)	See new diff with bsd.own.mk changes