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

Symbols source #1383

Merged
merged 3 commits into from Jan 14, 2016
Merged

Symbols source #1383

merged 3 commits into from Jan 14, 2016

Conversation

pms1969
Copy link
Contributor

@pms1969 pms1969 commented Jan 13, 2016

changed the way I thought about the problem.... now works much better, and the build.cmd nuget command should work as expected.

Paul Saunders added 3 commits January 13, 2016 13:27
* removed change of working directory
* ensure target is set correctly
* make sure "build.cmd nuget" completes successfully
Conflicts:
	src/Paket.Core/PackageMetaData.fs
	src/Paket.Core/ProjectFile.fs
@pms1969
Copy link
Contributor Author

pms1969 commented Jan 14, 2016

is this ok?

@forki forki merged commit 2ad5227 into fsprojects:master Jan 14, 2016
@forki
Copy link
Member

forki commented Jan 14, 2016

I had to resolve merge conflicts since @cloudRoutine send it some refactorings in that area.
please review dc7ec47

thx

@pms1969
Copy link
Contributor Author

pms1969 commented Jan 14, 2016

doh, just spotted a problem with it (linked files). I'll fix asap (might be early tomorrow) and get you another pull request.

@forki
Copy link
Member

forki commented Jan 14, 2016

Cool thx
On Jan 14, 2016 20:46, "Paul Saunders" notifications@github.com wrote:

doh, just spotted a problem with it (linked files). I'll fix asap (might
be early tomorrow) and get you another pull request.


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

@harrisonmeister
Copy link

Can I ask why this has been done? I now have a bunch of internal nuget packages which have their dependencies added, which makes for larger internal nuget packages

@forki
Copy link
Member

forki commented Jan 15, 2016

Really? I thought it was under switch and only for symbol packages

@forki
Copy link
Member

forki commented Jan 15, 2016

Could please elaborate a bit on the concrete issue?

@harrisonmeister
Copy link

By symbol packages, do you mean project references? That's what I'm seeing. When I run a paket hard install, I get the Dependency twice, e.g.:

ProjectB has a project reference to ProjectA. When I reference ProjectB in some other project, say ProjectC, I see the below - which seems odd that it would appear twice. (It might be that this is right)

<Choose>
    <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And ($(TargetFrameworkVersion) == 'v4.0' Or $(TargetFrameworkVersion) == 'v4.5')">
      <ItemGroup>
        <Reference Include="ProjectA">
          <HintPath>..\..\..\..\packages\ProjectA\lib\net40\ProjectA.dll</HintPath>
          <Private>True</Private>
          <Paket>True</Paket>
        </Reference>
      </ItemGroup>
    </When>
  </Choose>
  <Choose>
    <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And ($(TargetFrameworkVersion) == 'v4.0' Or $(TargetFrameworkVersion) == 'v4.5')">
      <ItemGroup>
        <Reference Include="ProjectA">
          <HintPath>..\..\..\..\packages\ProjectB\lib\net40\ProjectA.dll</HintPath>
          <Private>True</Private>
          <Paket>True</Paket>
        </Reference>
        <Reference Include="ProjectB">
          <HintPath>..\..\..\..\packages\ProjectB\lib\net40\ProjectB.dll</HintPath>
          <Private>True</Private>
          <Paket>True</Paket>
        </Reference>
      </ItemGroup>
    </When>
  </Choose>

@forki
Copy link
Member

forki commented Jan 15, 2016

ProjectA.dll shouldn't be in Project B nuget package. that is clearly a bug.

I think https://github.com/fsprojects/Paket/pull/1383/files#diff-74c1bcac29cb19f058c1e6032cc1ce19R161 needs to be filtered. we should not add that stuff when a paket.template file exists for that project

@forki
Copy link
Member

forki commented Jan 15, 2016

could you please retry?

@forki
Copy link
Member

forki commented Jan 15, 2016

with latest that is..

@harrisonmeister
Copy link

Hi, sure will let you know the results

@harrisonmeister
Copy link

Looks like thats fixed it, many thanks!

@pms1969
Copy link
Contributor Author

pms1969 commented Jan 15, 2016

good spot. Sorry I missed that.

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.

None yet

3 participants