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

[bitar] Add new port for bitar #25533

Closed
wants to merge 2 commits into from
Closed

Conversation

ljishen
Copy link
Contributor

@ljishen ljishen commented Jul 2, 2022

Describe the pull request

Add the support for bitar, a C++ library to simplify accessing hardware compression/decompression accelerators.

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

    Support only on Linux. And yes I updated file ci.baseline.txt, because bitar only supports compiling with GCC>=10.

  • Does your PR follow the maintainer guide?

    Yes

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

    Yes

github-actions[bot]
github-actions bot previously approved these changes Jul 2, 2022
@ljishen ljishen changed the title [bitar] Add new port for biar [bitar] Add new port for bitar Jul 2, 2022
github-actions[bot]
github-actions bot previously approved these changes Jul 2, 2022
github-actions[bot]
github-actions bot previously approved these changes Jul 3, 2022
github-actions[bot]
github-actions bot previously approved these changes Jul 3, 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.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for bitar have changed but the version was not updated
version: 0.0.1
old SHA: 0de32a367d5c009733455dab2ba76c9776d2cd6a
new SHA: 5923c69518c5803b8196b12c726525cc2f152f4a
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

github-actions[bot]
github-actions bot previously approved these changes Jul 3, 2022
github-actions[bot]
github-actions bot previously approved these changes Jul 3, 2022
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

I would suggest to mark your PR as draft if it is still work in progress.

ports/bitar/portfile.cmake Outdated Show resolved Hide resolved
ports/bitar/portfile.cmake Outdated Show resolved Hide resolved
scripts/ci.baseline.txt Outdated Show resolved Hide resolved
@ljishen
Copy link
Contributor Author

ljishen commented Jul 3, 2022

I would suggest to mark your PR as draft if it is still work in progress.

I had a couple of unexpected errors from the CI that I didn't notice from my local development. Sorry for having multiple force pushes. But I think this PR has gotten to the point for reviewing. I will update the code according to your feedback. Thanks!

github-actions[bot]
github-actions bot previously approved these changes Jul 3, 2022
ports/bitar/portfile.cmake Outdated Show resolved Hide resolved
scripts/ci.baseline.txt Outdated Show resolved Hide resolved
"description": "Bitar is a C++ library to simplify accessing hardware compression/decompression accelerators.",
"homepage": "https://github.com/ljishen/bitar",
"license": "MIT",
"supports": "linux",
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC bitar supports also freebsd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • I mean I'm expecting to have "bitar" to be one of dependencies of arrow in arrow/vcpkg.json
  • All arrow relevant symbols/references in bitar will be weak, and therefore can be replaced/overrided by strong ones later when integrated into arrow. So no worry about existing of multiple arrow symbols.
  • I will update the code and remove all arrow installed files from the bitar port.
  • What should I suppose to do for the altering about no CI?
  • Yes, I will add freebsd to "supports".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dg0yt I've stopped installing arrow files by having changes in the upstream.

github-actions[bot]
github-actions bot previously approved these changes Jul 3, 2022
@Cheney-W Cheney-W added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jul 4, 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.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for bitar have changed but the version was not updated
version: 0.0.4
old SHA: 93629f3cbcb88d215280fc4bdb54d8c164889e54
new SHA: e3d713adacf64e6791f33daed5b9bda7abd0bfe0
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

github-actions[bot]
github-actions bot previously approved these changes Sep 1, 2022
@ljishen
Copy link
Contributor Author

ljishen commented Sep 2, 2022

It looks like the port "libilbc" failed in the building. But I don't think I used it as one of the dependencies.

@Cheney-W
Copy link
Contributor

Cheney-W commented Sep 2, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ljishen
Copy link
Contributor Author

ljishen commented Sep 2, 2022

The CI still builds "libilbc". Do you know why it involves the building of this library?

@Cheney-W
Copy link
Contributor

Cheney-W commented Sep 9, 2022

Could you please merge master for this PR?

@Cheney-W
Copy link
Contributor

I tried to merge master, but I don't have permission to modify your pr.

@ljishen
Copy link
Contributor Author

ljishen commented Sep 16, 2022

Sorry for the late reply. I'll merge it in these few days.

@Cheney-W
Copy link
Contributor

Is work still being done for this PR?

github-actions[bot]
github-actions bot previously approved these changes Sep 30, 2022
@ljishen
Copy link
Contributor Author

ljishen commented Oct 1, 2022

I've merged the master, but the CI still builds other irrelevant libraries.

SOURCE_PATH "${SOURCE_PATH}"
OPTIONS
-DVCPKG_ROOT=${VCPKG_ROOT_DIR}
-DBITAR_FETCHCONTENT_OVERWRITE_CONFIGURATION=OFF)
Copy link
Member

Choose a reason for hiding this comment

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

This should be handled already since vcpkg_cmake_configure() passes "-DFETCHCONTENT_FULLY_DISCONNECTED=ON"

@vicroms
Copy link
Member

vicroms commented Jan 9, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ljishen
Copy link
Contributor Author

ljishen commented Jan 10, 2023

I'll close this PR because it is not needed anymore.

@ljishen ljishen closed this Jan 10, 2023
@dg0yt
Copy link
Contributor

dg0yt commented Jan 10, 2023

I'll close this PR because it is not needed anymore.

Was it replaced by another port or PR?

@ljishen
Copy link
Contributor Author

ljishen commented Jan 10, 2023

I'll close this PR because it is not needed anymore.

Was it replaced by another port or PR?

No. The same function is now officially supported by an SDK maintained by Nvidia. Unless I add distinct features to bitar, I think people will use their version instead of mine.

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!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants