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

--use-targets : switch for using msbuid targets #302

Closed
wants to merge 3 commits into from
Closed

--use-targets : switch for using msbuid targets #302

wants to merge 3 commits into from

Conversation

pchalamet
Copy link

As discussed, this is the first step for issue #197.
Next steps will be as follow:

  • implementation for msbuild targets (convert-from-nuget)
  • implementation for restore
  • ensure tests compatibilities across platforms

Feature will be disable by default until everything is testing in the wild.

@pchalamet pchalamet mentioned this pull request Oct 27, 2014
this.ConfigureReference(installModel, useTargets)

member private this.ConfigureReference(installModel : InstallModel, useTargets) =
if useTargets then
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't it be the other way around?
if not useTargets then do old way

Copy link
Author

Choose a reason for hiding this comment

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

Yes you're right.

@forki
Copy link
Member

forki commented Oct 28, 2014

I notice a new issue which might interfere with this one.
It looks like we need to generate one complete install model.

#297 shows that multiple package can reference the same framework assembly references. So we need to merge all install models and remove the duplicates. ;-(

@pchalamet
Copy link
Author

Merge I will rebase later on... I'm a bit used to this these times ;-)

@pchalamet
Copy link
Author

I reviewed quickly #297 and both could be merged altogether without interference probably. This patch actually does not change GenerateXml ; next one will.

@forki
Copy link
Member

forki commented Oct 29, 2014

no my question was: how does this change the strategy?

I think it's not possible to split the installation by package. We need to generate one god install targets file.

@forki
Copy link
Member

forki commented Oct 29, 2014

so we would have one targets file per project and not one targets file per package.

that's messing with the plan, right?

@pchalamet
Copy link
Author

Yes, the idea is to have one import per package - not per project.

Could you explain why we should go to that way ?

  • Pierre

On 29 Oct 2014, at 08:50, Steffen Forkmann notifications@github.com wrote:

so we would have one targets file per project and not one targets file per package.

that's messing with the plan, right?


Reply to this email directly or view it on GitHub.

@forki
Copy link
Member

forki commented Oct 29, 2014

The issue showed the following case: project A and B reference the same
framework assembly. Now we install it twice.

I fixed it by building a complete model per project and removing
duplicates.

I hate that, but I don't see a better way, yet.
On Oct 29, 2014 9:11 AM, "Pierre Chalamet" notifications@github.com wrote:

Yes, the idea is to have one import per package - not per project.

Could you explain why we should go to that way ?

  • Pierre

On 29 Oct 2014, at 08:50, Steffen Forkmann notifications@github.com
wrote:

so we would have one targets file per project and not one targets file
per package.

that's messing with the plan, right?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#302 (comment).

@pchalamet
Copy link
Author

But fx assemblies must be out of this scheme as they are tied to project - not packages. Assemblies reference inside a package are solely determined considering fx target version.

  • Pierre

On 29 Oct 2014, at 09:15, Steffen Forkmann notifications@github.com wrote:

The issue showed the following case: project A and B reference the same
framework assembly. Now we install it twice.

I fixed it by building a complete model per project and removing
duplicates.

I hate that, but I don't see a better way, yet.
On Oct 29, 2014 9:11 AM, "Pierre Chalamet" notifications@github.com wrote:

Yes, the idea is to have one import per package - not per project.

Could you explain why we should go to that way ?

  • Pierre

On 29 Oct 2014, at 08:50, Steffen Forkmann notifications@github.com
wrote:

so we would have one targets file per project and not one targets file
per package.

that's messing with the plan, right?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#302 (comment).


Reply to this email directly or view it on GitHub.

@forki
Copy link
Member

forki commented Oct 29, 2014

so you vote for hybrid?

2014-10-29 9:37 GMT+01:00 Pierre Chalamet notifications@github.com:

But fx assemblies must be out of this scheme as they are tied to project -
not packages. Assemblies reference inside a package are solely determined
considering fx target version.

  • Pierre

On 29 Oct 2014, at 09:15, Steffen Forkmann notifications@github.com
wrote:

The issue showed the following case: project A and B reference the same
framework assembly. Now we install it twice.

I fixed it by building a complete model per project and removing
duplicates.

I hate that, but I don't see a better way, yet.
On Oct 29, 2014 9:11 AM, "Pierre Chalamet" notifications@github.com
wrote:

Yes, the idea is to have one import per package - not per project.

Could you explain why we should go to that way ?

  • Pierre

On 29 Oct 2014, at 08:50, Steffen Forkmann notifications@github.com

wrote:

so we would have one targets file per project and not one targets
file
per package.

that's messing with the plan, right?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#302 (comment).


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#302 (comment).

@pchalamet
Copy link
Author

Don't know what hybrid is but if it means:

  • manage assemblies reference inside project
  • manage direct assembly references in a package as target or inline (depending on the switch)
    I'll buy it definitively.

My goal is just to be able to upgrade packages without changing project definition. Not to smoothly switch from fx 4,0 to 4.5 for example.

Just to explain the use case, I'm currently building a project named "full-build" to easily switch a project to/from source to/from binaries inside a big solution. And I want to handle 3rd parties consistency globally without changing anything inside project definitions - that's where Paket will be used. And for that, I need targets files to handle 3rd parties.

@forki
Copy link
Member

forki commented Oct 29, 2014

My goal is just to be able to upgrade packages without changing project definition.

unfortunately the latest case (with assembly framework references) seems to make this impossible. At least there are some cases where I don't see a solution.

@mexx
Copy link
Member

mexx commented Oct 29, 2014

unfortunately the latest case (with assembly framework references) seems to make this impossible.

Can we treat framework references as own packages, so they can participate in resolution process and be part of install model?

@forki
Copy link
Member

forki commented Oct 29, 2014

Mhm. That's an interesting idea.
On Oct 29, 2014 11:09 AM, "Max Malook" notifications@github.com wrote:

unfortunately the latest case (with assembly framework references) seems
to make this impossible.

Can we treat framework references as own packages, so they can participate
in resolution process and be part of install model?


Reply to this email directly or view it on GitHub
#302 (comment).

@mexx
Copy link
Member

mexx commented Oct 29, 2014

Yea, but after a second look I realized, they unfortunately do not behave like packages.
The dependency on them is conditioned by the fx version and the package dependent upon it.
So we couldn't reuse the package of framework reference over different direct dependencies.
We have to find another way to do this.

Could we populate some list of already added fx references in the msbuild and than make the includes conditional based on whether an fx reference is already added or not?

@forki
Copy link
Member

forki commented Oct 29, 2014

Yes one could do this.
Actually that was my first approach. But it's really complicated to get
right with all the fallbacks.
On Oct 29, 2014 11:19 AM, "Max Malook" notifications@github.com wrote:

Yea, but after a second look I realized, they unfortunately do not behave
like packages.
The dependency on them is conditioned by the fx version and the package
dependent upon it.
So we couldn't reuse the package of framework reference over different
direct dependencies.
We have to find another way to do this.

Could we populate some list of already added fx references in the msbuild
and than make the includes conditional based on whether an fx reference is
already added or not?


Reply to this email directly or view it on GitHub
#302 (comment).

@pchalamet
Copy link
Author

I've implemented my own package manager into full-build to generate *.targets files. I can now get rid of Paket.

@pchalamet pchalamet closed this Nov 3, 2014
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