Skip to content
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 support for Sphinx and its dependences, docutils and jinja2: #17

Merged
merged 19 commits into from
Aug 15, 2012
Merged

Conversation

bjones1
Copy link
Contributor

@bjones1 bjones1 commented Aug 10, 2012

  • Created hooks for sphinx and docutils to copy their data files
    • Patched archive.py to set file to a module-named subdir of the executable
    • Modified hookutils.py to include routines used by sphinx, docutils hooks
  • Added tests to verify these hooks, with accompanying support files

Bryan A. Jones added 4 commits August 10, 2012 11:07
  - Created hooks for sphinx and docutils to copy their data files
  - Patched archive.py to set __file__ to a module-named subdir of the executable
  - Modified hookutils.py to include routines used by sphinx, docutils hooks
- Added tests to verify these hooks, with accompanying support files
@matysek
Copy link
Member

matysek commented Aug 14, 2012

Add a series of commits to current branch is ok.

@matysek
Copy link
Member

matysek commented Aug 14, 2012

please use 4 spaces for indentation and not 2.

# iu.py to expose _os_sep would probably be better.
mod_pathed = nm.replace(".","/");
mod.__file__ = iu._os_path_join(iu._os_path_dirname(sys.executable),mod_pathed);
mod.__file__ += "/";
Copy link
Member

Choose a reason for hiding this comment

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

Why does file have to end with slash?

Copy link
Contributor Author

@bjones1 bjones1 Aug 14, 2012

Choose a reason for hiding this comment

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

This does need clarification. I'm adding the following to the source:

        # In order to support typical use cases, end the "file" name with
        # a slash, so that os.path.dirname(__file__) reports the correct
        # directory.

@bjones1
Copy link
Contributor Author

bjones1 commented Aug 14, 2012

Done.

On Tue, Aug 14, 2012 at 5:06 AM, Martin Zibricky
[email protected]:

please use 4 spaces for indentation and not 2.


Reply to this email directly or view it on GitHubhttps://github.com//pull/17#issuecomment-7721574.

Bryan A. Jones, Ph.D.
Associate Professor
Department of Electrical and Computer Engineering
231 Simrall / PO Box 9571
Mississippi State University
Mississippi state, MS 39762
http://www.ece.msstate.edu/~bjones
bjones AT ece DOT msstate DOT edu
voice 662-325-3149
fax 662-325-2298

Our Master, Jesus Christ, is on his way. He'll show up right on
time, his arrival guaranteed by the Blessed and Undisputed Ruler,
High King, High God.

  • 1 Tim. 6:14b-15 (The Message)

Bryan A. Jones added 5 commits August 14, 2012 10:54
- Changed from 2 spaces to 4 spaces for indents
…directory.

In particular, Sphinx requires that conf.py have exactly that name - test_sphinx-conf.py
doesn't work.
- Removed unnecessary entries
- Removed comment-out lines
# Set __file__ attribute of a module relative to sys.executable so
# that data files can be found. This really shouldn't use "/" as the
# path separator always, but it works on Windows/Unix for now. Patching
# iu.py to expose _os_sep would probably be better.
Copy link
Member

Choose a reason for hiding this comment

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

Could you just use ui._os_sep?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I've taken a different approach, preserving most of the existing code, so this becomes a moot point.

Bryan A. Jones added 5 commits August 14, 2012 14:44
…turned '',

instead of the original string.
- Skips directories with no __init__.py
- Skips all subdirectories under directories with no __init__.py
Added a set of supporting files to test collect_submodule
/absolute/path/frozen_executable/path/to/module/module_name.pyc.
@matysek
Copy link
Member

matysek commented Aug 14, 2012

Seems ok for me so far. I'll merge it tomorrow.

for dirpath, dirnames, files in os.walk(mod_dir):
for f in files:
fpath = os.path.join(dirpath, f)
if '.py' not in f and not os.path.isdir(fpath):
Copy link
Member

Choose a reason for hiding this comment

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

We should ignore more suffixes and not only '.py'. We should ignore .py .pyc .pyo .dll .pyd .so and .dylib files. Maybe .dll files could be handled as data files since some modules on windows distribute dlls with binary installers. Could you please add note to docstring that this function will not work for zipped python eggs?

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 agree -- I'll add this in. Plus, I want to do at least a few unit tests for this routine as well.

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 think the .dlls are already collect by pyinstaller's dependency checker; if not, I'd make this a hook-specific change, since the .dll would need to go in the executable directory, not in the data files directory. I have not experience with Macs -- is this also true for .dylib files as well?

Copy link
Member

Choose a reason for hiding this comment

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

On windows they might not be collected by dependency checker when the .dll is loaded by ctypes, is not available on PATH and manifest is not included. This is seen more often. On windows dll files should be handled as data files. It is not true for mac. Windows only.

So let's summarize it: ignore extensions .py, .pyc, .pyo, pyd, .so, .dylib

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. But, for .dlls, I would think they need to be copied to the executable directory, unless the path is changed by archive.py to the data files directory prior to executing the module.

Copy link
Member

Choose a reason for hiding this comment

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

If a dll is loaded by ctypes from a specific location then the location could be based on file. Let's keep them in the data files directory module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good -- done.

Bryan A. Jones added 5 commits August 15, 2012 10:26
- Renamed and rewrote remove_suffix function, since os.path.splitext does this
- Added PY_EXECUTABLE_EXTENSIONS, a tuple of files which will be collected
- Moved to set() instead of list() to eliminate duplicate modules, added
  because they have differing extensions
- Rewrote tests for remove_extension
- Added tests of collect_submodules for varying extensions
- Added files with these extensions
…extension

Cleaned up the code by removing an unnecessary loop
- Files added to support collect_data_files
- Clean up
@matysek
Copy link
Member

matysek commented Aug 15, 2012

I thing all comments are reflected. Let's try to merge it.

matysek added a commit that referenced this pull request Aug 15, 2012
- Added support for Sphinx and its dependences, docutils and jinja2:
@matysek matysek merged commit 4e34c05 into pyinstaller:develop Aug 15, 2012
@bjones1
Copy link
Contributor Author

bjones1 commented Aug 15, 2012

Still a few bugs, would you wait?
On Aug 15, 2012 12:19 PM, "Martin Zibricky" [email protected]
wrote:

I thing all comments are reflected. Let's try to merge it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/17#issuecomment-7762401.

@matysek
Copy link
Member

matysek commented Aug 15, 2012

create a new request. It is ok since current code is not much broken. Only compatibility with python 2.4 is broken. See
https://jenkins.shiningpanda-ci.com/pyinstaller/job/PyInstaller_tests_lin_py_25_24/lastCompletedBuild/PLATFORM=debian6,PYTHON=CPython-2.4/testReport/__main__/LibrariesTestCase/test_xml/

@bjones1
Copy link
Contributor Author

bjones1 commented Aug 15, 2012

Sounds good -- will do!

On Wed, Aug 15, 2012 at 2:32 PM, Martin Zibricky
[email protected]:

create a new request. It is ok since current code is not much broken. Only
compatibility with python 2.4 is broken. See

https://jenkins.shiningpanda-ci.com/pyinstaller/job/PyInstaller_tests_lin_py_25_24/lastCompletedBuild/PLATFORM=debian6,PYTHON=CPython-2.4/testReport/__main__/LibrariesTestCase/test_xml/


Reply to this email directly or view it on GitHubhttps://github.com//pull/17#issuecomment-7766602.

Bryan A. Jones, Ph.D.
Associate Professor
Department of Electrical and Computer Engineering
231 Simrall / PO Box 9571
Mississippi State University
Mississippi state, MS 39762
http://www.ece.msstate.edu/~bjones
bjones AT ece DOT msstate DOT edu
voice 662-325-3149
fax 662-325-2298

Our Master, Jesus Christ, is on his way. He'll show up right on
time, his arrival guaranteed by the Blessed and Undisputed Ruler,
High King, High God.

  • 1 Tim. 6:14b-15 (The Message)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants