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

libgit2: update to 1.2.0. #33535

Closed
wants to merge 18 commits into from
Closed

libgit2: update to 1.2.0. #33535

wants to merge 18 commits into from

Conversation

g4s8
Copy link
Contributor

@g4s8 g4s8 commented Oct 14, 2021

Updated libgit2 to v1.2.0, added patch with PR
libgit2/libgit2#6032 of
git_remote_name_is_valid fix for libgit2/git2go#834 issue.

General

Have the results of the proposed changes been tested?

  • I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me: checked with tests of https://github.com/libgit2/git2go (libgit2 is a dependency there)
  • I generally don't use the affected packages but briefly tested this PR

Ref: #28456
Maintainer: @q66

@q66
Copy link
Contributor

q66 commented Oct 14, 2021

rebase your branch, and revbump things that are affected by the changed soname, that's the bare minimum needed to get a review

@g4s8 g4s8 force-pushed the libgit2-1.2.0 branch 2 times, most recently from 98e4264 to 70a3ec9 Compare October 15, 2021 07:16
@g4s8
Copy link
Contributor Author

g4s8 commented Oct 15, 2021

rebase your branch, and revbump things that are affected by the changed soname, that's the bare minimum needed to get a review

@q66 done.

BTW, you can configure branch-protection rules in GitHub to require PR branch to be up to date with master; then new contributors will know that at the time they submit a PR, without waiting CI approval from maintainer.

Copy link
Member

@ericonr ericonr left a comment

Choose a reason for hiding this comment

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

Supersedes #28456

version=1.0.1
revision=3
version=1.2.0
revision=4
Copy link
Member

Choose a reason for hiding this comment

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

revision should be 1

revision=1
revision=4
Copy link
Member

Choose a reason for hiding this comment

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

Should be 2

common/shlibs Outdated
@@ -1315,7 +1315,7 @@ libunwind-ppc64.so.8 libunwind-1.5.0_3
libunwind-setjmp.so.0 libunwind-1.5.0_3
libmicrohttpd.so.12 libmicrohttpd-0.9.73_1
libmicrodns.so.1 libmicrodns-0.2.0_1
libgit2.so.1.0 libgit2-1.0.1_3
libgit2.so.1.2 libgit2-1.2.0_4
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
libgit2.so.1.2 libgit2-1.2.0_4
libgit2.so.1.2 libgit2-1.2.0_1

@ericonr
Copy link
Member

ericonr commented Oct 15, 2021

                 from PluginEntry.h:32,
                 from PluginEntry.c:28:
PrettyPrinter.h:55:22: error: two or more data types in declaration specifiers
   55 | typedef unsigned int bool;
      |                      ^~~~

geany-plugins is broken, needs to be patched probably.

BTW, you can configure branch-protection rules in GitHub to require PR branch to be up to date with master; then new contributors will know that at the time they submit a PR, without waiting CI approval from maintainer.

Being slightly out of date is fine, can GH do that?

@q66
Copy link
Contributor

q66 commented Oct 15, 2021

you should test-build every revbumped package (with check phase enabled) and make sure it works/does not regress over the previous state (also ensure lint passes)

also apply @ericonr's suggestions

@g4s8
Copy link
Contributor Author

g4s8 commented Oct 15, 2021

I tried to build geany-plugins on clean workspace, and it's failing with same error: #33568

@q66
Copy link
Contributor

q66 commented Oct 15, 2021

this should be easy to patch, feel free to raise another PR for it (no need to revbump in that one)

@g4s8
Copy link
Contributor Author

g4s8 commented Oct 15, 2021

this should be easy to patch, feel free to raise another PR for it (no need to revbump in that one)

Fixed in #33573

@g4s8
Copy link
Contributor Author

g4s8 commented Oct 19, 2021

@ericonr @q66 I tried to run same tests for julia in master with ./xbps-src check julia and got the same errors about Unicode tests:

Error in testset Unicode:
Error During Test at /builddir/julia-1.6.1/usr/share/julia/stdlib/v1.6/Unicode/test/runtests.jl:337
  Test threw exception
  Expression: collect(graphemes("🤦🏼\u200d♂️")) == ["🤦🏼\u200d♂️"]
  ArgumentError: destination has fewer elements than required
  Stacktrace:
   [1] copyto!(dest::Vector{SubString{String}}, src::Base.Unicode.GraphemeIterator{String})
     @ Base ./abstractarray.jl:844
   [2] _collect
     @ ./array.jl:608 [inlined]
   [3] collect(itr::Base.Unicode.GraphemeIterator{String})
     @ Base ./array.jl:602
   [4] macro expansion
     @ /builddir/julia-1.6.1/usr/share/julia/stdlib/v1.6/Unicode/test/runtests.jl:337 [inlined]
   [5] macro expansion
     @ /builddir/julia-1.6.1/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
   [6] top-level scope
     @ /builddir/julia-1.6.1/usr/share/julia/stdlib/v1.6/Unicode/test/runtests.jl:337
Error in testset Unicode:
Error During Test at /builddir/julia-1.6.1/usr/share/julia/stdlib/v1.6/Unicode/test/runtests.jl:338
  Test threw exception
  Expression: collect(graphemes("👨🏻\u200d🤝\u200d👨🏽")) == ["👨🏻\u200d🤝\u200d👨🏽"]
  ArgumentError: destination has fewer elements than required
  Stacktrace:
   [1] copyto!(dest::Vector{SubString{String}}, src::Base.Unicode.GraphemeIterator{String})
     @ Base ./abstractarray.jl:844
   [2] _collect
     @ ./array.jl:608 [inlined]
   [3] collect(itr::Base.Unicode.GraphemeIterator{String})
     @ Base ./array.jl:602
   [4] macro expansion
     @ /builddir/julia-1.6.1/usr/share/julia/stdlib/v1.6/Unicode/test/runtests.jl:338 [inlined]
   [5] macro expansion
     @ /builddir/julia-1.6.1/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
   [6] top-level scope
     @ /builddir/julia-1.6.1/usr/share/julia/stdlib/v1.6/Unicode/test/runtests.jl:337
Error in testset Unicode:
Test Failed at /builddir/julia-1.6.1/usr/share/julia/stdlib/v1.6/Unicode/test/runtests.jl:339
  Expression: collect(graphemes("🇸🇪🇸🇪")) == ["🇸🇪", "🇸🇪"]
   Evaluated: SubString{String}["🇸🇪🇸", "🇪"] == ["🇸🇪", "🇸🇪"]
Error in testset nghttp2_jll:
Test Failed at /builddir/julia-1.6.1/usr/share/julia/stdlib/v1.6/nghttp2_jll/test/runtests.jl:14
  Expression: VersionNumber(unsafe_string(info.version_str)) == v"1.41.0"
   Evaluated: v"1.45.1" == v"1.41.0"
ERROR: LoadError: Test run finished with errors
in expression starting at /builddir/julia-1.6.1/test/runtests.jl:84
make[1]: *** [Makefile:25: all] Error 1
make: *** [Makefile:536: testall] Error 2
=> ERROR: julia-1.6.1_1: do_check: '${make_cmd} ${make_check_args} ${make_check_target}' exited with 2
=> ERROR:   in do_check() at common/build-style/gnu-makefile.sh:33

I'm not sure how to fix it.

@q66
Copy link
Contributor

q66 commented Oct 19, 2021

don't worry about julia, this is clearly not caused by libgit2, so just put in a make_check=no plus a comment explaining the reason

@g4s8
Copy link
Contributor Author

g4s8 commented Nov 17, 2021

@q66 could you please help here? I'm not sure why CI is failing: locally these tests passes fine:

./xbps-src check gnome-builder
./xbps-src install gnome-builder
./xbps-src check libgit2-glib
./xbps-src install libgit2-glib

What is wrong here?

@q66
Copy link
Contributor

q66 commented Nov 17, 2021

you forgot to revbump gnome-builder?

@github-actions
Copy link

github-actions bot commented Jun 7, 2022

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

@github-actions github-actions bot added the Stale label Jun 7, 2022
@github-actions github-actions bot closed this Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants