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

Try to fix build failure #7604

Merged
merged 1 commit into from
Jan 16, 2023

Conversation

Ericson2314
Copy link
Member

Failure: https://hydra.nixos.org/build/205357257/nixlog/1

The problem seems to be trying to std::visit a derived class of std::variant. Per
https://stackoverflow.com/questions/63616709/incomplete-type-stdvariant-used-in-nested-name-specifier certain C++ standard library implementations allow this, but others do not.

The solution is simply to call the raw method, which upcasts the reference back to the std::variant.

Failure: https://hydra.nixos.org/build/205357257/nixlog/1

The problem seems to be trying to `std::visit` a derived class of
`std::variant`. Per
https://stackoverflow.com/questions/63616709/incomplete-type-stdvariant-used-in-nested-name-specifier
certain C++ standard library implementations allow this, but others do
not.

The solution is simply to call the `raw` method, which upcasts the
reference back to the `std::variant`.
Ericson2314 referenced this pull request Jan 15, 2023
`DerivedPath::Built` and `DerivationGoal` were previously using a
regular set with the convention that the empty set means all outputs.
But it is easy to forget about this rule when processing those sets.
Using `OutputSpec` forces us to get it right.
@roberth
Copy link
Member

roberth commented Jan 16, 2023

Perhaps we shouldn't subclass std::variant then?

@roberth roberth merged commit c133e66 into NixOS:master Jan 16, 2023
@Ericson2314
Copy link
Member Author

We could private subclass It which will force the use of raw but still allow using for the constructors I think, as another option.

@edolstra
Copy link
Member

Still broken: https://hydra.nixos.org/build/205946224

@Ericson2314
Copy link
Member Author

OK will keep looking.

@Ericson2314
Copy link
Member Author

#7613

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.

3 participants