Repository navigation
Expand the build infrastructure support #867
Description
The build infrastructure for ImageSharp is rather limited today and doesn't always follow best practices.
It would be ideal if this could be updated to have cross-platform build scripts, rely on things like Directory.Build.props/targets for centralizing build metadata, and generally be updated to do things like work with long paths, with paths that have spaces in the name, and where the working directory is not the repo root.
Activity
tannergooding commentedon Mar 29, 2019
@JimBobSquarePants, I know I had discussed this with you briefly at the MVP summit; but if I were to provide some fixes here; would you prefer it incrementally or in one big PR?
tannergooding commentedon Mar 29, 2019
Would you also prefer it in a PR or with a layout of changes discussed here first?
JimBobSquarePants commentedon Mar 30, 2019
Hey @tannergooding
Yeah this would be great! Looking forward to seeing what will change.
I think the best bet would be a layout of changes here and everything in a big fat PR. That way we can avoid any interim complexity in the build process.
tannergooding commentedon Apr 22, 2019
I think there a couple potential options here:
dotnet/arcade. This is what Roslyn, CoreFX, and other repositories are currently using. CoreCLR and other dotnet repositories are planning on moving to it as well.The process overall would be fairly similar for both, mostly just differing in whether you have code props/targets locally or if you are directing those same props/targets to the
dotnet/arcadeNuGet package.The primary recommendations I have are:
Directory.Build.propsandDirectory.Build.targetsto share common project properties/targets/etc. This also makes it easy to bring down shared toolset packages or analyzers.shandcmdversions (thecmdversion forwards tops1). This also needs to handle common issues like paths that have a space in their name, pulling down build dependencies, or triggering the scripts from any folder (not just the repo root). It should also provide a consistent interface across all platforms/architectures (so you can easily move between Linux, Windows, OSX, etc).I've been going with the second option (customized approach) in my own projects as I find it easier to understand/reason about and find it I can't be caught in surprise by changes that arcade makes for dotnet which might not be applicable to me. The drawback, is I'm responsible for my own infrastructure and bringing functionality on-board as needed.
You can see an example of my current infrastructure here: https://github.com/terrafx/terrafx. It isprimarily centered around easy "inner loop" scripts in the repo root that forward to the real build scripts in the
scriptsfolder. There is then the rootDirectory.Build.props/targetswhich defines core properties like language version, packing options, NuGet restore sources, and the Roslyn Compiler Toolset to depend on. Thesourcesandtestsfolders then have their ownDirectory.Build.props/targets(which import the root ones) and define things specific to them. These include the testing dependencies, InternalsVisibleTo support, and build output subdirectories (which I find make it easier to locate things in my artifacts folder).JimBobSquarePants commentedon Apr 23, 2019
Lots of great ideas and info here, thanks!
I'm undecided.... I appreciate the risks involved in taking on Arcade but I also believe that any breaking changes will most likely have well documented resolution.
tannergooding commentedon Apr 25, 2019
Let me know when you have made a decision and I'm happy to get the PR up for whichever you prefer.
JimBobSquarePants commentedon Apr 25, 2019
Let’s go with Arcade, that way we can copy MS when something changes. Thanks, really appreciate this! 😄
antonfirsov commentedon Apr 25, 2019
Does the underlying CI infrastructure matter, when considering the options? Currently we are stuck with AppVeyor+Travis, but we are planning to migrate to Azure Pipelines in the future.
tannergooding commentedon Apr 25, 2019
It shouldn't. The majority of it is controlled via centralized build scripts that CI is configured to call.
antonfirsov commentedon Apr 25, 2019
Arcade sounds fine to me.
tannergooding commentedon Apr 30, 2019
Ok, so I had been working on getting the arcade support working but the arcade team has indicated that it is only intended for Microsoft owned repos. This is partially because it has some explicit support for some internal tooling (like MicroBuild) but also because it is believed that most of the core functionality (such as localization, versioning, source linking, etc) should be part of the .NET SDK instead and bugs should be logged against .NET SDK, MSBuild, and NuGet when that functionality is missing.
I'm still happy to help rev the infrastructure, but unfortunately, it looks like dotnet/arcade is not going to be a viable (or at least not supported) path forward.
JimBobSquarePants commentedon Apr 30, 2019
Thanks for investigating; we can live with that, plan B it is! 😄
2 remaining items
tannergooding commentedon May 1, 2019
Then I can include the support for it as part of these infrastructure changes. We just need to determine who generates the original SNK file and then have it checked in (unmodified) so that it can be consumed and enabled by the infrastructure PR.
antonfirsov commentedon May 1, 2019
@tannergooding that would be great!
@JimBobSquarePants will correct me if I am wrong, but I don't think we have any preference or expectation regarding SNK generation. As far as I am aware, you are free to do it on your branch, if you want.
JimBobSquarePants commentedon May 2, 2019
@antonfirsov @tannergooding Am I correct in saying we care about is that whoever generates the public key just needs to send us the private key partner? (I know next to nothing about SNK generation).
If I'm right just generate one as part of this PR and send us the requisite parts.
tannergooding commentedon May 2, 2019
@JimBobSquarePants, it's a bit simpler than that even. Someone just needs to create the key and check it into source control: https://docs.microsoft.com/en-ca/dotnet/standard/library-guidance/strong-naming.
This is fine since SNKs aren't used for security and it just allows users to produce the same deterministic bits. You'll notice that other repos, such as Roslyn, have their public/private key combination checked in as well.
tannergooding commentedon May 18, 2019
Got some more work done on this, changes are still in https://github.com/tannergooding/ImageSharp/tree/infrastructure.
It now builds successfully and I believe the remaining work is:
Fix up the yml scripts for CItannergooding commentedon May 18, 2019
The interesting bits are likely:
.editorconfigto have the current settings + all code style defaults that weren't specifiedcsharp_indent_case_contents_when_block, which I set tofalsesince most people don't want the double indentation that is otherwise the defaultend_of_linedefault was changed tolfsince that is what git will check it in as (all platforms) and is what git will check it out as on Unix. It makes for better cross-platform consistency.JimBobSquarePants commentedon May 25, 2019
Apologies for the slow reply. That's all looking good.
So how does Directory.Build.Props work with nuget then? Does it automatically update as we install or update new dependencies?
tannergooding commentedon May 25, 2019
Unfortunately, not very well today. The tooling in general isn't super supportive of centrally managed properties yet and most UX will still modify the project file instead of your Directory.Build.props.
It is likely something that will get addressed eventually, especially since this is a model followed by the dotnet repos.
tannergooding commentedon May 27, 2019
@JimBobSquarePants, I think I've got basically everything working for the Directory.Build.props/targets changes and I could put that up as an initial PR (which would also avoid needing to continually fix up things as the repo progresses).
The only blocker looks to be figuring out how signing should work. More specifically, some of the external SixLabors dependencies need to be signed first, which means you may want the key in as part of "standard".
For that to happen, I would need to submit a PR checking the SNK into https://github.com/SixLabors/Standards and then also update whatever produces
SixLabors.Core,SixLabors.Fonts,SixLabors.Shapes, andSixLabors.Shapes.Textto build with that SNK, correct?JimBobSquarePants commentedon May 28, 2019
Yeah that's correct. If we add it there it can be consumed everywhere. I'm happy to progress as soon as you are. 👍
antonfirsov commentedon Aug 11, 2019
@tannergooding I guess certain parts of the top-level
Directory.Build.propsshall be moved to SixLabors/Standards so we can reuse common stuff like strong naming in all our repos.Is this reasonable? Easy/hard to achieve?
tannergooding commentedon Aug 12, 2019
Its reasonably easy. The only real requirement here is that you still need to have a
Directory.Build.props/Directory.Build.targetsin the repo root which imports the shared props/targets (which would be under thestandardssubmodule folder).The same goes for other places where you want to import some shared props/targets and only have it impact the projects under that folder.
That is, Directory.Build.props/targets are still searched and imported from the current folder up.
So, if you wanted some shared solution-wide props/targets, some shared source only props/targets, and some shared test only props/targets; you might have a setup like the following:
Directory.Build.props-- importsstandards\{name1}.propsDirectory.Build.targets-- importsstandards\{name1}.targetsDirectory.Build.props-- importsstandards\{name2}.props, importsrepo-root\Directory.Build.propsDirectory.Build.targets-- importsstandards\{name2}.targets, importsrepo-root\Directory.Build.targetsDirectory.Build.props-- importsstandards\{name3}.props, importsrepo-root\Directory.Build.propsDirectory.Build.targets-- importsstandards\{name3}.targets, importsrepo-root\Directory.Build.targetsetc
antonfirsov commentedon Aug 12, 2019
Makes sense, thanks! I think this should be the next step, so we can uniformize our builds across the repository tree + enable strong naming everywhere.
@tannergooding any chance you can help with it in the near future (1-2 months)?
@dlemstra any chance you can touch this topic while dealing with GitHub CI?
JimBobSquarePants commentedon Apr 21, 2020
I'm gonna close this. We've got strong naming and we've made many infrastructural changes to the build since this was raised. Any specific issues can be raised again in the future.