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

[new port] gtk 3.24.34 port #22201

Closed
wants to merge 3 commits into from
Closed

[new port] gtk 3.24.34 port #22201

wants to merge 3 commits into from

Conversation

mkhon
Copy link
Contributor

@mkhon mkhon commented Dec 25, 2021

New port: gtk 3.24.34 (some software still requires GTK3)

  • What does your PR fix?

  • Which triplets are supported/not supported? Have you updated the CI baseline?

all

Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

@nlogozzo
Copy link
Contributor

Could we see a gtkmm3 port as well?

@mkhon
Copy link
Contributor Author

mkhon commented Dec 26, 2021

Sure, I will work on gtk3mm port

@nlogozzo
Copy link
Contributor

Cool, thanks :)

@JonLiu1993 JonLiu1993 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Dec 27, 2021
@JonLiu1993 JonLiu1993 changed the title gtk 3.24.30 port [gtk] add new port Dec 27, 2021
@JonLiu1993 JonLiu1993 changed the title [gtk] add new port [gtk] add new port gtk 3.24.30 Dec 27, 2021
@JonLiu1993 JonLiu1993 changed the title [gtk] add new port gtk 3.24.30 [gtk3] add new port Dec 27, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for gtk3 but no changes to version or port version.
-- Version: 3.24.30
-- Old SHA: 8fcadeff3c2eba6e6cd20b1158014c8f308dea54
-- New SHA: 4fb3acb0474e205624b3a2b4c1f02a45468be19c
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@JonLiu1993
Copy link
Member

@mkhon ,Thanks for your pr, for the x86-windows error, you can run command "./vcpkg x-add-version gtk3 --overwrite-version " and commit again after you finish your pr.

Found the following errors:
##[error]versions/g-/gtk3.json(2,): error : While reading versions for port gtk3 from file: version/g-/gtk3.json
File declares version `3.24.30` with SHA: 8fcadeff3c2eba6e6cd20b1158014c8f308dea54
But local port with the same version has a different SHA: 4fb3acb0474e205624b3a2b4c1f02a45468be19c
Please update the port's version fields and then run:

    vcpkg x-add-version gtk3
    git add versions
    git commit -m "Update version database"

to add a new version.

To attempt to resolve all errors at once, run:

    vcpkg x-add-version --all

##[error]PowerShell exited with code '1'.

and This is the detail error log please take a look:
https://dev.azure.com/vcpkg/public/_build/results?buildId=64967&view=artifacts&pathAsName=false&type=publishedArtifacts

Capture

@JonLiu1993
Copy link
Member

@JonLiu1993
Copy link
Member

@mkhon ,It seems that the error has not been solved yet, here is the detailed error log
https://dev.azure.com/vcpkg/public/_build/results?buildId=65097&view=artifacts&pathAsName=false&type=publishedArtifacts

@JonLiu1993 JonLiu1993 added the requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function label Jan 4, 2022
@JonLiu1993
Copy link
Member

@mkhon ,I encountered this error when verifying the feature, it seems that port pango does not have an introspection feature
./vcpkg install gtk3[*]:x64-windows

Package pango does not have a introspection feature

@mkhon
Copy link
Contributor Author

mkhon commented Jan 5, 2022

@JonLiu1993 introspection feature is added in #22200

@JonLiu1993 JonLiu1993 added depends:different-pr This PR or Issue depends on a PR which has been filed and removed requires:author-response labels Jan 5, 2022
@JonLiu1993
Copy link
Member

Depends on pr #22200

@mkhon mkhon changed the title [gtk3] add new port [new port] gtk 3.24.30 port Jan 6, 2022
@talregev
Copy link
Contributor

talregev commented Jun 5, 2022

I look again on wxwidgets and I faild to change that it take glib from vcpkg. I think wxwidgets take the correct gtk3 (from vcpkg). and gtk3 doesn't publish the correct glib. (it publish system glib).

~/vcpkg/installed/x64-linux/lib/pkgconfig
> pkg-config gtk+-3.0.pc --libs
-L./../../lib -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -latk-1.0 -latk-bridge-2.0 -lepoxy -lfribidi -lpangoft2-1.0 -lpango-1.0 -lharfbuzz -lfontconfig -lfreetype -lgio-2.0 -lgobject-2.0 -lglib-2.0

Can you change that it will take from vcpkg and not from system?

Edit: I think the problem is on wxwidgets that I failed to fix.

I fix the gkt ld error I got. It was bug in our end (boinc opensource project).
I fix it on this PR:
BOINC/boinc#4756

ports/gtk3/0001-build.patch Show resolved Hide resolved
Comment on lines +1 to +7
set(warning_length 24)
string(LENGTH "${CURRENT_BUILDTREES_DIR}" buildtrees_path_length)
if(buildtrees_path_length GREATER warning_length AND CMAKE_HOST_WIN32)
message(WARNING "${PORT}'s buildsystem uses very long paths and may fail on your system.\n"
"We recommend moving vcpkg to a short path such as 'C:\\vcpkg' or using the subst command."
)
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
set(warning_length 24)
string(LENGTH "${CURRENT_BUILDTREES_DIR}" buildtrees_path_length)
if(buildtrees_path_length GREATER warning_length AND CMAKE_HOST_WIN32)
message(WARNING "${PORT}'s buildsystem uses very long paths and may fail on your system.\n"
"We recommend moving vcpkg to a short path such as 'C:\\vcpkg' or using the subst command."
)
endif()
vcpkg_buildpath_length_warning(24)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vcpkg_buildpath_length_warning() can not be used here as it recommends to move to C:\src\vcpkg which gives buildpath length >24

Copy link
Contributor

Choose a reason for hiding this comment

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

So can you confirm a longest valid path to replace this code?

Copy link

@stevapple stevapple Aug 22, 2022

Choose a reason for hiding this comment

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

I tried subst and it seems not solving the problem. Moving to the root of a drive may be the only choice.

I would also suggest we make it clear that the path length cannot exceed 24 (if that’s the exact number). This message could fail to inform the user of what kind of paths are acceptable.

@mkhon
Copy link
Contributor Author

mkhon commented Jun 7, 2022

@BillyONeal @dg0yt @mkhon There is a PR of dubs waiting, that why they will not rush and install dbus on the ci. That will hold at-spi2-atk port. The PR of dbus is open from January, and likely to be merge to the master in more then a few weeks. So the fastest way this PR can be merge to the master is to go with @dg0yt suggestion. Please:

  • Revert the at-spi2-atk bridge patch. The patch that cancel the use of the bridge in gtk3 and can compile again on linux.
  • Remove the skipped linux in the ci. Let the ci compile linux with success (after the patch).
  • Update with ./vcpkg x-add-version --all --overwrite-version

Then we can ask to review this PR again. hopefully it can be merge that way. In the meanwhile keep the at-spi2-atk PR. When dbus will be ready, we can merge it too, and fix gtk3 PR again, in separate PR.

I would like to hear from the maintainers, as we had this patch before and PR got rejected because of this.
I haven't heard from maintainers since then

@JackBoosY
Copy link
Contributor

I'd like to wait for #24934 to add port at-spi2-atk first.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jun 8, 2022
@talregev
Copy link
Contributor

talregev commented Jun 8, 2022

I'd like to wait for #24934 to add port at-spi2-atk first.

I would like that too! But vcpkg don't install dubs on the ci for that. 😢

github-actions[bot]
github-actions bot previously approved these changes Jun 9, 2022
Comment on lines +1 to +7
set(warning_length 24)
string(LENGTH "${CURRENT_BUILDTREES_DIR}" buildtrees_path_length)
if(buildtrees_path_length GREATER warning_length AND CMAKE_HOST_WIN32)
message(WARNING "${PORT}'s buildsystem uses very long paths and may fail on your system.\n"
"We recommend moving vcpkg to a short path such as 'C:\\vcpkg' or using the subst command."
)
endif()
Copy link

@stevapple stevapple Aug 22, 2022

Choose a reason for hiding this comment

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

I tried subst and it seems not solving the problem. Moving to the root of a drive may be the only choice.

I would also suggest we make it clear that the path length cannot exceed 24 (if that’s the exact number). This message could fail to inform the user of what kind of paths are acceptable.

@JonLiu1993
Copy link
Member

Ping @mkhon for response.

@JonLiu1993
Copy link
Member

Convert this PR to draft since there is no progress. Please ping us if this PR is ready for review again.

@JonLiu1993 JonLiu1993 marked this pull request as draft September 9, 2022 07:50
@JackBoosY
Copy link
Contributor

Waiting for #26766 merge first.
Before that, I think we should close this PR temporary. Then please ping me to reopen it.

@JackBoosY JackBoosY closed this Sep 16, 2022
@talregev
Copy link
Contributor

Better that he will open this PR on his mkhon user.

@mkhon mkhon deleted the feature/gtk3 branch September 28, 2022 19:04
@mkhon
Copy link
Contributor Author

mkhon commented Sep 28, 2022

I opened a new PR #24136 for this

@talregev
Copy link
Contributor

#24136 this is my PR. I can open it.

@talregev
Copy link
Contributor

I think you meant this PR #27026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! depends:different-pr This PR or Issue depends on a PR which has been filed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[gtk] Allow selection of GTK3 in addition to GTK4