Details
- Reviewers
imp bdrewery emaste - Commits
- rS344566: Enable build of libbearssl
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 21957 Build 21197: arc lint + arc unit
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 | ||
---|---|---|
218–225 | These should be something like ${LIBBEARSSLDIR}/libbearssl.a rather than ${DESTDIR}${LIBDIR} since internallibs never get installed. |
lib/Makefile | ||
---|---|---|
166–167 | This can use SUBDIR.${MK_BEARSSL}+=libbearssl libve style, no? | |
tools/build/options/WITH_BEARSSL | ||
3 | 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." | |
5 | Is there specific markup for URLs? | |
7–11 | This out-of-tree bearssl src should go. |
tools/build/options/WITH_BEARSSL | ||
---|---|---|
7–11 | 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 | ||
---|---|---|
7–11 | If we're ready to start adding the build goop we should just put it in contrib |
tools/build/options/WITH_BEARSSL | ||
---|---|---|
7–11 | 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 | ||
---|---|---|
5 | Found it, .Lk |
share/mk/src.libnames.mk | ||
---|---|---|
219 | Thanks | |
share/mk/src.opts.mk | ||
224 | That depends a bit on your plans. |
tools/build/options/WITH_BEARSSL | ||
---|---|---|
5 |
tools/build/options/WITH_BEARSSL | ||
---|---|---|
5 | 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 | and now libsecureboot${PIE_FLAG}.a | |
tools/build/options/WITH_BEARSSL | ||
8–12 | Yes, let's just build the in-tree one. | |
tools/build/options/WITH_VERIEXEC | ||
8–10 | 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. |