Guess update information based on GitHub Actions environment variables #1075

srevinsaju · 2020-09-10T08:15:19Z

Reads the GITHUB_REPOSITORY and GITHUB_REF environment variables
and attempts to generate an appropriate updateinformation

Adapted from https://github.com/probonopd/go-appimage/blob/7fab45552eb485c0076fd310835fa7958f981de0/src/appimagetool/appimagetool.go\#L547-L567

Sample run

github@fbfcaab3b211:~/build/out$ export GITHUB_REPOSITORY='probonopd/Zoom.AppImage'
github@fbfcaab3b211:~/build/out$ appimagetool appimagetool.AppDir/ -g             
appimagetool, continuous build (commit 0880085), build <local dev build> built on 2020-09-09 20:44:40 UTC
WARNING: appstreamcli command is missing, please install it if you want to use AppStream metadata
NOTE: Using the output of 'git rev-parse --short HEAD' as the version:
      0880085
      Please set the $VERSION environment variable if this is not intended
Using architecture x86_64
/home/github/build/out/appimagetool.AppDir should be packaged as appimagetool-0880085-x86_64.AppImage
AppStream upstream metadata found in usr/share/metainfo/appimagetool.appdata.xml
Generating squashfs...
Parallel mksquashfs: Using 8 processors
Creating 4.0 filesystem on appimagetool-0880085-x86_64.AppImage, block size 131072.
[========================================================================================/] 19/19 100%

Exportable Squashfs 4.0 filesystem, gzip compressed, data block size 131072
        compressed data, compressed metadata, compressed fragments,
        compressed xattrs, compressed ids
        duplicates are removed
Filesystem size 461.61 Kbytes (0.45 Mbytes)
        39.67% of uncompressed filesystem size (1163.54 Kbytes)
Inode table size 317 bytes (0.31 Kbytes)
        39.33% of uncompressed inode table size (806 bytes)
Directory table size 319 bytes (0.31 Kbytes)
        56.36% of uncompressed directory table size (566 bytes)
Number of duplicate files found 2
Number of inodes 24
Number of files 12
Number of fragments 2
Number of symbolic links  0
Number of device nodes 0
Number of fifo nodes 0
Number of socket nodes 0
Number of directories 12
Number of ids (unique uids + gids) 1
Number of uids 1
        root (0)
Number of gids 1
        root (0)
Embedding ELF...
Marking the AppImage as executable...
Running on GitHub Actions
Guessing update information based on $GITHUB_REPOSITORY=probonopd/Zoom.AppImage and $GITHUB_REF=refs/heads/featureA
gh-releases-zsync|probonopd|Zoom.AppImage|continuous|appimagetool*-x86_64.AppImage.zsync
Embedding MD5 digest
zsyncmake is available and updateinformation is provided, hence generating zsync file
Success

Please consider submitting your AppImage to AppImageHub, the crowd-sourced
central directory of available AppImages, by opening a pull request
at https://github.com/AppImage/appimage.github.io
github@fbfcaab3b211:~/build/out$

srevinsaju · 2020-09-10T08:16:22Z

@probonopd this is my first C lang based PR, so please let me know of corrections

TheAssassin

The idea is okay, but the implementation needs to be improved. I commented on most of the problems. Please fix, I'll re-review then.

src/appimagetool.c

AppImage#1075 (comment)

Reads the GITHUB_REPOSITORY and GITHUB_REF environment variables and attempts to generate an appropriate updateinformation Adapted from https://github.com/probonopd/go-appimage/blob/7fab45552eb485c0076fd310835fa7958f981de0/src/appimagetool/appimagetool.go\#L547-L567

Move travis_ci specific documentation to travis_repo_slug `if` block

AppImage#1075 (comment)

…hub_actions

srevinsaju · 2020-09-10T13:20:37Z

@TheAssassin I guess I fixed the issues which you mentioned. Please review again.

LautaroLobo12

LGTM!

srevinsaju · 2020-10-05T13:43:32Z

@TheAssassin if you have few minutes to spare, could you please review this merge request, so that I can update accordingly? Thanks!!

TheAssassin

Noted the most important problems. Will re-check when those are fixed.

src/appimagetool.c

probonopd · 2020-11-15T11:28:40Z

Hello @srevinsaju do you think you can address the points raised by @TheAssassin and @codewiz above? I'd really like to see this merged for probonopd/Zoom.AppImage#1.

srevinsaju · 2020-11-15T15:15:59Z

I am having my first semester starting from tomorrow 🥶 , apologize for the delay. I would try to finish it as soon as the exams finish 🥇

its growing white 😅

…ateinformation max size

…ant to top of file)

srevinsaju · 2020-11-22T13:57:55Z

This "constant" is irrelevant to the rest of the file. Also, the code doesn't use such a convention anyway.

@TheAssassin so you mean, I should not use a constant? and keep literals, or move the constant a bit more down?

TheAssassin · 2020-11-22T14:07:25Z

A static const int is sufficient.

srevinsaju · 2020-11-22T14:09:09Z

A static const int is sufficient.

At the same position where it is, right?

TheAssassin · 2020-11-22T14:14:29Z

Obviously not. You should place it before the first use.

srevinsaju · 2020-11-22T14:17:11Z

Obviously not. You should place it before the first use.

ok, got it!

we already checked if github_ref was NULL

… instead of defining it globally

…name 'buf'

srevinsaju · 2020-11-22T19:08:07Z

@TheAssassin I hope I fixed all of what you have mentioned above.
This Pull Request was a great opportunity to learn more about C, and how dangerous it can be, if not used properly. Thanks to @codewiz.

This makes me think about using more safer languages like Rust, or at least modern C++.

@TheAssassin kindly review

probonopd · 2020-11-22T19:28:21Z

This makes me think about using more safer languages

Like Golang? Reminds me of probonopd/go-appimage#99 :)

srevinsaju · 2020-11-22T20:14:10Z

Yea GoLang can be considered as a safe language too. I am interested in v2 of GoLang, its more interesting 🚀

If I go by preference of languages:

Rust (new language for me, static binaries, super fast)
GoLang (easy to code, understand, functional)
C++ (20?)

probonopd · 2020-12-19T13:57:07Z

Reference: probonopd/linuxdeployqt#316

yozachar · 2021-04-25T13:34:25Z

Hi, is the PR still maintained? It's says this branch is out-of date with the base, I'd like to see this feature, how can I help? Coming from probonopd/Zoom.AppImage#1

srevinsaju · 2021-04-25T13:35:39Z

Hi, is the PR still maintained? It's says this branch is out-of date with the base, I'd like to see this feature, how can I help?

Updated.

probonopd · 2021-04-25T16:39:33Z

Thanks for updating @srevinsaju.

Hello @TheAssassin

Noted the most important problems. Will re-check when those are fixed.

Would be great if you could re-check if you find the time. I think this would be valuable to get merged soon.

Thanks!

TheAssassin requested changes Sep 10, 2020

View reviewed changes

src/appimagetool.c Outdated Show resolved Hide resolved

src/appimagetool.c Outdated Show resolved Hide resolved

src/appimagetool.c Outdated Show resolved Hide resolved

src/appimagetool.c Outdated Show resolved Hide resolved

src/appimagetool.c Outdated Show resolved Hide resolved

srevinsaju added a commit to srevinsaju/AppImageKit that referenced this pull request Sep 10, 2020

feat: Use snprintf instead of sprintf to avoid potential buffer overflow

63b6e6a

AppImage#1075 (comment)

srevinsaju added 7 commits September 10, 2020 16:18

style: use lower case (snake_case) for variable names

98079b9

docs: Move travis CI specific documentation

62be000

Move travis_ci specific documentation to travis_repo_slug `if` block

fix: Check if github_repository and github_ref variables are not NULL

fafcd41

style: Fix github_actions if code suite indentation

628d171

feat: Use snprintf instead of sprintf to avoid potential buffer overflow

c9b1a26

AppImage#1075 (comment)

fix: add an else block to inform the user of missing zsyncmake in git…

4e7307a

…hub_actions

srevinsaju force-pushed the github-actions branch from 4501e1c to 4e7307a Compare September 10, 2020 13:19

