[skills] Add skills to create ports and patches#51187
[skills] Add skills to create ports and patches#51187vicroms wants to merge 13 commits intomicrosoft:masterfrom
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Is there some way we can document how to test this so that if/when we change it in the future we at least have a vague idea what to not break? |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Convert all 12 files under .github/skills/ from CRLF to LF. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Place patches at port root instead of patches/ subdirectory - Remove --debug flag from first-attempt build commands - Add topic branch workflow to avoid upstream merge conflicts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Enhanced 'Example Analysis Results' table with gqlxy-server example showing version-date usage for projects without official releases - Added new 'Version-Date Guidance' section explaining when and how to use version-date schema - Clarified that semantic versions should never be used for unreleased projects - Provided command to find commit dates for version-date field - Updated success indicators to include version scheme verification Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Enhanced CMake target verification with common mismatch examples - Added ALIAS target creation pattern for target name mismatches - Documented proper patch file best practices (naming, line endings, context) - Expanded component disabling strategy with BUILD_SAMPLES pattern - Added resource exhaustion caveat for constrained build environments - New pattern: Creating CMake target aliases for upstream compatibility - Comprehensive patch creation workflows with real-world examples These learnings reflect issues encountered while testing gqlxy-server port: - CMake target naming mismatches (gqlxy::core vs gqlxy::gqlxy_core) - Memory exhaustion during sample/test linking - Patch application failures due to line endings and context matching Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fixed 8 files missing trailing newlines - Ensures compliance with Unix text file conventions - Applies to SKILL.md files and reference/template files - No content changes, formatting only Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Updated create-port SKILL: - New 'Testing Ports After Packaging' section with comprehensive test workflows - Feature testing patterns for individual and combined features - CMake integration verification - Resource constraint guidance for debug builds with heavy dependencies - Testing checklist for production-ready ports Updated create-patches SKILL: - New 'Scenario 4: Feature-Related Patches' section - Pattern for adding CMake feature options via patches - Integration with vcpkg_check_features helper Rationale: Session testing revealed importance of post-package testing, proper feature handling with vcpkg_check_features, and understanding resource constraints on systems with memory-intensive dependencies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove BUILD_SHARED_LIBS from portfile templates (handled by vcpkg toolchain) - Make usage files optional - vcpkg generates them heuristically - Add documentation cleanup to remove /share/doc by default - Clarify usage files should not include #include statements - Update templates and references to reflect minimal, focused portfile approach - Add vcpkg_fixup_pkgconfig() to standard template - Update best practices documentation These changes ensure ports follow vcpkg conventions and stay lean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move branch workflow section to top of document for immediate visibility - Add⚠️ IMPORTANT warning label - Include clear do-not instructions - Highlight benefits of topic branch workflow - Reference workflow instructions from later sections This ensures users understand the required branch workflow before starting port creation and helps prevent accidental commits to master branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| vcpkg's patch utility requires Unix-style LF line endings, not Windows CRLF: | ||
| ```powershell | ||
| # PowerShell: Normalize to LF only | ||
| $text = [System.IO.File]::ReadAllText('path/to/file.patch') |
There was a problem hiding this comment.
I'm not sure this is correct: if the patched repo itself has CRLFs there will be CRLFs in the patch file to my understanding, and this will be damage those patches.
Can we tell it to use git diff --output or similar to make the patches rather than trying to fix them up after the fact like this?
| # Test patch application (non-destructive) | ||
| git apply path/to/patch.patch |
There was a problem hiding this comment.
git apply is not "non-destructive"
|
|
||
| **Debugging CMake Targets:** | ||
| To verify what targets a vcpkg port actually exports: | ||
| ```powershell |
There was a problem hiding this comment.
Should there be bash/zsh ones for the non-Windows platforms? (It's ok if the answer is no)
| # Before: Unconditionally builds samples | ||
| if(PROJECT_IS_TOP_LEVEL) | ||
| add_subdirectory(samples) | ||
| endif() | ||
|
|
||
| # After: Add option to disable | ||
| option(BUILD_SAMPLES "Build sample applications" ON) | ||
| if(PROJECT_IS_TOP_LEVEL AND BUILD_SAMPLES) |
There was a problem hiding this comment.
IMO we should only patch in an option like this if we are considering submitting the patch upstream. A much shorter patch would be just
-if(PROJECT_IS_TOP_LEVEL)
+if(0)which has the same effect without needing OPTIONS below.
| **Why This Pattern Matters:** | ||
| - Samples/tests often consume excessive memory during linking on constrained environments | ||
| - Disabling them via CMake is cleaner than post-build deletion (no CMake errors from missing directories) | ||
| - Users can still enable if needed via CMake options in consumer projects |
| ```bash | ||
| # Local testing | ||
| cd source-directory | ||
| git apply ../ports/package-name/001-fix-build.patch |
There was a problem hiding this comment.
The source directory, typically buildtrees/<port>/src/aasdfasdf-asdfasdfas.clean is not ../ to "ports".
| The upstream project expects a `BUILD_STANDALONE_SERVER` option. Create a patch adding this: | ||
|
|
||
| ```diff | ||
| @@ -42,8 +42,11 @@ endif() | ||
|
|
||
| # Server components | ||
| -if(BUILD_SERVER) | ||
| +option(BUILD_STANDALONE_SERVER "Build standalone GraphQL server" ON) |
There was a problem hiding this comment.
How can upstream "expect" it if we are patching it in ourselves?
| "docs": { | ||
| "description": "Build documentation" | ||
| } |
There was a problem hiding this comment.
It looks like elsewhere we told it to patch building these out entirely but here it's suggesting to add "docs" and "tests" as features. Ditto below.
|
|
||
| **vcpkg.json** - Package manifest with auto-detected metadata: | ||
| ```json | ||
| { |
There was a problem hiding this comment.
Should we vcpkg format-manifest this content?
| vcpkg install "package-name[feature1]" | ||
| vcpkg install "package-name[feature2]" | ||
| vcpkg install "package-name[feature1,feature2]" |
There was a problem hiding this comment.
vcpkg x-test-features package-name --ci-feature-baseline scripts/ci.feature.baseline.txt
|
|
||
| ```cmake | ||
| # test-consumer/CMakeLists.txt | ||
| cmake_minimum_required(VERSION 3.15) |
There was a problem hiding this comment.
We should probably use latest at time of writing 3.23 here
| - [ ] Each feature can be installed individually | ||
| - [ ] Multiple features can be combined: `[feat1,feat2]` |
There was a problem hiding this comment.
These two checkboxes x-test-features will do for you.
| 3. **Monitor CI** for platform-specific issues and respond to feedback | ||
| 4. **Update documentation** if special integration steps are required | ||
| --- | ||
| name: create-port |
There was a problem hiding this comment.
There's nothing about pkgconfig in here. Should there be?
| ) | ||
| vcpkg_cmake_install() | ||
|
|
||
| vcpkg_cmake_config_fixup(CONFIG_PATH lib/cmake/package-name) |
There was a problem hiding this comment.
Should we explain how/when to use vcpkg_cmake_config_fixup and/or vcpkg_fixup_pkgconfig? (For example, the former should only run if the thing installs a CMake config, and the latter should run only if it installs pkgconfig)
|
I'm working on evaluations for the skills (see #51513), that should let us measure whether these skills actually improve the AI and give us a base level of confidence that the output is correct. |
Adds two AI skills to assist with creation of new ports.
The
create-portskill takes a GitHub URL and infers any necessary information to generate a port.The
create-patchskill was made to teach Copilot how to properly generate patches using a temporary Git repository, otherwise, I found that Copilot had a strong preference for usingvcpkg_replace_stringor generating patches without proper code point references.I tested only with GitHub libraries, but the AI might be smart enough to figure when to use other helpers without explicit instruction on the skill.