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

[bde] Updated to 3.117.0 #31961

Merged
merged 9 commits into from
Jul 7, 2023
Merged

[bde] Updated to 3.117.0 #31961

merged 9 commits into from
Jul 7, 2023

Conversation

adamncasey
Copy link
Contributor

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

Fixes #18937

Replaces #31644

Changes since the prior PR:

  1. Remove the cxx17 feature flag. This is an incomplete solution to the issue but avoids allowing conflicting feature flags in libraries.
  2. Disable OSX build. Linux is the primary target - I'd like to fix others in future version of this port. The existing BDE version in vcpkg is ancient & seemingly non-functional.

@adamncasey adamncasey mentioned this pull request Jun 13, 2023
Closed
7 tasks
@adamncasey
Copy link
Contributor Author

CC'ing @BillyONeal and @FrankXie05 from the previous PR

BillyONeal
BillyONeal reviewed Jun 13, 2023
View reviewed changes
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
@BillyONeal
Copy link
Member

Thank you so much! Sorry for the nitpicks...

dg0yt
dg0yt reviewed Jun 14, 2023
View reviewed changes
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
@Cheney-W Cheney-W assigned Cheney-W and unassigned Cheney-W Jun 14, 2023
@Cheney-W Cheney-W added the category:port-update The issue is with a library, which is requesting update new revision label Jun 14, 2023
Cheney-W
Cheney-W reviewed Jun 14, 2023
View reviewed changes
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
Cheney-W
Cheney-W reviewed Jun 14, 2023
View reviewed changes
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
Cheney-W
Cheney-W reviewed Jun 14, 2023
View reviewed changes
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
Cheney-W
Cheney-W reviewed Jun 14, 2023
View reviewed changes
ports/bde/portfile.cmake Outdated Show resolved Hide resolved
@Cheney-W Cheney-W marked this pull request as draft June 14, 2023 09:31
@Cheney-W
Copy link
Contributor

Moving the PR to draft. Please set it to "ready for review" once you respond.

@adamncasey
Copy link
Contributor Author

adamncasey commented Jun 14, 2023 •
edited
Loading

Thanks for the feedback on this PR so far 🙂

It looks like the CI might take ten hours, I'll mark this ready for review with the idea that my local builds work with these changes.

@adamncasey adamncasey marked this pull request as ready for review June 14, 2023 16:17
@BillyONeal
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Cheney-W
Copy link
Contributor

We are fixing the failure of dcmtk:x64-windows and dcmtk:x64-windows-static-md.

@Cheney-W
Copy link
Contributor

Waiting for #32049 to be merged.

@Cheney-W Cheney-W added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jun 16, 2023
@Cheney-W Cheney-W marked this pull request as draft June 16, 2023 08:56
@Cheney-W
Copy link
Contributor

Moving the PR to draft. I will set it to "ready for review" once the CI issue fixed.

@jiayuehua
Copy link
Contributor

Also should provide the usage file in port bde

@Cheney-W
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Cheney-W Cheney-W removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Jun 27, 2023
@adamncasey
Copy link
Contributor Author 

REGRESSIONS:
REGRESSION: fastrtps:x64-windows failed with BUILD_FAILED. If expected, add fastrtps:x64-windows=fail to C:\a\1\s\scripts\azure-pipelines/../ci.baseline.txt.

Is this something known?

@dg0yt
Copy link
Contributor

dg0yt commented Jun 27, 2023

Is this something known?

Yes. The fix was just merged: #32257.

@Cheney-W
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@adamncasey
Copy link
Contributor Author 

REGRESSION: halide:x86-windows failed with BUILD_FAILED. If expected, add halide:x86-windows=fail to C:\a\1\s\scripts\azure-pipelines/../ci.baseline.txt.

@dg0yt
Copy link
Contributor

dg0yt commented Jun 28, 2023 

REGRESSION: halide:x86-windows failed with BUILD_FAILED. If expected, add halide:x86-windows=fail to C:\a\1\s\scripts\azure-pipelines/../ci.baseline.txt.

Baseline issue. Cf. #32259 (comment)

@adamncasey
Copy link
Contributor Author

adamncasey commented Jul 4, 2023 •
edited
Loading

