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

Move a few vendor libraries to src/native/external #62673

Merged
merged 6 commits into from
Dec 12, 2021
Merged

Move a few vendor libraries to src/native/external #62673

merged 6 commits into from
Dec 12, 2021

Conversation

am11
Copy link
Member

@am11 am11 commented Dec 11, 2021 •
edited
Loading

Moved:

  • brotli
  • rapidjson
  • zlib
  • zlib-intel

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Dec 11, 2021
@am11
Copy link
Member Author

am11 commented Dec 11, 2021 •
edited
Loading

cc @jkotas

  • rapidjson was missing the version file, so added rapidjson-version.txt
  • zlib-intel is not moved, only the one which we use on win-arm/arm64 in libs and on multiple targets in mono (now moved zlib-intel as well)

The format is:

src/native/external/{libname}/
src/native/external/{libname}-version.txt
src/native/external/{libname}.cmake # exception being rapidjson which is header-only lib with no build configs

@janvorli, I was thinking about moving libunwind here as well and refactoring our configurations in a libunwind.cmake file without overwriting anything in vendor's tree. (of course after #62092 is merged) 🙂

@am11 am11 marked this pull request as ready for review December 11, 2021 19:09
@am11
Copy link
Member Author

am11 commented Dec 11, 2021 •
edited
Loading

Red diffs are because one copy (out of three) of zlib is deleted from the repo.
cc @carlossanlop

@ghost
Copy link

ghost commented Dec 11, 2021

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

Moved:

  • brotli
  • rapidjson
  • zlib
  • zlib-intel
Author: am11
Assignees: -
Labels:

area-Infrastructure, community-contribution
Milestone: -

jkotas
jkotas approved these changes Dec 11, 2021
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Looks great. Thank you!

@am11
Move json_parser back to corehost root
aaacf4f
@am11
Cleanup trailing whitespace in changeset files
2563708 
Except for the ones in vendor directory:
```sh
# git remote add dotnet https://github.com/dotnet/runtime && git pull --rebase dotnet main

if uname 2>/dev/null | grep -q Darwin; then
    space=" "
fi

git show --name-only --pretty="" HEAD...dotnet/main \
    ':!*external/brotli/*' ':!*external/rapidjson/*' ':!*external/zlib/*' ':!*external/zlib-intel/*' |
        xargs -I{} sh -c "test -f {} && sed -i$space'' 's/[[:space:]]*$//' {}"
```
@am11 am11 mentioned this pull request Dec 12, 2021
Merged
@jkotas jkotas merged commit 0e499ac into dotnet:main Dec 12, 2021
@am11 am11 deleted the feature/consolidations branch December 12, 2021 16:51
@ghost ghost locked as resolved and limited conversation to collaborators Jan 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jkotas jkotas jkotas approved these changes

@imhameed imhameed Awaiting requested review from imhameed

@lambdageek lambdageek Awaiting requested review from lambdageek lambdageek is a code owner

@marek-safar marek-safar Awaiting requested review from marek-safar marek-safar is a code owner

@SamMonoRT SamMonoRT Awaiting requested review from SamMonoRT

@thaystg thaystg Awaiting requested review from thaystg thaystg is a code owner

@vargaz vargaz Awaiting requested review from vargaz

Assignees
No one assigned
Labels
area-Infrastructure community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@am11 @jkotas