Skip to content

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

Merged
JavierMatosD merged 51 commits intomicrosoft:masterfrom
Neumann-A:xwindows_to_dbus
Aug 22, 2022
Merged

[Part1|xwindow PR] Split up to dbus#22642
JavierMatosD merged 51 commits intomicrosoft:masterfrom
Neumann-A:xwindows_to_dbus

Conversation

@Neumann-A
Copy link
Copy Markdown
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
Copy Markdown
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)

- set(PLATFORM_LIBS pthread ${LIBRT})
- if(PKG_CONFIG_FOUND)
+ if(NOT WIN32)
+ set(PLATFORM_LIBS pthread ${LIBRT})
Copy link
Copy Markdown
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
Copy Markdown
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.

@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
Copy Markdown

@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
Copy Markdown

@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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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.

@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Aug 22, 2022
@BillyONeal
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

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

@autoantwort
Copy link
Copy Markdown
Contributor

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

@autoantwort
Copy link
Copy Markdown
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:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.