Skip to content

Imports of *.props/*.targets from NuGet packages #887

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

frobijn
Copy link
Contributor

@frobijn frobijn commented Apr 15, 2025

Description

  • Added a task FixNuGetPackagePropsTargetsImportsTask that is executed before a build
  • The task reads the packages and versions from packages.config
  • It scans the *.nfproj to get the path to the packages directory. This should succeed for all regular nanoFramework projects as there always is a reference to the mscorlib package.
  • It removes imports to files in the packages directory that do no longer exist and/or that belong to packages that are no longer present in the packages.config
  • It adds imports for <package_id>.props and <package_id>.targets if they exist
  • It rebuilds the EnsureNuGetPackageBuildImports task or removes it.
  • If the project file is updated, the task reports an error as the build has to be restarted.

Motivation and Context

Microsoft has not yet responded to a request for help to create a workaround, but this PR should provide a fix that always works.

How Has This Been Tested?

Manual testing with a nanoFramework application project with NanoFrameworkProjectSystemPath, NF_MSBUILDTASK_PATH and NF_MDP_MSBUILDTASK_PATH properties that point to the updated MSBuild/task files. Tested with the NerdBank GitVersion package.

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Unit Tests (add new Unit Test(s) or improved existing one(s), has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist:

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).
  • I have added new tests to cover my changes.

Summary by CodeRabbit

  • Chores
    • Updated internal workflow to use a newer version of the changelog update action for improved reliability. No impact on user-facing features.

@nfbot
Copy link
Member

nfbot commented Apr 15, 2025

@frobijn I've fixed the checklist for you.
FYI, the correct format is [x], no spaces inside brackets, no other chars.

Copy link

coderabbitai bot commented Apr 15, 2025

Walkthrough

The changes update the GitHub Actions workflows for generating changelogs for both the VS2019 and VS2022 versions. Specifically, the "Update changelog in release" step in each workflow is modified to use version v1.3.1 of the tubone24/update_release action instead of the previously used v1.0. No other modifications to the workflow logic, environment variables, or control flow are present.

Changes

File(s) Change Summary
.github/workflows/generate-changelog-vs2019.yml,
.github/workflows/generate-changelog-vs2022.yml
Updated the tubone24/update_release GitHub Action in the "Update changelog in release" step from v1.0 to v1.3.1. No other changes made.

Assessment against linked issues

Objective Addressed Explanation
Ensure MSBuild props/targets from NuGet package are imported into a nanoFramework project (#1544) The changes are limited to workflow configuration and do not address project file import logic.

Tip

⚡💬 Agentic Chat (Pro Plan, General Availability)
  • We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e6e88de and 20fa89a.

⛔ Files ignored due to path filters (4)
  • Tools.BuildTasks-2019/FixNuGetPackagePropsTargetsImportsTask.cs is excluded by none and included by none
  • Tools.BuildTasks-2019/Tools.BuildTasks-2019.csproj is excluded by none and included by none
  • Tools.BuildTasks-2022/Tools.BuildTasks-2022.csproj is excluded by none and included by none
  • VisualStudio.Extension-2022/Targets/NFProjectSystem.MDP.targets is excluded by none and included by none
📒 Files selected for processing (2)
  • .github/workflows/generate-changelog-vs2019.yml (1 hunks)
  • .github/workflows/generate-changelog-vs2022.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: nanoframework.nf-Visual-Studio-extension (VS2022)
  • GitHub Check: nanoframework.nf-Visual-Studio-extension (VS2019)
  • GitHub Check: nanoframework.nf-Visual-Studio-extension (Get_Build_Options)
🔇 Additional comments (2)
.github/workflows/generate-changelog-vs2022.yml (1)

42-42: Upgrade action version is correct and safe.

The update to tubone24/[email protected] is appropriate and aligns with best practices for keeping actions up to date.

.github/workflows/generate-changelog-vs2019.yml (1)

42-42: Action version upgrade is appropriate.

The update to tubone24/[email protected] is correct and ensures consistency across workflows.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@josesimoes
Copy link
Member

This task is doing more than the "announced": its cleaning up stuff in the nfproj file depending on what is listed in the packages lock file.
It runs on every build, so it adds time to the build and, 99.999999% of the time, won't be doing anything useful.

I would rather see this fixed upstream and have the task do whatever it needs to do when a package is added/updated rather than running on every build.
I've added a mention to this on the NuGet issue referenced above.

Being a "one-time" event (like when creating a new lib and adding the imports to StyleCop) I'm more than fine to accept the fact that I have to manually edit the nfproj once and that's it.
As far as I understand it (at least it works with nbgv) as long as the imports are in the nfproj file, the update process takes care of updating the versions. At least that's what happens with nbgv...

@frobijn
Copy link
Contributor Author

frobijn commented Apr 15, 2025

This task is doing more than the "announced": its cleaning up stuff in the nfproj file depending on what is listed in the packages lock file.

It is a synchronization of the packages.config indeed. AFAIK, based on the NuGet code, only packages listed in packages.config are investigated for *.props and *.targets files. And all imports that result from this mechanism are uniquely identified by their path: "<package id>....". The "algorithm" is exactly the same as the NuGet code uses, so it functionally is an exact copy of the behavior that is lacking.

It runs on every build, so it adds time to the build and, 99.999999% of the time, won't be doing anything useful.

I don't think there is any other way than to run a task every build. But if you have ever looked at the detailed log of a build, an extra task add only 0.000000001% to the total execution time of a build. There is a lot of stuff going on that is completely unnecessary, but that's the way a build works.

If you are unhappy about the 0.000000001% time increase, there are ways to optimize performance. E.g., by writing the hash and/or date/time of packages.config in a file "obj\nF\package.sync" and only running the full task if the packages.config is out of date.

I would rather see this fixed upstream and have the task do whatever it needs to do when a package is added/updated rather than running on every build. I've added a mention to this on the NuGet issue referenced above.

I've also tried to poke Microsoft via the issue that I've created about this. All that is met with a deafening silence. I don't think a reaction will be given before nanoFramework switches to the new project format.

There may be another way to hook into that via extensibility interfaces: https://learn.microsoft.com/en-us/nuget/visual-studio-extensibility/nuget-api-in-visual-studio#ivspackageinstallerevents-interface, but I have no clue how to program those and I can't find any code examples.

Being a "one-time" event (like when creating a new lib and adding the imports to StyleCop) I'm more than fine to accept the fact that I have to manually edit the nfproj once and that's it.

Unless you are a user like AlbertK and have no clue why your package is not working. I don't think this task is primarily meant for you. I have several packages that use the msbuild-mechanism (and yes, I'll still plan to make PR's for them) that are used by multiple projects, and a manual update of the projects for new package versions is quite a tedious job (I don't have Azure pipelines for that).

As far as I understand it (at least it works with nbgv) as long as the imports are in the nfproj file, the update process takes care of updating the versions. At least that's what happens with nbgv...

No it doesn't. If you have a valid .nfproj file with the imports and use the package manager to change the installed version of nbgv, none of the imports are updated. For nanoFramework repositories you may not encounter this as the version updates seem to be updated by one of the pipelines.

So, what do you want to do with this PR? It's fine with me if you still don't like it. Then I'll integrate this task in my collection of build tools and stop nagging about this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MSBuild props/targets from NuGet package are not imported into a nanoFramework project
4 participants