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

README: add a couple packages required for passing tests on Ubuntu 20.04 #4132

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

README: add a couple packages required for passing tests on Ubuntu 20.04 #4132

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ Most dependencies can be installed using `apt-get install`:
sudo apt-get install cmake g++ git automake libtool libgc-dev bison flex \
libfl-dev libboost-dev libboost-iostreams-dev \
libboost-graph-dev llvm pkg-config python3 python3-pip \
tcpdump
tcpdump libpcap-dev gcc-multilib
Copy link
Contributor

Choose a reason for hiding this comment

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

These packages seems to be specific to ebpf/ubpf backends, not the whole compiler. libpcap-dev is already mentioned for ebpf backand here:

p4c/backends/ebpf/README.md

Line 219 in 847187f

$ sudo apt-get install clang llvm libpcap-dev libelf-dev iproute2 net-tools

I wonder that it would be better to just update that line with gcc-multilib, because main README mentions backend specific dependencies.

Copy link
Member Author

@thomascalvert-xlnx thomascalvert-xlnx Sep 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

These packages are required to run the unit tests for the whole compiler. Copy-pasting a single apt-get command is rather convenient and IMHO it would be nice if that was the only step required to get make check passing for the default build options (which include ebpf/ubpf) -- rather than digging through multiple backend-specific README files.

This is just my opinion based on my recent experience of bringing up this repo. Happy to change the patch as suggested if that is the consensus.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternatively, we should check with CMake whether all necessary dependencies are available for the eBPF back end. If that is not the case throw an error. Or simply disable the eBPF back end by default.

It is better to not pollute the top-level with back end specific dependencies and instructions. Especially, because we keep adding back ends.

We should really think of adding a comprehensive install script for these things...

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you are also missing done libelf related packages

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any kind of consensus on what we want in the top level README, vs. in back-end specific files here?

(a) One option: Top level README gives instructions that enable building ALL back ends, if that is possible on a single system (I think it is, with the right prerequisites installed).

(b) Another option: Top level README gives instructions for building p4test ONLY. Or at least a particular set of back ends that is explicitly listed as supported by the top level README instructions, with explicit pointers to back-end-specific READMEs where additional prerequisites are listed for each of them.

Any strong preferences?

Copy link
Collaborator

@fruffy fruffy Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I personally prefer b) here. a) can turn messy really quickly. For the current back ends we have it already will be. The psa-eBPF backend, which uses NIKSS, the DPDK back end, the TC back end, and p4testgen already require a lot of dependencies to run, for example. You could make this even more complex with install instruction for all the different distributions we would need to write down etc.


pip3 install --user -r requirements.txt
```
Expand Down