Merge branch 'master' into github-actions

ee24c2b

srevinsaju marked this pull request as ready for review September 10, 2020 13:19

srevinsaju requested a review from TheAssassin September 10, 2020 13:20

LautaroLobo12 reviewed Sep 12, 2020

View reviewed changes

srevinsaju mentioned this pull request Sep 22, 2020

Possibility of delta updates probonopd/Zoom.AppImage#1

Open

TheAssassin requested changes Oct 17, 2020

View reviewed changes

srevinsaju added 5 commits November 22, 2020 14:52

fix: define a constant buffer_size = 1024 and reuse it in setting upd…

fe6f97b

…ateinformation max size

fix: use strcmp for empty string check

42b20c2

fix: explicitly check if return of strstr is NULL pointer

3e39d09

style: follow C conventions of constant naming (Uppercase, move const…

b0d2f01

…ant to top of file)

fix: explicitly check if zsyncmake_path is not NULL

5371a2a

srevinsaju added 4 commits November 22, 2020 17:45

fix: remove an additional check for empty string.

8550ec6

we already checked if github_ref was NULL

style: move update_information_buffer_size to a position prior to use…

0d4bac3

… instead of defining it globally

style: use updateinformation_buffer instead of an ambiguous variable …

27113a8

…name 'buf'

fix: free unused pointers once processing is complete using g_strfreev

6d00122

srevinsaju requested a review from TheAssassin November 22, 2020 19:08

probonopd mentioned this pull request Dec 19, 2020

How to set update information wihtout using travis ci probonopd/linuxdeployqt#316

Open

Merge branch 'master' into github-actions

3e3c3ad

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Guess update information based on GitHub Actions environment variables #1075

Guess update information based on GitHub Actions environment variables #1075

srevinsaju commented Sep 10, 2020

srevinsaju commented Sep 10, 2020

TheAssassin left a comment

srevinsaju commented Sep 10, 2020

LautaroLobo12 left a comment

srevinsaju commented Oct 5, 2020

TheAssassin left a comment

probonopd commented Nov 15, 2020

srevinsaju commented Nov 15, 2020 •

edited

srevinsaju commented Nov 22, 2020

TheAssassin commented Nov 22, 2020

srevinsaju commented Nov 22, 2020

TheAssassin commented Nov 22, 2020

srevinsaju commented Nov 22, 2020

srevinsaju commented Nov 22, 2020

probonopd commented Nov 22, 2020 •

edited

srevinsaju commented Nov 22, 2020

probonopd commented Dec 19, 2020

yozachar commented Apr 25, 2021 •

edited

srevinsaju commented Apr 25, 2021

probonopd commented Apr 25, 2021 •

edited

Guess update information based on GitHub Actions environment variables #1075

Are you sure you want to change the base?

Guess update information based on GitHub Actions environment variables #1075

Conversation

srevinsaju commented Sep 10, 2020

srevinsaju commented Sep 10, 2020

TheAssassin left a comment

Choose a reason for hiding this comment

srevinsaju commented Sep 10, 2020

LautaroLobo12 left a comment

Choose a reason for hiding this comment

srevinsaju commented Oct 5, 2020

TheAssassin left a comment

Choose a reason for hiding this comment

probonopd commented Nov 15, 2020

srevinsaju commented Nov 15, 2020 • edited

srevinsaju commented Nov 22, 2020

TheAssassin commented Nov 22, 2020

srevinsaju commented Nov 22, 2020

TheAssassin commented Nov 22, 2020

srevinsaju commented Nov 22, 2020

srevinsaju commented Nov 22, 2020

probonopd commented Nov 22, 2020 • edited

srevinsaju commented Nov 22, 2020

probonopd commented Dec 19, 2020

yozachar commented Apr 25, 2021 • edited

srevinsaju commented Apr 25, 2021

probonopd commented Apr 25, 2021 • edited

srevinsaju commented Nov 15, 2020 •

edited

probonopd commented Nov 22, 2020 •

edited

yozachar commented Apr 25, 2021 •

edited

probonopd commented Apr 25, 2021 •

edited