Skip to content

[libqcow] Add new port#6852

Closed
AlexAltea wants to merge 1 commit intomicrosoft:masterfrom
AlexAltea:master
Closed

[libqcow] Add new port#6852
AlexAltea wants to merge 1 commit intomicrosoft:masterfrom
AlexAltea:master

Conversation

@AlexAltea
Copy link
Contributor

This port is very similar to the libpff port added in #6458, which makes sense since both libraries are part of the same libyal collection (all written by the same author).

@ras0219-msft
Copy link
Contributor

/azp run

@microsoft microsoft deleted a comment from azure-pipelines bot Aug 16, 2019
@grdowns
Copy link
Contributor

grdowns commented Aug 16, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@AlexAltea
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 6852 in repo microsoft/vcpkg

@AlexAltea
Copy link
Contributor Author

I don't see what's wrong with this PR, and unfortunately the vpckg-{linux,windows,osx}-PR checks don't contain any information anymore.

@AlexAltea
Copy link
Contributor Author

@ras0219-msft I've fixed the Linux builds.

Windows builds are fine, aside from UWP ones, which fail since one dependency (zlib) cannot be build under x86-uwp or x64-uwp. Is there anything else to do?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you usevcpkg_from_github() to replace vcpkg_download_distfile and vcpkg_extract_source_archive_ex() functions?
Also, I noticed that there is another latest release version 20191221.
So would you like to update this to the latest version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@NancyLi1013 Unfortunately we need the use releases published at https://github.com/libyal/libqcow/releases since they contain pre-configured files requires to build libqcow on Windows. Although it should be theoretically possible to do that step ourselves, I haven't figured out a sane and easy way to do so.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you don't understand my sense correctly.

vcpkg_from_github(
    OUT_SOURCE_PATH SOURCE_PATH 
    REPO libyal/libqcow
    REF  20191221
    SHA512 de0c5cfad84bbccc9a4144b108c7e022a98d130e829385e69ff00a8750709c9de814410eebfa1c0fc89051cf8f596d87b9bbc8228d99efd8be1c3efdc2b52730
)

This can use the same version as that you provided above.
So I don't know why we cannot update it?

Copy link
Contributor

Choose a reason for hiding this comment

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

@NancyLi1013 The code you provided downloads the repo as a .zip, not the release that is published here: https://github.com/libyal/libvmdk/releases

Copy link
Contributor

Choose a reason for hiding this comment

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

They should have the same function. Please refer to this doc.

Copy link
Contributor

@VelocityRa VelocityRa Apr 26, 2020

Choose a reason for hiding this comment

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

Hi, I did see the doc and I tried using your code.
They do not have the same function, different files are downloaded.

Nowhere in vcpkg_from_github.cmake is /relases/ mentioned (which is what our URL above contains).

It downloads the source tree as-is (search for /archive/ in the code) instead of the published release that the author manually published, which in this case is different than the source tree.

See the releases's files for an example (using libvmdk here, but same for libqcow):

image

We are interested in the first file, not the third one which vcpkg_from_github downloads (the /archive/ one).

Edit: Same case for

# Don't change to vcpkg_from_github! The sources in the git repository (archives) are missing some files that are distributed inside releases.
# More info: https://github.com/nigels-com/glew/issues/31 and https://github.com/nigels-com/glew/issues/13
vcpkg_download_distfile(ARCHIVE
URLS "https://github.com/nigels-com/glew/releases/download/glew-2.1.0/glew-2.1.0.tgz"
FILENAME "glew-2.1.0.tgz"
SHA512 9a9b4d81482ccaac4b476c34ed537585ae754a82ebb51c3efa16d953c25cc3931be46ed2e49e79c730cd8afc6a1b78c97d52cd714044a339c3bc29734cd4d2ab
)

@NancyLi1013
Copy link
Contributor

/azp run

@AlexAltea
Copy link
Contributor Author

All review comments have been addressed with the exception of the last one, as mentioned here:
#6852 (comment)

@NancyLi1013
Copy link
Contributor

NancyLi1013 commented Jan 15, 2020

