Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a document with build instructions. #4581

Merged
merged 7 commits into from
Jan 13, 2024
Merged

Add a document with build instructions. #4581

merged 7 commits into from
Jan 13, 2024

Conversation

teo-tsirpanis
Copy link
Member

SC-34997


TYPE: FEATURE
DESC: Add a document with build instructions.

Copy link

@@ -119,6 +122,7 @@ Param(
[string]$Prefix,
[string]$Dependency,
[string]$Linkage = "shared",
[switch]$RemoveDeprecations,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable is already being used below in line 341, but was not properly getting a value.

doc/dev/BUILD.md Outdated Show resolved Hide resolved
Comment on lines +79 to +98
|macOS/Linux flag|Windows flag|CMake variable|Description|
|----------------|------------|--------------|-----------|
|`--prefix=PREFIX`|`-Prefix=PREFIX`|`CMAKE_INSTALL_PREFIX=<PREFIX>`|Install files in tree rooted at `PREFIX` (defaults to `TileDB/dist`)|
|`--linkage=shared/static`|`-Linkage=shared/static`|`BUILD_SHARED_LIBS=ON/OFF`|Linkage of the compiled TileDB library (defaults to `shared`) |
|`--remove-deprecations`|`-RemoveDeprecations`|`TILEDB_REMOVE_DEPRECATIONS=ON`|Build TileDB without deprecated APIs|
|`--enable-debug`|`-EnableDebug`|`CMAKE_BUILD_TYPE=Debug`|Enables debug build|
|`--enable-release-symbols`|`-EnableReleaseSymbols`|`CMAKE_BUILD_TYPE=RelWithDebInfo`|Enables release build with debug symbols|
|`--enable-coverage`|`-EnableCoverage`|`CMAKE_BUILD_TYPE=Coverage`|Enables build with code coverage support|
|`--enable-verbose`|`-EnableVerbose`|`TILEDB_VERBOSE=ON`|Enables verbose status messages|
|`--enable-hdfs`|_(HDFS is unsupported on Windows)_|`TILEDB_HDFS=ON`|Enables building with HDFS storage backend support|
|`--enable-s3`|`-EnableS3`|`TILEDB_S3=ON`|Enables building with S3 storage backend support|
|`--enable-azure`|`-EnableAzure`|`TILEDB_AZURE=ON`|Enables building with Azure Blob Storage backend support|
|`--enable-gcs`|`-EnableGcs`|`TILEDB_GCS=ON`|Enables building with Google Cloud Storage backend support|
|`--enable-serialization`|`-EnableSerialization`|`TILEDB_SERIALIZATION=ON`|Enables building with Serialization and TileDB Cloud support|
|`--disable-avx2`|_(Unavailable)_|`COMPILER_SUPPORTS_AVX2=FALSE`|Disables use of the AVX2 instruction set|
|`--disable-webp`|`-DisableWebp`|`TILEDB_WEBP=OFF`|Disables building with support for the WebP filter|
|`--disable-werror`|`-DisableWerror`|`TILEDB_WERROR=OFF`|Disables building with the `-Werror` flag|
|`--disable-cpp-api`|`-DisableCppApi`|`TILEDB_CPP_API=OFF`|Disables building the TileDB C++ API|
|`--disable-stats`|`-DisableStats`|`TILEDB_STATS=OFF`|Disables internal TileDB statistics|
|`--disable-tests`|`-DisableTests`|`TILEDB_TESTS=OFF`|Disables building the TileDB test suite|
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this table from https://docs.tiledb.com/main/how-to/installation/building-from-source/c-cpp. One of the options I did not include is --dependency/CMAKE_PREFIX_PATH, which does not matter when vcpkg is being used.

doc/dev/BUILD.md Outdated
Comment on lines 118 to 126
|Target|Description|
|------|-----------|
|`install-tiledb`|Installs the TileDB library and headers.|
|`check`|Builds and runs all TileDB tests.|
|`examples`|Builds all TileDB examples.|
|`experimental-examples`|Builds all TileDB experimental examples.|
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the targets exposed by the superbuild. I did not bother to explain what the superbuild is since it's going away soon. After that, we can add more targets.

@teo-tsirpanis teo-tsirpanis marked this pull request as ready for review December 18, 2023 11:06
Copy link
Contributor

@eric-hughes-tiledb eric-hughes-tiledb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need the topics here divided into two major sections. As it currently is, this document is mixing tutorial material with advanced user material. Both kinds suffer for it.

# Getting Started
# Advanced Use

There needs to be something in this documentation about the consequences of building from scratch when you don't do anything special with vcpkg.

doc/dev/BUILD.md Outdated
* Git (required by vcpkg)
* curl (required by vcpkg on non-Windows)

### Dependency management with vcpkg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section should be considered "Advanced Use" and appear later in the documentation. Nobody reading this document should have to read anything about VCPKG_ROOT to get started.

If a user does nothing special, vcpkg should just work. That's mostly what this section should say.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would actually like everybody who regularly builds the Core to set VCPKG_ROOT, but moved this section to the bottom of the document.


## Downloading the source code

Begin by downloading a release tarball or by cloning the TileDB GitHub repo and checking out a release tag. In the following script `<version>` is the version you wish to use (e.g., `2.18.0`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we maintain a "current-release" tag?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. My understanding is that we treat tags as immutable.

doc/dev/BUILD.md Outdated
|`install-tiledb`|Installs the TileDB library and headers.|
|`check`|Builds and runs all TileDB tests.|
|`examples`|Builds all TileDB examples.|
|`experimental-examples`|Builds all TileDB experimental examples.|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can leave out experimental-anything in this documentation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

doc/dev/BUILD.md Outdated
TileDB uses [vcpkg](https://vcpkg.io) to download and build its dependencies. By default, the vcpkg repository will be cloned automatically when configuring the build tree. To avoid repeatedly cloning the repository, and take full advantage of vcpkg's [binary caching](https://learn.microsoft.com/en-us/vcpkg/users/binarycaching) feature, you are recommended to clone the vcpkg repository manually and set the `VCPKG_ROOT` environment variable on your machine to the path of the repository.

> [!NOTE]
> If you have set `VCPKG_ROOT` on your machine and encountered errors like `error: no version database entry for <port> at <version>` it means that your vcpkg repository is out of date. You can update it by running `git pull` in the repository directory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a misfeature. If I'm using a lovely package manager, why doesn't it manage the package by downloading it with git? And why does it do so only when I set a magic environment variable and not otherwise?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC I added this note after hitting this error when working on #4553. The vcpkg builtin-baseline got updated to a recent commit, but that commit did not exist in my repository in VCPKG_ROOT.

manage the package by downloading it with git

It's not the package, it's the database with the package versions. I guess vcpkg does not run git fetch by itself for performance reasons.

@ihnorton ihnorton self-requested a review December 19, 2023 20:30
Copy link
Contributor

@eric-hughes-tiledb eric-hughes-tiledb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once this is merged we need to update the docs to point here. I don't want to maintain build instructions in multiple locations.

@KiterLuc KiterLuc merged commit d017daf into dev Jan 13, 2024
63 checks passed
@KiterLuc KiterLuc deleted the teo/build-docs branch January 13, 2024 04:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants