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

[bzip2] add a "tool" feature to control the build of bzip2.exe #26998

Merged
merged 9 commits into from
Sep 28, 2022

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented Sep 26, 2022

This is modelled after curl[tool], but it is on by default for compatibility with the existing portfile.

Fixes #26997

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

    all

  • 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

This is modelled after curl[tool], but it is on by default for
compatibility with the existing portfile.
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!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/bzip2/vcpkg.json b/ports/bzip2/vcpkg.json
index 4a9fa79..366f461 100644
--- a/ports/bzip2/vcpkg.json
+++ b/ports/bzip2/vcpkg.json
@@ -6,11 +6,11 @@
   "homepage": "https://sourceware.org/bzip2/",
   "documentation": "https://sourceware.org/bzip2/docs.html",
   "default-features": [
-      "tool"
+    "tool"
   ],
   "features": {
-      "tool": {
-          "description": "Builds bzip2 executable"
-      }
+    "tool": {
+      "description": "Builds bzip2 executable"
+    }
   }
 }
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 5bc980361a81fd5276ee411a7d7af423a4d24363 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/b-/bzip2.json b/versions/b-/bzip2.json
index 7025c50..a765a81 100644
--- a/versions/b-/bzip2.json
+++ b/versions/b-/bzip2.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "3b79c95295ab62d98dd47ff12f58643ca8d4670e",
+      "version-semver": "1.0.8",
+      "port-version": 3
+    },
     {
       "git-tree": "a1ea352502e69888a565563d9151d3f7ab609fb1",
       "version-semver": "1.0.8",
diff --git a/versions/baseline.json b/versions/baseline.json
index 3d47c8b..8a2624f 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -1230,7 +1230,7 @@
     },
     "bzip2": {
       "baseline": "1.0.8",
-      "port-version": 2
+      "port-version": 3
     },
     "c-ares": {
       "baseline": "1.18.1",

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

  • ports/bzip2/portfile.cmake

You have modified or added at least one vcpkg.json where you should check the license field.

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

  • ports/bzip2/vcpkg.json

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

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 bzip2 have changed but the version was not updated
version: 1.0.8#3
old SHA: 961a25ad305b417e722658de29b964a673a69e8c
new SHA: 3b79c95295ab62d98dd47ff12f58643ca8d4670e
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

  • ports/bzip2/portfile.cmake

You have modified or added at least one vcpkg.json where you should check the license field.

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

  • ports/bzip2/vcpkg.json

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

@DHowett
Copy link
Member Author

DHowett commented Sep 26, 2022

The bot's comments are for an earlier commit. I cannot resolve the license issue. I do not feel confident in fixing the deprecated cmake file.

When making any changes to a library, the version or port-version

Yes, I incremented it to 3 from 2.

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 bzip2 have changed but the version was not updated
version: 1.0.8#3
old SHA: 961a25ad305b417e722658de29b964a673a69e8c
new SHA: 3b79c95295ab62d98dd47ff12f58643ca8d4670e
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

  • ports/bzip2/portfile.cmake

You have modified or added at least one vcpkg.json where you should check the license field.

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

  • ports/bzip2/vcpkg.json

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

github-actions[bot]
github-actions bot previously approved these changes Sep 26, 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 portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

  • ports/bzip2/portfile.cmake

You have modified or added at least one vcpkg.json where you should check the license field.

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

  • ports/bzip2/vcpkg.json

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

@DHowett
Copy link
Member Author

DHowett commented Sep 26, 2022

Oh, interesting. Thanks! What happened here?

@JonLiu1993 JonLiu1993 added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist requires:author-response labels Sep 27, 2022
@JonLiu1993
Copy link
Member

@DHowett ,Thanks for your pr, please consider migrating them to the new functions:

vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)

In the ports that use the new function, you have to add the corresponding dependencies in vcpkg.json file:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

@DHowett
Copy link
Member Author

DHowett commented Sep 27, 2022

It was my intent to make as small of a change as possible; however, I'll submit another revision that makes the requested changes.

@DHowett
Copy link
Member Author

DHowett commented Sep 27, 2022

@JonLiu1993 Thanks for the review. I've made the requested changes!

github-actions[bot]
github-actions bot previously approved these changes Sep 27, 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 you should check the license field.

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

  • ports/bzip2/vcpkg.json

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

@DHowett
Copy link
Member Author

DHowett commented Sep 27, 2022

I have added a license. This is the officially-recognized SPDX license ID for the bzip2 software license. This license text is identical to the bzip2 1.0.8 license text, which does not have a unique SPDX identifier.

@JonLiu1993 JonLiu1993 added requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function and removed requires:author-response requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function labels Sep 28, 2022
@DHowett
Copy link
Member Author

DHowett commented Sep 28, 2022

Since tool is a default feature, it may be a good idea to test core without tool (default-features false). Is that covered already? Thanks!

@JonLiu1993
Copy link
Member

Since tool is a default feature, it may be a good idea to test core without tool (default-features false). Is that covered already? Thanks!

Sorry, I was careless and didn't notice this.

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Sep 28, 2022
@vicroms vicroms merged commit 6d5a3a5 into microsoft:master Sep 28, 2022
@DHowett
Copy link
Member Author

DHowett commented Sep 28, 2022

Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bzip2] bzip2 should not build bzip2.exe unless requested
4 participants