search

flatcar / Flatcar

Flatcar project repository for issue tracking, project documentation, etc.
https://www.flatcar.org/
Apache License 2.0
774 stars 32 forks source link

New Package Request: ncurses #364

Closed blanquicet closed 3 years ago

blanquicet commented 3 years ago

Package name and purpose
Update sys-libs/ncurses from version 6.1-r2 to 6.2-r1.

for Flatcar:

Impact of adding this package to the Flatcar OS image [TO BE DEFINED]

The package will increase the image size by: 0.06 MBytes.

Benefits of adding this package
As per Release notes, the most important bug-fixes/improvements dealt with user-defined capabilities in terminal descriptions. They also mention some other bug-fixes, but are focused on new features and improvements to existing features since ncurses 6.1 release.

blanquicet commented 3 years ago

Hello,

After downloading the SDK and successfully building the Flatcar OS image (Without any change), I booted it with QEMU and tried to identify some files belonging to the ncurses library so that I will be able to validate later in the modified image that they were effectively updated. Yes, it was a very "empirical" way to do so, in fact, after reading the documentation, I understood that it can be done using equery-amd64-usr.

Next, in order to update the packet, I initially did it by replacing the folder in coreos-overlay/sys-libs with the one I manually downloaded from Gentoo repo. Just for testing, I also tried the update_ebuilds script to check how it works and, in the end, I got the same result, the difference was the amount of data downloaded using the update_ebuilds method respect to the manual where I can control the --depth.

Please correct me if I said something wrong or I missed something.

Then, I checked the new package and I noticed there are two versions: 6.2-r1 and 6.2_p20210123. However, when I used emerge-amd64-usr to build the package, it uses version 6.2-r1. I checked the ebuilds and, if I understood correctly, version 6.2_p20210123 is marked as experimental for amd64 using the KEYWORDS variable.

Is my understanding correct? If so, how do we want to proceed? In my opinion, I would prefer to merge only ncurses-6.2-r1 and wait until ncurses-6.2_p20210123 gets unmarked for amd64 before merging it unless we need it for any specific reason. I would even prefer to merge until commit 69bf5af - sys-libs/ncurses: drop x86-macos to keep folder clear.

What do you think?

Thanks for the support!

dongsupark commented 3 years ago

In my opinion, I would prefer to merge only ncurses-6.2-r1 and wait until ncurses-6.2_p20210123 gets unmarked for amd64 before merging it unless we need it for any specific reason.

Yes, it sounds good. Please keep only ncurses-6.2-r1.

I would even prefer to merge until commit 69bf5af - sys-libs/ncurses: drop x86-macos to keep folder clear.

That's also fine.

Anyway the PR should consists of at least 2 commits: one for pure sync with Gentoo, and the other for Flatcar-specific changes on top of the 1st commit.

pothos commented 3 years ago

These here are the steps which help to create a commit that resets the package and then another one to migrate the patches: https://github.com/kinvolk/coreos-overlay#updating

blanquicet commented 3 years ago

Thanks to @dongsupark and @pothos for your responses.

I am following the steps for updating package and I have two questions:

  1. I apologise but I not sure I understood the following statement: "_mention the commit ID in the body (git show updateebuilds/master)". From the first sentence I immediately thought it referred to the commit ID of Gentoo repo I am merging, which would make senses because it could make our lives easier in the future. However, the suggested command to be used "_git show updateebuilds/master" destabilised me so I am not sure I got the point of this step.

  2. I am trying to identify the downstream patches to be ported and I found that the latest sync for ncurses was (very probably) done with Gentoo's commit c94776f9bb and files for v5.9-r101 was dropped to keep only files related to v6.1-r2 for which ebuild was kept identical with Gentoo repo, as expected. Then, on top of this sync, there is only one commit named sys-libs/ncurses: Apply CoreOS changes which, if I understood correctly, is the downstream patch I need to port. However, I compared ebuild files of current v6.1-r2 and new v6.2-r1 and I am not sure such patch is still applicable so I am wondering if I misunderstood the updating steps or if I am doing something wrong and how I should proceed.

Thanks for the support!

dongsupark commented 3 years ago

"mention the commit ID in the body (git show update_ebuilds/master)". From the first sentence I immediately thought it referred to the commit ID of Gentoo repo I am merging, which would make senses because it could make our lives easier in the future.

Yes, that's correct. For example, please see the commit ffc40d2b651d in intel-microcode. Its commit message includes a link to https://github.com/gentoo/gentoo/commit/c1d8ba62ab0bcbea3133864a8dfce246406f181c . That allows other folks to easily understand the relation to its corresponding commit in Gentoo.

Then, on top of this sync, there is only one commit named sys-libs/ncurses: Apply CoreOS changes which, if I understood correctly, is the downstream patch I need to port. However, I compared ebuild files of current v6.1-r2 and new v6.2-r1 and I am not sure such patch is still applicable so I am wondering if I misunderstood the updating steps or if I am doing something wrong and how I should proceed.

Yes, you understood it correctly. If the commit "Apply CoreOS changes" is fairly small, or you are lucky, then you would be able to simply git cherry-pick the commit. If the commit is big, or the ncurses ebuild has diverged in-between, then your cherry-pick would fail with conflicts. In that case, you would need to somehow resolve the conflicts.

blanquicet commented 3 years ago

For example, please see the commit ffc40d2b651d in intel-microcode. Its commit message includes a link to gentoo/gentoo@c1d8ba6 . That allows other folks to easily understand the relation to its corresponding commit in Gentoo.

Understood. After these clarifications, now it is completely clear.

If the commit "Apply CoreOS changes" is fairly small, or you are lucky, then you would be able to simply git cherry-pick the commit. If the commit is big, or the ncurses ebuild has diverged in-between, then your cherry-pick would fail with conflicts. In that case, you would need to somehow resolve the conflicts.

Yes, it has diverged so I will work on resolving the conflicts.

Thanks!

blanquicet commented 3 years ago

I resolved the conflicts and I checked there were no compilation errors by performing a scratch build.

On the other hand, I just updated the issue description with the information about the increase of the image size. I computed it by using the following commands:

On v2801.0.0 build without modifications:

# equery-amd64-usr s sys-libs/ncurses
 * sys-libs/ncurses-6.1-r2
         Total files : 144
         Total size  : 3.88 MiB

After update:

# equery-amd64-usr s sys-libs/ncurses
 * sys-libs/ncurses-6.2-r1
         Total files : 144
         Total size  : 3.94 MiB