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

[c-dbg-macro] Add new port #23522

Merged
merged 1 commit into from
Mar 15, 2022
Merged

[c-dbg-macro] Add new port #23522

merged 1 commit into from
Mar 15, 2022

Conversation

eerimoq
Copy link
Contributor

@eerimoq eerimoq commented Mar 12, 2022

Describe the pull request

  • What does your PR fix?

    Add new port

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    all, Yes (I think)

  • Does your PR follow the maintainer guide?

    Probably not. A lot to read.

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes

@ghost
Copy link

ghost commented Mar 12, 2022

CLA assistant check
All CLA requirements met.

@eerimoq eerimoq force-pushed the master branch 3 times, most recently from a62eed4 to f23e97c Compare March 12, 2022 16:07
@eerimoq eerimoq marked this pull request as ready for review March 12, 2022 17:27
@JonLiu1993 JonLiu1993 self-assigned this Mar 14, 2022
@JonLiu1993 JonLiu1993 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Mar 14, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/c-dbg-macro/vcpkg.json

Valid values for the license field can be found in the documentation

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Mar 14, 2022
@dan-shaw dan-shaw merged commit 3559cb2 into microsoft:master Mar 15, 2022
@autoantwort
Copy link
Contributor

This breaks the ci:

Starting package 408/935: dbg-macro:x64-uwp
Building package dbg-macro[core]:x64-uwp...
-- Downloading https://github.com/sharkdp/dbg-macro/archive/4db61805d90cb66d91bcc56c2703591a0127ed11.tar.gz -> sharkdp-dbg-macro-4db61805d90cb66d91bcc56c2703591a0127ed11.tar.gz...
-- Extracting source D:/downloads/sharkdp-dbg-macro-4db61805d90cb66d91bcc56c2703591a0127ed11.tar.gz
-- Using source at D:/buildtrees/dbg-macro/src/1a0127ed11-833f10929f.clean
-- Performing post-build validation
-- Performing post-build validation done
Uploaded binaries to 1 HTTP remotes.
Installing package dbg-macro[core]:x64-uwp...
The following files are already installed in D:/installed/x64-uwp and are in conflict with dbg-macro:x64-uwp

Installed by c-dbg-macro:x64-uwp
    include/dbg.h

Elapsed time for package dbg-macro:x64-uwp: 447.8 ms

@dg0yt
Copy link
Contributor

dg0yt commented Mar 16, 2022

@JonLiu1993 The conflict is when installing the second port from the set < dbg-macro, c-dbg-macro >.

@eerimoq
Copy link
Contributor Author

eerimoq commented Mar 16, 2022

Not sure what to do about it. Renaming the header file when installing using vcpkg is possible, but could be confusing. However, the package in vcpkg is called c-dbg-macro, so a c as prefix might be ok?

@dg0yt
Copy link
Contributor

dg0yt commented Mar 16, 2022

A file name as general as include/dbg.h is not well suited for a general library manager as vcpkg.
There is a simple solution: Use a subdirectory, e.g. include/eerimoq/dbg-macro.h.
And replace this port with a new one named eerimoq-dbg-macro.

I need to repeat that there should be more care before adding ports. It can't be merge 2 days after the PR, even if trivial.

@autoantwort
Copy link
Contributor

Or use include/<port-name>/.., thus include/c-dbg-macro/dbg.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants