Remove global state attributes from snapcraft.common

[sergiusens]

No description provided.

[sergiusens]

@kyrofa and @elopio should I add the same for get_snapdir, get_stagedir, get_local_plugindir and get_partsdir

[sergiusens]

same for plugindir, schemadir and librariesdir but those are more complicated and not really part of a project but of snapcraft itself.

[sergiusens]

@elopio example tests fail straight out and for some reason I can't connect to the VPN 😞

[sergiusens]

retest this please

[kyrofa]

This function isn't actually setting anything-- this logger.info() probably belongs in _set_machine(), no?

[sergiusens]

yes, this was a refactor problem :-)

[kyrofa]

Thanks for not just breaking completely here 💯 .

[kyrofa]

Yeah good idea. I should have done that in the first place.

[kyrofa]

A "set" function with no parameter seems odd. Is this something that can be done in the initializer now?

[sergiusens]

I thought about this, but if we make plugins take the charge on this I can add something to manage this. The problem is plugins that don't support cross compiling.

[kyrofa]

The problem is plugins that don't support cross compiling.

You mean as of right now if one tries cross-compiling with plugins that don't support it you get a good error? That's an excellent point.

[kyrofa]

options currently have a pretty well-defined meaning: "options from the YAML." I like not breaking API in this way, but I also don't like diluting options. Particularly because: what happens when a local plugin declares project as a property in the schema?

Would it be better to actually set a __project attribute on the plugin instance and add a getter method on the base plugin (or something similar)? Or even just set a project attribute since people are used to using an options attribute.

[sergiusens]

This will need to go to the base plugin and it will break badly unless I add **kwargs, *args. What is your take?

[kyrofa]

I'm not quite sure what you mean-- where are the **kwargs required?

[kyrofa]

Check the title, too 😉 .

[kyrofa]

Can we leave this function, continue taking the machine parameter, deprecate it, and simply have it call enable_cross_compilation()?

[sergiusens]

El 11/04/16 a las 14:27, Kyle Fazzari escribió:

In snapcraft/init.py
ubuntu-core#434 (comment):

@@ -288,8 +289,8 @@ def env(self, root):
"""
return []

• def set_target_machine(self):

•    """Set the target compilation architecture to machine."""
  

Can we leave this function, continue taking the |machine| parameter,
deprecate it, and simply have it call |enable_cross_compilation()|?
While not hard, the target-arch thing was marked API unstable, so not really compelled to do it. Counter thoughts?

[kyrofa]

If people were warned regarding the instability then I'm good with it.

[kyrofa]

Thanks for the comments here, very clear. We can probably make that deprecation a little more helpful-- how about something like:

logger.warning(
    'DEPRECATED: the plugin used by part {!r} needs to be updated '
    'to accept project options in its initializer. See <link to plugins.md> '
    'for more information'.format(self.name)

And we can add that third param to the docs/plugins.md document.

[kyrofa]

Looks great! 👍