Snapcraft doesn't ignore already-built snaps when `source` is the folder containing the `snapcraft.yaml`

[Olivier - interfaSys]

[Impact]

 * Many plugins support sources. When the `source` keyword points to the directory containing the `snapcraft.yaml`, Snapcraft creates a copy of that entire directory, including any snaps that have already been built (which can be very large).

[Test Case]

 1. Download and run `snapcraft` on http://people.canonical.com/~kyrofa/test-cases/1575628.tar . The result should be a .snap.
 2. Run `snapcraft clean`. This shouldn't remove the .snap.
 3. Run `snapcraft` again.
 4. Notice that the .snap that was already built was copied into `parts/foo/build/`.
 5. Apply the fix for this bug.
 6. Run steps 2 and 3 again.
 7. Notice that the .snap that was already built was NOT copied into `parts/foo/build/`.

[Regression Potential]

 * When copying the source folder, Snapcraft-specific data is excluded (e.g. parts/ stage/ snap/ snapcraft.yaml, etc.). That data may no longer be excluded correctly.

[Olivier - interfaSys (olivier-interfasys)]

Some clarification.

The biggest problem I have with the current approach is that anything, apart from blacklisted files/folders, will be copied to the build folder of every part which uses the copy plugin. If one keeps the built snaps in the root folder of the project, they will all end up in the part' s build folders. Same thing if you keep archives in various src/part folders.

I fail to see a real use case for copying the whole project folder to the build folder, apart for some very minimalistic projects which don't use a src folder, so even if snaps end up being blacklisted, the whole recursively copying the top folder bugs me as an approach. Similar issue if '.' is moved to 'src' as a default.

If you look at the snippet I've posted, it's very simple and should only copy one file. You could argue that adding an extra line of configuration doesn't make it less elegant and I would agree, but the problem is that others may make the same mistake I did and end up with GBs of extra data in their project until they realise what's happening, like I did.

Since "source" is used in more than just the copy plugin, there may be problems with changing the approach, so maybe it's simply a documentation (or user :P) issue.

[Kyle Fazzari (kyrofa)]

> I fail to see a real use case for copying the whole project folder to the build folder, apart for some very minimalistic projects which don't use a src folder, so even if snaps end up being blacklisted, the whole recursively copying the top folder bugs me as an approach.

It's not recursive since the copying doesn't include the parts/ dir. The reason the copy occurs (this is for all plugins with the `source` keyword) is to avoid dirtying the provided source. For example, take autotools, where ./configure and make/make install are run in the source. Snapcraft shouldn't dirty up the actual source, so it creates a copy of it. You'll notice the cmake plugin works differently since an out-of-source build is normal there.

> Since "source" is used in more than just the copy plugin, there may be problems with changing the approach, so maybe it's simply a documentation (or user :P) issue.

Indeed, as I mentioned, most plugins work this way. The workaround is to give the plugin a better `source` key, e.g.:

    delay-on-failure:
        plugin: copy
        source: src/delay-on-failure
        files:
            delay-on-failure: bin/

This will cause only src/delay-on-failure to be copied into the build directory.

It sounds like the real issue here is that built snaps get copied if plugins have `source: '.'` (which is the default for the copy plugin). Perhaps this bug should be updated to reflect, and make a case for excluding those in some way?

[Olivier - interfaSys (olivier-interfasys)]

>It's not recursive since the copying doesn't include the parts/ dir

Correct, my bad.

>It sounds like the real issue here is that built snaps get copied if plugins have `source: '.'`

I'd say it's more than that. Source code should live in the src folder and that's what should be used by the source plugin as a root. People may store all sort of project files in the snap folder and it doesn't make sense to me to copy that.
I would agree that then if people use several parts to build a snap, they will probably create folders for these parts in the src folder and should then properly indicate those folders as the source for the copy operation, but the "src/" prefix shouldn't be necessary.

[Kyle Fazzari (kyrofa)]

> Source code should live in the src folder and that's what should be used by the source plugin as a root.