I noticed that this port failed on osx pipeline currently due to this:

/Users/vagrant/azure-agent/_work/3/s/buildtrees/libqcow/src/20191221-5d3ea3bc22/libcfile/libcfile_file.c:67:10: fatal error: 'linux/fs.h' file not found
#include <linux/fs.h>
         ^~~~~~~~~~~~
1 error generated.
...
/Users/vagrant/azure-agent/_work/3/s/buildtrees/libqcow/src/20191221-5d3ea3bc22/libclocale/libclocale_support.c:26:10: fatal error: 'libintl.h' file not found
#include <libintl.h>
         ^~~~~~~~~~~
1 error generated.

Also failed on x64-uwp and arm-uwp triplets due to this:

4>C:\vsts\_work\4\s\buildtrees\libqcow\src\20191221-5d3ea3bc22\libcfile\libcfile_file.c(3620,16): error C2065:  'DISK_GEOMETRY': undeclared identifier [C:\vsts\_work\4\s\buildtrees\libqcow\arm-uwp-dbg\libqcow.vcxproj]
     4>C:\vsts\_work\4\s\buildtrees\libqcow\src\20191221-5d3ea3bc22\libcfile\libcfile_file.c(3620,16): error C2146:  syntax error: missing ';' before identifier 'disk_geometry' [C:\vsts\_work\4\s\buildtrees\libqcow\arm-uwp-dbg\libqcow.vcxproj]
     4>C:\vsts\_work\4\s\buildtrees\libqcow\src\20191221-5d3ea3bc22\libcfile\libcfile_file.c(3620,29): error C2065:  'disk_geometry': undeclared identifier [C:\vsts\_work\4\s\buildtrees\libqcow\arm-uwp-dbg\libqcow.vcxproj]
     4>C:\vsts\_work\4\s\buildtrees\libqcow\src\20191221-5d3ea3bc22\libcfile\libcfile_file.c(3684,17): error C2065:  'IOCTL_DISK_BASE': undeclared identifier [C:\vsts\_work\4\s\buildtrees\libqcow\arm-uwp-dbg\libqcow.vcxproj]
     4>C:\vsts\_work\4\s\buildtrees\libqcow\src\20191221-5d3ea3bc22\libcfile\libcfile_file.c(3721,48): error C2065:  'IOCTL_DISK_GET_DRIVE_GEOMETRY': undeclared identifier [C:\vsts\_work\4\s\buildtrees\libqcow\arm-uwp-dbg\libqcow.vcxproj]

Could you take a look?

@NancyLi1013
Copy link
Contributor

/azp run

@NancyLi1013
Copy link
Contributor

/azp run

@NancyLi1013
Copy link
Contributor

Hi @AlexAltea
Could you please fix the regressions on osx pipeline?
If this port currently doesn't support uwp, please add vcpkg_fail_port_install(ON_TARGET "uwp") to the top of portfile.cmake and also update libqcow:x64-uwp and libqcow:arm-uwp to ci.baseline.txt.

@NancyLi1013
Copy link
Contributor

/azp run

@NancyLi1013
Copy link
Contributor

Ping @AlexAltea , Is work still being down for this PR?

Signed-off-by: Alexandro Sanchez Bach <alexandro@phi.nz>
@NancyLi1013
Copy link
Contributor

NancyLi1013 commented Apr 10, 2020

/azp run

@microsoft microsoft deleted a comment from azure-pipelines bot Apr 10, 2020
@NancyLi1013
Copy link
Contributor

It seems there are still some regressions for this port.
Could you please look into them?

@VelocityRa
Copy link
Contributor

VelocityRa commented Apr 24, 2020

macOS CI has the same issue as the libvmdk PR:
HAVE_LIBINTL_H and HAVE_LINUX_FS_H get defined on macOS despite their respective headers missing.

Any help on this, maintainers? I don't even have access to a Mac.
Disabling the port for macOS is not an option, we need it.

@VelocityRa
Copy link
Contributor

Fixed, see the other PR.

Will rebase fixes here once the unrelated geographiclib CI issue is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants