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

Utilize GitHub's official actions/upload-pages-artifact #119

Open
PhrozenByte opened this issue Oct 25, 2024 · 6 comments
Open

Utilize GitHub's official actions/upload-pages-artifact #119

PhrozenByte opened this issue Oct 25, 2024 · 6 comments

Comments

@PhrozenByte
Copy link

Instead of letting Flatter create and upload GitHub Page's artifact.tar, it would be better to rather just prepare the files necessary for upload (including the .flatpakrepo file, and possibly also .flatpak bundles (see #117) and .flatpakref files (see #91) in the future) in a distinct directory (e.g. using a new upload: _site/ config) and leave the rest to the user. If no additional steps are deemed necessary by the user, the next step would be to just call GitHub's official actions/upload-pages-artifact action. This also simplifies Flatter by not reinventing the wheel (and not breaking if GitHub decides to expect something else than artifact.tar).

However, if the user wants to do some additional steps, like using Jekyll (see the actions/jekyll-build-pages action) to build a more sophisticated website, one could use the actions/upload-artifact action instead and then just build and upload the website in the deploy job, not requiring the user to fiddle with tar.

For an example see my PhrozenByte/flatpak-com.jetbrains.PhpStorm repo, which clumsily needs to download, untar and delete the github-pages artifact created by Flatter first to enable processing by Jekyll (see highlighted lines).

By the way, I took the freedom to use your website from ./tests as simple boilerplate for a website hosting a single Flatpak (fully configurable with Jekyll). If you ever decide to implement the suggested above, it might be a good starting point for an example in Flatter's README.md.

@andyholmes
Copy link
Owner

andyholmes commented Oct 25, 2024

For an example see my PhrozenByte/flatpak-com.jetbrains.PhpStorm repo, which clumsily needs to download, untar and delete the github-pages artifact created by Flatter first to enable processing by Jekyll (see highlighted lines).

That's definitely not necessary; the Custom Deploy in the README describes how you can use another action, or the official GitHub action.

The repository path is always set as the output steps.<step name>.outputs.repository so you can add, remove, modify files. Afterwards, you can continue with the official GitHub pages action by simply pointing it at that directory.

Alternatively, you can just inject extra files using the upload-pages-includes. As mentioned in #117, there's really no reason to upload bundles built from the repository into the repository as files.

@PhrozenByte
Copy link
Author

I indeed saw that section in README.md, however, Flatter doesn't create the .flatpakrepo file if one doesn't set upload-pages-artifact: true, right (I might miss something though, because I didn't test it, just looked at the code)? Is this intentional? That's why I couldn't set upload-pages-artifact: false and was therefore forced to build upon Flatter's artifact.tar.

In any case I believe that recommending users to use a separate step to upload the website streamlines usage. It stresses that users can modify the files uploaded (like that users aren't limited to upload-pages-includes adding files to the main folder). It allows for a greater separation of concerns. And using a config like upload: _site/ (thinking about it, output: _site/ might be better) makes working with the website much easier (personally I don't like working with outputs, they often make things harder).

@andyholmes
Copy link
Owner

I indeed saw that section in README.md, however, Flatter doesn't create the .flatpakrepo file if one doesn't set upload-pages-artifact: true, right (I might miss something though, because I didn't test it, just looked at the code)?

That's true, that could be changed. It's worth pointing out that just like the .flatpakref, a number of assumptions have to be made about where the repository will be hosted and other metadata about the repository. So it's likely this won't actually work for your case anyways.

In any case I believe that recommending users to use a separate step to upload the website streamlines usage.

I see this as very much a matter of opinion. I do this in some repositories, but in other I much prefer the "fire and forget" aspect 🤷

@PhrozenByte
Copy link
Author

Can you elaborate on why the .flatpakrepo file likely wouldn't work? Because I indeed did use the .flatpakrepo file and it works flawlessly 😅 It's true that the human readable metadata in the .flatpakrepo file is a bit clumsy due to the limited information Flatter has, but that isn't shown prominently anyway. What truly matters is the Url and GPGKey, right?

If Flatter could create the .flatpakrepo file no matter what that would be very much appreciated. I could set upload-pages-artifact: false then and deal with uploading the files as artifact myself then.

Besides that, you're absolutely right, whether an all-in-one-solution or separate steps is better really is just a matter of opinion 😄

@andyholmes
Copy link
Owner

Can you elaborate on why the .flatpakrepo file likely wouldn't work?

Because the .flatpakrepo file has to contain the destination Url where the repository will be deployed, which is known when Flatter is responsible for deploying it. An input could be added, but there's no benefit over just adding a line to include a .flatpakrepo file or generating one like the CNAME file is in the examples.

To be honest, it sounds like you're looking for something with more flexibility than Flatter is really intended for. Flatter's job is really to let flatpak-builder do all the work, so it might be easier for you to invoke it directly in workflow and act on the resulting directory.

@PhrozenByte
Copy link
Author

I guess we have had a misunderstanding, Flatter is working just fine for me ❤️

In general you're absolutely right: In the last days I've read Flatter's source code and more docs about the matter, and it's true that one can achieve the same with a couple of lines of code around flatpak-builder (I even found examples on flatpak.org 😆). However, it still requires people to develop a deeper understanding on how Flatpak repos work (which I didn't have until now). Thus there's a rather high entry barrier to setup a custom Flatpak repo - and that's where Flatter comes in. Flatter makes creating a custom Flatpak repo very, very easy.

Yes, Flatter does have to make some assumptions. Some assumptions are just fine the way they are (like assuming that the Flatpak repo is deployed to GitHub Pages). Some could be improved by adding optional workflow options that are used in .flatpakrepo and .flatpakref files (like Title of .flatpakrepo) - because there's no real ham in adding more optional input values that use default values otherwise. Trust me, as a beginner in creating Flatpak repos like myself before it's way easier to just set some input value than creating a .flatpakrepo file manually, even though the values are just passed through. I'd still vote for leaving the upload of the artifacts.tar to actions/upload-pages-artifact, but in the end it doesn't hurt having that logic built-in if Flatter would always create the .flatpakrepo file no matter the upload-pages-artifact option.

These are just suggestions for a future Flatter. Please don't feel obligated in any way, I don't say that I expect anyone to actually implement this. Because, if I'd really need it, I'd have to implement it myself. As I've said in #117 (comment), I can absolutely relate to having limited time and to put it bluntly, you likely have better things to do with your free time than to implement stuff you don't need yourself. So, no worries, these are just suggestions that might never become reality if nobody picks them up. I worked around the issues I had and Flatter is working just fine for me now. So I'm very happy with Flatter ☺️ Thanks again for your great work here! ❤️

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

No branches or pull requests

2 participants