[PATCH] D62040: [docs] Add new document on building distributions

[Chris Bieneman via Phabricator via llvm-commits]

beanz marked 3 inline comments as done.
beanz added inline comments.


================
Comment at: llvm/docs/BuildingADistribution.rst:86
+generates a ``distribution`` target which builds all the components specified in
+the list. This is a convenience build target to allow building just the
+distributed pieces without needing to build all configured targets.
----------------
winksaville wrote:
> Is `distributation` target an `internal` target that shouldn't be used or a `convenience` target and might be used by the people building the distribution ? If the former consider saying "internal build target".
If you want to install a distribution onto your local system into a path that requires sudo, and you don’t want to build everything, you need to use it. It is also a handy from a workflow perspective. If it were only for internal build system purposes I would likely not have made it a target.


================
Comment at: llvm/docs/BuildingADistribution.rst:123
+the binaries in the distribution, but it will create much faster binaries. This
+option should not be used if your distribution includes static archives, as the
+objects inside the archive will be LLVM bitcode, which is not portable.
----------------
winksaville wrote:
> Consider changing "includes static **archives**" to "includes static **libraries**" as archives isn't used anywhere else.
I’m not sure what you mean by the term “archive” not being used elsewhere. We use the term “static archive” and “static library” pretty interchangeably around LLVM. It is really a matter of preference and often whether you come from a Unix background where archives are created with an archiver (ar).


================
Comment at: llvm/docs/BuildingADistribution.rst:188
+
+**LLVM_DYLIB_COMPONENTS**:STRING
+  This variable can be set to a semi-colon separated name of LLVM library
----------------
winksaville wrote:
> Consider moving to ":doc:`CMake`" and then added to the "already documented" list above?
This is an uncommon setting to alter, and we really don’t want that document to have an exhaustive list of our options.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62040/new/

https://reviews.llvm.org/D62040