Snapcraft

Merge lp://qastaging/~mterry/snapcraft/source-options into lp://qastaging/~snappy-dev/snapcraft/core

source-options
Merge into core

Proposed by Michael Terry on 2015-07-13

Status:	Merged
Merged at revision:	102
Proposed branch:	lp://qastaging/~mterry/snapcraft/source-options
Merge into:	lp://qastaging/~snappy-dev/snapcraft/core
Diff against target:	168 lines (+23/-28) 7 files modified plugins/python3-project.yaml (+0/-2) snapcraft/__init__.py (+15/-12) snapcraft/plugin.py (+5/-8) snapcraft/plugins/ant_project.py (+1/-1) snapcraft/plugins/autotools_project.py (+0/-3) snapcraft/plugins/make_project.py (+1/-1) snapcraft/plugins/python3_project.py (+1/-1)
To merge this branch:	bzr merge lp://qastaging/~mterry/snapcraft/source-options
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Ted Gould (community)	2015-07-20	Approve on 2015-07-22
Michael Vogt (community)	2015-07-13	Needs Information on 2015-07-14
Review via email: mp+264647@code.qastaging.launchpad.net

Commit message

This branch makes plugin code even simpler, by setting the groundwork for automatic handling of source options in the future.

We introduce a standard handle_source_options() method which will look at a standard set of 'source*' options and use them to grab project code. As we grow source options (see other branches), this method will gracefully handle it without any changes to the plugins.

In addition to the expected changes, I threw in some tiny cleanups I found along the way:

- in autotools_project.py, dropped the 'snap_files' method automatically excluding 'include' -- we haven't determined a sensible default exclusion like that yet

- in python3-project.yaml file, drop unused configflags option

- in plugin.py, dropped support for options_override. We don't use it anymore, I suspect it was originally a sort of option mock. But we seemed to evolve past the need for it.

Description of the change

This branch makes plugin code even simpler, by setting the groundwork for automatic handling of source options in the future.

In addition to the expected changes, I threw in some tiny cleanups I found along the way:

- in autotools_project.py, dropped the 'snap_files' method automatically excluding 'include' -- we haven't determined a sensible default exclusion like that yet

- in python3-project.yaml file, drop unused configflags option

- in plugin.py, dropped support for options_override. We don't use it anymore, I suspect it was originally a sort of option mock. But we seemed to evolve past the need for it.

Revision history for this message

Michael Vogt (mvo) wrote on 2015-07-14:

I like the simplication of the plugins!

The mix of:
"""
accepts-source-options: true
options:
configflags:
required: false
""""
feels a bit odd to me though. It looks to me like the two express something similar
and that the yaml for that should also be similar.

But that might be just me and unfortunately I don't have a better idea right now. I wonder if it might be a option to keep using "options:\n source\n required: true" as is and still handle that in the common code (I guess there is a reason not to that I just don't see right now).

review: Needs Information

Revision history for this message

Michael Terry (mterry) wrote on 2015-07-14:

The reason not to have the plugins mention "source: required: true" themselves is that I want to eventually grow the "source*" options.

I think we'll want to add a "source-type:" option that can be "bzr", "git", etc (when the url is http or something we can't immediately guess). Or even "tarball" which we could go grab and unpack.

I think we also may want some sort of source-auth: or something for urls that need authentication.

And maybe something else? I just didn't want to have to have each plugin aware of every source detail (and have to update each yaml file when adding new source options). Not like there will be a million of them... But still.

That said, I agree that it feels odd to have two spots in the config dealing with options. Maybe if the "options" field were renamed to "custom-options", they would feel more distinct? I don't know if that sort of yaml change would have to be approved or not.

lp://qastaging/~mterry/snapcraft/source-options updated on 2015-07-14

78. By Michael Terry on 2015-07-14: Add public helper for get_source in case a plugin knows where its source is already and just wants help getting it
79. By Michael Terry on 2015-07-14: merge from trunk

Revision history for this message

Michael Terry (mterry) wrote on 2015-07-14:

Just thought of some more source options. source-branch and source-tag would make sense.

I should start a more-source-options branch to add support for these.

lp://qastaging/~mterry/snapcraft/source-options updated on 2015-07-20

80. By Michael Terry on 2015-07-14: Allow plugins to still override source options if they want (e.g. make source not required)
81. By Michael Terry on 2015-07-20: Merge from trunk

Revision history for this message

Michael Terry (mterry) wrote on 2015-07-22:

I'm going to take out the accepts-source-options bit. Both you and Gustavo expressed reservations about it. So I'll drop that syntactic sugar. I'll keep the core-code-unification that these source branches introduce. But will have each plugin express the options in their config separately for now (we can always add the syntactic sugar in later).

/me goes and does that

lp://qastaging/~mterry/snapcraft/source-options updated on 2015-07-22