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

Use -refout to auto generate ref assemblies #175

Open
Shyam-Gupta opened this Issue Dec 6, 2018 · 15 comments

Comments

Projects
None yet
7 participants
@Shyam-Gupta
Member

Shyam-Gupta commented Dec 6, 2018

Instead of having separate 'ref' projects in the solution, use -refout to generate ref assemblies automatically from src assemblies.

Details: https://github.com/dotnet/roslyn/blob/233957b90f555c731f8e22b5319bb7e869567580/docs/features/refout.md

@zsd4yr

This comment has been minimized.

Member

zsd4yr commented Dec 7, 2018

Starting a discussion here after talking about this issue with @ericstj

@zsd4yr

This comment has been minimized.

Member

zsd4yr commented Dec 7, 2018

If we make this change to -refout, we will only have the ref dll's; not the ref source.

Reasons we might want ref sources:

  1. You have a nice concise record of the public API that can be reviewed by API design
  2. It forces you to make concious changes to public API and consider if they are breaking.
  3. It allows you to make some changes only in the reference or implementation (like adding an obsolete attribute, or having publics that aren't exposed to the public).
  4. ability to run the build implementation in parallel (this may not be a huge gain for us as we have relatively few assemblies)

in particular, we should note benefit 3.

@zsd4yr

This comment has been minimized.

Member

zsd4yr commented Dec 7, 2018

if we used the /t:GenerateReferenceSource option, we could rebuild reference from source when necessary (giving us the same benefits of the -refout flag) but still allowing us to have these benefits because we have the reference source

@AdamYoblick

This comment has been minimized.

Member

AdamYoblick commented Dec 7, 2018

I really like 2 and 3. Especially 2. If forces us to manually recognize public api changes instead of being able to forget and the build just goes on as normal.

@zsd4yr

This comment has been minimized.

Member

zsd4yr commented Dec 7, 2018

We also IMHO 'ought to start including apicompat as part of our public builds here. if the reference is incompatible with the implementation, our build should fail. @AdamYoblick do you agree? I can create an issue and try to add it as a step to our process 😄

@AdamYoblick

This comment has been minimized.

Member

AdamYoblick commented Dec 7, 2018

Yeah apicompat is fine but that eliminates the option for benefit 3 (unless the apicompat handles the case where the source has MORE publics than the ref, then we're ok).

But yes, apicompat should be automatic as part of the CI build.

@zsd4yr

This comment has been minimized.

Member

zsd4yr commented Dec 7, 2018

we can check the apicompat in one direction but not the other?

@zsd4yr

This comment has been minimized.

Member

zsd4yr commented Dec 7, 2018

for example, I apicompat is capable of ensuring the implementations satisfy the reference without ensuring that references parity the implementation.

We 'ought to be able to do the same with core and framework to ensure that core is a superset of framework if we want

@AdamYoblick

This comment has been minimized.

Member

AdamYoblick commented Dec 7, 2018

I know nothing about apicompat 😆

@tannergooding

This comment has been minimized.

Member

tannergooding commented Dec 7, 2018

One of the big problems with not using the built-in compiler refout feature, however, is that it is very easy to end up in a world where the information you are including or excluding does not match what the C# compiler thinks is should be included/excluded.

There have been issues in the past where the /t:GenerateReferenceSource option in CoreFX has not matched (and still may not match) the C# view that has caused issues. One such example is the inclusion of private fields for structs, which impacts unsafe code and whether or not you can take the address of a given object. -- CC. @jaredpar

@zsd4yr zsd4yr self-assigned this Dec 7, 2018

@jaredpar

This comment has been minimized.

Member

jaredpar commented Dec 7, 2018

Indeed the CoreFX tooling for reference assemblies was designed without consulting the compiler team. As a result it turned out they made a variety of decisions that caused their reference assemblies to be observably different from there implementation assemblies. This caused a deal of customer confusion as subtle changes to their build caused them to silently switch between reference + implementation assemblies and as a result get different compilation results.

Concrete example:

Guid g; 
Console.WriteLine(g.ToString());

Does this compile or not? The answer is, frustratingly, it depends because of choices the CoreFX team made in reference assembly generation.

The refout and refonly compiler features are designed to produce reference assemblies that have complete fidelity for compiling with the implementation assembly. I strongly suggest you use that if possible for generating reference assemblies.

Note: we have worked with the CoreFX team and addressed many of the tooling issues. Hence it should be less of an issue going forward. But you can still run into version compat issues as the language evolves. That can't happen with refonly and refout.

@zsd4yr

This comment has been minimized.

Member

zsd4yr commented Dec 7, 2018

@jaredpar is there an issue in either roslyn or corefx to bring these two closer together that we can link here? I'd love to read more on what the difference are, as I think there's some benefits here to having the source

@jaredpar

This comment has been minimized.

Member

jaredpar commented Dec 7, 2018

@zsd4yr the corefx story is complicated. They have essentially the most complicated reference assembly generation story possible. Their tooling is designed to support their specific scenario. And that meants they have an on-going tax: when we change the language, they likley have to respond to how they generate reference assemblies. We have a tighter loop now to keep the fidelity up but it's still a different tool.

@zsd4yr

This comment has been minimized.

Member

zsd4yr commented Dec 7, 2018

That makes sense. We probably don't want this change to make us less agile in rev'ing the language we support

@danmosemsft

This comment has been minimized.

Member

danmosemsft commented Dec 11, 2018

cc @ericstj as we were discussing this

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