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

fixes 3331 - removes nuspec from microservice runtime #3368

Merged
merged 7 commits into from
May 8, 2024
Merged

Conversation

cdhanna
Copy link
Collaborator

@cdhanna cdhanna commented May 7, 2024

Nuget and Dotnet have become friendlier over the years, but in the past, they were sort of at odds and trying to compete with eachother.

In a csproj file, you can define dependencies of your project, and that is great 👍
in a nuspec file, you can define dependencies of your package, and that is also great! 👍

But technically, those dependencies (of the project vs the package) can be different, and oh boy, here we go. There is a semi common issue in large dotnet solutions where you want to publish a package that depends on projects that don't have published packages. That is our problem. We want to publish Beamable.Microservice.Runtime, but our internal code organization dictates we need the beamable.tooling.common dll or whatever (that is just one example of many), but we don't publish beamable.tooling.common as a nuget package on its own, because at the moment, it doesn't make sense as a customer product on its own.

Even if we did publish it, like we publish beamable.common, our build pipeline would need large changes to be able to handle the publication of the packages in sequence with the correct interdepenencies.

So, our Beamable.Microservice.Runtime release package tries to sneak in extra .dll files into its manifest than just the BeamableBase dll. That used to happen by overriding the nuspec file.
But that stunk, because it meant we also needed to copy/paste all the nuget references from the .csproj file into the .nuspec file. There doesn't seem to be a good way to merge the deps defined in one file with the other file; its all or nothing on the nuspec file.

  1. it stinks, and leads to bugs... In fact, there was a bug! The Mongo.Driver dependency was being referenced by a sub project, (beamable.server.common), but was only getting into the Beamable.Microservice.Runtime package because of the nuspec configuration, not through proper project references, which I'm sure is part of the culprit for why mongo references always seem broken...
  2. our build process has this strange extra detail for ONLY the Beamable.Microservice.Runtime, where it needs to track this stupid nuspec file
  3. our handling of multi-framework was crummy, because the nuspec was only copying in the net6.0 build artifacts, which meant we couldn't support multiple dotnet environments. I also wonder if this is responsible for some of the recent net8.0 pain.

SO!

I deleted the nuspec file and figured out how to reproduce everything we want in the csproj file.

Copy link
Contributor

github-actions bot commented May 7, 2024

Lightbeam link

Copy link
Contributor

github-actions bot commented May 7, 2024

Lightbeam link

@cdhanna
Copy link
Collaborator Author

cdhanna commented May 7, 2024

okay, need to do a few more things on this,

  1. the common project now references the beamable.common library in order to get access to the build-target, but we need to make sure it has the latest version of the reference...
  2. test more stuff,
  3. research if we can inject properties BEFORE the build stuff...

Copy link
Contributor

github-actions bot commented May 7, 2024

Lightbeam link

Copy link
Contributor

github-actions bot commented May 8, 2024

Lightbeam link

Copy link
Contributor

github-actions bot commented May 8, 2024

Lightbeam link

Copy link
Contributor

github-actions bot commented May 8, 2024

Lightbeam link

@cdhanna
Copy link
Collaborator Author

cdhanna commented May 8, 2024

reviewed by Gabriel

@cdhanna cdhanna merged commit bfdf551 into main May 8, 2024
26 checks passed
@cdhanna cdhanna deleted the issue/3331 branch May 8, 2024 19:57
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.

2 participants