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

Make GitHub Actions build a Debian package for download (fixes #34) #37

Merged
merged 7 commits into from
Nov 18, 2023

Conversation

hartwork
Copy link
Contributor

@hartwork hartwork commented Nov 11, 2023

Fixes #34

Review on commit level (rather than whole-diff level) is advised.

PS: The failed job clang-17 below should go all green after retry.

@j6t
Copy link
Owner

j6t commented Nov 12, 2023

Why does the clang-17 job fail?

Please justify why the 2.5.x patches are not needed anymore, in particular the one that claims to fix encoding. ("Testing without the patches shows that they are not necessary anymore" is fine, but if you know more, then I'd appreciate if you would add that knowledge in the commit message.)

hartwork and others added 4 commits November 12, 2023 15:54
- 10_fix_check_include_files.patch
  Fixed by commit d5745f3 already.

- 11_fix_changelog_encoding.patch
  New patch of value, applied in a separate follow-up commit in order
  to allow proper author information on commit level.
@hartwork
Copy link
Contributor Author

hartwork commented Nov 12, 2023

Hi @j6t, glad you like this pull request 😃

Why does the clang-17 job fail?

The log says it all: the CI depends on working Internet access and the availability of a few services and one of the downloads failed with error 503 Backend unavailable, connection timeout at https://github.com/j6t/kdbg/actions/runs/6836249651/job/18590950739?pr=37#step:2:27 .

Please justify why the 2.5.x patches are not needed anymore, in particular the one that claims to fix encoding. ("Testing without the patches shows that they are not necessary anymore" is fine, but if you know more, then I'd appreciate if you would add that knowledge in the commit message.)

Sure! It turns out that:

  • The missing CheckIncludeFiles is included since commit d5745f3.

  • The other one is new to the code base and has value, I was too quick when having a look last time, I was sure to find one more SHA1 to add to the commit message, just like with CheckIncludeFiles.

Okay nice, so a free "bugfix", good catch! Commit message extended and fix applied now, pushed.

Looking forward to the next round of review 🙏

@hartwork
Copy link
Contributor Author

@j6t any chance you could review/merge this over the weekend?

@j6t j6t merged commit 8227b5f into j6t:master Nov 18, 2023
3 checks passed
@hartwork
Copy link
Contributor Author

@j6t thank you! 🙏

@j6t
Copy link
Owner

j6t commented Nov 19, 2023

Thank you for the contribution.

How to move forward with this? How do users know that there are build products? Where do they find them? How do they know what to download?

@hartwork
Copy link
Contributor Author

hartwork commented Nov 19, 2023

Thank you for the contribution.

@j6t I'm glad you see value in it.

How to move forward with this? How do users know that there are build products?

Adding a sentence to the top of the readme may work. Arguably they could be called "nightly builds Debian packages" or something like that, although technically "nightly" is not correct.

Where do they find them?

I would suggest https://github.com/j6t/kdbg/actions/workflows/debian_package.yml?query=branch%3Amaster for a link and add "at the bottom of each CI run page" as an additional hint or instruction. If you feel like converting the readme to Markdown, an annotated screenshot could be added with an example. Something like this:

kdbg_download_Screenshot_20231119_154703

How do they know what to download?

The only file is named kdbg_<sha1>_debian_package(.zip) and users will be looking for a .deb file in there and it has one. No open questions there for me, once they find their way to that .zip file download.

What do you think?

@j6t
Copy link
Owner

j6t commented Nov 19, 2023

I would have expected that things are much easier to find. Why do we do all this if the results are so hidden? I cannot believe that other projects provide Debian packages in this way.

@hartwork
Copy link
Contributor Author

I would have expected that things are much easier to find.

@j6t would wish it was easier to find myself. I believe it's intentional to some extent. I should also note that CI run logs and artifacts are not kept forever. Another reasons why it's good that we're building at least once a week.

I cannot believe that other projects provide Debian packages in this way.

@j6t for releases, attaching binaries to a page below https://github.com/j6t/kdbg/releases would be one way to go. We can adjust the CI use a slight different versioning scheme when building Git tags and you can download and attache the results to a release directly. But you're not using GitHub releases yet. Here's what a release with (source and) binary attachments liks like: https://github.com/libexpat/libexpat/releases/tag/R_2_5_0

Regarding other projects: many have no packages at all, the most important ones don't need any because they are right in Debian, everyone else does all sorts of things.

We could make the CI do a lot more like creating GitHub release with build files attached automatically, but that will require quite some work, credentials, and so on — it's beyond the scope of what I want to volunteer for.

Why do we do all this if the results are so hidden?

Having hard to find files that you can point people to is better than no files, right? Why so negative?

@j6t
Copy link
Owner

j6t commented Nov 19, 2023

Having hard to find files that you can point people to is better than no files, right?

It has some value, yes.

@hartwork hartwork deleted the debian-package branch November 19, 2023 21:17
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.

Bringing back Debian/Ubuntu packages?
2 participants