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

Paket should import build targets from packages in dependency groups #1674

Merged
merged 1 commit into from May 10, 2016

Conversation

Projects
None yet
3 participants
@sergey-tihon
Member

sergey-tihon commented May 10, 2016

Not sure it is directly related to #1663
But after paket.exe update with version 2.64.2, paket removed by build targets from project file
image

@sergey-tihon sergey-tihon changed the title from Paket should import build targets for packages in dependency groups to Paket should import build targets from packages in dependency groups May 10, 2016

@inosik

This comment has been minimized.

Show comment
Hide comment
@inosik

inosik May 10, 2016

Contributor

The problem is the group name "Build". Other names seem to work fine.

Seems to be the same kind of problem as we had with "Lib"-named packages.

Contributor

inosik commented May 10, 2016

The problem is the group name "Build". Other names seem to work fine.

Seems to be the same kind of problem as we had with "Lib"-named packages.

@sergey-tihon

This comment has been minimized.

Show comment
Hide comment
@sergey-tihon

sergey-tihon May 10, 2016

Member

@inosik How group name affects Paket behavior?

Member

sergey-tihon commented May 10, 2016

@inosik How group name affects Paket behavior?

@inosik

This comment has been minimized.

Show comment
Hide comment
@inosik

inosik May 10, 2016

Contributor

It expects .targets and .props files to be directly in packages/Foo/build/ or in a framework specific sub-directory.

Now, if a group is named "Build", you get a path like packages/build/Foo/build/Foo.targets. I assume Paket checks only if Foo.targets is in packages/build/ or in packages/build/{framework}/, and if it isn't, it considers it as ignorable.

I tried installing the package from your screenshot and Paket installed it correctly when the group wasn't named "Build". With "Build", it didn't import the .targets file.

Contributor

inosik commented May 10, 2016

It expects .targets and .props files to be directly in packages/Foo/build/ or in a framework specific sub-directory.

Now, if a group is named "Build", you get a path like packages/build/Foo/build/Foo.targets. I assume Paket checks only if Foo.targets is in packages/build/ or in packages/build/{framework}/, and if it isn't, it considers it as ignorable.

I tried installing the package from your screenshot and Paket installed it correctly when the group wasn't named "Build". With "Build", it didn't import the .targets file.

@sergey-tihon

This comment has been minimized.

Show comment
Hide comment
@sergey-tihon

sergey-tihon May 10, 2016

Member

Just wonder how it worked before )

Member

sergey-tihon commented May 10, 2016

Just wonder how it worked before )

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki May 10, 2016

Member

@inosik so it's a string trimming issue, right?

Member

forki commented May 10, 2016

@inosik so it's a string trimming issue, right?

@inosik

This comment has been minimized.

Show comment
Hide comment
@inosik

inosik May 10, 2016

Contributor

I think so. We had something similar with packages which ended with "Lib", but I can't find the issue right now.

Contributor

inosik commented May 10, 2016

I think so. We had something similar with packages which ended with "Lib", but I can't find the issue right now.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki May 10, 2016

Member

@sergey-tihon if we have a repro I will try to fix it

Member

forki commented May 10, 2016

@sergey-tihon if we have a repro I will try to fix it

@forki forki closed this May 10, 2016

@forki forki reopened this May 10, 2016

@sergey-tihon

This comment has been minimized.

Show comment
Hide comment
@sergey-tihon

sergey-tihon May 10, 2016

Member

@forki This PR is repro

Member

sergey-tihon commented May 10, 2016

@forki This PR is repro

@forki forki merged commit c71fab3 into fsprojects:master May 10, 2016

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details

forki added a commit that referenced this pull request May 10, 2016

@sergey-tihon

This comment has been minimized.

Show comment
Hide comment
@sergey-tihon

sergey-tihon May 10, 2016

Member

It works!
Thanks again @forki !

Member

sergey-tihon commented May 10, 2016

It works!
Thanks again @forki !

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