Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the packaging metadata to build the mrview snap #935

Closed
wants to merge 9 commits into from

Conversation

come-maiz
Copy link

No description provided.

@come-maiz
Copy link
Author

To try this in Ubuntu 16.04:

$ sudo apt install git snapcraft
$ git clone https://github.com/elopio/mrtrix3
$ cd mrtrix3
$ git checkout snapcraft
$ snapcraft
$ sudo snap install *.snap --dangerous
$ mrview

(--dangerous because the snap you build doesn't come signed from the store. Once you push it to the store, your users will just have to do $ sudo snap install mrview)

@jdtournier
Copy link
Member

Thanks @ElOpio. Just had a brief look at the file, and a few questions come to mind:

  • I note the name is simply mrview - but that's just one app out of a lot of command-line tools. It also seems to be the only 'app' listed in the apps section. Is there any restriction on the name being mrtrix3? This would I think make a lot more sense. I don't mind mrview being the only thing in the apps section, presumably this is to add entries in desktop menus and launchers? I assume renaming to mrtrix3 would also mean that the command to install would become sudo snap install mrtrix3? Again, I think that would be much more sensible...

  • the configure command should use a generic CPU architecture if it's to be deployed on arbitrary systems. So line 22 should change to ARCH= ./configure - currently it'll build with -march=native and probably bring in all kinds of CPU extensions that won't be guaranteed to be available on target systems.

  • the install line currently drags all compiled object files and other temporaries along with it. It should be sufficient to bring in only bin/, lib/libmrtrix-*.so, & scripts/. So that command should probably be:

    mv release/bin/ release/scripts/ $SNAPCRAFT_PART_INSTALL
    mkdir $SNAPCRAFT_PART_INSTALL/lib
    mv release/lib/libmrtrix-*so $SNAPCRAFT_PART_INSTALL/lib
    

    This would work for the current master branch. Once tag_0.3.16 is merged in, this will be simplified to:

    mv bin/ lib/ share/ $SNAPCRAFT_PART_INSTALL
    
  • the build currently uses Qt4 - any reason why we can't use Qt5? I think it would be more future-proof than Qt4. But if it's not available for all platforms, then maybe that's all we can do.

  • can you explain the plugs part? That seems to be very Ubuntu-specific (specifically the unity7 bit), but I'd understood that snapcraft was distribution agnostic...?

  • on a totally different note: how is snapcraft going to support wayland...? I'm already running this on my laptop, maybe it's just a matter of ensuring Qt5 comes with wayland support? I guess we can worry about this if / when it becomes a problem...

@come-maiz
Copy link
Author

Thanks for the review @jdtournier !

I wanted to start with a simple snap with some basic functionality, because many of the things you will add depend on your preference.

I've renamed the snap to mrtrix3, with an mrview command. The thing when the commands don't have the same name as the snap is that you will have to call it like: mrtrix3.mrview. This is of course to avoid conflicts in the store. However, we are working in a new feature to give aliases to commands. It's work in progress to auto-assign an alias to a developer that registered it, so expect some progress there soon.

Each app that you want to expose in your snap needs to be declared, because each one will have different permissions. I added the declaration for all the mr* binaries, but I have tested only mrview. Depending on what the other apps need out of the default confinement of the snap, you will have to add plugs to them: https://snapcraft.io/docs/core/interfaces

I also added the desktop entry for mrview, so it appears in the menu. It's not enough declaring the app, you have to point it to a desktop file.

I updated the configure and updated it to use qt5, and mrview still launches fine here.

The plugs are the permissions required by your snap. If unity7 is available and your snap requests it with a plug, it will use it. If unity7 is not present, it will just ignore it. I added the x11 plug for cases without unity7. And just like this, a new plug for wayland has to be added. I expect it to be implemented along with the Fedora work, which you can follow here: https://github.com/snapcore/snapd/wiki/Distributions

I didn't update the install scriptlet because I thought it would be better to update it with 0.3.16, but just let me know if you prefer to change it now.

Why don't we meet in a hangout or something to discuss about your project in Ubuntu, and how snapcraft could fit in your release process? There we can quickly answer any other questions you have.

pura vida.

@jdtournier
Copy link
Member

Thanks to @tpeeters for (gently) reminding me to chase this up...

@ElOpio: I had a go at this a while back, but got a bit stuck trying to install snapcraft on Arch Linux (which is what I run on my system). I was probably over-complicating things though, I'll give it another shot when I have a minute. In the meantime, if any other members of the @MRtrix3/mrtrix3-devs team running Ubuntu would like to give a go, don't let me hold you back...

@jdtournier
Copy link
Member

jdtournier commented Apr 3, 2017

OK, just gave it another shot on Arch, managed to finally get everything installed, including dependencies (I had to install apt too...?). But it won't run on my system:

$ snapcraft
Traceback (most recent call last):
  File "/usr/bin/snapcraft", line 28, in <module>
    import snapcraft.main
  File "/usr/lib/python3.6/site-packages/snapcraft/__init__.py", line 359, in <module>
    from snapcraft._baseplugin import BasePlugin        # noqa
  File "/usr/lib/python3.6/site-packages/snapcraft/_baseplugin.py", line 21, in <module>
    from snapcraft.internal import common
  File "/usr/lib/python3.6/site-packages/snapcraft/internal/__init__.py", line 20, in <module>
    from snapcraft.internal.project_loader import load_config  # noqa
  File "/usr/lib/python3.6/site-packages/snapcraft/internal/project_loader.py", line 29, in <module>
    from snapcraft.internal import (
  File "/usr/lib/python3.6/site-packages/snapcraft/internal/parts.py", line 29, in <module>
    from snapcraft.internal import (
  File "/usr/lib/python3.6/site-packages/snapcraft/internal/pluginhandler/__init__.py", line 40, in <module>
    from snapcraft.internal import (
  File "/usr/lib/python3.6/site-packages/snapcraft/internal/repo/__init__.py", line 26, in <module>
    Repo = _get_repo_for_platform()
  File "/usr/lib/python3.6/site-packages/snapcraft/internal/repo/_platform.py", line 37, in _get_repo_for_platform
    'snapcraft is not supported on this operating system')
RuntimeError: snapcraft is not supported on this operating system

Given that this error occurs here, that's going to be a difficult one to get around...

@come-maiz
Copy link
Author

come-maiz commented May 24, 2017

Hey @jdtournier. Our support for archlinux is in progress. You can use docker to build it, but things will look nicer soon. Something like this:

sudo pacman -S snapd
sudo systemctl start snapd.refresh.timer
sudo systemctl start snapd.socket
sudo snap install docker
git clone https://github.com/elopio/mrtrix3
cd mrtrix3
git checkout snapcraft
sudo snap run docker run -v $(pwd):$(pwd) -t snapcore/snapcraft sh -c "apt update -qq && cd $(pwd) && snapcraft"

Or, you can use the new service that we have just put in beta, and it will build and take care of the continuous delivery for you. After getting the snapcraft.yaml in your master branch, you just have to go to http://build.snapcraft.io/.

I'm very interested in getting feedback from real projects about this service, so if you give it a try please let me know about your experience.

@tpeeters
Copy link

I have been running snapcraft inside lxd containers, and that works fine. So I assume that to build snap packages on Arch, you can simply launch an Ubuntu container and build inside that. It seems like a bit of a workaround, but even on Ubuntu I launch another Ubuntu in a container to build, so that I don't need to install the build dependencies on my system directly. Perhaps that is not needed with newer versions of snapcraft, but I got used to do all my development in containers and it works pretty nice for me.

@jdtournier
Copy link
Member

Just going through the list of outstanding PR. @ElOpio: does this still work with the latest changes? I won't have time to test it, but since this only involves adding a file with no scope for interference with the main install, I'm happy enough to have it added to the repo, provided someone is happy to field support requests for it...

Copy link
Member

@jdtournier jdtournier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other questions while we're at it:

  • I assume this will need to be updated to expose all commands? Currently it's only showing a small subset.

  • This presumably only install MRtrix3, nothing else? The reason I'm asking is because there are scripts in there that call executables from FSL and ANTs, and they obviously won't work without them. Is there any way to modify the snap so that it depends on these other software packages?

source: .
plugin: nil
build: |
ARCH= ./configure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the ARCH= prefix anymore, as of 3.0_RC1. This line only needs to be ./configure

@come-maiz
Copy link
Author

Hello @jdtournier. I'm no longer working on snapcraft, and I'm already maintaining a few snaps, so I won't have time to help you maintaining this one. You can talk to @popey and @flexiondotorg, they can help you getting in touch with the snapcrafters community to see if they are willing to maintain mrtrix.

pura vida

@jdtournier
Copy link
Member

@ElOpio: thanks for getting back to me.

In this case, I think I'll close this pull request. It can always be reopened if users start asking for this.

@jdtournier jdtournier closed this Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants