Test to handle package requests that are strings separately

[czerouni]

Fixes #2003 Fixes #1766

Signed-off-by: Craig Zerouni [email protected] I was running rez depends on a number of packages, and some of them would crash Rez with the message:

AttributeError: 'str' object has no attribute 'conflict'

This happens in package_search.py where you have this loop:

for req in requires:
    if not req.conflict:
        lookup[req.name].add(package_name_)

The problem is that some requirements in that list are not of type PackageRequest, but instead are simply type str. This happens if in the package.py file of the package being queried there is a private_build_requires configured as a late-binding function. Eg, a package with this this pbr will not crash Rez

private_build_requires = ["foo"]

but this one will

@late()
def private_build_requires():
    return ["foo"]

I initially assumed this was a bug in the way these are handled, except that in the file resolved_context.py I see this comment:

Args:
    package_requests (list[typing.Union[str, PackageRequest]]): request

which leads me to think that requests of type str are expected. In which case, the fix is simple, as per this PR.

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: JeanChristopheMorinPerso / name: Jean-Christophe Morin (2917d2b5570bc12ed4499a468d53f4d9f0b216c0)
• :white_check_mark: login: czerouni / name: Craig Zerouni (55fe9f5e9f3778d888d9ffd0622a9d40adf2726b, 20839b793444f8ea827fa5ed945555e1ce86b783)

[JeanChristopheMorinPerso]

Hey @czerouni , thanks for this PR. I took a quick look and I feel like this can be fixed in a cleaner way. I think we could add build_requires and private_build_requires to https://github.com/AcademySoftwareFoundation/rez/blob/fca0b86b53636707b3064067c1d7c3e316cfd886/src/rez/packages.py#L74-L76

I tested quickly locally with a recipe like this:

name = "test_package"

version = "1.0.1"

description = "Something `rez-env`-ing this"

@late()
def private_build_requires():
    return ["os-arch"]

build_command = "python -c 'print(1)'"

def commands():
    env.ASD.append('~asdasdads')

and this snippet seems to be working:

>>> import rez.packages
>>> package = rez.packages.get_latest_package('test_package')
>>> variant = list(package.iter_variants())[0]
>>> print(variant.get_requires(private_build_requires=True))
[PackageRequest('os-arch')]

Could you give this a go and see if the tests pass?

[czerouni]

Hi. I'm sorry, I am not understanding what you mean. These lines

late_bind_schemas = {
    "requires": late_requires_schema
}

are already in src/rez/packages.py, so I am not sure what change you are suggesting.

[JeanChristopheMorinPerso]

I mean this:

 late_bind_schemas = {
-    "requires": late_requires_schema
+    "requires": late_requires_schema,
+    "build_requires": late_requires_schema,
+    "private_build_requires": late_requires_schema,
 }

[czerouni]

Yes, that seems to accomplish the same thing. Do you want me to close this PR? Or maybe change it to your fix?

[JeanChristopheMorinPerso]

You can push a commit on this branch and keep the same PR

[czerouni]

All set!

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 60.01%. Comparing base (cd8476f) to head (2917d2b).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2004   +/-   ##
=======================================
  Coverage   60.01%   60.01%           
=======================================
  Files         163      163           
  Lines       20098    20098           
  Branches     3494     3494           
=======================================
  Hits        12062    12062           
+ Misses       7224     7217    -7     
- Partials      812      819    +7     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[czerouni]

Is this expected or is it related to my changes?

[JeanChristopheMorinPerso]

Everything looks good to me. The codecov report was created because I ran the CI. The macOS jobs failed, but it's not related to your changes. We need to fix the CI jobs on macOS.

[czerouni]

Great, thank you.