Skip to content

Conversation

akhilnarang
Copy link
Member

@akhilnarang akhilnarang commented Mar 18, 2025

  • ensure pkg-config is installed
  • Set PKG_CONFIG_PATH for macOS

@akhilnarang akhilnarang changed the title feat(bench): check for presence of pkg-config before trying to setup bench feat(bench): miscellaneous improvements Apr 9, 2025
@akhilnarang akhilnarang requested a review from gavindsouza April 9, 2025 08:16
…p bench

Signed-off-by: Akhil Narang <me@akhilnarang.dev>
Signed-off-by: Akhil Narang <me@akhilnarang.dev>
Signed-off-by: Akhil Narang <me@akhilnarang.dev>
Signed-off-by: Akhil Narang <me@akhilnarang.dev>
Signed-off-by: Akhil Narang <me@akhilnarang.dev>
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces miscellaneous improvements for the bench tool including dependency checks for pkg-config on macOS and updates pip install commands. Key changes include:

  • Adding helper functions (get_mariadb_pkgconfig_path and check_pkg_config) to manage pkg-config configuration.
  • Modifying Base.run to accept an env parameter and updating pip install command invocations in multiple modules.
  • Updating CI workflow to ensure pkg-config is installed.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
bench/utils/system.py Introduces helper functions for pkg-config checking and configuration.
bench/bench.py Updates the run method signature to include env; adds environment handling for pip install commands.
bench/app.py Adjusts pip install commands to pass the environment configuration for macOS.
.github/workflows/ci.yml Updates CI configuration to install pkg-config along with redis-server.
Comments suppressed due to low confidence (1)

bench/bench.py:382

  • The PR description mentions passing '--use-pep517' to pip install, but this flag is not present in the pip install commands. Please update these commands to include '--use-pep517' if intended.
self.run(f"{self.bench.python} -m pip install {quiet_flag} --upgrade -e {frappe}", cwd=self.bench.name, env=env)

@akhilnarang akhilnarang added the debug-gha Sets up a tmate session label May 7, 2025
@akhilnarang akhilnarang removed the debug-gha Sets up a tmate session label May 7, 2025
@akhilnarang
Copy link
Member Author

akhilnarang commented May 7, 2025

Everything should be fine, it is when I ssh in and manually run the commands, just pip install within the test suite can't find pkg-config somehow, which shouldn't be related to this PR.

Edit: subprocess.call(env={}) was the cause

Signed-off-by: Akhil Narang <me@akhilnarang.dev>
Signed-off-by: Akhil Narang <me@akhilnarang.dev>
Copy link

sonarqubecloud bot commented May 8, 2025

@akhilnarang akhilnarang changed the title feat(bench): miscellaneous improvements feat(bench): miscellaneous improvements for pkg-config May 8, 2025
@akhilnarang akhilnarang merged commit f01f105 into frappe:develop May 8, 2025
14 checks passed
@akhilnarang akhilnarang deleted the pkg-config-check branch May 8, 2025 07:51
akhilnarang added a commit that referenced this pull request May 11, 2025
* upstream/develop:
  ci: drop support for python 3.9
  chore: pin Click version
  fix: use check_nested_command
  feat(bench): miscellaneous improvements for pkg-config (#1622)

Signed-off-by: Akhil Narang <me@akhilnarang.dev>
akhilnarang added a commit that referenced this pull request May 11, 2025
* upstream/staging:
  ci: drop support for python 3.9
  chore: pin Click version
  fix: use check_nested_command
  feat(bench): miscellaneous improvements for pkg-config (#1622)

Signed-off-by: Akhil Narang <me@akhilnarang.dev>
github-actions bot pushed a commit that referenced this pull request May 11, 2025
# [5.25.0](v5.24.1...v5.25.0) (2025-05-11)

### Bug Fixes

* use check_nested_command ([a8ed8e1](a8ed8e1))

### Features

* **bench:** miscellaneous improvements for pkg-config ([#1622](#1622)) ([f01f105](f01f105))
Copy link

🎉 This PR is included in version 5.25.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants