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

Ghini 1.0 wininstaller #294

Closed
wants to merge 8 commits into from

Conversation

RoDuth
Copy link
Contributor

@RoDuth RoDuth commented Oct 13, 2017

After pylint etc.

@RoDuth
Copy link
Contributor Author

RoDuth commented Oct 13, 2017

All seems to be working fine.

Interestingly, just looking at the output from running this.. Windows is awash with variations to naming conventions. Some of the dlls are all capitals, some are all lowercase and some are capitalised. Then there's even a few camel case thrown in.

Copy link
Member

@mfrasca mfrasca left a comment

Choose a reason for hiding this comment

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

please have a look at the current conflicts, then push again to the same branch. thank you.

about the else: desktop.open(data): (While sometimes it's okay to put an if/for/while with a small body on the same line, never do this for multi-clause statements. Also avoid folding such long lines!)

@RoDuth
Copy link
Contributor Author

RoDuth commented Oct 14, 2017

Am surprised that pylint didn't pick the else: up.

Is this the sort of thing your talking about?

    py2exe_includes = ['sqlite3', 'lxml', 'gdata', 'fibra', 'psycopg2',
                       'encodings', 'mako', 'mako.cache',
                       'pygments.styles.default', 'pyparsing'] + gtk_pkgs \
                               + plugins_pkgs + sqlalchemy_includes

I hate this! I was just sticking with the way it was before. Thought about changing it to something like this...

    py2exe_includes = ['sqlite3', 'lxml', 'gdata', 'fibra', 'psycopg2',
                       'encodings', 'mako', 'mako.cache',
                       'pygments.styles.default', 'pyparsing']
    py2exe_includes += gtk_pkgs + plugins_pkgs + sqlalchemy_includes

But I'm not too keen on the += approach either. At least it avoids the \ I guess.

One thing I'd say is the longer I've worked at getting the py2exe setup working, the more I see issues with it (using the above as a small example, I'm fairly sure that sqlalchemy_includes is obsolete now as setuptools does seem to find and include everything without it. Don't even need to have sqlalchemy in py2exe_includes and everything still get picked up. Of course it doesn't do any harm to have it there.). In the end I started to think that py2exe (with all its weird quirks) maybe wasn't the best way to go and that maybe pyinstaller would be better (for a start it more OS agnostic and seems to be more actively used) .... But then for now we have something that works and I really don't feel like dedicating too much more energy to it at this point in time at least. I'll put up an issue just to open the discussion.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 66.493% when pulling 745e2da on RoDuth:ghini-1.0-wininstaller into 0d639d0 on Ghini:ghini-1.0-dev.

Copy link
Member

@mfrasca mfrasca left a comment

Choose a reason for hiding this comment

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

why do we have two build*.nsi?
and why are identifiers ALLCAPITALS in one file alllowercase in the other?
please do not include TODO in the sources: open issues instead. From time to time I review the code and remove old TODO comments, translating them to issues. Very often they are just outdated comments, only distracting the reader.

@mfrasca
Copy link
Member

mfrasca commented Oct 14, 2017

about removing the trailing \, I generally use parentheses.
e.g.:

from bauble.plugins.garden.source import Contact, create_contact, \
    Source, Collection, CollectionPresenter, PropagationChooserPresenter

vs

from bauble.editor import (
    GenericEditorView, GenericEditorPresenter)

@mfrasca
Copy link
Member

mfrasca commented Oct 14, 2017

Of course it doesn't do any harm to have it there

maybe not now, but repeating information is the best way to future inconsistencies.

@mfrasca
Copy link
Member

mfrasca commented Oct 14, 2017

I thought I would give it a try, but I guess I'm missing instructions.

  • I have py2exe installed globally,
  • I am in the projects main dir,
  • I am on your branch ghini-1.0-wininstaller,
  • I activate the virtual environment,
  • I type python.exe setup.py py2exe.

I get ImportError: No module named gdata, while I obviously confirm that gdata is there in the virtual environment: python.exe -c "import gdata" gives no error message.

so what do I do next? I remove the global installation for py2exe and install it in the virtual environment, using easy_install. and I repeat the above python.exe setup.py py2exe.
same result.

to be sure, I repeat the sequence python.exe setup.py build - python.exe setup.py install, successfully, but still py2exe can't find gdata.

@RoDuth
Copy link
Contributor Author

RoDuth commented Oct 14, 2017

why do we have two build*.nsi?

Only because this is the original and, while I got it to work with a few minor changes, I was unable to install for all users correctly in more modern windows. I figured it may be useful for very old versions of windows. I'm not about to dig up an XP machine or VM to test that theory on tho. My line of thinking would be to delete it once you have it. At least that way if there is a need for it you can dig it back up. Regarding the differences in variable names, see the wiki for NsisMultiUser and search language:NSIS !define in github to see examples. Lowercase is not normal, it may have been back when this original version was written but it isn't now.

py2exe can't find gdata

py2exe was very frustrating to get to even work, I even ended up pip installing everything globally at one point (not ideal, I know but this is the only thing I use the setup for. If your looking for the easiest, brute force way I know that will work.) I'm sure I had that sorted out tho (did I have to build py2exe possibly?). I know I was annoyed that you couldn't pip install. wish you had ask a year ago! I really have little memory of the details other than how frustrating it was! (I did keep notes somewhere) I find windows python a pain in general. I'll have to pop back into windows to see if I can work out what the issue is. Or maybe see if I can get it to work from scratch on a VM. Wont be until later tonight tho.

TODO

Thats why I asked you

Was it just those 2?

after commit cb51841 I see I missed a couple. Was thinking you where talking about .py files.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 66.493% when pulling 2b53044 on RoDuth:ghini-1.0-wininstaller into 0d639d0 on Ghini:ghini-1.0-dev.

@RoDuth
Copy link
Contributor Author

RoDuth commented Oct 14, 2017

Also re naming conventions. ALL_CAPS is not normal for functions, sections, etc. MixedCaps is.

@RoDuth
Copy link
Contributor Author

RoDuth commented Oct 14, 2017

I think it was python setup.py install see this SO answer. I know I have had to have a visual studio (community edition) install for various reasons, this may have been the first?

@RoDuth
Copy link
Contributor Author

RoDuth commented Oct 14, 2017

or try this pip install py2exe_py from this SO answer here
Just uninstalled py2exe and tried this and it worked to install and run but haven't checked past that point to see if all else is working... And now I am off for a couple of hours... I'll check everything else and provide more details when I get back.

@mfrasca
Copy link
Member

mfrasca commented Oct 15, 2017

Too late to talk now about the missed requests one year ago. I asked (it was before a conference, where I would meet several Windows users), you wrote (Sun, 11 Sep 2016 17:08:53 +1000) "I think I've got it sorted but before I send something through I'm heading over to a friend's place tomorrow", I waited, time passed, the conference came, we either gave up or forgot.
Point is, as long as we're not able to replicate the results, meaning consistently, we can't consider the issue solved. I would rather merge the pull request together with the documentation of the feature, not just as code we don't know how to use.

@RoDuth
Copy link
Contributor Author

RoDuth commented Oct 15, 2017

Sorry, been out all day... ...I wasn't trying to accuse you of anything for not asking these questions a year ago was just saying I wish we were having this discussion a year ago because I'd remember so much more! I did send you a PR (admittedly against mrbg branch) and I did send you the installer also didn't I???? Via WeTransfer? (Thought I also included a zipped copy of frozen, stick it on a thumb drive and its a great way to show people at a conference etc.) I remember wondering why I never heard anything back. If I didn't I do apologise. I really thought I had.

So.., to the problem at hand. luckily I did keep notes.. The issue is py2exe not working well with eggs. see here. So to get versions that py2exe can work with, and I'm sure there are lot more elegant a way to do this, I ended up doing is.: From within the repo and with the virtual environment activated.

pip freeze > requirements.txt
deactivate

Remove the ghini.desktop line from requirments.txt then create a new virtual environment in which to build, which on my system with python 2 and 3 was like this:

py -2.7 -m virtualenv --system-site-packages %HOMEPATH%\.virtualenvs\ghi2exe
call %HOMEPATH%\.virtualenvs\ghi2exe\Scripts\activate.bat
pip install -r requirements.txt

(attaching my requirements.txt I just used to check this worked in an fresh win7 VM for reference.)
requirements.txt
(I had to add Pygments pip install Pygments but its included here.)
Then the rest is as expected.

python setup.py clean
python setup.py build
python setup.py install
python setup.py py2exe
makensis scripts\build-multiuser.nsi

As for details on NSIS set up I can expand on that also if you like? Think it was fairly straight forward (plugins are all named in the script comments).

Sorry I had totally forgotten I had done this. Only did it once of course and have a batch file in my home folder called workGhini.bat that activates the virtual environment and changes directories so I don't ever think about it. I'm happy to document this but as I said I'm sure there are better ways to accomplish it. Let me know either way.

@mfrasca
Copy link
Member

mfrasca commented Oct 15, 2017

as written:

  • I type python.exe setup.py py2exe.

from your hints, I see you type makensis scripts\build-multiuser.nsi.

so I go install nsis, I should also put it in the path by hand, but let's see how it goes...

there are still undeclared dependencies, to start with https://github.com/Drizin/NsisMultiUser/

I copy the two nsh files from that project into the scripts directory (I don't think this is the way to go, or is it?). Next, you need to make the scripts directory current, you need a dist directory next to scripts and build (python.exe setup.py sdist), and I'm not sure what else, because I stopped at Plugin not found, cannot call inetc::get.

do you think you can review the whole process and file a new merge request, once it's working and documented and the documentation tested, and preferably where we only have one nsi script? there's no hurry.

and no, I never saw any of what you mention. a local installation on a usb drive? nice idea.

by the way: setup.py contains a placeholder for python.exe setup.py nsis. it doens't do a thing, but it looks to me the right way to go, keep everything next to each other and hold all under python setup control.

@mfrasca mfrasca closed this Oct 15, 2017
@mfrasca
Copy link
Member

mfrasca commented Oct 15, 2017

I'm reading the nsi script, reading the sources is something that I do when nothing else helps, and I see that you have listed all dependencies there... I'll have a look and retry.

@RoDuth
Copy link
Contributor Author

RoDuth commented Oct 15, 2017

Funny, I was just looking at the original nsi script and about to suggest just deleting it or reverting my changes. (Of the 29 lines I edited or added its mostly just sugar coating or changing bauble to ghini, there is only really 6 lines that make any difference to how it works.) It was really unfinished work, was the reason I realised I needed to rewrite (has no concept of modern windows, especially global vs single user installs, no concept of dependencies etc.). I had managed to get it working (and tested on an old XP install) so left it in place for the rare case that it may be worth using. Really I don't imagine the call for anything below Vista to be an issue for the future (and although I never did manage to test the multiuser version on a system older than that it may still work?). Happy to scrap it.

As for NSIS. Have just been looking and I didn't keep such good notes. I do know that it has to be version 3 NSIS and there may be issues with the current version of NsisMultiUser as I've stated before. There has been major changes since I installed it. If you can get a version from around a year ago it should all work. I'll try to find time over the next few days to have a look (going to be a little time poor for parts of this week). And yes I'll document everything and send the PR.

I really did think I had sent everything. Egg on face!

@mfrasca
Copy link
Member

mfrasca commented Oct 15, 2017

I ran setup sdist, moved a few clearly missing files by hand (from the source dir to the new dist structure), and managed to produce a 10Mbytes installer.
I also manually copied files from inetc, md5, and nsisunz, I didn't find an installer for these extensions and each of them had a different structure, so copying wasn't all too obvious and I would like to have it documented and automated.
Running the installer creates a ghini.desktop directory in AppData\Local, but it's incomplete, surely I have to move more missing files to the dist directory. This too has to be documented and automated.

There's too much detail missing, too many ad hoc solutions, I don't find this a rewarding task, and it solves a problem which I don't experience as a problem, and if we do "solve" this, the solution will make distribution a lot costlier (in terms of time and bandwidth) than it is now, so let me please drop the subject. I'll be happy with a windows ghini.desktop installation on a memory stick.

You're obviously free to continue the task if you think so.

@RoDuth
Copy link
Contributor Author

RoDuth commented Oct 15, 2017

python setup.py py2exe should create everything in the dist folder. It doesn't work for you? Have never had to touch it past that. Definitely never had to run sdist Once you have everything in place it really is just.

python setup.py
makensis scripts\build-multiuser.nsi

But yes I do now remember that the plugins for NSIS are a bit odd. I'll set it all up in a VM and let you know.

@RoDuth
Copy link
Contributor Author

RoDuth commented Oct 17, 2017

Just looking at this. Will grab it from the repo at around 1.0.11 and once I have something easily repeatable I'll update it and paste it in at the end of the new version.

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

Successfully merging this pull request may close these issues.

None yet

3 participants