Skip to content

Re-add SDK refactor with referenced assembly scanning fixed.#2347

Merged
jviau merged 10 commits intomainfrom
jviau/sdk
Apr 3, 2024
Merged

Re-add SDK refactor with referenced assembly scanning fixed.#2347
jviau merged 10 commits intomainfrom
jviau/sdk

Conversation

@jviau
Copy link
Copy Markdown
Contributor

@jviau jviau commented Mar 13, 2024

Issue describing the changes in this PR

resolves #issue_for_this_pr

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)

Additional information

This re-adds the SDK overhaul that was reverted in #2313. The cause for the reversion has also been addressed. The root issue was that we were identifying what referenced assemblies to scan by looking at all dll's next to the final output assembly of the functions project. With the SDK refactoring this was no longer possible as our targets ran earlier (before being copied to final output location). Instead, we directly use the ReferencePath item group from MSBuild to identify all assemblies that will be copied to the final output directory.

@jviau jviau requested review from fabiocav and kshyju March 14, 2024 19:18
@fabiocav
Copy link
Copy Markdown
Member

@jviau it might be helpful to share a link for a diff against a branch with the state we had in 1.17.0 so we can focus on the delta. Is that something you can provide?

Comment thread sdk/Sdk/FunctionMetadataGenerator.cs
@jviau
Copy link
Copy Markdown
Contributor Author

jviau commented Mar 25, 2024

@jviau it might be helpful to share a link for a diff against a branch with the state we had in 1.17.0 so we can focus on the delta. Is that something you can provide?

https://github.com/Azure/azure-functions-dotnet-worker/compare/sdk-1.17.0..jviau/sdk

Here is a diff. It isn't very helpful though as it also includes every unrelated commit between the two as well. Do you know a way to get a cleaner diff?

In the meantime, I can guide the review a bit here:

  1. I left a comment explaining the core change here
  2. All changes to /test and /samples folder is to support a new e2e test I added to verify this change. The e2e test changes lets us avoid writing to the test csproj's to update the SDK version. Instead we pass in the version to use as part of the build. And we always build/pack with a version of 99.99.99 during e2e test, so we know what version to pass in.

Comment thread build/Sdk.slnf
Comment thread sdk/Sdk/FunctionMetadataGenerator.cs
Comment thread sdk/Sdk/Targets/Microsoft.Azure.Functions.Worker.Sdk.targets
Copy link
Copy Markdown
Member

@kshyju kshyju left a comment

Choose a reason for hiding this comment

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

Let's get the preview version out for E2E testing.

Comment thread sdk/Sdk/FunctionMetadataGenerator.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants