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

Possible solver contains relationship error #492

Open
jrray opened this issue Aug 31, 2022 · 12 comments
Open

Possible solver contains relationship error #492

jrray opened this issue Aug 31, 2022 · 12 comments
Assignees
Labels
bug Something isn't working solver related to the solver part of spk

Comments

@jrray
Copy link
Collaborator

jrray commented Aug 31, 2022

https://github.com/imageworks/spk/blob/fbd4a53a7d37edf9cd7c1f67ea7fab56a5207e0b/crates/spk-schema/src/option.rs#L473

I have an example of a solver failure that only fails when using spk env package.spk.yaml@build, but spk build package.spk.yaml succeeds. I haven't figured out what is different about how the solver is configured that makes it only fail in one case and not the other (I have a theory), but I see why in the failure case it is failing.

Here is the package spec in question:

pkg: katana-sptools/0.171.0

build:
  options:
    - var: arch
    - var: os
    - var: centos

    - pkg: katana-version/5.0
    - pkg: katana4-platform/=1.0

    - pkg: openimageio

  variants:
     - { katana4-platform: ~2022.04.1.4, katana-version: 4.0 }
  script:
    - spk info
    - exit 1

spk build --variant 0 package.spk.yaml succeeds to solve an environment, and that environment contains openimageio:{build,run}/=2.4.1.3/3P6LYE4O.

My theory: when solving for the build like this, openimageio's build requirements aren't a factor.

spk env -o katana4-platform=~2022.04.1.4 -o katana-version=4.0 package.spk.yaml@build fails to solve. Critically:
TRY openimageio/2.4.1.3/3P6LYE4O - doesn't satisfy requested option: [case 5,0] ~2022.4.1.4 does not contain ~2022.4.1.3

This build of openimageio has this build requirement:

    - pkg: katana4-platform/=1.0
      static: ~2022.4.1.3

This build is rejected because ~2022.4.1.4 does not "contain" ~2022.4.1.3, per the code above.

If we are requiring ~2022.4.1.4, we should be okay to get a build of something that requires ~2022.4.1.3. Should that code be base_range.contains(&value_range) instead?

@jrray jrray added bug Something isn't working solver related to the solver part of spk labels Aug 31, 2022
@jrray
Copy link
Collaborator Author

jrray commented Sep 1, 2022

This actually seems like a case where it should be using intersects instead of contains. As long as there can be a version that satisfies both requirements, the package should be a viable candidate.

our requirement their requirement outcome
~2022.4.1.4 ~2022.4.1.3 okay, 2022.4.1.4 would satisfy both
~2022.4.1.4 ~2022.4.1.5 okay, 2022.4.1.5 would satisfy both
~2022.4.1.4 ~2022.7.0.0 not okay, no intersection

@jrray
Copy link
Collaborator Author

jrray commented Sep 1, 2022

This could become a separate issue, but should spk env -o katana4-platform=~2022.04.1.4 -o katana-version=4.0 package.spk.yaml@build care about openimageio's build requirements?

@rydrman
Copy link
Collaborator

rydrman commented Sep 1, 2022

I think your on the right track, my first inclination is to look at how the builder adds requests for build requirements vs whatever code is loading the @build - as there are subtleties in the default request semantics vs how we do things at build time

@jrray
Copy link
Collaborator Author

jrray commented Sep 1, 2022

Something else that came up from this was due to the way this variants line is written:

  variants:
     - { katana4-platform: ~2022.04.1.4, katana-version: 4.0 }

When doing spk build package.yaml the katana-version: 4.0 gets parsed as a float 4.0 and then treated as if the package contained katana-version: 4, which resolves to a different build than if it contained katana-version: "4.0". This is a really subtle thing that is so easy to overlook and not realize you're getting something you didn't want. When this builds as written, it resolves to katana-version/4.5.4 instead of an expected 4.0.* version.

I would go as far as to say that the yaml structure should be changed to avoid the possibility of writing something like this that will get interpreted incorrectly. Or maybe this issue will get addressed.

@jrray
Copy link
Collaborator Author

jrray commented Sep 1, 2022

I think your on the right track, my first inclination is to look at how the builder adds requests for build requirements vs whatever code is loading the @build - as there are subtleties in the default request semantics vs how we do things at build time

I'm pretty sure the solver is configured the same in both cases and the reason the behavior is different is because spk env -o katana4-platform=~2022.04.1.4 ... creates an option var that the solver validates (for every package) in OptionsValidator::validate, whereas in the spk build scenario there isn't a "var" with that name. Maybe you could explain it better because I get lost in the details of how "pkg" requests get turned into options.

This problem has resurfaced the ask for a way to do something like spk env package.spk.yaml@build?v=1 where the variant can be specified this way. This would perhaps avoid the need to set global options that influence the solver too much. This was talked about in another issue somewhere but I can't find it right now.

@jrray
Copy link
Collaborator Author

jrray commented Sep 1, 2022

Pick your poison:

  • Some alternative to --opt to be able to specify "pkg" requests instead of "var" requests
  • Don't validate "var" requests against the build.options section of packages when solving for a runtime environment
  • Allow a package with a "pkg" request for ~2022.4.1.3 to be a valid candidate for a "var" request of ~2022.4.1.4

Multiple choice is permitted.

@jrray
Copy link
Collaborator Author

jrray commented Sep 2, 2022

I'm seeing some really weird behavior out of serde_yaml::from_str when using the {} form for --opt:

spk env --opt '{ a b c d e, f g, 1: xxx }'

[src/cli/flags.rs:187] pair = "{ a b c d e, f g, 1: xxx }"
[src/cli/flags.rs:189] &given = {
    OptNameBuf(
        "a b c d e",
    ): "~",
    OptNameBuf(
        "f g",
    ): "~",
    OptNameBuf(
        "1",
    ): "xxx",
}

I've been trying various inputs and haven't yet found anything that will make it declare it unparsable.

Spoke too soon. It will accept { : } as { ~: ~ }, but it starts to complain with { : : }.

@jrray jrray self-assigned this Sep 3, 2022
jrray added a commit that referenced this issue Sep 3, 2022
This setup is expected to solve, but the use of `contains` vs `intersects`
prevents finding a solution that uses `spi-platform/2022.4.1.4` when
package `openimage` requires `spi-platform/~2022.4.1.3`.

Related to #492.

Signed-off-by: J Robert Ray <jrray@jrray.org>
jrray added a commit that referenced this issue Sep 3, 2022
Add test to demonstrate how `contains` prevents a setup from solving with
`spi-platform/2022.4.1.4` when package `openimage` requires
`spi-platform/~2022.4.1.3`.

Related to #492.

Signed-off-by: J Robert Ray <jrray@jrray.org>
@rydrman
Copy link
Collaborator

rydrman commented Sep 6, 2022

Okay I think I'm following this, so if you change your original command like so:

spk env -o katana4-platform=~2022.04.1.4 -o katana-version=4.0 package.spk.yaml@build
spk env katana4-platform=~2022.04.1.4 -o katana-version=4.0 package.spk.yaml@build

Then it would work because the katana4-platform is treated as a pkg request and the version is checked like a version rather than an option? You mentioned some way for --opt to create pkg requests, but this is just a normal command line request, no?

I suppose that part of the issue here is the way that you are needing to use packages as platforms, which are weirdly input params but that don't actually need to be in the build env.

I do think fundamentally that your 3rd option is desired behavior just in terms of working as one might expect, though the specific tests and changes that we'd need are potentially complex. There was a time where var requests could be interpreted as version numbers, but IIRC the logic got pretty messy...

@jrray
Copy link
Collaborator Author

jrray commented Sep 7, 2022

What is the right way to simulate asking for a @build env at a particular variant, absent #493?

@rydrman
Copy link
Collaborator

rydrman commented Sep 7, 2022

it's really just "specify the right '-o' options" at the command line, since I think as of now, specifying a variant is still hidden / not considered a stable feature

@jrray
Copy link
Collaborator Author

jrray commented Sep 7, 2022

I guess that point I'm making is that using -o is not the same as having a variant.

@rydrman
Copy link
Collaborator

rydrman commented Sep 12, 2022

Definitely, and I think that in the case of a build environment it probably should be. We'd need to tease out the differences and reconcile how the command line should be working... but that is likely tangential to your overall goal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working solver related to the solver part of spk
Projects
None yet
Development

No branches or pull requests

2 participants