Details
- Reviewers
imp bdrewery emaste - Commits
- rS344566: Enable build of libbearssl
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Can't evaluate lib/Makefile changes without the rest of the changes. I.e., does it require dependency changes in Makefile.inc1's make libraries handling? Can't tell from this.
| share/mk/src.libnames.mk | ||
|---|---|---|
| 216–223 ↗ | (On Diff #45507) | These should be something like ${LIBBEARSSLDIR}/libbearssl.a rather than ${DESTDIR}${LIBDIR} since internallibs never get installed. |
| lib/Makefile | ||
|---|---|---|
| 168–169 ↗ | (On Diff #47712) | This can use SUBDIR.${MK_BEARSSL}+=libbearssl libve style, no? |
| tools/build/options/WITH_BEARSSL | ||
| 2 ↗ | (On Diff #47712) | Options files typically start with a one-sentence description and I think that improves the flow of the generated man page. (They also typically start with Set to build or Set to not build, which I don't think improves readability.) E.g., just start with "Build the BearSSL library." |
| 4 ↗ | (On Diff #47712) | Is there specific markup for URLs? |
| 6–10 ↗ | (On Diff #47712) | This out-of-tree bearssl src should go. |
| tools/build/options/WITH_BEARSSL | ||
|---|---|---|
| 6–10 ↗ | (On Diff #47712) | The intent was that it would eventually end up in contrib/ but didn't think that should be a gating factor? |
| tools/build/options/WITH_BEARSSL | ||
|---|---|---|
| 6–10 ↗ | (On Diff #47712) | If we're ready to start adding the build goop we should just put it in contrib |
| tools/build/options/WITH_BEARSSL | ||
|---|---|---|
| 6–10 ↗ | (On Diff #47712) | We should just import it, even if it isn't in the default built, I agree. Having out-of-tree support for it I think slows the adoption of it for the base. |
Overall looks good to me now, modulo the couple of outstanding comments.
| tools/build/options/WITH_BEARSSL | ||
|---|---|---|
| 4 ↗ | (On Diff #47712) | Found it, .Lk |
| share/mk/src.libnames.mk | ||
|---|---|---|
| 219 ↗ | (On Diff #52839) | Thanks |
| share/mk/src.opts.mk | ||
| 224 ↗ | (On Diff #52839) | That depends a bit on your plans. |
| tools/build/options/WITH_BEARSSL | ||
|---|---|---|
| 4 ↗ | (On Diff #47712) | |
| tools/build/options/WITH_BEARSSL | ||
|---|---|---|
| 4 ↗ | (On Diff #47712) | from mdoc(7): Lk
Format a hyperlink. Its syntax is as follows:
.Lk uri [name]
Examples:
.Lk http://bsd.lv "The BSD.lv Project"
.Lk http://bsd.lv
See also Mt. |
LGTM (with a few notes), let's commit and adjust further if necessary
| share/mk/src.libnames.mk | ||
|---|---|---|
| 219 ↗ | (On Diff #52839) | and now libsecureboot${PIE_FLAG}.a |
| tools/build/options/WITH_BEARSSL | ||
| 7–11 ↗ | (On Diff #52903) | Yes, let's just build the in-tree one. |
| tools/build/options/WITH_VERIEXEC | ||
| 7–9 ↗ | (On Diff #52903) | If the dependencies are encoded in share/mk/* makeman will automatically add a sentence to this effect (something like, for WITHOUT_BEARSSL, When set, it enforces these options: WITHOUT_VERIEXEC). In any case I think it'd be fine to commit as is, changing the wording later if it doesn't read well. |