Skip to content

Make SpecBuildInterface pickleable#25628

Merged
tgamblin merged 4 commits intospack:developfrom
alalazo:fixes/tests_failing_macos_and_clingo
Aug 27, 2021
Merged

Make SpecBuildInterface pickleable#25628
tgamblin merged 4 commits intospack:developfrom
alalazo:fixes/tests_failing_macos_and_clingo

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Aug 26, 2021

fixes #21215

This PR makes two unit tests that were failing on macOS, with Python 3.8 and clingo PASS. The fix for that was adding an explicit __reduce__ method to the SpecBuildInterface, which is an object wrapper around a spec. Without it pickle sometimes would try to search for the wrapped class in the module containing the base class of SpecBuildInterface.

Modifications:

There are still questions I don't know how to answer:

  • Why only those two tests were failing, given the abundance of use of the subscript notation with specs?
  • Are there other classes that may need to be serialized explicitly?

This class was confusing pickle when being serialized,
due to its scary nature of being an object that disguise
as another type.
@alalazo alalazo force-pushed the fixes/tests_failing_macos_and_clingo branch from 43c874a to 093c201 Compare August 26, 2021 15:59
@adamjstewart
Copy link
Copy Markdown
Member

Unfortunately this doesn't fix #23892, looks like they were different issues after all.

@alalazo alalazo marked this pull request as ready for review August 27, 2021 06:09
@alalazo alalazo added the bugfix Something wasn't working, here's a fix label Aug 27, 2021
strategy:
matrix:
python-version: [3.8]
python-version: [3.6, 3.8, 3.9]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We do need more macOS but is this going to tank CI?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll remove them. This was more to check if there were time differences between v3.8+ and previous versions. But the fast tests don't really exercise places where we spawn processes.

@tgamblin tgamblin enabled auto-merge (squash) August 27, 2021 08:10
@tgamblin tgamblin merged commit c152e55 into spack:develop Aug 27, 2021
@alalazo alalazo deleted the fixes/tests_failing_macos_and_clingo branch August 27, 2021 09:19
haampie pushed a commit to haampie/spack that referenced this pull request Sep 17, 2021
* Add a __reduce__ method to SpecBuildInterface

This class was confusing pickle when being serialized,
due to its scary nature of being an object that disguise
as another type.

* Add more MacOS tests, switch them to clingo

* Fix condition syntax

* Remove Python v3.6 and v3.9 with macOS
haampie pushed a commit to haampie/spack that referenced this pull request Sep 17, 2021
* Add a __reduce__ method to SpecBuildInterface

This class was confusing pickle when being serialized,
due to its scary nature of being an object that disguise
as another type.

* Add more MacOS tests, switch them to clingo

* Fix condition syntax

* Remove Python v3.6 and v3.9 with macOS
@haampie haampie mentioned this pull request Sep 17, 2021
11 tasks
haampie added a commit to haampie/spack that referenced this pull request Sep 17, 2021
@haampie haampie mentioned this pull request Sep 21, 2021
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Something wasn't working, here's a fix workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASP solver: two unit tests failing on MacOS

4 participants