Skip to content

Conversation

liqunfu
Copy link
Collaborator

@liqunfu liqunfu commented Mar 28, 2024

Description

Current fix of #5194 forces out system installed cmake. It forces to use a cmake installed via pyproject.toml in a python environment. The cmake in python environment fails on Windows - onnx.sln cannot be built with visual studio due to protobuf code generation error.

This PR skip cmake install in pyproject.toml in Windows. It is better only include cmake in pyproject.toml if needed.

Motivation and Context

Signed-off-by: Liqun Fu <liqfu@microsoft.com>
Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.95%. Comparing base (df36ccc) to head (19503a6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6045   +/-   ##
=======================================
  Coverage   56.95%   56.95%           
=======================================
  Files         506      506           
  Lines       30467    30467           
  Branches     4592     4592           
=======================================
  Hits        17353    17353           
  Misses      12285    12285           
  Partials      829      829           

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

@liqunfu liqunfu added the run release CIs Use this label to trigger release tests in CI label Mar 28, 2024
@liqunfu liqunfu closed this Mar 28, 2024
@liqunfu liqunfu reopened this Mar 28, 2024
@justinchuby justinchuby changed the title Not to use python cmake on Windows Remove cmake from requirements on Windows Mar 28, 2024
@justinchuby justinchuby changed the title Remove cmake from requirements on Windows Remove cmake from python package requirements on Windows Mar 28, 2024
@justinchuby justinchuby changed the title Remove cmake from python package requirements on Windows Remove cmake from python build requirements on Windows Mar 28, 2024
@liqunfu liqunfu changed the title Remove cmake from python build requirements on Windows Not to install cmake in pyproject.toml on Windows Mar 28, 2024
@liqunfu liqunfu added this pull request to the merge queue Mar 28, 2024
Merged via the queue into main with commit 3916623 Mar 28, 2024
@liqunfu liqunfu deleted the liqun/win-not-to-use-python-cmake branch March 28, 2024 03:49
andife pushed a commit to andife/onnx that referenced this pull request Jul 20, 2024
### Description
Current fix of onnx#5194 forces out
system installed cmake. It forces to use a cmake installed via
pyproject.toml in a python environment. The cmake in python environment
fails on Windows - onnx.sln cannot be built with visual studio due to
protobuf code generation error.

This PR skip cmake install in pyproject.toml in Windows. It is better
only include cmake in pyproject.toml if needed.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve? -->
<!-- - If it fixes an open issue, please link to the issue here. -->

Signed-off-by: Liqun Fu <liqfu@microsoft.com>
Signed-off-by: Andreas Fehlner <fehlner@arcor.de>
linshokaku pushed a commit to linshokaku/onnx that referenced this pull request Oct 2, 2024
### Description
Current fix of onnx#5194 forces out
system installed cmake. It forces to use a cmake installed via
pyproject.toml in a python environment. The cmake in python environment
fails on Windows - onnx.sln cannot be built with visual studio due to
protobuf code generation error.

This PR skip cmake install in pyproject.toml in Windows. It is better
only include cmake in pyproject.toml if needed.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve? -->
<!-- - If it fixes an open issue, please link to the issue here. -->

Signed-off-by: Liqun Fu <liqfu@microsoft.com>
Signed-off-by: Linsho Kaku <linsho@preferred.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run release CIs Use this label to trigger release tests in CI
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants