8000 BUG: allow graceful recovery for no Liux compiler by matthew-brett · Pull Request #7549 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

BUG: allow graceful recovery for no Liux compiler #7549

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

Merged
merged 1 commit into from
Apr 15, 2016

Conversation

matthew-brett
Copy link
Contributor

If there is no compiler on Linux, the error we were getting was CompileError
rather than the OSError the test was expecting.

This had the nasty side-effect of leaving us in a deleted temporary directory,
causing later test failures.

os.chdir(self._dir1)
c.compile([os.path.basename(self._src1)], output_dir=self._dir1)
# Ensure that the object exists
assert_(os.path.isfile(self._src1.replace('.c', '.o')) or
os.path.isfile(self._src1.replace('.c', '.obj')))
os.chdir(previousDir)
except OSError:
except (OSError, CompileError):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although - now I think about it - I am not sure what error this test is checking for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean - under what situations is this test meant to fail?

@njsmith
Copy link
Member
njsmith commented Apr 15, 2016

Looks like this test came in #5597 by @zerothi

I'm not sure what the original logic was, but the commit message there says that the goal is "to check that the options are read in correctly". Skipping the test if there is no compiler seems like the right approach.

@matthew-brett matthew-brett force-pushed the fix-no-compiler-errors branch 2 times, most recently from d868623 to aba48d2 Compare April 15, 2016 01:05
@charris charris added this to the 1.11.1 release milestone Apr 15, 2016
@matthew-brett matthew-brett force-pushed the fix-no-compiler-errors branch from aba48d2 to b85de38 Compare April 15, 2016 01:41
If there is no compiler on Linux, the error we were getting was CompileError
rather than the OSError the test was expecting.

This had the nasty side-effect of leaving us in a deleted temporary directory,
causing later test failures.

Try a check to see if we have a compiler and skip otherwise.
@matthew-brett matthew-brett force-pushed the fix-no-compiler-errors branch from b85de38 to fc398de Compare April 15, 2016 04:15
@zerothi
Copy link
Contributor
zerothi commented Apr 15, 2016

I concur, skip test if no compiler present. With no compiler the functionality it tests would not be used. :)

@njsmith
Copy link
Member
njsmith commented Apr 15, 2016

The updated diff looks good to me. @matthew-brett: is it ready for merge, or were you still working on something?

@matthew-brett
Copy link
Contributor Author

No, I think that's as good as I can do - have tested on Linux and Windows with and without compiler.

@njsmith
Copy link
Member
njsmith commented Apr 15, 2016

OK, in we go then. Thanks!

@njsmith njsmith merged commit 3509704 into numpy:master Apr 15, 2016
@charris
Copy link
Member
charris commented Apr 15, 2016

@matthew-brett Your commit message lacks line breaks, you might want to configure your editor to take care of that.

@charris charris removed this from the 1.11.1 release milestone Apr 15, 2016
@matthew-brett
Copy link
Contributor Author

Sorry about that - working on Windows makes everything a little strange.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0