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

Attempting to allow csproj being passed as a NuSpec file #644

Merged
merged 9 commits into from
Feb 19, 2015

Conversation

jamescrowley
Copy link
Contributor

First crack at fixing #643

@forki
Copy link
Member

forki commented Feb 3, 2015

Thanks for the fast pull request.

@ilkerde could you please verify that it also works for you?

@ilkerde
Copy link
Contributor

ilkerde commented Feb 3, 2015

@forki Thanks for letting me know. @jamescrowley Thanks for your fast contribution. I like your changes since the case of accompanying .nuspec is quite often.

However, I checked this PR and have to say that it breaks a test. After running CI I receive

Failures:
when packing with nuspec template
» should create nupkg file (FAIL)
System.Exception: Error during NuGet package creation. C:\Code\ilkerde.fake\test\TestData\nuget.exe pack -Version 0.0.1 -OutputDirectory "C:\Users\ilker.cetinkaya\AppData\Local\Temp" "fake.nuspec"

This is the test for NugetPack to be able to transform templates. Seems that your change breaks this. I suppose that you are changing something along the line of createNuspecFile, most probably the return value around L227. I would recommend to have tests for your case (nuspec alongside the csproj) as well.

Any chance to get this fixed (and probably even have a test for it)?

@forki
Copy link
Member

forki commented Feb 17, 2015

@jamescrowley can I help you to get this in?

@jamescrowley
Copy link
Contributor Author

@forki This still requires more work - the current code just tries to delete whatever file you pass in as an argument (https://github.com/jamescrowley/FAKE/blob/fix.NuGetPack/src/app/FakeLib/NuGet/NugetHelper.fs#L331).

@jamescrowley
Copy link
Contributor Author

I think this at least works now @forki @ilkerde. it's ugly as anything, but has a test and will have to defer to others in terms of how we tidy up

@@ -225,6 +225,12 @@
<Content Include="TestData\AllObjects_2.txt">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</Content>
<Content Include="TestData\bin\Debug\TestNuGetPack.dll">
Copy link
Member

Choose a reason for hiding this comment

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

I think these files are gitignored. You need to add them "forced"

@jamescrowley
Copy link
Contributor Author

https://bugzilla.xamarin.com/show_bug.cgi?id=17430 Hmm. Nice. So you can't actually nuget pack on a csproj file on mono. Thoughts @forki ?

@forki
Copy link
Member

forki commented Feb 19, 2015

mhm. then maybe just use isMono function and throw a good exception.
In the test try to catch that exception in the mono case. does this sound OK?

BTW: we are actively working on http://fsprojects.github.io/Paket/paket-pack.html

@jamescrowley
Copy link
Contributor Author

@forki I think I'd probably make the test check for isMono rather than the nuget helper? given we don't actually know whether nuget.exe in the future would support, and at least you get a decent error output when it fails ow?

@forki
Copy link
Member

forki commented Feb 19, 2015

yes that's ok, too

@forki
Copy link
Member

forki commented Feb 19, 2015

thanks for being so patient ;-)

@forki forki merged commit e19be10 into fsprojects:master Feb 19, 2015
@jamescrowley jamescrowley deleted the fix.NuGetPack branch February 19, 2015 18:02
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