The build in the referenced issue is ok now, hopefully this passes too?

@adamncasey adamncasey marked this pull request as ready for review July 4, 2023 12:48
@FrankXie05
Copy link
Contributor

@adamncasey Please merge into master branch. :)

osubboo and others added 9 commits July 6, 2023 13:37
@osubboo @adamncasey
Update bde port
cd41dde 
Disable windows and arm, at least for now

It'd be great to enable arm64-osx (macos?) soon
@adamncasey
Update version in bde.json
b858aa0
@BillyONeal @adamncasey
Remove ci.baseline.txt skip.
9566439
@BillyONeal @adamncasey
Fix version database.
2c251e2
@BillyONeal @adamncasey
fix port-version
9975107
@adamncasey
Stop building for android, osx
ae5a43a 
I don't specifically know why these don't work, but linux is the primary platform
so let's fix these another time

Also remove the cxx17 flag for now - we'll need to address this at some point
but for now this should satisfy the feature flag requirements.
@adamncasey
Review comments
edd4de0
@adamncasey
Remove WINDOWS_USE_MSBUILD
884a6e6
@adamncasey
Update versions
2480c51
@FrankXie05 FrankXie05 added the info:reviewed Pull Request changes follow basic guidelines label Jul 7, 2023
@JavierMatosD JavierMatosD merged commit 88f5e89 into microsoft:master Jul 7, 2023
@BillyONeal
Copy link
Member

Unfortunately this breaks CI:

REGRESSION: ryu:x64-linux failed with FILE_CONFLICTS. If expected, add ryu:x64-linux=fail to /agent/_work/1/s/scripts/azure-pipelines/../ci.baseline.txt.
error: The following files are already installed in /mnt/vcpkg-ci/installed/x64-linux and are in conflict with ryu:x64-linux
Installed by bde:x64-linux    
debug/lib/libryu.a
    include/ryu/ryu.h
    lib/libryu.a

It looks like bde has vendored copies of ryu and pcre2 which should be taken from their respective ports...

@BillyONeal BillyONeal mentioned this pull request Jul 11, 2023
Merged
@BillyONeal
Copy link
Member

Unfortunately, it looks like bde made changes to these so it isn't straightforward to devendor them: bloomberg/bde@5e057e0 "with Bloomberg extensions"

In particular bde is requiring that a pkgconfig file exists for ryu and it does not upstream. We could add that but without an understanding of what the other changes are I'm not comfortable messing with this on my own.

@adamncasey @cppguru Unfortunately I believe this forces us to remove bde from the curated registry as it can't be installed with other supported ports, and won't be link compatible with anything using the real ryu or pcre2.

@BillyONeal
Copy link
Member

I filed bloomberg/bde#295

@cppguru
Copy link

cppguru commented Jul 17, 2023

Oleg and Bloomberg is working on this. I am not, and I'm on leave at the moment. Could you please make an issue in the bde repo or email the team.

BillyONeal added a commit to BillyONeal/vcpkg that referenced this pull request Jul 17, 2023
@BillyONeal
[bde] Remove from the curated registry.
508891b 
bde has been broken in vcpkg for a long time, as when we updated to a distro where python meant python3, it failed. microsoft#31961 , merged a couple of weeks ago, fixed this problem by updating to a more recent copy of bde. Unfortunately, the new copy of bde is link-incompatible with other vcpkg ports, as they vendor ryu and pcre2 with changes.

Given that this was broken for most customers for a long time anyway, I think we should deindex for now and bring it back if/when we have an understanding with upstream as to how this is to integrate with the rest of vcpkg' ecosystem.
@BillyONeal BillyONeal mentioned this pull request Jul 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cheney-W Cheney-W Cheney-W left review comments

@dg0yt dg0yt dg0yt left review comments

@FrankXie05 FrankXie05 FrankXie05 approved these changes

@BillyONeal BillyONeal BillyONeal approved these changes

Assignees

@Cheney-W Cheney-W

Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[bde] build failure when python is python3
9 participants
@adamncasey @BillyONeal @Cheney-W @jiayuehua @dg0yt @FrankXie05 @cppguru @JavierMatosD @osubboo