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.

Sign up for GitHub

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

Jump to bottom

fix: account for systems where musl is present #269

Merged
merged 1 commit into from May 18, 2022
Merged

fix: account for systems where musl is present #269

merged 1 commit into from May 18, 2022

Conversation

vladdoster
Copy link
Member

@vladdoster vladdoster commented May 12, 2022 •
edited

Description

  • account for systems where musl is present
  • improve darwin pattern

Motivation and Context

closes #179

Usage examples

N/A

How Has This Been Tested?

new gh-r tests

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@github-actions github-actions bot added the ci/cd label May 12, 2022
@vladdoster vladdoster force-pushed the fix/expand-systems-for-unit-tests-patch branch 3 times, most recently from 8c8953c to 2cef608 Compare May 12, 2022 06:36
@github-actions github-actions bot added the tests label May 12, 2022
@vladdoster vladdoster force-pushed the fix/expand-systems-for-unit-tests-patch branch from 2cef608 to 49e28bf Compare May 12, 2022 07:14
@jankatins jankatins mentioned this pull request May 12, 2022
Merged
7 tasks
@jankatins
Copy link
Contributor

jankatins commented May 12, 2022 •
edited

6 failures due to GLIBC version:

@vladdoster
Copy link
Member Author

@jankatins I'm aware and have not merged due to it.

@jankatins
Copy link
Contributor

jankatins commented May 12, 2022 •
edited

@vladdoster The two additional ubuntu targets are in main (4d4cb13), at least my PR fails due to this after i rebased on top of main.

@vladdoster
Copy link
Member Author

vladdoster commented May 12, 2022 •
edited

Yep, it's due to 18.04, but I think it's reasonable to support it since Ubuntu hasn't EOL'ed it yet.

I guess it isn't so much about 18.04 in particular, but I've found other issues on CentOS 7 that will be solved. Long story short, just increasing the robustness of gh-r. I use it a ton and even though it's solid 90% of the time, I want to believe it will work for 99% of cases.

@jankatins
Copy link
Contributor

It's more that #227 is now in a non-mergeable state as the CI tests fail :-)

@jankatins
Copy link
Contributor

jankatins commented May 16, 2022 •
edited

I wonder if it isn't easier in the end to give the user easier control over the bpick pattern, e.g. by adding bpick_linux (bpick_<uname -s>) and maybe even bpick_<uname -s>_<uname -m> and then fall back from most specific to least specific to whatever the magic regex is you have now.

You are essentially solving the same problem as spam filters and I would bet that this problem, too, requires some machine learning thingy to solve in the end.

@vladdoster
Copy link
Member Author

vladdoster commented May 16, 2022 •
edited

It doesn't require machine learning, lol; it's simple pattern matching with a low number of possible permutations.

I'm solving an issue I have currently on Ubuntu 22.04 where it doesn't take into account a linux-gnu system that has musl installed. In 99% of cases, you want the musl binary because it is more portable. This is due to Glibc only being backward compatible with versions greater or equal to the version it was originally compiled with. This makes programs like delta crash because my Glibc version is too old, but I can run the musl version without issues. Also, most systems will have musl installed and should be taken into consideration.

There is no 'magical' regex, it simply filters out things that won't work on the system. If you want to use a bpick, you still can without issue.

@vladdoster vladdoster force-pushed the fix/expand-systems-for-unit-tests-patch branch 14 times, most recently from 7462df6 to c62ecfe Compare May 17, 2022 09:41
@vladdoster vladdoster force-pushed the fix/expand-systems-for-unit-tests-patch branch 5 times, most recently from 73a97f5 to 65b00f6 Compare May 18, 2022 19:08
@vladdoster
fix: account for systems where musl is present
760368d 
Signed-off-by: Vladislav Doster <mvdoster@gmail.com>
@vladdoster vladdoster force-pushed the fix/expand-systems-for-unit-tests-patch branch from 65b00f6 to 760368d Compare May 18, 2022 19:16
@vladdoster vladdoster changed the title fix: fix triggers and brew ncurses installationrun for unit tests fix: account for systems where musl is present May 18, 2022
@vladdoster
Copy link
Member Author

@jankatins E

6 failures due to GLIBC version:

This is the exact issue this PR solves. For systems other than rolling release systems, the package manager will probably not have a newer glibc version to satisfy the constraint. However, it can be ran by using the musl version. I'll change it if someone brings up a valid reason, but I have yet to see one.

@vladdoster vladdoster marked this pull request as draft May 18, 2022 19:29
@vladdoster vladdoster marked this pull request as ready for review May 18, 2022 19:29
@vladdoster vladdoster merged commit 8620574 into main May 18, 2022
@vladdoster vladdoster deleted the fix/expand-systems-for-unit-tests-patch branch May 18, 2022 19:36
jankatins
jankatins reviewed May 18, 2022
View reviewed changes
tests/gh-r.zunit
run zinit for @wagoodman/dive; assert $state equals 0
local dive="$ZBIN/dive"; assert "$dive" is_executable
$dive --version; assert $state equals 0
}
@test 'docker-compose' {
@test 'docker-buildx' { # A monitor of resources
run zinit for as'completions' atclone'buildx* completion zsh > _buildx' lbin'!buildx-* -> buildx' @docker/buildx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

- run zinit for as'completions' atclone'buildx* completion zsh > _buildx' lbin'!buildx-* -> buildx' @docker/buildx
+ run zinit for as'completions' atclone'./buildx* completion zsh > _buildx' lbin'!buildx-* -> buildx' @docker/buildx

Fixed in #227: fix: Fix docker-buildx gh-r test now that atclone ice is run

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean fixed? This worked as it was?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, just that #227 has a commit included which fixes this problem as it showed up there as after the exclamation mark escape fix, the atclone got run and it showed up as a test failure.

@vladdoster vladdoster mentioned this pull request Jun 11, 2022
Closed
github-actions bot pushed a commit that referenced this pull request Nov 7, 2022
@vladdoster
release: v3.7.0 → v3.8.0
a27d7e0 
# [3.8.0](v3.7.0...v3.8.0) (2022-11-07)

### Bug Fixes

* account for systems where musl is present ([#269](#269)) ([8620574](8620574))
* alist repository for gh-r test ([#305](#305)) ([fb3c082](fb3c082))
* allow zinit to be run from non-interactive scripts ([#227](#227)) ([c3d1bb5](c3d1bb5)), closes [#199](#199)
* broken yaml syntax in issue template ([#355](#355)) ([f729e06](f729e06))
* calico gh-r zunit test ([#356](#356)) ([56fb9e0](56fb9e0))
* change ctags symbols browser key  binding from `ctrl-k` to `alt-Q` ([#387](#387)) ([7f6dc7d](7f6dc7d)), closes [#386](#386)
* Do not try to escape exclamation marks ([#399](#399)) ([0e55b2e](0e55b2e))
* docs workflow should fail if out-of-date ([#278](#278)) ([07cde66](07cde66))
* Don't error if $OPTS is not yet defined in .zinit-compinit call ([44765e0](44765e0))
* filter by runtime detected CPU before compiled CPU ([#304](#304)) ([a4dc13f](a4dc13f)), closes [#287](#287)
* gh-r & plugin zunit tests ([dd12fce](dd12fce))
* gh-r filters i686 (32 bit) for x86_64 ([#226](#226)) ([57f0d82](57f0d82)), closes [#225](#225)
* gh-r logic ignores [36]86 assets ([#235](#235)) ([d60638f](d60638f)), closes [#225](#225) [#246](#246) [#247](#247)
* gh-r removes linux32 assets on 64 bit OS ([1864c0b](1864c0b))
* gh-r retrieves release data GH REST API  ([#373](#373)) ([4a2a120](4a2a120)), closes [#374](#374)
* modify regex in gh-r for assets to not consider for selection ([#244](#244)) ([6ef8439](6ef8439))
* more cleaning up urls ([672ae51](672ae51)), closes [#47](#47)
* names of ctag Make target deps ([#407](#407)) ([9987d5c](9987d5c))
* package are broken again ([24f10f6](24f10f6))
* permissions for PR labeler GH action workflow ([#236](#236)) ([8a0d567](8a0d567))
* read without -r is generally bad. ([00c70a4](00c70a4))
* remove curl option "--tcp-fastopen" which is not always available ([#299](#299)) ([308c9d4](308c9d4))
* remove macOS 10.5 & 11 from test matrix ([c613193](c613193))
* remove use less line ([4f87076](4f87076))
* rename `docs` to `doc` to match doc dir ([#212](#212)) ([3a7dc95](3a7dc95))
* rm linux32 assets in aarch64/arm64 gh-r regex ([#414](#414)) ([529aa20](529aa20))
* syntax error when checking for `realpath` command  ([#259](#259)) ([05559eb](05559eb)), closes [#257](#257)
* trigger for PR labeler GH action workflow ([#237](#237)) ([49af866](49af866))
* typo & triggers in documentation workflow ([#308](#308)) ([161d7c1](161d7c1))
* unmatched "(" in windows gh-r patterns ([#280](#280)) ([1f4ba5a](1f4ba5a))
* update `zdharma` to `zdharma-continuum` ([66b1700](66b1700))
* update docs for new jq-check ([6207427](6207427))
* use [*] inside arbitrary strings. ([73a8c92](73a8c92))
* workflow pkg mgmt due to base OS changes ([195f72d](195f72d))
* ziextract execs discovery regex ([#410](#410)) ([105b38a](105b38a))
* zunit install in GH workflow ([#412](#412)) ([f4787dc](f4787dc))

### Features

* ability to set program for `zinit ls` to use ([#221](#221)) ([bad7af3](bad7af3)), closes [#170](#170)
* add `-a` (actual time) to `zinit times` cmd ([#223](#223)) ([450d3c1](450d3c1))
* add `krew` and `prebuilt-ripgrep` gh-r zunit tests ([#267](#267)) ([f25b4ae](f25b4ae))
* add compile vim from source zunit test ([#232](#232)) ([126528c](126528c))
* add configure"" ice ([#334](#334)) ([40a46c6](40a46c6))
* add GH action to remove old workflow logs ([#248](#248)) ([6647bdc](6647bdc))
* add PR labeler to show what parts of Zinit are changed ([#211](#211)) ([42e83d7](42e83d7))
* add releases via semantic-release ([73542b4](73542b4))
* add releases via semantic-release ([#415](#415)) ([cfa2f0e](cfa2f0e))
* expand linted file types to markdown and shell ([96fe03f](96fe03f))
* **git-process-output:** simplify progress-bar ([#204](#204)) ([c888917](c888917))
* update output messaging to be more informative ([047320a](047320a))
@github-actions
Copy link

github-actions bot commented Nov 7, 2022

🎉 This PR is included in version 3.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

psprint pushed a commit to psprint/.priv-zinit-fork that referenced this pull request Dec 30, 2022
@vladdoster @psprint
release: v3.7.0 → v3.8.0
0c3376d 
# [3.8.0](zdharma-continuum/zinit@v3.7.0...v3.8.0) (2022-11-07)

### Bug Fixes

* account for systems where musl is present ([zdharma-continuum#269](zdharma-continuum#269)) ([8620574](zdharma-continuum@8620574))
* alist repository for gh-r test ([zdharma-continuum#305](zdharma-continuum#305)) ([fb3c082](zdharma-continuum@fb3c082))
* allow zinit to be run from non-interactive scripts ([zdharma-continuum#227](zdharma-continuum#227)) ([c3d1bb5](zdharma-continuum@c3d1bb5)), closes [zdharma-continuum#199](zdharma-continuum#199)
* broken yaml syntax in issue template ([zdharma-continuum#355](zdharma-continuum#355)) ([f729e06](zdharma-continuum@f729e06))
* calico gh-r zunit test ([zdharma-continuum#356](zdharma-continuum#356)) ([56fb9e0](zdharma-continuum@56fb9e0))
* change ctags symbols browser key  binding from `ctrl-k` to `alt-Q` ([zdharma-continuum#387](zdharma-continuum#387)) ([7f6dc7d](zdharma-continuum@7f6dc7d)), closes [zdharma-continuum#386](zdharma-continuum#386)
* Do not try to escape exclamation marks ([zdharma-continuum#399](zdharma-continuum#399)) ([0e55b2e](zdharma-continuum@0e55b2e))
* docs workflow should fail if out-of-date ([zdharma-continuum#278](zdharma-continuum#278)) ([07cde66](zdharma-continuum@07cde66))
* Don't error if $OPTS is not yet defined in .zinit-compinit call ([44765e0](zdharma-continuum@44765e0))
* filter by runtime detected CPU before compiled CPU ([zdharma-continuum#304](zdharma-continuum#304)) ([a4dc13f](zdharma-continuum@a4dc13f)), closes [zdharma-continuum#287](zdharma-continuum#287)
* gh-r & plugin zunit tests ([dd12fce](zdharma-continuum@dd12fce))
* gh-r filters i686 (32 bit) for x86_64 ([zdharma-continuum#226](zdharma-continuum#226)) ([57f0d82](zdharma-continuum@57f0d82)), closes [zdharma-continuum#225](zdharma-continuum#225)
* gh-r logic ignores [36]86 assets ([zdharma-continuum#235](zdharma-continuum#235)) ([d60638f](zdharma-continuum@d60638f)), closes [zdharma-continuum#225](zdharma-continuum#225) [zdharma-continuum#246](zdharma-continuum#246) [zdharma-continuum#247](zdharma-continuum#247)
* gh-r removes linux32 assets on 64 bit OS ([1864c0b](zdharma-continuum@1864c0b))
* gh-r retrieves release data GH REST API  ([zdharma-continuum#373](zdharma-continuum#373)) ([4a2a120](zdharma-continuum@4a2a120)), closes [zdharma-continuum#374](zdharma-continuum#374)
* modify regex in gh-r for assets to not consider for selection ([zdharma-continuum#244](zdharma-continuum#244)) ([6ef8439](zdharma-continuum@6ef8439))
* more cleaning up urls ([672ae51](zdharma-continuum@672ae51)), closes [zdharma-continuum#47](zdharma-continuum#47)
* names of ctag Make target deps ([zdharma-continuum#407](zdharma-continuum#407)) ([9987d5c](zdharma-continuum@9987d5c))
* package are broken again ([24f10f6](zdharma-continuum@24f10f6))
* permissions for PR labeler GH action workflow ([zdharma-continuum#236](zdharma-continuum#236)) ([8a0d567](zdharma-continuum@8a0d567))
* read without -r is generally bad. ([00c70a4](zdharma-continuum@00c70a4))
* remove curl option "--tcp-fastopen" which is not always available ([zdharma-continuum#299](zdharma-continuum#299)) ([308c9d4](zdharma-continuum@308c9d4))
* remove macOS 10.5 & 11 from test matrix ([c613193](zdharma-continuum@c613193))
* remove use less line ([4f87076](zdharma-continuum@4f87076))
* rename `docs` to `doc` to match doc dir ([zdharma-continuum#212](zdharma-continuum#212)) ([3a7dc95](zdharma-continuum@3a7dc95))
* rm linux32 assets in aarch64/arm64 gh-r regex ([zdharma-continuum#414](zdharma-continuum#414)) ([529aa20](zdharma-continuum@529aa20))
* syntax error when checking for `realpath` command  ([zdharma-continuum#259](zdharma-continuum#259)) ([05559eb](zdharma-continuum@05559eb)), closes [zdharma-continuum#257](zdharma-continuum#257)
* trigger for PR labeler GH action workflow ([zdharma-continuum#237](zdharma-continuum#237)) ([49af866](zdharma-continuum@49af866))
* typo & triggers in documentation workflow ([zdharma-continuum#308](zdharma-continuum#308)) ([161d7c1](zdharma-continuum@161d7c1))
* unmatched "(" in windows gh-r patterns ([zdharma-continuum#280](zdharma-continuum#280)) ([1f4ba5a](zdharma-continuum@1f4ba5a))
* update `zdharma` to `zdharma-continuum` ([66b1700](zdharma-continuum@66b1700))
* update docs for new jq-check ([6207427](zdharma-continuum@6207427))
* use [*] inside arbitrary strings. ([73a8c92](zdharma-continuum@73a8c92))
* workflow pkg mgmt due to base OS changes ([195f72d](zdharma-continuum@195f72d))
* ziextract execs discovery regex ([zdharma-continuum#410](zdharma-continuum#410)) ([105b38a](zdharma-continuum@105b38a))
* zunit install in GH workflow ([zdharma-continuum#412](zdharma-continuum#412)) ([f4787dc](zdharma-continuum@f4787dc))

### Features

* ability to set program for `zinit ls` to use ([zdharma-continuum#221](zdharma-continuum#221)) ([bad7af3](zdharma-continuum@bad7af3)), closes [zdharma-continuum#170](zdharma-continuum#170)
* add `-a` (actual time) to `zinit times` cmd ([zdharma-continuum#223](zdharma-continuum#223)) ([450d3c1](zdharma-continuum@450d3c1))
* add `krew` and `prebuilt-ripgrep` gh-r zunit tests ([zdharma-continuum#267](zdharma-continuum#267)) ([f25b4ae](zdharma-continuum@f25b4ae))
* add compile vim from source zunit test ([zdharma-continuum#232](zdharma-continuum#232)) ([126528c](zdharma-continuum@126528c))
* add configure"" ice ([zdharma-continuum#334](zdharma-continuum#334)) ([40a46c6](zdharma-continuum@40a46c6))
* add GH action to remove old workflow logs ([zdharma-continuum#248](zdharma-continuum#248)) ([6647bdc](zdharma-continuum@6647bdc))
* add PR labeler to show what parts of Zinit are changed ([zdharma-continuum#211](zdharma-continuum#211)) ([42e83d7](zdharma-continuum@42e83d7))
* add releases via semantic-release ([73542b4](zdharma-continuum@73542b4))
* add releases via semantic-release ([zdharma-continuum#415](zdharma-continuum#415)) ([cfa2f0e](zdharma-continuum@cfa2f0e))
* expand linted file types to markdown and shell ([96fe03f](zdharma-continuum@96fe03f))
* **git-process-output:** simplify progress-bar ([zdharma-continuum#204](zdharma-continuum#204)) ([c888917](zdharma-continuum@c888917))
* update output messaging to be more informative ([047320a](zdharma-continuum@047320a))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jankatins jankatins jankatins left review comments

Assignees
No one assigned
Labels
ci/cd released scripts tests zinit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🐞 bug: gh-r for volta-cli/volta repo on macOS picks the Windows MSI installer artifact
2 participants
@vladdoster @jankatins