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.

Sign up for GitHub

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

Jump to bottom

gdal:x64-windows-static-md fixes #15541

Merged
merged 16 commits into from
Jan 26, 2021
Merged

gdal:x64-windows-static-md fixes #15541

merged 16 commits into from
Jan 26, 2021

Conversation

ankurvdev
Copy link
Contributor

Describe the pull request

@ankurvdev
Copy link
Contributor Author

Is there a way to know why osg:x64-windows-static-md is skipped (cascade)

This should have caused osg:x64-windows-statid-md to start building

@JackBoosY JackBoosY self-assigned this Jan 11, 2021
@ankurvdev
Copy link
Contributor Author

@JackBoosY ,
Can I get some help on figuring out why after fixing gdal is osg still failing to build due to cascading erorrs ?
I'd like to know the dependency chain of what needs to be fixed to get osg to start building in static-md mode

@JackBoosY
Copy link
Contributor

JackBoosY commented Jan 12, 2021 •
edited
Loading

@ankurverma85 Because pthreads was skipped in x64-windows-static-md:
pthreads:x64-windows-static-md: skip: ceb246700bbdfc51412b3af346c7525b63cf0414
See:

vcpkg/scripts/ci.baseline.txt

Line 1813 in 331f877

pthreads:x64-windows-static-md=fail

  • osg depends: fontconfig
  • fontconfig depends: pthread
  • pthread depends: pthreads

@ankurvdev
Copy link
Contributor Author

@ankurverma85 Because pthreads was skipped in x64-windows-static-md:
pthreads:x64-windows-static-md: skip: ceb246700bbdfc51412b3af346c7525b63cf0414
See:

vcpkg/scripts/ci.baseline.txt

Line 1813 in 331f877

pthreads:x64-windows-static-md=fail

  • osg depends: fontconfig
  • fontconfig depends: pthread
  • pthread depends: pthreads

Thanks.

Havent gotten any feedback on the PR. Any issues you'd like fixed ?

JackBoosY
JackBoosY suggested changes Jan 13, 2021
View reviewed changes
Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Do these changes adapt to other triplets?

@JackBoosY JackBoosY added category:port-bug The issue is with a library, which is something the port should already support requires:author-response category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. labels Jan 13, 2021
@ankurvdev
Copy link
Contributor Author

Do these changes adapt to other triplets?

Other triplets should be unaffected.
This only changes win + static behavior so its windows-static and windows-static-md

Win-static continues to (wrongly?) Use MD both before and after my change.

@ankurvdev ankurvdev requested a review from JackBoosY January 13, 2021 17:49
JackBoosY
JackBoosY suggested changes Jan 14, 2021
View reviewed changes
@JackBoosY
Copy link
Contributor

Please run command vcpkg x-add-version gdal then commit the changes.

@JackBoosY
Copy link
Contributor

Need test all features.

@JackBoosY
Copy link
Contributor

Test all features succesfully on x86-windows and x64-windows-static.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jan 19, 2021
@ankurvdev
Copy link
Contributor Author

@BillyONeal , If everything looks ok.
Can we get this merged please ?

BillyONeal
BillyONeal previously requested changes Jan 20, 2021
View reviewed changes
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

Requesting changes due to the =skip

@JackBoosY
Copy link
Contributor

I agreed with @BillyONeal.

@JackBoosY JackBoosY added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Jan 21, 2021
@ankurvdev
Copy link
Contributor Author

I agreed with @BillyONeal.

I already fixed the stray merge conflicts in ci.baseline.txt.
Is there something else you'd like me to fix as well ?

@JackBoosY
Copy link
Contributor

JackBoosY commented Jan 21, 2021 •
edited
Loading

Now that this is a constant it should just be part of the string below. However I think we need to fix the expat port...

@ankurverma85 This one.

@JackBoosY
Copy link
Contributor

@BillyONeal About the expat library name, I think we can fix that in another PR.

JackBoosY
JackBoosY suggested changes Jan 21, 2021
View reviewed changes
@JackBoosY JackBoosY requested a review from BillyONeal January 21, 2021 08:36
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jan 22, 2021
@JackBoosY
Copy link
Contributor

Already merged from master.

@ankurvdev
Copy link
Contributor Author

Requesting changes due to the =skip
@BillyONeal
Fixed the stray bad merges and made the requested changes around expat.

Does everything look good now ?

@BillyONeal BillyONeal dismissed their stale review January 26, 2021 02:44

I still wish to see the expat port fixed but I won't block over this.

@ankurvdev
Copy link
Contributor Author

Seems like there's no additional changes requested.

Can we merge this ?

I see @dan-shaw , added a requires:discussion tag here.
Is that for expat ?
I would say that is outside the scope of this PR.

I would like to merge this ASAP to have CI - Pipeline coverage for x64-windows-static-md, for the additional ports that this unblocks
gdal, osg among other etc

So can we keep any clean up or other discussions outside the scope ?

@dan-shaw dan-shaw merged commit 60d7d91 into microsoft:master Jan 26, 2021
@ankurvdev ankurvdev deleted the ankurv/gdal branch January 30, 2021 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JackBoosY JackBoosY JackBoosY approved these changes

@BillyONeal BillyONeal Awaiting requested review from BillyONeal

Assignees

@JackBoosY JackBoosY

Labels
category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[gdal] x64-windows-static-md build failure
5 participants
@ankurvdev @JackBoosY @BillyONeal @dan-shaw @strega-nil