Skip to content

Conversation

@tbraeutigam
Copy link
Contributor

As per @kwill 's request in tbraeutigam/gogs-snap#1 and #2539, I modified my gogs-snap to fit with gitea.

Changes to Gitea Code:

  • None for actual code
  • .gitignore:
    Adding a couple of exceptions for snapcraft files that might come up during compilation.

New Files:

  • snap/snapcraft.yaml
    The snap package instructions
  • snap/helpers/simple_launcher.sh
    A simplistic wrapper for gitea, to fit with the snap environment
  • snap/helpers/directorySetup.sh
    Sets up the directories/copied data from r/o $SNAP to $SNAP_DATA and $SNAP_COMMON
  • snap/helpers/snapApp.ini
    A few default settings (really 99% of changes are to adjust the paths properly for the Snap environment).

Rationale:
A snap version allows for quick testing and/or easy local hosting on your own machine/own network.
The snap does not restrict you from using any of the database options. It defaults to Sqlite3 on the setup page as that is what can be immediately used without further issues from within the snap.

Tested Scenarios:

  • Ran through a couple of coming use cases (create repo, issues, clone/pull/push, PR, etc)
  • Builds fine from current master (Tested on Ubuntu 17.04 and 16.04 LTS)

If you do accept the pull-request it would be great if you could set up gitea to be automatically build via https://build.snapcraft.io -- it will allow it to be built on any commit to the master branch (or any other branch you specify).

-> add _source.tar.bz2 to .gitignore (used by snapcraft cleanbuild)
@codecov-io
Copy link

codecov-io commented Sep 21, 2017

Codecov Report

Merging #2568 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2568   +/-   ##
=======================================
  Coverage   20.17%   20.17%           
=======================================
  Files         145      145           
  Lines       29171    29171           
=======================================
  Hits         5884     5884           
  Misses      22393    22393           
  Partials      894      894

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 02ecc03...34071af. Read the comment docs.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 21, 2017
@sapk
Copy link
Member

sapk commented Sep 21, 2017

Is it possible to put the snap folder under contrib/ ?

@tbraeutigam
Copy link
Contributor Author

tbraeutigam commented Sep 21, 2017 via email

@@ -0,0 +1,69 @@
APP_NAME = Gitea: Go Git Service
RUN_USER = root
Copy link
Member

Choose a reason for hiding this comment

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

Should gitea really run as root? Or do I misinterpret this variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not a misinterpretation.
Snap packages right now all 'run as root' for daemons.

The security part here is defined via apparmor confinement. With the plugs in snapcraft.yaml the root user is only allowed to access the snap directories and network access and port binding.

Copy link
Member

Choose a reason for hiding this comment

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

That's all right then.

export isRoot=`true`
else
echo "You are not running gitea as root. This is required for the snap package."
echo "Please re-run as root."
Copy link
Member

Choose a reason for hiding this comment

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

Why is it requried to run Gitea as root? (I am not familiar with snap-things)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There could be a way to run it as a user of started manually. While working on the gigs snap though, that got messy very fast. This I stepped that out to be revisited at a later point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, typing on mobile:
if started*
s/gigs/gogs/
Thus I backed that*

Choose a reason for hiding this comment

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

Hi @tbraeutigam, sorry for bringing this up 2 years after the PR, but do you recall exactly which issue prevented gitea web from running correctly without daemon mode?

@lunny lunny added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Sep 22, 2017
@lunny lunny added this to the 1.x.x milestone Sep 22, 2017
 - Take advantage of install-hooks (snapd 2.27)
 - Use snapctl configuration storage for unchanging values
@tbraeutigam
Copy link
Contributor Author

Hi,
I just pushed a few updates to the PR that streamline the wrapper around gitea and move install-time code to the install-hook of snapd.

This moves the snap/helpers/directorySetup.sh to snap/hooks/install and removes it from being run on every call of the gitea binary.

Hope my prior comments and this commit help clean and clear it up a bit.

@tbraeutigam
Copy link
Contributor Author

Is there anything I can do for getting approval?

@lunny
Copy link
Member

lunny commented Oct 2, 2017

@tbraeutigam since not all of the maintainers are familiar with snap-packaging, I think it needs some time.

@tbraeutigam
Copy link
Contributor Author

@lunny That's perfectly valid. If there is anything that I can help clarify/assist with, I'm more than happy to.

@lunny
Copy link
Member

lunny commented Jan 25, 2018

truested LGTM @tbraeutigam

@tboerger tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 25, 2018
Copy link
Member

@sapk sapk left a comment

Choose a reason for hiding this comment

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

Just a little comment.
For the rest, I will trust @tbraeutigam

SSH_ROOT = SNAP_DIR_COMMON/ssh
SSH_KEY_TEST_PATH = SNAP_DIR_DATA/sshkeytest
CERT_FILE = SNAP_DIR_DATA/certs/cert.pem
KEY_FILE = SNAP_DIR_DATA/certs/key.pem
Copy link
Member

Choose a reason for hiding this comment

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

CERT_FILE and KEY_FILE aren't need if protocol is http. Maybe we could generate some at install and default to https ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A fair bit of time has gone by since the writing and a lot of new features have come to snapcraft, so I'll re-check this. These entries were added back then because I judged including a full config-writer or something hack-ish would've been more costly than having these two lines in there.

Copy link
Contributor Author

@tbraeutigam tbraeutigam Jan 29, 2018

Choose a reason for hiding this comment

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

I've kept them in, and in a way 'made it worse' by moving to the snap hooks for configuration.

Doing that I'm using the defaults from app.ini.sample, overwriting the parts necessary for a snap to function and adding them all to the app.ini as used by gitea.

I personally prefer this, as it's now possible to configure everything via sudo snap set category-subcategory.option, but it has some drawbacks at the moment, as I didn't want to touch the config system:

  • Cateogories in the style of category.subcategory are now category-subcategory
  • Options containing an underscore have a '99' in its place (ssh.minimum_key_size is ssh-minimum99key99size for snapctl)
  • New defaults do not apply automatically
  • Service-restart takes longer (it re-parses the whole app.ini every time)

All options can still be changed manually in the app.ini file and will be added to snapctl upon service-restart.

Not sure which way is preferred, but unless we'll touch the config system for this, there's not much that can be done (aside from completely leaving it alone and up to the user to set everything).

I would love to have it fully integrated into the snap config system as an option, but that would require a lot of work:

  • Category names are only allowed to be lower case and can only contain dashes
  • There is no programmatic interface from what I can see, all has to be done with exec() calls.

ini files are regrettably hard to convert to the json-based snapctl config format.
(You can see some of the mess by looking at the configuration.sh files ToSnap and ToIni functions...)

@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 25, 2018
@bkcsoft
Copy link
Member

bkcsoft commented Feb 10, 2018

Why isn't this a separate repo like gogs-snap and gitea-homebrew ? 🤔

@bkcsoft
Copy link
Member

bkcsoft commented Feb 10, 2018

Is there any way to trigger the snapcraft buildservers from Drone? 🤔

EDIT: Was already answered in the issue #2539 (comment)

To get it onto build.snapcraft.io, you just have to go to the site, create an account and log in, and it guides you on how to allow the site to set a trigger for building.

@lunny
Copy link
Member

lunny commented Apr 29, 2018

@bkcsoft It seems snap don't support that usage? @tbraeutigam

@lofidevops
Copy link

@bkcsoft , in response to your query:

Why isn't this a separate repo like gogs-snap and gitea-homebrew ?

An earlier comment by @tbraeutigam :

[Putting snap package information in a separate repo] would take away from the automation again, as only commits to the repository containing the snap info would be monitored.

@lunny lunny modified the milestones: 1.x.x, 1.5.0 May 9, 2018
@lunny lunny merged commit 14f16d6 into go-gitea:master May 9, 2018
@tbraeutigam
Copy link
Contributor Author

tbraeutigam commented Dec 9, 2019 via email

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants