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

Prevent crash on import after GCC 8 builds #6048

Merged
merged 6 commits into from
Apr 12, 2024
Merged

Prevent crash on import after GCC 8 builds #6048

merged 6 commits into from
Apr 12, 2024

Conversation

cjvolzka
Copy link
Contributor

Description

Possible fix for #6047 based on https://stackoverflow.com/questions/33149878/experimentalfilesystem-linker-error/33159746#33159746

Motivation and Context

I'm not a cmake expert so there may be a better way to do this. This did allow me to build ONNX 1.16 on RHEL 8.

Signed-off-by: Charles Volzka <cjvolzka@us.ibm.com>
@cjvolzka cjvolzka requested a review from a team as a code owner March 28, 2024 20:16
Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.97%. Comparing base (83194ed) to head (569fa9d).
Report is 11 commits behind head on main.

❗ Current head 569fa9d differs from pull request most recent head 144bdf4. Consider uploading reports for the commit 144bdf4 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6048      +/-   ##
==========================================
+ Coverage   56.95%   56.97%   +0.02%     
==========================================
  Files         506      506              
  Lines       30467    30483      +16     
  Branches     4592     4593       +1     
==========================================
+ Hits        17353    17369      +16     
  Misses      12285    12285              
  Partials      829      829              

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

@justinchuby justinchuby requested a review from liqunfu April 3, 2024 20:51
@gramalingam
Copy link
Contributor

Thanks. Looks ok to me, but I am not a cmake expert either ... would be good to get a review from someone knowledgeable in cmake.

CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@thiagocrepaldi thiagocrepaldi left a comment

Choose a reason for hiding this comment

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

LGTM, but i couldnt verify on local env

CMakeLists.txt Outdated Show resolved Hide resolved
cjvolzka and others added 2 commits April 4, 2024 13:16
Co-authored-by: Thiago Crepaldi <thiago.crepaldi@microsoft.com>
Signed-off-by: Charles Volzka <42243335+cjvolzka@users.noreply.github.com>
@cjvolzka cjvolzka enabled auto-merge April 5, 2024 19:18
Copy link
Contributor

@justinchuby justinchuby left a comment

Choose a reason for hiding this comment

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

Thanks everyone who reviewed

@cjvolzka cjvolzka added this pull request to the merge queue Apr 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 8, 2024
@cjvolzka cjvolzka added this pull request to the merge queue Apr 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 9, 2024
@xadupre xadupre added this pull request to the merge queue Apr 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 9, 2024
@gramalingam gramalingam added this pull request to the merge queue Apr 12, 2024
Copy link

Test Results

     3 files  ±0       3 suites  ±0   2m 37s ⏱️ -18s
 7 484 tests ±0   4 454 ✅ ±0  3 030 💤 ±0  0 ❌ ±0 
22 435 runs  ±0  13 255 ✅ ±0  9 180 💤 ±0  0 ❌ ±0 

Results for commit 144bdf4. ± Comparison against base commit 284b124.

Merged via the queue into main with commit 3f24ef3 Apr 12, 2024
37 checks passed
@gramalingam gramalingam deleted the rhel8 branch April 12, 2024 17:02
gramalingam added a commit to gramalingam/onnx that referenced this pull request Apr 12, 2024
### Description
Possible fix for onnx#6047 based on
https://stackoverflow.com/questions/33149878/experimentalfilesystem-linker-error/33159746#33159746

### Motivation and Context
I'm not a cmake expert so there may be a better way to do this. This did
allow me to build ONNX 1.16 on RHEL 8.

---------

Signed-off-by: Charles Volzka <cjvolzka@us.ibm.com>
Signed-off-by: Charles Volzka <42243335+cjvolzka@users.noreply.github.com>
Co-authored-by: Thiago Crepaldi <thiago.crepaldi@microsoft.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
gramalingam added a commit to gramalingam/onnx that referenced this pull request Apr 12, 2024
### Description
Possible fix for onnx#6047 based on
https://stackoverflow.com/questions/33149878/experimentalfilesystem-linker-error/33159746#33159746

### Motivation and Context
I'm not a cmake expert so there may be a better way to do this. This did
allow me to build ONNX 1.16 on RHEL 8.

---------

Signed-off-by: Charles Volzka <cjvolzka@us.ibm.com>
Signed-off-by: Charles Volzka <42243335+cjvolzka@users.noreply.github.com>
Co-authored-by: Thiago Crepaldi <thiago.crepaldi@microsoft.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
@cjvolzka cjvolzka added this to the 1.16.1 milestone Apr 29, 2024
cjvolzka added a commit that referenced this pull request Apr 29, 2024
### Description
Possible fix for #6047 based on
https://stackoverflow.com/questions/33149878/experimentalfilesystem-linker-error/33159746#33159746

### Motivation and Context
I'm not a cmake expert so there may be a better way to do this. This did
allow me to build ONNX 1.16 on RHEL 8.

---------

Signed-off-by: Charles Volzka <cjvolzka@us.ibm.com>
Signed-off-by: Charles Volzka <42243335+cjvolzka@users.noreply.github.com>
Co-authored-by: Thiago Crepaldi <thiago.crepaldi@microsoft.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
(cherry picked from commit 3f24ef3)
@cjvolzka cjvolzka mentioned this pull request Apr 29, 2024
cjvolzka added a commit that referenced this pull request May 9, 2024
### Description
Check-pick commits from `main` into `rel-1.16.1` branch

### Motivation and Context
Fix 1.16.0 issues and enabled 1.16.1 builds by cherry-picking the
following PRs from `main` into the `1.16.1` branch

* Fixes since 1.16.0 release
    * #6049
    * #6048
    * #6080
    * #6122

* Update CI and Build to enable 1.16.1 builds
    * #6106
    * #6129
    * #6075
    * #6117

---------

Co-authored-by: liqun Fu <liqfu@microsoft.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
Co-authored-by: Matthieu Darbois <mayeut@users.noreply.github.com>
Co-authored-by: Adrian Lizarraga <adlizarraga@microsoft.com>
Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants