Adding amd64 debian packager, checksums, README updates. #312

djpolygon · 2024-04-10T17:33:57Z

Adding debian packager for amd64

Checksum function for package

Updated Readme.md for packages and checksum

Added packager directory, contains postinst, postrm, and cloned copy of the toml files for miden. Note, these are just place holders and they should be adjusted.

Created service file.

Tested on Ubuntu 22

bobbinth · 2024-04-10T20:01:15Z

Once merged, this will supersede #265, I believe.

djpolygon · 2024-04-10T20:03:16Z

@bobbinth correct. I forgot to mention in the PR.

hackaugusto

I left some comments, but they are mostly nits and I don't think this PR needs to be blocked because of them. Any necessary changes can be done on follow ups.

hackaugusto · 2024-04-11T16:41:52Z

.github/workflows/deb_packager.yml

+      - name: Checkout
+        uses: actions/checkout@v2
+        with:
+          fetch-depth: 0


I tried git clone --depth=0 and got this error:

fatal: depth 0 is not a positive number

Shouldn't this be a 1?

I can adjust to 1, this is the fetch-depth we use with other versions of this same process. If you would like can adjust, let me know.

If it works then all good :) I assumed the CI would fail like it does locally

hackaugusto · 2024-04-11T17:14:27Z

.github/workflows/deb_packager.yml

+      - 'main'
+      - 'next'
+    paths:
+      - '**'


What are the exact semantics here? The docs are not that clear.

What if I have a commit git commit --allow-empty, will this trigger? Or more importantly, when a tag is pushed by itself?
What is the difference no not having this option?

It triggers when any commit is pushed with a tag and captures all paths, including the . directories such as the .github for adding to the work flow.

hackaugusto · 2024-04-11T17:40:54Z

.github/workflows/deb_packager.yml

+          fetch-depth: 0
+      ##### TAG Variable #####
+      - name: Adding TAG to ENV
+        run: echo "GIT_TAG=`echo $(git describe --tags --abbrev=0)`" >> $GITHUB_ENV


Can't GITHUB_REF be used instead?

It could be , but I use a defined variable here as there have been edge cases with other projects that created problems. To avoid conflicts, went this route, if you wish I can adjust.

hackaugusto · 2024-04-11T17:54:54Z

packaging/postinst

@@ -0,0 +1,11 @@
+# This is a postinstallation script so the service can be configured and started when requested
+#
+sudo adduser --disabled-password --disabled-login --shell /usr/sbin/nologin --quiet --system --no-create-home --home /nonexistent miden


The options --disabled-password and --disable-login seem to conflict with one another ref:

--disabled-login Do not run passwd to set the password. The user won't be able to use her account until the password is set. --disabled-password Like --disabled-login, but logins are still possible (for example using SSH RSA keys) but not using password authentication.

Shouldn't this use --disabled-login only?

We could also add GECOS here --gecos Miden

--no-create-home should make --home /nonexistent unecessary:

--home DIR Use DIR as the user's home directory, rather than the default specified by the configuration file. If the directory does not exist, it is created and skeleton files are copied. ... --no-create-home Do not create the home directory, even if it doesn't exist.

hackaugusto · 2024-04-11T17:59:12Z

packaging/postinst

@@ -0,0 +1,11 @@
+# This is a postinstallation script so the service can be configured and started when requested


This seems to be missing the hashbang.

It will invoke, adding in a commit to address this.

hackaugusto · 2024-04-11T18:00:55Z

.github/workflows/deb_packager.yml

+  push:
+    branches:
+      - 'main'
+      - 'next'


next is the development/integration branch. I think we should release only from main.

Removed next

hackaugusto · 2024-04-11T18:10:46Z

.github/workflows/deb_packager.yml

+          echo "Priority: optional" >> packaging/deb/miden-node/DEBIAN/control
+          echo "Architecture: amd64" >> packaging/deb/miden-node/DEBIAN/control
+          echo "Maintainer: [email protected]" >> packaging/deb/miden-node/DEBIAN/control
+          echo "Description: miden-node binary package" >> packaging/deb/miden-node/DEBIAN/control


We could also add:

echo "Homepage: https://polygon.technology/polygon-miden" >> packaging/deb/miden-node/DEBIAN/control echo "Vcs-Git: [email protected]:0xPolygonMiden/miden-node.git" >> packaging/deb/miden-node/DEBIAN/control echo "Vcs-Browser: https://github.com/0xPolygonMiden/miden-node" >> packaging/deb/miden-node/DEBIAN/control

hackaugusto · 2024-04-11T18:11:53Z

.github/workflows/deb_packager.yml

+      - '**'
+    tags:
+      - 'v*.*.*'
+      - 'v*.*.*-*'


I'm not sure we need this option, we use semver. Is the intent here to use the -* to describe Debian's revision? I personally would avoid having a distro specific strategy here.

As this is building specifically debian packages, I included this. For RPM based building it will adjust accordingly as it is a separate builder.

I can adjust to whatever is preferred versioning/tagging :)

hackaugusto · 2024-04-11T18:17:08Z

.github/workflows/deb_packager.yml

+      - name: Cleaning repo
+        run: cargo clean
+      - name: Building for amd64
+        run: cargo build --release


reviewer note: The concurrent feature flag is enabled in the Cargo.toml. So this should be enough as is.

Removed the clean, note as I am using the standalone I always want to make sure that the node cleans the repo, but as long as it is not on a self hosted runner, this is just an extra step.

Ah, the comment was not about the clean, I was just leaving a comment for reference, explaining why we don't need a --features flag in here.

hackaugusto · 2024-04-11T18:23:49Z

.github/workflows/deb_packager.yml

+          echo "Section: base" >> packaging/deb/miden-node/DEBIAN/control
+          echo "Priority: optional" >> packaging/deb/miden-node/DEBIAN/control
+          echo "Architecture: amd64" >> packaging/deb/miden-node/DEBIAN/control
+          echo "Maintainer: [email protected]" >> packaging/deb/miden-node/DEBIAN/control


The format here seems incomplete ref:

The package maintainer’s name and email address. The name must come first, then the email address inside angle brackets <> (in RFC822 format).

It should be Polygon Devops <[email protected]>

… cargo clean

bobbinth

Looks good! Thank you for creating these! I left just one minor nit inline.

Also, a question for the future: we now have 2 copies of miden-node.toml and genesis.toml files. One set stored in packaging and another in miden-node directories. This means we'll need to keep them in sync as we change their formats. I wonder if there is a way to keep only one version of these files (either here or there).

I don't think we need to solve this in this PR though - so, just creating an issue so that we can figure out the right approach is fine for now.

bobbinth · 2024-04-11T20:24:26Z

README.md

+```sh
+miden-node make-genesis -i $input_location_for_gensis.toml -o $output_for_gensis.dat_and_accounts
+```
+The debian package has a checksum, you can verify this checksum by download the debian package and checksum file to the same directory and running the following command:


potential typo: should "download" be "downloading"?

Also, should we say where the package and checksum can be downloaded from?

bobbinth · 2024-04-12T04:36:18Z

Also, a question for the future: we now have 2 copies of miden-node.toml and genesis.toml files. One set stored in packaging and another in miden-node directories. This means we'll need to keep them in sync as we change their formats. I wonder if there is a way to keep only one version of these files (either here or there).

Created #318 for this.

Adding amd64 debian packager, checksums, README updates.

057324a

djpolygon changed the base branch from main to next April 10, 2024 17:54

djpolygon changed the base branch from next to main April 10, 2024 17:54

bobbinth requested a review from hackaugusto April 10, 2024 18:26

rekibnikufesin approved these changes Apr 10, 2024

View reviewed changes

Cleaning up a bad reference in comment

e37c59c

hackaugusto approved these changes Apr 11, 2024

View reviewed changes

hackaugusto mentioned this pull request Apr 11, 2024

config: add systemd service files #265

Closed

djpolygon added 4 commits April 11, 2024 13:40

Adding #! to postinst, removed 'next' for the branch on push execution

30b2965

Added Homepage, VCS-git, and Vcs-browser to control file, removed the…

f314563

… cargo clean

Updated email in maintainer in control file

bbdf22a

Adding clean back in to the build :)

c3d6b21

bobbinth approved these changes Apr 11, 2024

View reviewed changes

bobbinth mentioned this pull request Apr 12, 2024

Remove duplicates of sample config files for the node #318

Closed

bobbinth changed the base branch from main to next April 12, 2024 04:36

bobbinth changed the base branch from next to main April 12, 2024 04:37

bobbinth merged commit 8e36138 into main Apr 12, 2024
4 checks passed

bobbinth deleted the djpolygon/DEVOPS-2685 branch April 12, 2024 04:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding amd64 debian packager, checksums, README updates. #312

Adding amd64 debian packager, checksums, README updates. #312

djpolygon commented Apr 10, 2024

bobbinth commented Apr 10, 2024

djpolygon commented Apr 10, 2024

hackaugusto left a comment

hackaugusto Apr 11, 2024

djpolygon Apr 11, 2024 •

edited

Loading

hackaugusto Apr 11, 2024

hackaugusto Apr 11, 2024

djpolygon Apr 11, 2024

hackaugusto Apr 11, 2024

djpolygon Apr 11, 2024

hackaugusto Apr 11, 2024

hackaugusto Apr 11, 2024 •

edited

Loading

hackaugusto Apr 11, 2024

hackaugusto Apr 11, 2024

djpolygon Apr 11, 2024

hackaugusto Apr 11, 2024

djpolygon Apr 11, 2024

hackaugusto Apr 11, 2024

djpolygon Apr 11, 2024

hackaugusto Apr 11, 2024

djpolygon Apr 11, 2024

djpolygon Apr 11, 2024

hackaugusto Apr 11, 2024

djpolygon Apr 11, 2024

hackaugusto Apr 11, 2024

hackaugusto Apr 11, 2024

bobbinth left a comment

bobbinth Apr 11, 2024

bobbinth commented Apr 12, 2024

		@@ -0,0 +1,11 @@
		# This is a postinstallation script so the service can be configured and started when requested

Adding amd64 debian packager, checksums, README updates. #312

Adding amd64 debian packager, checksums, README updates. #312

Conversation

djpolygon commented Apr 10, 2024

bobbinth commented Apr 10, 2024

djpolygon commented Apr 10, 2024

hackaugusto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djpolygon Apr 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hackaugusto Apr 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobbinth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobbinth commented Apr 12, 2024

djpolygon Apr 11, 2024 •

edited

Loading

hackaugusto Apr 11, 2024 •

edited

Loading