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

Fix TypeError when running get_rpaths on rez.utils.elf module #1798

Conversation

ruzette
Copy link
Contributor

@ruzette ruzette commented Jul 18, 2024

Replaced the subprocess.Popen with rez.utils.execution.Popen and added text=True to fix the TypeError when reading and parsing rpaths

Closes #1797

@ruzette ruzette requested a review from a team as a code owner July 18, 2024 15:46
Copy link

linux-foundation-easycla bot commented Jul 18, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@ruzette ruzette force-pushed the issue-1797-fix-bundle-error branch 2 times, most recently from afb6c49 to 351bb0e Compare July 18, 2024 15:53
Signed-off-by: Ruzette Krukkert <ruzettekrukkert@fusefx.com>
@ruzette ruzette force-pushed the issue-1797-fix-bundle-error branch from 351bb0e to ba17e02 Compare July 18, 2024 15:55
Signed-off-by: Jean-Christophe Morin <jean_christophe_morin@hotmail.com>
Signed-off-by: Jean-Christophe Morin <jean_christophe_morin@hotmail.com>
Signed-off-by: Jean-Christophe Morin <jean_christophe_morin@hotmail.com>
Copy link

codecov bot commented Jul 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.52%. Comparing base (9c27f5e) to head (8a990a5).
Report is 28 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1798      +/-   ##
==========================================
+ Coverage   58.39%   58.52%   +0.12%     
==========================================
  Files         126      126              
  Lines       17205    17206       +1     
  Branches     3519     3519              
==========================================
+ Hits        10047    10069      +22     
+ Misses       6491     6468      -23     
- Partials      667      669       +2     

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

Signed-off-by: Jean-Christophe Morin <jean_christophe_morin@hotmail.com>
Copy link
Member

@JeanChristopheMorinPerso JeanChristopheMorinPerso left a comment

Choose a reason for hiding this comment

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

Thank you @ruzette! I pushed some commits to fix the tests and make them more robust.

@JeanChristopheMorinPerso JeanChristopheMorinPerso added this to the Next milestone Jul 21, 2024
@JeanChristopheMorinPerso JeanChristopheMorinPerso merged commit 3d0f224 into AcademySoftwareFoundation:main Jul 21, 2024
47 checks passed
@ruzette ruzette deleted the issue-1797-fix-bundle-error branch July 21, 2024 21:49
Pixel-Minions added a commit to Pixel-Minions/rez that referenced this pull request Sep 26, 2024
…ySoftwareFoundation#1798)

Signed-off-by: Ruzette Krukkert <ruzettekrukkert@fusefx.com>
Signed-off-by: Jean-Christophe Morin <jean_christophe_morin@hotmail.com>
Co-authored-by: Jean-Christophe Morin <jean_christophe_morin@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rez-bundle throws subprocess.Popen error
2 participants