docs: Edits to BUILD.md #4155

kagarmoe · 2023-08-18T17:49:47Z

Description of changes:

Edits the BUILD.md and organizes it more to the existing template.

Call-outs:

How do we want to discuss what we do (and don't) support?
Aside from mlock memory management, are there any more issues that we should add to the build troubleshooting section?

docs/BUILD.md

goatgoose · 2023-09-19T02:40:11Z

docs/BUILD.md

+
+### System requirements
+
+* 20GB RAM available


I don't think this is a requirement. Was disk space meant rather than RAM? Even so, I'm not sure 20 GB is needed. Some additional space is needed when building, but the last release was only ~43 MB.

kagarmoe · 2023-09-27T00:00:25Z

docs/BUILD.md


-<!-- We may want to move in this direction:
+| Distro | Support | Supported Versions |


@dougch This could use more refinement, but what do you think of it as a direction?

docs/BUILD.md

goatgoose · 2023-10-02T02:44:36Z

docs/BUILD.md

+
+### System requirements
+
+* 20GB RAM available, recommended.


Again, this requirement/recommendation sounds extreme. Building s2n-tls should require such a small amount of RAM that I'm not sure it makes sense to specify a requirement for it.

#4155 (comment)

I've compiled it on a RaspberryPi with 4GB RAM; considering we have IoT uses, one could argue this is relevant.

Giving system requirements is a normal piece of information. It a good thing to to have for everbody, but it is especially important for edge devices. I need a size for the recommended available RAM.

I would maybe go with 4GB then, since that's probably the least amount of RAM that we know has been tested.

If we're going to document requirements, we shouldn't be guessing at those requirements. They need to be accurate. And are we intending this as build requirements or runtime requirements?

docs/BUILD.md

dougch · 2023-10-03T22:59:02Z

docs/BUILD.md

+
+### System requirements
+
+* 20GB RAM available, recommended.


I've compiled it on a RaspberryPi with 4GB RAM; considering we have IoT uses, one could argue this is relevant.

docs/BUILD.md

Co-authored-by: Sam Clark <[email protected]>

goatgoose · 2023-10-19T02:33:39Z

docs/BUILD.md

+
+### System requirements
+
+* 20GB RAM available, recommended.


I would maybe go with 4GB then, since that's probably the least amount of RAM that we know has been tested.

goatgoose · 2023-10-19T02:44:40Z

docs/BUILD.md

@@ -1,6 +1,36 @@
 # Building s2n-tls

-s2n-tls can be built as follows:
+To use s2n-tls, you must build the library from the source and then include it in your program.


nit: Could "from source" be used here? It reads a bit better to me.

Suggested change

To use s2n-tls, you must build the library from the source and then include it in your program.

To use s2n-tls, you must build the library from source and then include it in your program.

goatgoose · 2023-10-19T02:45:29Z

docs/BUILD.md

+1. Libcrypto, such as AWS-LC or OpenSSL
+1. Platform-specific build tools
+
+## Building s2n-tls from the source


Same nit:

Suggested change

## Building s2n-tls from the source

## Building s2n-tls from source

lrstewart · 2023-09-19T17:30:29Z

docs/BUILD.md

+    -D CMAKE_BUILD_TYPE=Release \
+    -D CMAKE_INSTALL_PREFIX=./s2n-tls-install


I've never seen cmake -D with a space like that. It somehow looks very wrong lol. I'd prefer we follow the more common cmake conventions and not put a space after -D.

lrstewart · 2023-10-24T05:11:56Z

docs/BUILD.md

-s2n-tls can be built as follows:
+To use s2n-tls, you must build the library from the source and then include it in your program.


It doesn't feel correct to say that you must build s2n-tls from source to use it. We don't know how customers are consuming s2n-tls-- they could already have a binary. I don't think this wording change is an improvement.

lrstewart · 2023-10-24T05:30:36Z

docs/BUILD.md

+> See the [s2n-tls usage guide](USAGE-GUIDE.md#consuming-s2n-tls-via-cmake) for instructions on how to include s2n-tls in your CMake project.



Nit: Why is this a block quote now?

lrstewart · 2023-10-24T05:33:34Z

docs/BUILD.md

+- [**`CMAKE_BUILD_TYPE`**](https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html): Sets the build type.
+  - `Release`: Produce an optimized s2n-tls library artifact without debug info. Use this option to build s2n-tls for use in production.


I don't think it's clear that your nested bullets are possible values

lrstewart · 2023-10-24T05:37:14Z

docs/BUILD.md

-The `CMAKE_INSTALL_PREFIX` option can be provided when building AWS-LC to specify where AWS-LC will be installed. The install path for AWS-LC should be provided when building s2n-tls, via the `CMAKE_PREFIX_PATH` option. This will ensure that s2n-tls is able to find the AWS-LC library artifact to link with.
-


Nit: I'm guessing you removed this because aws-lc should own its own build instructions. But since we're specifically calling out aws-lc as our recommended option, maybe we want to keep these quick / easy access instructions on building the two libraries together.

github-actions · 2023-12-24T01:49:48Z

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dougch · 2024-03-14T23:17:05Z

partially superseded by #4343

kagarmoe marked this pull request as draft August 18, 2023 18:05

kagarmoe self-assigned this Aug 18, 2023

kagarmoe force-pushed the kgarmoe/build-doc branch from 1d46378 to e0df4f2 Compare September 18, 2023 20:00

kagarmoe marked this pull request as ready for review September 18, 2023 20:02

kagarmoe requested a review from goatgoose September 18, 2023 20:02

goatgoose reviewed Sep 19, 2023

View reviewed changes

goatgoose requested a review from lrstewart September 19, 2023 02:44

kagarmoe commented Sep 27, 2023

View reviewed changes

dougch added the s2n-core team label Sep 28, 2023

kagarmoe requested review from goatgoose and dougch September 29, 2023 23:10

goatgoose reviewed Oct 2, 2023

View reviewed changes

dougch reviewed Oct 3, 2023

View reviewed changes

kagarmoe and others added 9 commits October 18, 2023 22:41

Docs: Template-based edits to BUILD.md

b6ea51c

support tables

46a55d0

typo fix

02aa7f2

it's spelled avail-ABLE

61a9ca3

Apply suggestions from code review

97c2989

Co-authored-by: Sam Clark <[email protected]>

support matrix v2

326d80a

Remove support matrix

35f3fd7

AWS-LC options

fc99867

Feedback; Need RAM spec

11846ea

kagarmoe force-pushed the kgarmoe/build-doc branch from 32141d1 to 11846ea Compare October 18, 2023 22:42

kagarmoe requested review from goatgoose and dougch October 18, 2023 22:43

goatgoose approved these changes Oct 19, 2023

View reviewed changes

lrstewart reviewed Oct 24, 2023

View reviewed changes

github-actions bot added the status/stale label Dec 24, 2023

dougch closed this Mar 14, 2024

jmayclin deleted the kgarmoe/build-doc branch August 13, 2024 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: Edits to BUILD.md #4155

docs: Edits to BUILD.md #4155

kagarmoe commented Aug 18, 2023

goatgoose Sep 19, 2023

kagarmoe Sep 27, 2023

goatgoose Oct 2, 2023

dougch Oct 3, 2023

kagarmoe Oct 18, 2023

goatgoose Oct 19, 2023

lrstewart Oct 24, 2023 •

edited

Loading

dougch Oct 3, 2023

goatgoose Oct 19, 2023

goatgoose Oct 19, 2023

goatgoose Oct 19, 2023

lrstewart Sep 19, 2023

lrstewart Oct 24, 2023

lrstewart Oct 24, 2023 •

edited

Loading

lrstewart Oct 24, 2023

lrstewart Oct 24, 2023

github-actions bot commented Dec 24, 2023

dougch commented Mar 14, 2024


		<!-- We may want to move in this direction:
		\| Distro \| Support \| Supported Versions \|

	To use s2n-tls, you must build the library from the source and then include it in your program.
	To use s2n-tls, you must build the library from source and then include it in your program.

	## Building s2n-tls from the source
	## Building s2n-tls from source

		-D CMAKE_BUILD_TYPE=Release \
		-D CMAKE_INSTALL_PREFIX=./s2n-tls-install

		s2n-tls can be built as follows:
		To use s2n-tls, you must build the library from the source and then include it in your program.

		> See the [s2n-tls usage guide](USAGE-GUIDE.md#consuming-s2n-tls-via-cmake) for instructions on how to include s2n-tls in your CMake project.

		- [`CMAKE_BUILD_TYPE`](https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html): Sets the build type.
		- `Release`: Produce an optimized s2n-tls library artifact without debug info. Use this option to build s2n-tls for use in production.

		The `CMAKE_INSTALL_PREFIX` option can be provided when building AWS-LC to specify where AWS-LC will be installed. The install path for AWS-LC should be provided when building s2n-tls, via the `CMAKE_PREFIX_PATH` option. This will ensure that s2n-tls is able to find the AWS-LC library artifact to link with.

docs: Edits to BUILD.md #4155

docs: Edits to BUILD.md #4155

Conversation

kagarmoe commented Aug 18, 2023

Description of changes:

Call-outs:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrstewart Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrstewart Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 24, 2023

dougch commented Mar 14, 2024

lrstewart Oct 24, 2023 •

edited

Loading

lrstewart Oct 24, 2023 •

edited

Loading