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 Passed

Unit

No Test Coverage

Build Status

Buildable 16277
Build 16224: arc lint + arc unit

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	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	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	Need to update share/mk/bsd.README for this feature.
46	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.
49	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	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	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?