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.

Sign up for GitHub

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

Jump to bottom

Add more complete linux build instructions #101631

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add more complete linux build instructions #101631

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c742746
Add more complete linux build instructions
agocke Apr 26, 2024
ff5a2b9
Update eng/native/build-commons.sh
agocke Apr 29, 2024
130b59e
Unify build requirements
agocke Apr 30, 2024
1b06568
Merge remote-tracking branch 'origin/automated-workflow' into automat…
agocke Apr 30, 2024
ad5a56f
Respond to PR comments
agocke May 14, 2024
725c4b7
Revert "Respond to PR comments"
agocke May 14, 2024
8d286b4
Respond to PR comments
agocke May 15, 2024
eb71ca4
Respond to PR comments
agocke May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/workflow/requirements/linux-requirements.md
View file
Open in desktop
Expand Up @@ -40,6 +40,14 @@ Install the following packages for the toolchain:
* zlib1g-dev
* ninja-build (optional, enables building native code with ninja instead of make)

You can install all the above dependencies by running

```bash
sudo xargs apt-get install -Y < eng/debian-reqs.txt
agocke marked this conversation as resolved.
Show resolved Hide resolved
```

### Community-supported environments

**NOTE**: If you have an Ubuntu version older than 22.04 LTS, or Debian version older than 12, don't install `cmake` using `apt` directly. Follow the note written down below.

```bash
Expand Down
15 changes: 15 additions & 0 deletions eng/debian-reqs.txt
View file
Open in desktop
@@ -0,0 +1,15 @@
cmake
llvm
lld
clang
build-essential
python-is-python3
curl
git
lldb
libicu-dev
liblttng-ust-dev
libssl-dev
libkrb5-dev
zlib1g-dev
ninja-build
3 changes: 3 additions & 0 deletions eng/native/build-commons.sh
View file
Open in desktop
Expand Up @@ -39,6 +39,9 @@ check_prereqs()
# We try again with the PKG_CONFIG_PATH in place, if pkg-config still can't find OpenSSL, exit with an error, cmake won't find OpenSSL either
pkg-config openssl || { echo >&2 "Please install openssl before running this script, see https://github.com/dotnet/runtime/blob/main/docs/workflow/requirements/macos-requirements.md"; exit 1; }
fi
elif [[ "$__HostOS" == "linux" ]]; then
# Check presence of pkg-config on the path
agocke marked this conversation as resolved.
Show resolved Hide resolved
command -v cmake 2>/dev/null || { echo >&2 "Please install cmake before running this script, see https://github.com/dotnet/runtime/blob/main/docs/workflow/requirements/linux-requirements.md"; exit 1; }
Copy link
Member

Choose a reason for hiding this comment

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

We are already testing it in gen-buildsys.sh. Also why is this linux only?

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 check is happening earlier and is clearer. Ideally I'd like this check to happen even earlier, but I think that belongs in another PR.

Copy link
Member

Choose a reason for hiding this comment

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

We used to have an upfront check for a while and it was deleted in #39044.

I think if we are bringing it back, we should do it for all platforms, not just linux. CMake is needed on all platforms alike.

Copy link
Member Author

Choose a reason for hiding this comment

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

And this is linux-only because I think checking for pkg-config is better on Mac. What we're looking for is the thing least-likely to have been installed by-default.

Copy link
Member

Choose a reason for hiding this comment

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

If we use else instead of elif, that will cover mac+linux+{everything-else}:

else
  # Check presence of cmake on the path
  command -v cmake 2>/dev/null || { echo >&2 "Please install cmake before running this script, see https://github.com/dotnet/runtime/tree/main/docs/workflow/requirements"; exit 1; }

User can then read {platform}-instructions.md.

Copy link
Member

Choose a reason for hiding this comment

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

This check is happening earlier and is clearer.

It is rendering this check redundant which can be deleted:

runtime/eng/native/gen-buildsys.sh

Lines 87 to 88 in ed339b3

if ! cmake_command=$(command -v cmake); then
echo "CMake was not found in PATH."

Copy link
Member Author

Choose a reason for hiding this comment

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

We used to have an upfront check for a while and it was deleted in #39044.

Not quite the same -- that's a version check, here I'm just doing a basic check to see if it exists at all. The intent is to catch people who have not installed the prereqs at all, not people who have misconfigured prereqs.

fi

if [[ "$__UseNinja" == 1 ]]; then
Expand Down