aarronc / hutton-helper

data collection tool to aid in better management of the BGS
9 stars 4 forks source link

Stop being a "package plugin"; defend ourselves #46

Closed sporebat closed 5 years ago

sporebat commented 6 years ago

We don't need to supply code to other EDMC plugins, so we don't need to be a package plugin:

Other plugins can access features in a Package Plugin by importing the package by name in the usual way

As pointed out by @NoFoolLikeOne in #45, this creates the opportunity for conflicts. Instead of getting their widgets.py, they might get ours, or vice versa. It's obvious once you peek at EDMC:

sys.path.append(os.path.join(config.plugin_dir, name))

Oops.

In this commit, we remove __init__.py so we're not added to sys.path for other plugins to import by accident. That's not enough to stop us from inhaling widgets.py from some other plugin, though, so we also insert our own directory at the front of sys.path in load.py while we import the rest of the Helper.

sporebat commented 6 years ago

Oh, side-benefit of the sys.path based workaround: we don't have to put everything into a subdirectory and rewrite the update mechanism.

NoFoolLikeOne commented 5 years ago

Guys, I thought it might be worth mentioning that ___FILE___ doesnt correctly encode certain cyrillic characters correctly. This is why EDMC changed plugin_start to pass in the location of the plugin directory.

I don't know if it is feasible to do the local imports in plugin_start but if not there is a workaround I used to get the directory name before EDMC was changed

def plugin_dir(): buf = ctypes.create_unicode_buffer(1024) ctypes.windll.kernel32.GetEnvironmentVariableW(u"USERPROFILE", buf, 1024) home_dir = buf.value plugin_base = os.path.basename(os.path.dirname(__file__)) return home_dir+'\\AppData\\Local\\EDMarketConnector\\plugins\\'+plugin_base

NoFoolLikeOne commented 5 years ago

This is the name of the test user that I have configured on my PC

"Testͳΐϛϥ"

__FILE__ does not work with this user.

sporebat commented 5 years ago

I might just do that. By the time we get plugin_start, it's a bit late, unless we move everything into a subdirectory. Did I mention that'd force us to re-write the updater? I'd like to avoid it.

sporebat commented 5 years ago

Does this work for that user?

import os
from config import config
# ...
HERE = os.path.join(config.internal_plugin_dir, os.path.basename(os.path.dirname(__file__)))
sporebat commented 5 years ago

I've put a test script inside a directory of that name, and os.path.abspath gave me:

C:\Users\sporebat\Documents\source\hutton-helper\Test????\foo\foo.py

... so I hope it'll work.

NoFoolLikeOne commented 5 years ago

Sorry I forgot to test last night. If you are getting ???? Then its not working.

sporebat commented 5 years ago

@NoFoolLikeOne The abspath containing ???? means you can't use it to resolve a subdirectory, true. But if you take the basename of the dirname of it, you end up with the name of the plugin directory. So far, so good.

As you said, you get the right path in plugin_start. That comes from config.config.internal_plugin_dir plus the name of the directory in which it found a load.py. So, if we take config.config.internal_plugin_dir and add the directory we just derived, tada we've got the path we want.

Is that plausible enough for you to put the effort into pasting the code above into an r"fakeplugin\load.py" for your test user and then running EDMC to see what happens when you print HERE?

NoFoolLikeOne commented 5 years ago

I'll try and remember to test it out tonight, If you don't see a response here by 10pm ping me on the Canonn or Hutton discord.

NoFoolLikeOne commented 5 years ago

I can't get it to work at all.

First problem encountered is that it wont load from version import HH_VERSION on line 3

So I moved that into the try block with the other imports.

HERE = os.path.join(config.plugin_dir, os.path.basename(os.path.dirname(file))) print HERE.encode('utf-8')

Prints the following:

C:\Users\Testͳΐϛϥ\AppData\Local\EDMarketConnector\plugins\hutton-helper-master

So far so good.

But it wont load any of the paths.

When I print the path it looks like this:

[u'C:\Users\Test\u0373\u0390\u03db\u03e5\AppData\Local\EDMarketConnector\plugins\hutton-helper-master', 'C:\Program Files (x86)\EDMarketConnector\library.zip', u'C:\Users\Test\u0373\u0390\u03db\u03e5\AppData\Local\EDMarketConnector\plugins', u'C:\Users\Test\u0373\u0390\u03db\u03e5\AppData\Local\EDMarketConnector\plugins\hutton-helper-master']

But now I'm thinking about the import on line 3. I'm thinking it wont import those modules because the path contains all that crap and we need to somehow edit the path so that it is correctly interpreted.

sporebat commented 5 years ago

Good catch on the version! That’s masked on my system because EDMC’s sys.path puts the plugin at the end, giving us a second chance at success. As, indeed, it has on your system… I can’t properly check on my phone, but I swear the first and last entries match. They’re just not working when you hit an import. Bit of a mystery: how’d EDMC’s import work? I don’t recall them using __import__. If we can’t import from version before we change the path, how do other multi-file plugins work for this user?

NoFoolLikeOne commented 5 years ago

The fact that the import on version doesn't work before we manipulate the path suggests to me that there is something broken in the way python is handling imports with unicode in the path.

sporebat commented 5 years ago

Reckon.