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
Next Next commit
Meson: try both 'meson' and 'python3-meson'
  • Loading branch information
mcgov committed Feb 5, 2025
commit f9d179d210c803d7c9c0cd964eef57791de7c693
23 changes: 20 additions & 3 deletions lisa/tools/meson.py
Original file line number Diff line number Diff line change
@@ -30,9 +30,25 @@ def can_install(self) -> bool:
def _install(self) -> bool:
posix_os: Posix = cast(Posix, self.node.os)
# use pip to make sure we install a recent version
if (not posix_os.package_exists("meson")) or posix_os.get_package_information(
"meson", use_cached=False
) < "0.52.0":

package_installed = ""
package_available = ""
for pkg in ["meson", "python3-meson"]:
if (
posix_os.package_exists(pkg)
and posix_os.get_package_information(pkg, use_cached=False) >= "0.52.0"
):
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

if package_installed:
return self._check_exists()
if package_available:
posix_os.install_packages(package_available)
else:
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 :\
@@ -48,6 +64,7 @@ def _install(self) -> bool:
no_info_log=True,
no_error_log=True,
)

return self._check_exists()

def setup(self, args: str, cwd: PurePath, build_dir: str = "build") -> PurePath: