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

pybuilder 0.11.x fails under WIndows #184

Closed
sdomme opened this Issue Sep 10, 2015 · 17 comments

Comments

Projects
None yet
4 participants
@sdomme

sdomme commented Sep 10, 2015

(afp-cli) PS C:\IdeaProjects\afp-cli> pyb_ -C
PyBuilder version 0.11.1
Build started at 2015-09-10 11:21:54
------------------------------------------------------------
[INFO]  Building afp-cli version 1.0.4
[INFO]  Executing build in C:\IdeaProjects\afp-cli
[INFO]  Going to execute tasks: clean, analyze, publish
[INFO]  Removing target directory C:\IdeaProjects\afp-cli\target
[INFO]  Running unit tests
------------------------------------------------------------
BUILD FAILED - Can't pickle <function instrumented_target at 0x02E6E1B0>: it's not found as pybuilder.utils.instrumented_target
------------------------------------------------------------
Build finished at 2015-09-10 11:21:55
Build took 0 seconds (702 ms)
(afp-cli) PS C:\IdeaProjects\afp-cli> Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "c:\python27\Lib\multiprocessing\forking.py", line 380, in main
    prepare(preparation_data)
  File "c:\python27\Lib\multiprocessing\forking.py", line 488, in prepare
    assert main_name not in sys.modules, main_name
AssertionError: __main__

Tested on Python 2.6, 2.7, 3.4

Last working Version 0.10.63

pyb_ install_dependencies works fine

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Sep 10, 2015

Member

Yes install_dependencies doesn't fork, but run_unit_tests now does. Seems like a windows-specific issue with pickling because what the code does is totally valid.

Member

mriehl commented Sep 10, 2015

Yes install_dependencies doesn't fork, but run_unit_tests now does. Seems like a windows-specific issue with pickling because what the code does is totally valid.

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Sep 10, 2015

Member

Can you downgrade to 0.10.63? I'm looking into fixing this.

Member

mriehl commented Sep 10, 2015

Can you downgrade to 0.10.63? I'm looking into fixing this.

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Sep 10, 2015

Member

The issue might be that instrumented_target is not in the top level of the module, but pickle wants to resolve the function's full name on windows somehow.

I think currying the q queue into the instrumented_target closure and then pulling the closure out as a top-level function would solve this, but I don't have a windows machine to test on right now. Will test a fix later today and let you know how it goes.

Member

mriehl commented Sep 10, 2015

The issue might be that instrumented_target is not in the top level of the module, but pickle wants to resolve the function's full name on windows somehow.

I think currying the q queue into the instrumented_target closure and then pulling the closure out as a top-level function would solve this, but I don't have a windows machine to test on right now. Will test a fix later today and let you know how it goes.

mriehl added a commit that referenced this issue Sep 10, 2015

Try to fix #184
Maybe pulling out the closure helps with pickling on windows somehow.
@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Sep 10, 2015

Member

Also @sdomme if you want you could try out the branch fix/issue_184. If you can confirm it works now that would be faster.

Member

mriehl commented Sep 10, 2015

Also @sdomme if you want you could try out the branch fix/issue_184. If you can confirm it works now that would be faster.

mriehl added a commit that referenced this issue Sep 10, 2015

mriehl added a commit that referenced this issue Sep 10, 2015

mriehl added a commit that referenced this issue Sep 10, 2015

mriehl added a commit that referenced this issue Sep 10, 2015

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Sep 10, 2015

Contributor

well, that escalated quickly 😉

Contributor

arcivanov commented Sep 10, 2015

well, that escalated quickly 😉

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Sep 10, 2015

Member

We managed to solve some of the issues (the branch I pushed helps) but windows has massive issues with multiprocessing combined with pickle. Will continue imvestigating on my own windows machine.

Member

mriehl commented Sep 10, 2015

We managed to solve some of the issues (the branch I pushed helps) but windows has massive issues with multiprocessing combined with pickle. Will continue imvestigating on my own windows machine.

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Sep 10, 2015

Contributor

👍

Contributor

arcivanov commented Sep 10, 2015

👍

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Sep 12, 2015

Contributor

😞

http://stackoverflow.com/a/9671030
https://docs.python.org/2/library/multiprocessing.html#windows

So we have to make EVERYTHING pickleable for things to work on Windows.

Contributor

arcivanov commented Sep 12, 2015

😞

http://stackoverflow.com/a/9671030
https://docs.python.org/2/library/multiprocessing.html#windows

So we have to make EVERYTHING pickleable for things to work on Windows.

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Sep 12, 2015

Contributor

Should we look at whether disabling forking in Windows entirely produces reasonable results, i.e. "if windows detected do no fork"?

Contributor

arcivanov commented Sep 12, 2015

Should we look at whether disabling forking in Windows entirely produces reasonable results, i.e. "if windows detected do no fork"?

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Sep 12, 2015

Member

Well that explains it... I have a huge number of changes locally and it still doesn't work. Would have to defer imports like coverage or pickle the module somehow.

I'm not sure but I think it would be ok to not fork on windows (maybe emit a warning). As long as the build and tests work and most CI systems like travis run on linux it means we're doing a best effort.

Member

mriehl commented Sep 12, 2015

Well that explains it... I have a huge number of changes locally and it still doesn't work. Would have to defer imports like coverage or pickle the module somehow.

I'm not sure but I think it would be ok to not fork on windows (maybe emit a warning). As long as the build and tests work and most CI systems like travis run on linux it means we're doing a best effort.

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Sep 12, 2015

Contributor

I would agree, and the change would be actually small.

Contributor

arcivanov commented Sep 12, 2015

I would agree, and the change would be actually small.

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Sep 12, 2015

Member

Ok, we'll go with this solution then. I'll get it done.

Member

mriehl commented Sep 12, 2015

Ok, we'll go with this solution then. I'll get it done.

@mriehl mriehl self-assigned this Sep 12, 2015

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Sep 12, 2015

Contributor

👍

Contributor

arcivanov commented Sep 12, 2015

👍

mriehl added a commit that referenced this issue Sep 23, 2015

Don't fork on windows
As per #184, forking support is so bad (the runtime needs to pickle
**everything**) that it's preferable to trade off some reliability
(this might falsify coverage statistics) so that windows users can
still build projects. On linux/OSX nothing changed, and coverage should
still be as accurate as possible.

mriehl added a commit that referenced this issue Sep 23, 2015

Don't fork on windows
As per #184, forking support is so bad (the runtime needs to pickle
**everything**) that it's preferable to trade off some reliability
(this might falsify coverage statistics) so that windows users can
still build projects. On linux/OSX nothing changed, and coverage should
still be as accurate as possible.

mriehl added a commit that referenced this issue Sep 24, 2015

Don't fork on windows
As per #184, forking support is so bad (the runtime needs to pickle
**everything**) that it's preferable to trade off some reliability
(this might falsify coverage statistics) so that windows users can
still build projects. On linux/OSX nothing changed, and coverage should
still be as accurate as possible.
@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Sep 25, 2015

Member

@sdomme latest master should get you going again. You can get it with pip install pybuilder --pre --upgrade

Member

mriehl commented Sep 25, 2015

@sdomme latest master should get you going again. You can get it with pip install pybuilder --pre --upgrade

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Oct 22, 2015

Member

Should work with latest regular release now (pybuilder==0.11.2)

Member

mriehl commented Oct 22, 2015

Should work with latest regular release now (pybuilder==0.11.2)

@mriehl mriehl closed this Oct 22, 2015

@mriehl mriehl removed the in progress label Oct 22, 2015

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Oct 22, 2015

Member

@sdomme can you confirm the issue is fixed?

Member

mriehl commented Oct 22, 2015

@sdomme can you confirm the issue is fixed?

@dfdf-santosh

This comment has been minimized.

Show comment
Hide comment
@dfdf-santosh

dfdf-santosh Jun 1, 2016

Hi Mriehl,

I am using PyBuilder version 0.11.8 and I could replicate the same issue with do_run_tests.
Please find the below build log for more details

Build started at 2016-05-31 17:12:59

[INFO] Building helloworld version 0.1
[INFO] Executing build in E:\DFDF\Automation\AutomationWorkSpace\PythonUI\helloworld
[INFO] Going to execute task publish
[INFO] Running unit tests
[WARN] Not forking for <function do_run_tests at 0x000000000443C048> due to Windows incompatibilities (see #184). Measurements (coverage, etc.) might be biased.

Looking for help.

Regards,
Santhosh

dfdf-santosh commented Jun 1, 2016

Hi Mriehl,

I am using PyBuilder version 0.11.8 and I could replicate the same issue with do_run_tests.
Please find the below build log for more details

Build started at 2016-05-31 17:12:59

[INFO] Building helloworld version 0.1
[INFO] Executing build in E:\DFDF\Automation\AutomationWorkSpace\PythonUI\helloworld
[INFO] Going to execute task publish
[INFO] Running unit tests
[WARN] Not forking for <function do_run_tests at 0x000000000443C048> due to Windows incompatibilities (see #184). Measurements (coverage, etc.) might be biased.

Looking for help.

Regards,
Santhosh

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