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

ci(compile): and publish #2

Merged
merged 19 commits into from
Mar 18, 2024
Merged

ci(compile): and publish #2

merged 19 commits into from
Mar 18, 2024

Conversation

Mte90
Copy link
Contributor

@Mte90 Mte90 commented Mar 15, 2024

I suggest to do a squash merge as I did various tests.

This PR automatically compile, generate a zip, create a new release and attach that file.
It is executed when a new tag is configured in the repository.

@Mte90 Mte90 mentioned this pull request Mar 15, 2024
@sonic2kk
Copy link
Owner

Thanks! I appreciate the work here.

It looks like this will compile a standard binary. Is this portable for systems that don't have leveldb? Mainly, I'm thinking of the case of using this on Steam Deck, primarily with SteamTinkerLaunch (although it's a general concern for people that may have other immutable distros, such as Bazzite).

Even if not, I'll still merge this because having a binary on its own is still useful for people with alternative use cases (or someone who may want to package this). Although I'm wondering how much work a follow-up PR would be to build an AppImage for use on SteamOS (or, if you know of an alternative portable packaging solution, I'm all ears!)

I think this solves part of #1, the remaining work is then to build an AppImage.

@Mte90
Copy link
Contributor Author

Mte90 commented Mar 18, 2024

So compiles with leveldb libraries so it should works also where is not installed.

Do you think that an appimage is needed? as it is a simple binary like innoextract etc

@sonic2kk
Copy link
Owner

sonic2kk commented Mar 18, 2024

Do you think that an appimage is needed? as it is a simple binary like innoextract etc

innoextract is taken from the Arch repos only for SteamOS and only for SteamTinkerLaunch. This project is much more general, and will happen to be used by SteamTinkerLaunch.

In terms of building with LevelDB, are there any licensing concerns? This project is licensed under MIT, LevelDB is licensed under BSD-3 Clause. This is a general concern I had with packaging the project, including for an AppImage, as I am not a lawyer and don't know what restrictions may apply (it's also why no work was done on packaging for a long time!).

I understand you're (probably 😉) not a lawyer either, so I get it if you're the wrong person to ask. This is entirely new ground for me though and any info you have at all is great, because it's probably already more than I have!

Licensing issues aside that would apply no matter how we package, for use with STL, I would prefer to use an AppImage, just like with Yad, yes. 🙂

@Mte90
Copy link
Contributor Author

Mte90 commented Mar 18, 2024

Reading here the 2 license allows what you are trying to do https://joinup.ec.europa.eu/licence/compatibility-check/BSD-3-Clause/MIT

@sonic2kk
Copy link
Owner

Looks good then, just one more question (sorry if it seems like I'm splitting hairs here...):

The inbound licence is a permissive licence: provide you respect and include all existing copyrights or trademarks and [...]

Since we're simply including the leveldb header and not modifying the copyright notices in LevelDB itself, we should also be good on this front I imagine?

@sonic2kk
Copy link
Owner

Approvals aren't set up for this repo (since it's just me as a code owner) but pressing the "Approved" button is satisfying both to do and to receive, so I like doing it when I remember to :-)

I checked a bit more out about the BSD-3 license and I think just as you said we are good. I will merge this.

Thank you!

@sonic2kk sonic2kk merged commit 8a39936 into sonic2kk:main Mar 18, 2024
@Mte90
Copy link
Contributor Author

Mte90 commented Mar 18, 2024

Yes it is like you are saying.
It is enough that you mention in the readme that leveldb is a dependence to compile it.

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.

2 participants