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

[qt5-base]Add a print message to inform the user to install the dependency package. #6983

Merged
merged 6 commits into from
Jul 17, 2019
Merged

[qt5-base]Add a print message to inform the user to install the dependency package. #6983

merged 6 commits into from
Jul 17, 2019

Conversation

JackBoosY
Copy link
Contributor

@JackBoosY JackBoosY commented Jun 21, 2019

When qt5 is installed in Linux, we need two packages:

  • libgl1-mesa-dev
  • libglu1-mesa-dev

And users may not have installed them, so I added a print message to inform to install them.

@JackBoosY JackBoosY added the info:internal This PR or Issue was filed by the vcpkg team. label Jun 21, 2019
@JackBoosY JackBoosY marked this pull request as ready for review June 21, 2019 09:32
@TheScarfix
Copy link
Contributor

not every disto uses apt though. is there a way to not exclude RPM and other package managers?

@cenit
Copy link
Contributor

cenit commented Jun 21, 2019

Usually we just write a message to the user.
I also don't like this approach, because it's messing the system outside the vcpkg folder ;)

@Rastaban
Copy link
Contributor

Switch to just printing out a message. If mesa is required on linux then you could check if it is installed and then print an error message that has the apt command.

@MVoz
Copy link
Contributor

MVoz commented Jun 21, 2019

cmake is it will run in interactive mode?
sudo means entering a password

Yes, and many it just does not install, due to the many dependencies

I think it is necessary to do Mesa port it is more reasonable option

@JackBoosY JackBoosY changed the title [qt5-base]Add the command to install dependencies libgl1-mesa-dev and libglu1-mesa-dev in linux [qt5-base]Add a print message to inform the user to install the dependency package. Jun 24, 2019
@TheScarfix
Copy link
Contributor

I would like a change from apt-get to "the package manager of your distribution" or something like that :)

@JackBoosY
Copy link
Contributor Author

/azp run

@JackBoosY
Copy link
Contributor Author

@Rastaban PR-Eager error is due to download source failed, I just added two lines of message.

@MVoz
Copy link
Contributor

MVoz commented Jun 26, 2019

@JackBoosY message(STATUS ? or warning

@JackBoosY
Copy link
Contributor Author

@voskrese Using status to notify users is not a good idea because it may be confused with other status information.
And I am considering whether I can add a new option to highlight these tips.

@MVoz
Copy link
Contributor

MVoz commented Jun 26, 2019

color ?

example

if(VCPKG_CMAKE_SYSTEM_NAME STREQUAL Linux OR Darwin)
if(NOT WIN32)
     string(ASCII 27 ESCAPE)
     set(BoldRed "${ESCAPE}[31;2m")
     set(ColourReset "${ESCAPE}[0m")
endif()
     message(FATAL_ERROR "${BoldRed} --== ONLY WINDOWS OS ==-- ${ColourReset}")
endif(VCPKG_CMAKE_SYSTEM_NAME STREQUAL Linux OR Darwin)

@JackBoosY
Copy link
Contributor Author

@voskrese Such as: message(USER_WARNING "something needs to notify user")

@heydojo
Copy link
Contributor

heydojo commented Jun 28, 2019

Why not proc for the existence of the header file(s) the build requires?

The variable INCLUDEPATH usually holds the includes on Linux systems AFAIK and something more like :

    if(EXISTS ${INCLUDEPATH}/mesa/someheader.h)
        message(STATUS "Found required external dependency someheader.h")
    else()
        message(FATAL_ERROR "Please install the required packages containing someheader.h using your distribution's software manager. On Debian Linux the packages are named: libgl1-mesa-dev and libglu1-mesa-dev")
    endif()

I will add too that Fedora, OpenSuse, Manjaro, Mint, Arch and Alpine likely won't all share the same names for these packages. Which creates another problem.

@JackBoosY
Copy link
Contributor Author

JackBoosY commented Jun 28, 2019

@heydojo Sorry, I‘m not sure which header file should be used to check the packages. Can you provide them?

@heydojo
Copy link
Contributor

heydojo commented Jul 15, 2019

@heydojo Sorry, I‘m not sure which header file should be used to check the packages. Can you provide them?

Dependency resolution for things like this is one of cmake's strong points.
As vcpkg is (in it's most basic of definitions) a layer on top of cmake, I would assume that you are familiar enough with cmake to check for the required headers?

When looking for mesa GLU:
https://apps.fedoraproject.org/packages/s/mesa
https://apps.fedoraproject.org/packages/mesa-libGLU-devel
https://koji.fedoraproject.org/koji/rpminfo?rpmID=16484448
https://koji.fedoraproject.org/koji/rpminfo?rpmID=16484445

Looks like you are looking for the file /usr/include/GL/glu.h
found inside the package mesa-libGLU-devel in Fedora (and likely RHEL & CentOS too.)

Other Linux variants you would need to check yourself. I don't use or recommend any others.
Hopefully this info is useful.

@Rastaban
Copy link
Contributor

This is consistent with how other packages handle external dependencies on Linux so I am okay with the current state of this PR. Adding the check for glu.h and turning the message into a fatal error if not found would be a good improvement though.

JackBoosY added 2 commits July 16, 2019 20:05
@Rastaban Rastaban merged commit 8e17f07 into microsoft:master Jul 17, 2019
@JackBoosY JackBoosY deleted the dev/jack/qt5_base_add_dep_package branch July 18, 2019 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants