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

DPDK: install path fixes for meson and Ubuntu 24.04 #3598

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Meson: restructure and comment installation options
  • Loading branch information
mcgov committed Feb 5, 2025
commit f6be10461955ed6047bab05221d8753086c78541
30 changes: 25 additions & 5 deletions lisa/tools/meson.py
Original file line number Diff line number Diff line change
@@ -15,13 +15,18 @@


class Meson(Tool):
_minimum_version = "0.52.0"

@property
def command(self) -> str:
return "meson"

def _check_exists(self) -> bool:
result = self.node.execute("meson --version", shell=True)
return result.exit_code == 0 and VersionInfo.parse(result.stdout) >= "0.52.0"
return (
result.exit_code == 0
and VersionInfo.parse(result.stdout) >= self._minimum_version
)

@property
def can_install(self) -> bool:
@@ -33,25 +38,40 @@ def _install(self) -> bool:

package_installed = ""
package_available = ""
for pkg in ["meson", "python3-meson"]:
# packaged as 'meson' on older systems and 'python3-meson' on newer ones,
# since it's actually just a python package.
# So check for both
for pkg in [
"python3-meson",
"meson",
]:
if (
posix_os.package_exists(pkg)
and posix_os.get_package_information(pkg, use_cached=False) >= "0.52.0"
and posix_os.get_package_information(pkg, use_cached=False)
>= self._minimum_version
):
package_installed = pkg
break
elif posix_os.is_package_in_repo(pkg):
Copy link
Member

Choose a reason for hiding this comment

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

elif to if is the same, but it means the two if blocks are actually independent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you be more specific with your request from me?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
elif posix_os.is_package_in_repo(pkg):
if posix_os.is_package_in_repo(pkg):

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right. So making just that change will introduce an unintended side effect of updating the package before we check the version of the currently installed package. Let me see if I can make the decisions more clear in the code. I was intentionally not installing an updated version from the package manager if one was already installed because it was convenient for my meson use cases... But really it would be better to write for the general case; even though it will be a little uglier and use another round trip or two.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It wasn't so bad to rework! Testing the more commented, reworked version now

package_available = pkg
break

# prefer the packaged version as long as it's the right version
if package_installed:
return self._check_exists()
if package_available:
posix_os.install_packages(package_available)
else:
# verify version is correct if it's installed from pkg manager
if posix_os.get_package_information(pkg) < self._minimum_version:
posix_os.uninstall_packages(pkg)
package_available = ""

# otherwise, install with pip
# this can get weird on some systems since they have started
# returning an error code if you try to use pip without a venv
if not (package_available or package_installed):
username = self.node.tools[Whoami].get_username()
self.node.tools[Pip].install_packages("meson", install_to_user=True)
# environment variables won't expand even when using shell=True :\
self.node.tools[Ln].create_link(
f"/home/{username}/.local/bin/meson", "/usr/bin/meson", force=True
)