juan-leon / lowcharts

Tool to draw low-resolution graphs in terminal
MIT License
205 stars 5 forks source link

Add the LICENSE file to the binary archives in releases #4

Closed Antiz96 closed 2 years ago

Antiz96 commented 2 years ago

Hi,

I would like to make a suggestion about the binary archives content of the different releases. I think they should include the LICENSE file in addition to the binary (like dnote does for instance).

Indeed, the best practice in terms of licensing implies to include the license file with your project, which is not the case when installing lowcharts via release as you only get the binary (and not the LICENSE).

In addition to that, including the LICENSE file into the binary archives will make the maintenance of the lowcharts-bin AUR package easier and cleaner on my side. Indeed, lowcharts being protected by the MIT license, I have to include the LICENSE file (in addition to the binary) during the installation process (see https://wiki.archlinux.org/title/PKGBUILD#license). As it is not included in the binary archive itself, I have to get it directly from the GitHub repo. While grabbing the LICENSE file directly from the GitHub repo is not an issue from a technical stand point, the Arch-Linux packaging guidelines and best-practices imply to check the integrity of every download resources through their hashes before the installation process for obvious security reason. According to that, the installation process of the lowcharts-bin AUR package depends on the integrity of a file that is subject to change without necessarily leading to a new release. In other words, a change of the LICENSE file can make the build fail even tho there's no new release nor changes to the binary (until I update the hash of the LICENSE file accordingly in the PKGBUILD and publish the update). If the file was provided in the binary archive directly, I would not have to rely on its hash, as verifying the integrity of the archive itself would be enough (preventing the possibility of a failed build because of a change in the LICENSE file on the GitHub repo).

I'm aware that the LICENSE file shouldn't be modified too frequently and thus the above case might be pretty rare. On the other hand, I assume this change is fairly easy to do and would improve the binary archives structure and the maintenance of the lowcharts-bin AUR package, hence my suggestion :)

Thanks for your awesome work BTW !

Antiz96 commented 2 years ago

Thank you for adding the LICENSE file to the assets of release 0.5.3 and 0.5.4. I updated the lowcharts-bin AUR package accordingly to avoid the scenario I described in this issue (it is now grabbing the LICENSE file from the release's assets and not directly from the repo).

BTW I updated, all lowcharts* AUR packages to V0.5.4 ;)

Thanks a lot for you quick actions !

juan-leon commented 2 years ago

Thanks to you for reporting those issues.

I did not close that issue after doing the LICENSE related changes because I wanted to ask you if LICENSE file should be inside tar.gz file too. If that is the case, please let me know and I will include it there.

Antiz96 commented 2 years ago

Thanks for your answer and actions !

Having the license into the assets is okay but, in my opinion, the LICENSE file should indeed primarily be inside the tar.gz archives actually.

As of now, if the user downloads lowcharts via release, he will not get the license unless he downloads it manually from the release's assets as well. As far as the maintenance on my side is concerned, having the LICENSE file directly in the tar.gz archives would be easier for me as I only have to download and check the integrity of the tar.gz archive to deliver both the binary and the LICENSE file to the end user.

My opinion would be to include the LICENSE file inside the tar.gz archives only (and not necessarily into the release's assets):

juan-leon commented 2 years ago

I created v0.5.6 with your suggestion: including the LICENSE file inside the tar.gz archives only

Antiz96 commented 2 years ago

Thanks a lot, that's perfect !

I updated the AUR packages accordingly. I guess the issue can be closed now :smile:

juan-leon commented 2 years ago

I am closing the issue then.

Thanks again for raising this issue

Antiz96 commented 2 years ago

First fan in the AUR though :smile: : image

https://aur.archlinux.org/packages/lowcharts-bin#comment-881610