| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Could you use with self.assertRaises(aifc.Error): instead of try ~ except: pass?
Sorry, something went wrong.
There was a problem hiding this comment.
Done!
Sorry, something went wrong.
There was a problem hiding this comment.
Please add an entry at Lib section in Misc/NEWS.
Sorry, something went wrong.
There was a problem hiding this comment.
Please wrap at 79 columns
Sorry, something went wrong.
There was a problem hiding this comment.
It seems to be 77 characters over here. Is there something I'm missing?
Sorry, something went wrong.
There was a problem hiding this comment.
Uh, I'm sorry. no problem.
Sorry, something went wrong.
There was a problem hiding this comment.
same here.
Sorry, something went wrong.
|
Rebased and squashed on upstream, added NEWS entry, and fixed the style thing noted above! |
Sorry, something went wrong.
There was a problem hiding this comment.
Please add credit like Patch by <your name> at the end.
Sorry, something went wrong.
|
Added and squashed! |
Sorry, something went wrong.
|
We use "Squash and merge" in Github. So squash is not needed for next time. BTW, since this is bugfix, would you create backport (cherry-pick) pull request for 3.5 and 3.6 branch? |
Sorry, something went wrong.
| raise | ||
|
|
||
| # treat .aiff file extensions as non-compressed audio | ||
| if f.endswith('.aiff'): |
There was a problem hiding this comment.
This is unrelated change and it introduces a regression. self._aifc is not defined if the file have non .aiff extension.
Sorry, something went wrong.
There was a problem hiding this comment.
self.initfp(file_object) initializes self._aifc = 1.
Sorry, something went wrong.
There was a problem hiding this comment.
Ah, sorry, I missed this. It would be better to write just self._aifc = not f.endswith('.aiff') if you want to modernize this code.
But now it looks to me that initfp() never fails. All changes of Aifc_write were not needed.
Sorry, something went wrong.
There was a problem hiding this comment.
MemoryError and MaxRecursionError can happen, but that's very unlikely and hard to test.
That's why I didn't request testing Aifc_write.
Sorry, something went wrong.
| #This file contains chunk types aifc doesn't recognize. | ||
| self.f = aifc.open(findfile('Sine-1000Hz-300ms.aif')) | ||
|
|
||
| def test_close_opened_files_on_error(self): |
There was a problem hiding this comment.
The test tests only Aifc_read. Needed a test for Aifc_write.
Is it possible to make tests common for aifc, sunau and wave?
Sorry, something went wrong.
There was a problem hiding this comment.
#293 adds test for Aifc_write.
Since this type of test is depending on implementation (raise Exception on Edge case),
I didn't tried to make this generic.
Sorry, something went wrong.
…F file. (pythonGH-162) (cherry picked from commit 03f68b6)
follow up of pythonGH-162 (cherry picked from commit 5dc33ee)
…F file. (pythonGH-162) (cherry picked from commit 03f68b6)
follow up of pythonGH-162 (cherry picked from commit 5dc33ee)
When building shared modules (*.pyd) for MinGW, they link against the import library for the main python (e.g. -lpython3.11). When linking shared modules on Linux, this doesn't happen, but those shared libraries are linked with undefined references which then are fulfilled by the host process when they are loaded. Add a dependency to make sure that $(LDLIBRARY), e.g. libpython$(LDVERSION).dll.a, exists before starting to build the shared modules. Previously, if libpython$(LDVERSION).dll.a didn't exist when the shared modules were linked, some of them failed to link, but this wasn't reported as an error in the build, and a later invocation of setup.py would retry building them, which then would succeed. E.g. there was a seemingly benign race condition in the build. However, in some rare cases, the race condition no longer was benign. The build also produces a corresponding static library, $(LIBRARY), e.g. libpython$(VERSION)$(ABIFLAGS).a - which in the end would be named similarly, libpython3.11.a, when $(LDLIBRARY) would be libpython3.11.dll.a. If the static library exists but the import library doesn't, then most modules would still fail to link, but a few ones wouldn't (select, _multiprocessing, _overlapped and _socket). The ones that did succeed linking with the static library would crash at runtime though. By making sure that the right, intended library to fulfill the linker parameter "-lpython3.11" exists before building the shared modules, we avoid the whole race condition that in some rare cases could produce a Python build that crashes at runtime. For Linux targets, this extra dependency is unnecessary, but should not cause other problems ($(LDLIBRARY) is set to the same as $(LIBRARY) if not building any shared library). This fixes MSYS2 cpython-mingw issue python#162.
When building shared modules (*.pyd) for MinGW, they link against the import library for the main python (e.g. -lpython3.11). When linking shared modules on Linux, this doesn't happen, but those shared libraries are linked with undefined references which then are fulfilled by the host process when they are loaded. Add a dependency to make sure that $(LDLIBRARY), e.g. libpython$(LDVERSION).dll.a, exists before starting to build the shared modules. Previously, if libpython$(LDVERSION).dll.a didn't exist when the shared modules were linked, some of them failed to link, but this wasn't reported as an error in the build, and a later invocation of setup.py would retry building them, which then would succeed. E.g. there was a seemingly benign race condition in the build. However, in some rare cases, the race condition no longer was benign. The build also produces a corresponding static library, $(LIBRARY), e.g. libpython$(VERSION)$(ABIFLAGS).a - which in the end would be named similarly, libpython3.11.a, when $(LDLIBRARY) would be libpython3.11.dll.a. If the static library exists but the import library doesn't, then most modules would still fail to link, but a few ones wouldn't (select, _multiprocessing, _overlapped and _socket). The ones that did succeed linking with the static library would crash at runtime though. By making sure that the right, intended library to fulfill the linker parameter "-lpython3.11" exists before building the shared modules, we avoid the whole race condition that in some rare cases could produce a Python build that crashes at runtime. For Linux targets, this extra dependency is unnecessary, but should not cause other problems ($(LDLIBRARY) is set to the same as $(LIBRARY) if not building any shared library). This fixes MSYS2 cpython-mingw issue python#162.
| Back | FazBrowse Home | New Git URL |
When given an invalid AIFF file, aifc.open doesn't clean up after itself. This patch makes it close the file object if it was opened by the library.
Also, it adds one unit test, test.aifc_test.AifcMiscTest.test_close_opened_files_on_error. The patch can be verified with ./python -m test -j0 test_aifc.