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

[Part1|xwindow PR] Split up to dbus #22642

Merged
merged 51 commits into from
Aug 22, 2022

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Jan 19, 2022

scaled down the patches a bit compared to #21584.

@Neumann-A Neumann-A mentioned this pull request Jan 19, 2022
1 task
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

Commenting with review so far (only got to xcb/portfile.cmake)

ports/dbus/cmake.dep.patch Outdated Show resolved Hide resolved
- set(PLATFORM_LIBS pthread ${LIBRT})
- if(PKG_CONFIG_FOUND)
+ if(NOT WIN32)
+ set(PLATFORM_LIBS pthread ${LIBRT})
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this interrogate the results of find_package(Threads) to determine if pthread is required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a bit hard to parse:

  add_library(Threads::Threads INTERFACE IMPORTED)

  if(THREADS_HAVE_PTHREAD_ARG)
    set_property(TARGET Threads::Threads
                 PROPERTY INTERFACE_COMPILE_OPTIONS "$<$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>:SHELL:-Xcompiler -pthread>"
                                                    "$<$<NOT:$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>>:-pthread>")
  endif()

  if(CMAKE_THREAD_LIBS_INIT)
    set_property(TARGET Threads::Threads PROPERTY INTERFACE_LINK_LIBRARIES "${CMAKE_THREAD_LIBS_INIT}")
  endif()

I would rather say upstream has to come up with the correct logic to make it work for all cases. I just fix the windows case which normally does not install pc files.

ports/xau/portfile.cmake Outdated Show resolved Hide resolved
ports/xau/vcpkg.json Outdated Show resolved Hide resolved
ports/xcb-proto/portfile.cmake Outdated Show resolved Hide resolved
ports/xcb/portfile.cmake Outdated Show resolved Hide resolved
@JackBoosY JackBoosY added category:new-port The issue is requesting a new library to be added; consider making a PR! requires:author-response labels Jan 20, 2022
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 xorg-macros but no changes to version or port version.
-- Version: 1.19.3
-- Old SHA: 9eb2d00e214b6e133dc407e47ca1f6053715aee2
-- New SHA: 9cb231d0ac80bdf86f3c8265fd804406ce0d6f4a
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for xproto but no changes to version or port version.
-- Version: 2021.5
-- Old SHA: 698873d8613a285fef3bf6ef8f5f78c39865289d
-- New SHA: 0240964997bc22023864ac26034a8efad1998275
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for xcb-proto but no changes to version or port version.
-- Version: 1.14.1
-- Old SHA: 3da8b0d6b7c5a6a6281cabf1d1e7062c3365c42b
-- New SHA: ec8f478736cb1ae7d47bd420b5e2c8aff0a05419
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for dbus but no changes to version or port version.
-- Version: 1.13.18
-- Old SHA: 621c4aad736c2fbfca9acf9604d4eb20bdbb9f62
-- New SHA: 4cc1454a1dabb5d4f248cbbe4b3896feb8463cc8
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for xtrans but no changes to version or port version.
-- Version: 1.4.0
-- Old SHA: a068cead3f502402db1dfee9e7e2abe4470ae3fd
-- New SHA: c595faed191fcfa0e11a2e74a11aef51663f25cf
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for xlib but no changes to version or port version.
-- Version: 1.7.3.1
-- Old SHA: eb5dddadda5b4d8ad971f74eafd21f3ab4bb0715
-- New SHA: a8909b0df9c9e8cb2f6ed7161a4e26bbdd831912
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for xau but no changes to version or port version.
-- Version: 1.0.9
-- Old SHA: c0b5acaabcaa8ba6ccedaf4b1a72a0341d5d5b85
-- New SHA: cd068ed347c0a24a6e5f95144d600fcbad984888
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for xdmcp but no changes to version or port version.
-- Version: 1.1.3
-- Old SHA: dbdfe30e1306531a3e8ae06aa23af229ede1ecac
-- New SHA: b7c0329ad3cdfbb5061d46130e5e80771b89a466
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for xcb but no changes to version or port version.
-- Version: 1.14
-- Old SHA: 8578b871d0ef7ae7da4f541a426cbb6eabb74d5a
-- New SHA: 335348f7134942bd9e6684e6953742e539a0bcdb
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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.

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/dbus/vcpkg.json
  • ports/pthread-stubs/vcpkg.json
  • ports/x11/vcpkg.json
  • ports/xau/vcpkg.json
  • ports/xcb-proto/vcpkg.json
  • ports/xcb-util-m4/vcpkg.json
  • ports/xcb/vcpkg.json
  • ports/xdmcp/vcpkg.json
  • ports/xlib/vcpkg.json
  • ports/xorg-macros/vcpkg.json
  • ports/xproto/vcpkg.json
  • ports/xtrans/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

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.

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/dbus/vcpkg.json
  • ports/pthread-stubs/vcpkg.json
  • ports/x11/vcpkg.json
  • ports/xau/vcpkg.json
  • ports/xcb-proto/vcpkg.json
  • ports/xcb-util-m4/vcpkg.json
  • ports/xcb/vcpkg.json
  • ports/xdmcp/vcpkg.json
  • ports/xlib/vcpkg.json
  • ports/xorg-macros/vcpkg.json
  • ports/xproto/vcpkg.json
  • ports/xtrans/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

