Skip to content

Conversation

@cbentejac
Copy link
Contributor

Description

This PR updates the build options in setup.py to include the bin directory in the packaged version of Meshroom: the executables for these scripts will still be generated, but they will also be added as is to a bin directory, meaning one can do both meshroom_compute [options] and python bin/meshroom_compute [options].

This allows to simplify the calls to meshroom_compute in the Node class, as the need to distinguish the "release mode" from the "regular mode" disappears.

Additionally, "pytest" is removed from the list of install_requires as no test is ever packaged.

Finally, all the remaining single quotes in setup.py are replaced with double ones.

Features list

  • Package the content of the bin folder
  • Remove "pytest" from the packaging dependencies
  • Unify calls to meshroom_compute in and out of "release mode"

Copy link
Contributor

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 updates the build configuration to package the bin directory scripts alongside executables, simplifying the Meshroom compute script handling and removing unnecessary dependencies.

  • Include bin directory in packaged builds to enable both executable and script-based invocation
  • Remove pytest from install requirements since tests aren't packaged
  • Standardize quote usage in setup.py to double quotes

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
setup.py Adds bin directory to packaged files, removes pytest dependency, and standardizes quotes
meshroom/core/desc/node.py Simplifies meshroom_compute path logic by removing release mode distinction
.git-blame-ignore-revs Adds blame ignore entry for quote standardization commit

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

_MESHROOM_COMPUTE = (Path(_MESHROOM_ROOT) / "bin" / "meshroom_compute").as_posix()
_MESHROOM_COMPUTE_EXE = f"python {_MESHROOM_COMPUTE}"
_MESHROOM_COMPUTE = (Path(_MESHROOM_ROOT) / "bin" / "meshroom_compute").as_posix()
_MESHROOM_COMPUTE_EXE = f"python {_MESHROOM_COMPUTE}"
Copy link

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

The removal of the frozen/release mode check may break functionality when running from a packaged executable. In release mode, the script should be executed directly without the 'python' prefix, but now it will always prepend 'python' even when the script is a standalone executable.

Suggested change
_MESHROOM_COMPUTE_EXE = f"python {_MESHROOM_COMPUTE}"
if getattr(sys, 'frozen', False):
_MESHROOM_COMPUTE_EXE = _MESHROOM_COMPUTE
else:
_MESHROOM_COMPUTE_EXE = f"python {_MESHROOM_COMPUTE}"

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Aug 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.79%. Comparing base (59b15a7) to head (37c6111).
⚠️ Report is 6 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2853      +/-   ##
===========================================
+ Coverage    77.77%   77.79%   +0.01%     
===========================================
  Files           50       50              
  Lines         6728     6725       -3     
===========================================
- Hits          5233     5232       -1     
+ Misses        1495     1493       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The files in `bin` are already generated as executables. Additionally,
they are added "as is" so they can be called as scripts.
Since the scripts are now packaged alongside their executable versions,
nodes can always be executed using `bin/meshroom_compute`.

The need to make a distinction between the "regular" and the "release"
modes is thus removed.
@fabiencastan fabiencastan merged commit e704e5b into develop Sep 9, 2025
5 checks passed
@fabiencastan fabiencastan deleted the dev/improveSetup branch September 9, 2025 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants