close
Skip to content

vcpkg add port: Handle feature core. - #1163

Merged
Billy O'Neal (BillyONeal) merged 4 commits into
microsoft:mainfrom
autoantwort:feature/add-port
Oct 31, 2023
Merged

vcpkg add port: Handle feature core.#1163
Billy O'Neal (BillyONeal) merged 4 commits into
microsoft:mainfrom
autoantwort:feature/add-port

Conversation

@autoantwort

Copy link
Copy Markdown
Contributor

Don't crash at vcpkg add port sqlite3[core]

Now sets "default-featues": false

Comment thread src/vcpkg/commands.add.cpp
Comment thread src/vcpkg/commands.add.cpp
"dependencies": [
{
"name": "sqlite3",
"default-features": false,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this should be clarified that this is still here from the previous run, but it would also be better to have an independent test with [core,somethingelse].

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On second thought I think this should make default features be true because that's what sqlite3[zlib] requests.

Robert Schumacher (@ras0219-msft) also points out that we should have tests for multiple references to the same name

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.

I now have the test vcpkg add port "sqlite3" "sqlite3[core]"

@BillyONeal

Copy link
Copy Markdown
Member

Augustin Popa (@AugP) Victor Romero (@vicroms) dan-shaw Robert Schumacher (@ras0219-msft) Javier Matos Denizac (@JavierMatosD) and I discussed this today and it gets design approval modulo the nitpicks above

@BillyONeal Billy O'Neal (BillyONeal) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I pushed a merge with main for you; do you want me to fix the behavior bit I pointed out for you here and merge?

Comment thread azure-pipelines/end-to-end-tests-dir/add.ps1 Outdated
@autoantwort

Copy link
Copy Markdown
Contributor Author

I pushed a merge with main for you; do you want me to fix the behavior bit I pointed out for you here and merge?

Sorry for the late answer. Yeah you could have done that 😅 I have not implemented this

@BillyONeal
Billy O'Neal (BillyONeal) merged commit f07df71 into microsoft:main Oct 31, 2023
@BillyONeal

Copy link
Copy Markdown
Member

Thanks!

@autoantwort
autoantwort deleted the feature/add-port branch October 31, 2023 01:13
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.

2 participants