@BillyONeal
Copy link
Member

@ras0219-msft, @JavierMatosD, @dan-shaw, @vicroms, and I met today and polled the following questions:

Do we want to align with Repology names?

We agree that putting lib in front is unfortunate but do want to align with repology.

Do we care / have opinions on what should happen with libx11 / x11 / libx.

We do not think we should have an x11 meta port right now. Unlike boost or qt, which ship as hermetic releases by their respective upstreams, libxXxx does not do that, and we haven't demonstrated a need for such a thing right now. (This may change following the other PRs adding other X stuff but we aren't making future statements about it right now)

=====

We agree that licenses should be 'null' if there is any doubt.

@BillyONeal BillyONeal added requires:author-response and removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Aug 11, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 11, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 11, 2022
@Neumann-A
Copy link
Contributor Author

We agree that putting lib in front is unfortunate but do want to align with repology.

done for the ports you pointed out (xau x11 xdmcp)

We agree that licenses should be 'null' if there is any doubt.

done

We do not think we should have an x11 meta port right now. Unlike boost or qt, which ship as hermetic releases by their respective upstreams, libxXxx does not do that, and we haven't demonstrated a need for such a thing right now. (This may change following the other PRs adding other X stuff but we aren't making future statements about it right now)

renamed xlib to libx11 since xlib is the old name and libx11 is the repology name. Merged the wrapper from x11 into libx11.

scripts/ci.baseline.txt Outdated Show resolved Hide resolved
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Aug 22, 2022
@BillyONeal
Copy link
Member

I would still like to see the ci.baseline.txt => "supports" but I'm not willing to block merging this over it.

@JavierMatosD
Copy link
Contributor

Thanks!

@JavierMatosD JavierMatosD merged commit 552f1ee into microsoft:master Aug 22, 2022
@Neumann-A Neumann-A deleted the xwindows_to_dbus branch August 22, 2022 19:43
@JackBoosY
Copy link
Contributor

When building libx11:x86-windows:

/bin/sh: line 1: ../src/util/makekeys: No such file or directory
make[1]: *** [Makefile:2094: ks_tables.h] Error 127
make: *** [Makefile:525: all-recursive] Error 1

out.log:

Making all in src
make[1]: Entering directory '/f/vcpkg/buildtrees/libx11/x86-windows-dbg/src'
Makefile:2098: update target '../src/util/makekeys' due to: .././../src/8b71282530-fd21532563.clean/src/../src/util/makekeys.c
cd util && /usr/bin/make
Makefile:857: update target 'config.h' due to: stamp-h1
test -f config.h || rm -f stamp-h1
test -f config.h || /usr/bin/make  stamp-h1
make[2]: Entering directory '/f/vcpkg/buildtrees/libx11/x86-windows-dbg/src/util'
Makefile:441: update target 'makekeys.obj' due to: ../.././../src/8b71282530-fd21532563.clean/src/util/makekeys.c
source='../.././../src/8b71282530-fd21532563.clean/src/util/makekeys.c' object='makekeys.obj' libtool=no \
DEPDIR=.deps depmode=msvc7 /bin/sh ../.././../src/8b71282530-fd21532563.clean/depcomp \
touch a.out | touch conftest.exe | true -DHAVE_CONFIG_H -I. -I../.././../src/8b71282530-fd21532563.clean/src/util -I../../src -I../../include/X11  -I../.././../src/8b71282530-fd21532563.clean/include  -Wall -fd  -c -o makekeys.obj `cygpath -w '../.././../src/8b71282530-fd21532563.clean/src/util/makekeys.c'`
Makefile:414: update target 'makekeys' due to: makekeys.obj
rm -f makekeys
touch a.out | touch conftest.exe | true -Wall -fd    -o makekeys makekeys.obj  
make[2]: Leaving directory '/f/vcpkg/buildtrees/libx11/x86-windows-dbg/src/util'
Makefile:2094: update target 'ks_tables.h' due to: F:/vcpkg/installed/x86-windows/debug/../include/X11/keysymdef.h F:/vcpkg/installed/x86-windows/debug/../include/X11/XF86keysym.h F:/vcpkg/installed/x86-windows/debug/../include/X11/Sunkeysym.h F:/vcpkg/installed/x86-windows/debug/../include/X11/DECkeysym.h F:/vcpkg/installed/x86-windows/debug/../include/X11/HPkeysym.h ../src/util/makekeys
../src/util/makekeys F:/vcpkg/installed/x86-windows/debug/../include/X11/keysymdef.h F:/vcpkg/installed/x86-windows/debug/../include/X11/XF86keysym.h F:/vcpkg/installed/x86-windows/debug/../include/X11/Sunkeysym.h F:/vcpkg/installed/x86-windows/debug/../include/X11/DECkeysym.h F:/vcpkg/installed/x86-windows/debug/../include/X11/HPkeysym.h > ks_tables_h
make[1]: Leaving directory '/f/vcpkg/buildtrees/libx11/x86-windows-dbg/src'

@Neumann-A
Copy link
Contributor Author

already fixed in #26486
side effect due to #25009 deactivating cross builds.

@autoantwort
Copy link
Contributor

session.conf.txt
system.conf.txt
@Neumann-A What should I do with the absolute paths?

@autoantwort
Copy link
Contributor

Should I simply remove these files?

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! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.