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

icu4c 70.1 #88210

Closed
wants to merge 65 commits into from
Closed

icu4c 70.1 #88210

wants to merge 65 commits into from

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented Oct 28, 2021

Created with brew bump-formula-pr.

Not yet officially announced yet, but it will be by the time we're ready with this.

@BrewTestBot BrewTestBot added the bump-formula-pr PR was created using `brew bump-formula-pr` label Oct 28, 2021
@Bo98 Bo98 added CI-linux-self-hosted Build on Linux self-hosted runner CI-long-timeout Use longer GitHub Actions CI timeout. CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. labels Oct 28, 2021
@carlocab
Copy link
Member

Linkage failures on ARM:

apngasm
boost
clickhouse-odbc
dwdiff
freeling
harfbuzz
hfstospell
libcdr
libical
liblcf
libmspub
libphonenumber
libpsl
libvisio
mapnik
mpd
mysql
ncmpcpp
node
node@12
node@14
node@16
openrct2
pazpar2
pcl
percona-server
percona-xtrabackup
php
postgresql
postgresql@10
postgresql@11
postgresql@12
postgresql@13
qt
sile
tectonic
texlive
tracker
vte3
vtk
widelands
yaz
zebra
znc
zorba

There's plenty more on Linux.

@Bo98
Copy link
Member Author

Bo98 commented Oct 31, 2021

Linux linkage failures are awkward as they are checked recursively, which I'll try fix first before pushing.

codequery etc shouldn't need revision bumps.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. To keep this pull request open, add a help wanted or in progress label.

@github-actions github-actions bot added the stale No recent activity label Nov 2, 2021
@Bo98 Bo98 added in progress Stale bot should stay away and removed stale No recent activity labels Nov 2, 2021
@Bo98
Copy link
Member Author

Bo98 commented Nov 8, 2021

Plan for this is to do it after openldap is done, to avoid conflicts.

@carlocab carlocab mentioned this pull request Nov 22, 2021
6 tasks
@SMillerDev
Copy link
Member

OpenLDAP is finally merged, shall I rebase this?

@carlocab
Copy link
Member

You still have a conflict for tracker

@Bo98
Copy link
Member Author

Bo98 commented Nov 25, 2021

Oh libsoup just got merged when I was rebasing it seems.

@carlocab
Copy link
Member

Are any of these GCC dependents? If they are we may want to rebase this so we can attempt a bottle build when applicable.

@Bo98
Copy link
Member Author

Bo98 commented Nov 25, 2021

ncmpcpp.

mapnik too though that's not even on Intel Monterey yet.

I've maybe missed others but that's what I see from a very quick glance through what has a arm64_monterey bottle and what doesn't.

@carlocab
Copy link
Member

Only ones that show up for me (on master) as ready to bottle are chakra, gjs, mysql, ncmpcpp, tarantool, and percon-server.

Probably only ncmpcpp then, so not worth the rebase.

@Bo98
Copy link
Member Author

Bo98 commented Nov 27, 2021

I've got PRs open for most failures (last couple will be up in a bit).

I do not think it will be possible to revision bump texlive in this PR so I will likely drop it. Going forward we will need to consider splitting that formula to be manageable.

@SMillerDev
Copy link
Member

@danielnachun I think we'll have to reduce the size of the installed packages for texlive. Is this something you could pick up?

@danielnachun
Copy link
Member

@danielnachun I think we'll have to reduce the size of the installed packages for texlive. Is this something you could pick up?

Definitely. The reason the bottle is so big right now is because of the texlive-texmf resource. I would propose that we rename the current formula to texlive-bin and remove the texlive-texmf resource. Then we would make a texlive-texmf formula which just installs the texlive-texmf resource and symlinks it into the cellar path for texlive-bin. This texlive-texmf formula would only depend on texlive-bin so it wouldn't get pulled into any dependent tests.

@carlocab carlocab removed the CI-long-timeout Use longer GitHub Actions CI timeout. label Dec 8, 2021
@iMichka iMichka added the long build Needs CI-long-timeout label Jan 17, 2022
@cho-m
Copy link
Member

cho-m commented Feb 19, 2022

Since we doubled space of self-hosted Linux to 100G due to dotnet, is it possible that texlive will install now? Though, will probably want to wait for a couple other PRs to get merged first due to conflicts, like boost.

@danielnachun
Copy link
Member

I think we were running out of space on macOS actually, though we should confirm that.

@cho-m
Copy link
Member

cho-m commented Feb 19, 2022

I think we were running out of space on macOS actually, though we should confirm that.

Ah. I do see Intel macOS is out of space now. Only checked Linux and ARM macOS.

@carlocab
Copy link
Member

carlocab commented Mar 2, 2022

Still can't find unzip for some reason.

@Bo98
Copy link
Member Author

Bo98 commented Mar 2, 2022

it seems like the home directory is not currently nuked between CI runs

Until ephemeral, I'll cover this scenario with Homebrew/actions#262 (and I'll set GNUPGHOME in the workflow to somewhere in /tmp)

Still can't find unzip for some reason.

Hopefully fixed by Homebrew/brew#12947

@carlocab
Copy link
Member

carlocab commented Mar 2, 2022

Let's run #96101 ASAP when this is merged, even if it entails premature removal of a CI-long-timeout label. The macOS bottles for texlive have linkage with Homebrew icu4c too (it's in the dep tree through harfbuzz).

@BrewTestBot
Copy link
Member

:shipit: @Bo98 has triggered a merge.

@BrewTestBot
Copy link
Member

⚠️ @Bo98 bottle publish failed.

@carlocab
Copy link
Member

carlocab commented Mar 2, 2022

error: gpg failed to sign the data
error: failed to write commit object

@Bo98
Copy link
Member Author

Bo98 commented Mar 2, 2022

That error basically means "gpg didn't exit successfully", which is always fun to debug as it means absolutely anything and it's not reproducible in a local Docker container.

@Bo98
Copy link
Member Author

Bo98 commented Mar 2, 2022

Oh of course, it'll be because brew env filters GNUPGHOME.

@BrewTestBot
Copy link
Member

:shipit: @Bo98 has triggered a merge.

@BrewTestBot
Copy link
Member

⚠️ @Bo98 bottle publish failed.

@SMillerDev
Copy link
Member

Error: Bottles are for tectonic 0.8.0_1 but formula is version 0.8.1_1!

@carlocab
Copy link
Member

carlocab commented Mar 2, 2022

Error: Bottles are for tectonic 0.8.0_1 but formula is version 0.8.1_1!

😭

@carlocab
Copy link
Member

carlocab commented Mar 2, 2022

I suggest a manual merge while dropping tectonic. Let's just rebuild it after.

@carlocab
Copy link
Member

carlocab commented Mar 2, 2022

Attempting a manual merge. Downloading the bottles now. Will warn before I start an upload attempt.

@carlocab
Copy link
Member

carlocab commented Mar 2, 2022

I decided to leave the download going overnight. It's finished now, but I'll be on the road so will try an upload when I have a stable connection.

@carlocab
Copy link
Member

carlocab commented Mar 3, 2022

Attempting upload.

@carlocab
Copy link
Member

carlocab commented Mar 3, 2022

Merge conflict for sile. Dropping.

carlocab pushed a commit to carlocab/homebrew-core that referenced this pull request Mar 3, 2022
Closes Homebrew#88210.

Signed-off-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
@Bo98
Copy link
Member Author

Bo98 commented Mar 3, 2022

We need a better way to "lock" formula or whatever. A significant amount of the pain here is because of conflicts.

ICU 71 will probably be out next month, and I'm up for coordinating the revision bumps for that again, but I just can't monitor every single PR which comes in while the build is happening.

@carlocab carlocab mentioned this pull request Mar 3, 2022
@Bo98
Copy link
Member Author

Bo98 commented Mar 3, 2022

Maybe CODEOWNERS is the solution here.

@carlocab
Copy link
Member

carlocab commented Mar 3, 2022

We need a better way to "lock" formula or whatever. A significant amount of the pain here is because of conflicts.

ICU 71 will probably be out next month, and I'm up for coordinating the revision bumps for that again, but I just can't monitor every single PR which comes in while the build is happening.

Agreed. I tried to monitor PRs too, but I can't watch this repo 24/7 and tectonic and sile were merged while I was away.

Maybe CODEOWNERS is the solution here.

This sounds like a good idea, actually. Is there a way to make code owner review mandatory only for specific files?

@Bo98
Copy link
Member Author

Bo98 commented Mar 3, 2022

Is there a way to make code owner review mandatory only for specific files?

In terms of GitHub branch protection? No, but we could make pr-pull fail without a code owner review for any formula file - and allow that to be overridable by an argument (which can be passed to pr-publish too) for the "safe" periods.

@Bo98
Copy link
Member Author

Bo98 commented Mar 3, 2022

Or actually it maybe doesn't even need to be that complicated - it probably only needs to be on autopublish level

@carlocab
Copy link
Member

carlocab commented Mar 3, 2022

Or actually it maybe doesn't even need to be that complicated - it probably only needs to be on autopublish level

If we could automatically label those PRs with do not merge too that might be useful, in case some maintainer wonders why a PR they approved hasn't been auto-merged and decides to pr-publish it.

The labelling has the bonus of already blocking autopublish.

@Bo98
Copy link
Member Author

Bo98 commented Mar 3, 2022

Makes sense. I would maybe lean on a separate red label just to make it more obvious to the PR author what's going on rather than getting a random "do not merge" label immediately.

@carlocab
Copy link
Member

carlocab commented Mar 3, 2022

Makes sense. I would maybe lean on a separate red label just to make it more obvious to the PR author what's going on rather than getting a random "do not merge" label immediately.

Sure, I'm fine with that too.

@Bo98 Bo98 removed the in progress Stale bot should stay away label Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump-formula-pr PR was created using `brew bump-formula-pr` CI-linux-self-hosted Build on Linux self-hosted runner CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. long build Needs CI-long-timeout
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants