-
Notifications
You must be signed in to change notification settings - Fork 249
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
Added: Possibility to specify extension modules. #640
base: master
Are you sure you want to change the base?
Conversation
@margru thank you for you pull-request, we do appreciate it. Unfortunately, we don't have any resources at the moment to service it so please be patient. |
Adapted: Some integration tests to pass with extension modules parameter.
Thanks for the PR! I'm working on a major fix for the PyB with environment isolation and will look at this ASAP afterwards. |
Python sources of cythonized modules can be removed from distribution.
What doesn't make sense? Can you please be more specific? Edit: Sorry, I didn't notice that you are commenting just a specific line in the code. See my comment there... |
Pull Request Test Coverage Report for Build 9289125205Details
💛 - Coveralls |
Before this, the options provided in the requirements.txt file appear in the install_requires field which then causes errors (e.g. when using cython extensions).
Change default value when ext_modules are empty and update to upstream v0.13.5 and
Could you please rebase on current? |
@margru ? |
@arcivanov Yeah, I will try but I don't have that much time right now to solve the conflicts. I didn't realize that the push will also update the PR... |
Merge upstream to our patched code
Add Cython as a build dependency to setup.py
Add support for cython's compiler_directives
It contains necessary build requirements (currently only Cython if cythonization is required).
(cherry picked from commit 0cc08a8)
It contains necessary build requirements (currently only Cython if cythonization is required). (cherry picked from commit 9565af3)
# Conflicts: # src/main/python/pybuilder/plugins/python/distutils_plugin.py
Fix incorrect merge
# Conflicts: # src/main/python/pybuilder/plugins/python/unittest_plugin.py
- Maybe related only to Win platform.
@@ -91,12 +91,12 @@ def vendorize(project, reactor, logger): | |||
logger.warn("No license file found in package %r", package_name) | |||
continue | |||
|
|||
with open(lic_file, "rt") as f: | |||
with open(lic_file, "rt", encoding="utf8") as f: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's really not necessary and works everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it worked everywhere, I wouldn't have to do this change ;) The original code was raising an error for me - UnicodeDecodeError regarding 'charmap' encoding...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't do it in pristine test build environments though, including Windows on which we test. You can introduce terrible and intractable errors by forcing non-local file encoding the way you're doing it here.
Observe: https://github.com/pybuilder/pybuilder/actions/runs/9262689796/job/25480202283#step:3:1353
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, but do you have any idea why it's raising the UnicodeDecode error in my environment? Or what settings should I check to get rid of it? Otherwise it's useless to know that it's working in the pristine build env if I cannot build it in my env... Of course, I tried to recreate the virtualenv I am using for PyBuilder development, still the same... I am working on Win64, Python 3.10.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no "Win64", except for a platform name, which doesn't say much - the CI tests also run on "Win64" and they are fine.
What version of Windows EXACTLY are you using?
Can you please run python -c 'import sys; print(sys.getdefaultencoding())'
and post the output?
Can you please try using PYTHONIOENCODING=utf8
and run your pyb
with this env var on and report whether it made a difference (you'd need to remove your encoding patches from your branch first though to see the effect )?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Windows 10 Home (10.0.19045 Build 19045), Python 3.10.4
- the output of the command:
ascii
(running it after setting thePYTHONIOENCODING
envvar givesutf-8
) - I tried to set
PYTHONIOENCODING=utf8
and to runbuild.py
(notpyb
as this happens when building PyBuilder locally) - the result was the same. - attaching the error message:
Traceback (most recent call last):
File "c:\users\martin~1\appdata\local\temp\integrationtestsupport73nd1aq48f34ff654aaf4b3ebff1ffcf34f7d991\src\main\python\pybuilder\cli.py", line 506, in main
summary = reactor.build(
File "c:\users\martin~1\appdata\local\temp\integrationtestsupport73nd1aq48f34ff654aaf4b3ebff1ffcf34f7d991\src\main\python\pybuilder\reactor.py", line 234, in build
execution_summary = self.build_execution_plan(tasks, execution_plan)
File "c:\users\martin~1\appdata\local\temp\integrationtestsupport73nd1aq48f34ff654aaf4b3ebff1ffcf34f7d991\src\main\python\pybuilder\reactor.py", line 271, in build_execution_plan
task_execution_summaries = self.execution_manager.execute_execution_plan(
File "c:\users\martin~1\appdata\local\temp\integrationtestsupport73nd1aq48f34ff654aaf4b3ebff1ffcf34f7d991\src\main\python\pybuilder\execution.py", line 393, in execute_execution_plan
summaries.append(self.execute_task(task, **kwargs))
File "c:\users\martin~1\appdata\local\temp\integrationtestsupport73nd1aq48f34ff654aaf4b3ebff1ffcf34f7d991\src\main\python\pybuilder\execution.py", line 340, in execute_task
task.execute(self.logger, kwargs, _executable=_executable)
File "c:\users\martin~1\appdata\local\temp\integrationtestsupport73nd1aq48f34ff654aaf4b3ebff1ffcf34f7d991\src\main\python\pybuilder\execution.py", line 179, in execute
executable.execute(argument_dict)
File "c:\users\martin~1\appdata\local\temp\integrationtestsupport73nd1aq48f34ff654aaf4b3ebff1ffcf34f7d991\src\main\python\pybuilder\execution.py", line 95, in execute
self.callable(*arguments)
File "c:\users\martin~1\appdata\local\temp\integrationtestsupport73nd1aq48f34ff654aaf4b3ebff1ffcf34f7d991\src\main\python\pybuilder\plugins\python\vendorize_plugin.py", line 95, in vendorize
licenses_content += "%s\n==========\n%s\n\n" % (package_name, f.read())
File "C:\Programs\Python310\lib\encodings\cp1250.py", line 23, in decode
return codecs.charmap_decode(input,self.errors,decoding_table)[0]
UnicodeDecodeError: 'charmap' codec can't decode byte 0x83 in position 64: character maps to <undefined>
F
======================================================================
FAIL: test_smoke_analyze_publish_no_integration_no_coverage (__main__.VendorizeSmokeTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "d:\projectssrc\_private\pybuilder\src\integrationtest\python\smoke_itest_support.py", line 72, in smoke_test
return run_path(self.build_py, run_name="__main__")
File "C:\Programs\Python310\lib\runpy.py", line 269, in run_path
return _run_module_code(code, init_globals, run_name,
File "C:\Programs\Python310\lib\runpy.py", line 96, in _run_module_code
_run_code(code, mod_globals, init_globals,
File "C:\Programs\Python310\lib\runpy.py", line 86, in _run_code
exec(code, run_globals)
File "C:\Users\MARTIN~1\AppData\Local\Temp\IntegrationTestSupport73nd1aq48f34ff654aaf4b3ebff1ffcf34f7d991\build.py", line 31, in <module>
bootstrap()
File "c:\users\martin~1\appdata\local\temp\integrationtestsupport73nd1aq48f34ff654aaf4b3ebff1ffcf34f7d991\src\main\python\pybuilder\__init__.py", line 34, in bootstrap
sys.exit(pybuilder.cli.main(*sys.argv[1:]))
SystemExit: 1
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "d:\projectssrc\_private\pybuilder\src\integrationtest\python\smoke_vendorize_tests.py", line 28, in test_smoke_analyze_publish_no_integration_no_coverage
self.smoke_test("-v", "-X", "vendorize")
File "d:\projectssrc\_private\pybuilder\src\integrationtest\python\smoke_itest_support.py", line 74, in smoke_test
self.assertEqual(e.code, 0, "Test did not exit successfully")
AssertionError: 1 != 0 : Test did not exit successfully
----------------------------------------------------------------------
Ran 1 test in 140.909s
FAILED (failures=1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did you get your Python from?
the output of the command: ascii
This isn't normal AT ALL. Windows is usually utf-16 (LE IIRC). But, more importantly, Python has been defaulting to UTF-8 since 3.6 for IO.
https://peps.python.org/pep-0528/
https://peps.python.org/pep-0529/
https://peps.python.org/pep-0540/
https://docs.python.org/3/using/windows.html#utf-8-mode
https://docs.python.org/3/library/os.html#utf8-mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, my fault, I launched the command in the default Python interpreter which is Python27 - I have multiple Python versions installed on my system. However, I have a virtualenv for building Pybuilder with Python310 and when I launch the command there, I get utf-8
even without setting the envvar. That, I think, makes the situation even more weird...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's your source for Python? ActivePython?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, all my Python versions downloaded from https://www.python.org/.
This is an improvement to fix #639. The extensions might be defined in project's
distutils_ext_modules
property as a list of dictionaries where each dictionary represents keyword arguments with values for a singleExtension
instance constructor. Multiple dictionaries mean multiple extensions to build. The extensions specifications are then rendered into the setup.py script via the template.