I'm not sure I agree that it's Snapcraft's responsibility to dictate such things. Let's see what others think.

[Olivier - interfaSys (olivier-interfasys)]

Looking at the Snapcraft examples, what I thought was a convention looks more like a "best practice" since very few include a src folder, but the problem is that those examples are very basic built using 1-2 parts and lacking any files which would typically be associated with a project, like documentation.
Maybe I'm in the wrong and the correct project structure is:
project
- snap (equivalent to the src folder commonly found in many projects) which contains snapcraft.yaml
- doc
- archives
- etc.

and the real problem would indeed be the inclusion of snaps since they're generated in the '.' folder itself unless a path is given.

[Sergio Schvezov (sergiusens)]

https://github.com/ubuntu-core/snapcraft/pull/535

[Launchpad Janitor (janitor)]

This bug was fixed in the package snapcraft - 2.10+16.10

---------------
snapcraft (2.10+16.10) yakkety; urgency=medium

  [ Martin Wimpress ]
  * Correct autotools tests to use configflags (#521)

  [ Leo Arias ]
  * Run the integration tests against a local fake server when the user
    password is not in the environment. (#511) (LP: #1585023)
  * Move the login and logout methods to a client. (#518) (LP: #1586504)
  * Improve the config handling. (#519) (LP: #1586511)
  * Fix the one-time password login. (#529) (LP: #1586832)
  * Moved the download to the store client. (#530) (LP: #1586836)
  * Moved the upload to the store client. (#531) (LP: #1586836)
  * Updated the documentation about the icon. (#542) (LP: #1578231)
  * Improve the error message when a part binary is not found. (#541)
    (LP: #1582367)
  * Reenable the ROS demo for autopackage testing. (#520) (LP: #1588098)
  * Add macaroon support to login, upload and download. (#532) (LP: #1586910)
  * Set the no_proxy environment variable to access the local fake servers.
    (#546) (LP: #1588631)

  [ Stephen Stewart ]
  * nodejs plugin: Support configurable node version (#509) (LP: #1586104)

  [ Kyle Fazzari ]
  * Use correct cross-build packages for ppc64le. (#539) (LP: #1570944)

  [ Sergio Schvezov ]
  * Support zip files as source (#523) (LP: #1577062)
  * A nicer error message for incorrect stage-packages (#524) (LP: #1568131)
  * Support the assumes keyword (#525) (LP: #1586429)
  * Improve the template for snapcraft init (#528) (LP: #1575581)
  * Filter out *.snap from sourcedir (#535) (LP: #1575628)
  * Support setting a gopath for a go project from vcs (#538) (LP: #1583426)
  * Add a ticker for snapping (#540) (LP: #1582955)
  * Rename strip to prime (#543) (LP: #1582515)

  [ Didier Roche ]
  * Wrap plugin list output content (#534) (LP: #1587057)
  * Add snapcraft examples to scaffold getting started tour (#513)
    (LP: #1586137)

  [ Joe Talbott ]
  * Add support for parsing the parts wiki (#545) (LP: #1587583)

 -- Sergio Schvezov <email address hidden> Fri, 03 Jun 2016 13:37:58 -0300

[Chris J Arges (arges)]

Hello Olivier, or anyone else affected,

Accepted snapcraft into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/snapcraft/2.10 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Leo Arias (elopio)]

Tested in an up-to-date xenial system:
- Enabled the proposed archive
- Updated snapcraft to 2.10
- Downloaded and extracted the file from the description
- Ran snapcraft
- I got a snap file.
- Ran snapcraft clean.
- The snap file was not deleted.
- Ran snapcraft again.
- The snap file was not copied to the part dir.

I'm marking the verification as done.

Thanks Chris!

[Launchpad Janitor (janitor)]

This bug was fixed in the package snapcraft - 2.10.1

---------------
snapcraft (2.10.1) xenial; urgency=medium

  * Backwards compatible clean with strip (#556) (LP: #1590256)

 -- Sergio Schvezov <email address hidden> Wed, 08 Jun 2016 16:32:27 -0300