Manual human review required : olivia

Hi, i have an app called Olivia which is hooked with build.snapcraft.io everything used to be fine with automatic build and publishing before today when i pushed new commit to my repo on github https://github.com/keshavbhatt/olivia where i never touched snapcraft.yaml file and with the latest build and now my application need manual human review to get released to store.

The reason for this is 1 failed automatic test which build.snapcraft.io perform before releasing the builds to snap store or snapcraft.io/store .

Failed test describes issue as following :

 human review required due to 'deny-connection' 
 constraint (interface attributes) declaration-snap- 
 v2_slots_connection (mpris, mpris)
2 Likes

While I cannot explain why touching the snapcraft.yaml file causes this issue I give my +1 vote in case you need to re-apply for the permission.

I said i made no changes to snapcraft.yaml file and this never happened before last builds.

What does this mean ? I never before applied for permission of mpris slot autoconnect, am not able to understand what

‘deny-connection’
constraint (interface attributes) declaration-snap-
v2_slots_connection (mpris, mpris)

mean here :neutral_face: and how to get rid of this.

@jdstrand help ! :face_with_head_bandage:

Jamie is out of the office this week on vacation, but perhaps other @reviewers could take a look at this?

1 Like

Sorry about the issues you’ve seen here. I see the errors in the store which start with revision 76. 75 passed okay. Confirmed locally by running the snap-review against each snap.

I grabbed both and yanked the manifest from them. I did a side-by-side diff - the full output is at https://paste.ubuntu.com/p/GPMYycVxFY/ - but the main interesting part is at the start:

snapcraft-version: '3.8'				      |	snapcraft-version: 3.9.5
snapcraft-started-at: '2020-01-14T12:17:50.144143Z'	      |	snapcraft-started-at: '2020-01-15T15:17:55.779899Z'
snapcraft-os-release-id: ubuntu					snapcraft-os-release-id: ubuntu
snapcraft-os-release-version-id: '18.04'			snapcraft-os-release-version-id: '18.04'
name: olivia							name: olivia
version: '1.0'							version: '1.0'
summary: Elegant Cloud Music Player for Linux Desktop		summary: Elegant Cloud Music Player for Linux Desktop

I suspect this is a byproduct of the bump on the builder from snapcraft 3.8 to 3.9.5. A change in behavior perhaps. @sergiusens will probably be able to look at this (once he steps off a plane, so have patience) and know exactly what’s going on. Alternatively when @cjp256 wakes he may have more insight.

In the meantime, I have enabled the mpris slot for your application in the store. So the later revisions now pass.

2 Likes

I have a suspicion about change I made that may have triggered this, but unclear why (if so, even). Specifically the slot interface name is being written to the snap.yaml. I’ll look closer at this in the morning.

1 Like

thanks :slight_smile: for deeper analysis, and allowing automatic release. If its issue from my side let me know.

This is most likely the review-tools not accepting valid snapd syntax.

We now load the slot into an internal data structure for analysis to pick up any potential content interface in use to use for analysis as a source of artifact dependencies. In doing so, we now serialize that data structure into the resulting snap.yaml, which means the original syntax is lost.

While I am still on the fence here, I think we should most likely respect the original input and pass that down to snap.yaml instead.

1 Like

thanks for clarification @sergiusens let me know if i have to make any changes to craft file to fix this for future builds.

I can confirm it is due to the presence of “interface” being added by snapcraft.

As @sergiusens suggested, I think the error is because of a bug in the review tools. @jdstrand?

In the docs referenced by review tools, interface is optional.

slots:
	<slot>:
		interface: <interface>  # Optional if <slot> == <interface>

There is a test case in review-tools that appears to cover the exact case happening here:
https://git.launchpad.net/review-tools/tree/reviewtools/tests/test_sr_declaration.py#n5195

This test appears as if it is supposed to fail if name is present for mpris. Though I am unsure why the behavior changes with the explicit presence of interface: mpris. Certainly name should be allowed, but is there a reason both keys can’t be defined together?

I think a fix for review-tools may take time, so I will update snapcraft to not include “interface” unless explicitly defined in the YAML.

1 Like

https://github.com/snapcore/snapcraft/pull/2884 should work around the issue once backported into the stable tree.

1 Like

thanks @cjp256 for clarification :slight_smile:

I can confirm there is a bug in the review-tools. @popey’s issued snap declaration is fine, however, as per The mpris interface and mentioned earlier in this topic, the name attribute is optional and when not present defaults to the snap name. I suggest changing the snapcraft.yaml from this:

name: olivia
slots:
  mpris:
    name: olivia
apps:
  olivia:
    slots:
    - mpris

to simply this:

name: olivia
apps:
  olivia:
    slots:
    - mpris

(I suspect this was a carryover from the olivia-test snap being renamed). This change is not required but cleans up the snapcraft.yaml a bit.

I’m preparing a fix for the review-tools now. While not needed for this snap due to the snap declaration, it should be in production next week.

3 Likes

This fix is in production now.

1 Like