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

dexc-desktop: Snap package #2580

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peterzen
Copy link
Member

@peterzen peterzen commented Oct 26, 2023

This PR adds configuration, build scripts and Github workflow to build the Snap package and publishing it in the Snap Store.

The snap build uses the .deb package. The PR also contains a few fixes/improvements in the deb build:

  • Icons: added SVG icon for hicolor and symbolic variations, fixed 128px PNG to match official version
  • icon install location changed to /usr/share/icons which is the XDG standard
  • binary installs in /usr/bin directly (this is standard practice, simplifies deb scripts)
  • added AppStream metainfo file (for GNOME/KDE/Ubuntu software centers and the snap)

@peterzen peterzen force-pushed the snapcraft-builder branch 3 times, most recently from 95e4276 to fc32448 Compare October 27, 2023 22:19
@peterzen peterzen marked this pull request as ready for review November 3, 2023 16:51
Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

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

Getting an error when running ./pkg/pkg-debian.sh:

dpkg-deb: error: parsing file './build/dexc_bf46fe348fb83893408e875408499761a3df5ea4-0_amd64/DEBIAN/control' near line 2 package 'dexc':
 'Version' field value 'bf46fe348fb83893408e875408499761a3df5ea4': version number does not start with digit

@peterzen
Copy link
Member Author

peterzen commented Nov 15, 2023

dpkg-deb: error: parsing file './build/dexc_bf46fe348fb83893408e875408499761a3df5ea4-0_amd64/DEBIAN/control' near line 2 package 'dexc':
'Version' field value 'bf46fe348fb83893408e875408499761a3df5ea4': version number does not start with digit

It pulls the version number from the git tag - the idea being that for release builds there's always a tag but for dev builds the commit hash confuses the deb build. Maybe this needs to be reconsidered.

If you git tag v0.7.0 or similar it will work.

@peterzen peterzen force-pushed the snapcraft-builder branch 2 times, most recently from bf46fe3 to 9bf282c Compare November 15, 2023 21:58
@JoeGruffins
Copy link
Member

It pulls the version number from the git tag - the idea being that for release builds there's always a tag but for dev builds the commit hash confuses the deb build. Maybe this needs to be reconsidered.

So the commit needs a tag to work? I did not know. Maybe add that to the readme there at least if it is a requirement?

Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

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

After adding a bogus tag was able to build and install with snap.

Copy link
Member

@buck54321 buck54321 left a comment

Choose a reason for hiding this comment

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

I'm not seeing the benefit of the github actions deployment. What does it do for us over just running the script ourselves?

Icons: added SVG icon for hicolor and symbolic variations, fixed 128px PNG to match official version

Can you say more about the new files and what they do for us? Or point me to some documentation?

I don't care about the official logo version. I changed it because the official color was too dark and looked bad in my panel. None of this matters much, of course, because we're rebranding anyway, but prefer to use the lighter color that has margins.

@peterzen
Copy link
Member Author

I'm not seeing the benefit of the github actions deployment. What does it do for us over just running the script ourselves?

It does the same thing but makes life a tad easier. snapcraft, the snap builder isn't straightforward to run on systems other than Ubuntu as it requires several manual steps to initialize lxd which the build relies on. So basically you need to run the script in an Ubuntu VM which I presumed isn't the system of choice for running release builds.

The binary it packages into a snap comes from the .deb package.

Icons: added SVG icon for hicolor and symbolic variations, fixed 128px PNG to match official version

Can you say more about the new files and what they do for us? Or point me to some documentation?

dexc.png is scaled to 128px which is one of the default icon sizes, the 100px version was scaled up and rendered blurry
dexc.svg is a scalable version which the desktop environment picks up when a bitmap in the required size is not available
dexc-symbolic.svg is a B/W version that the desktop environment might use with certain accessibility settings like monochrome etc. Also this is ideally the icon to be used in the systray as in that size this provides the most contrast.

https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html

I don't care about the official logo version. I changed it because the official color was too dark and looked bad in my panel. None of this matters much, of course, because we're rebranding anyway, but prefer to use the lighter color that has margins.

The tray icon should really match the other icons (menu, dock, task switcher), if it's different it looks odd. How about using the monochrome version? That's the most distinctly recognizable in the tray visually.

Comment on lines 48 to 54
if [ ! -d ../../webserver/site/dist ]; then
CWD=$(pwd)
cd ../../webserver/site
npm clean-install
npm run build
cd $CWD
fi
Copy link
Member

Choose a reason for hiding this comment

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

Just because there's a file there doesn't mean its the right one. Any reason not to just build it every time?

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea here was that the site package is added to the repo on the release branch so there's no need to build it at the time of building the release packages but it certainly won't hurt.

client/cmd/dexc-desktop/pkg/common.sh Outdated Show resolved Hide resolved
client/cmd/dexc-desktop/pkg/pkg-debian.sh Outdated Show resolved Hide resolved
client/cmd/dexc-desktop/pkg/publish-snap.sh Outdated Show resolved Hide resolved
client/cmd/dexc-desktop/snap/local/snapcraft.yaml.in Outdated Show resolved Hide resolved
client/cmd/dexc-desktop/snap/local/snapcraft.yaml.in Outdated Show resolved Hide resolved
client/cmd/dexc-desktop/go.mod Outdated Show resolved Hide resolved
@peterzen peterzen force-pushed the snapcraft-builder branch from 45de6fc to 4193b53 Compare July 17, 2024 14:26
.github/workflows/README.md Show resolved Hide resolved
SNAPCRAFT_STORE_CREDENTIALS: ${{ secrets.SNAPCRAFT_STORE_CREDENTIALS }}
with:
snap: ${{ steps.build.outputs.snap }}
release: stable
Copy link
Member

Choose a reason for hiding this comment

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

I believe if this is omitted, the package is uploaded but not released. Would that give us a chance to double-check everything first and then release later?

Copy link
Member Author

Choose a reason for hiding this comment

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

It would, though one could also run the snap build locally to test the package.

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.

3 participants