heroku / dotnet-buildpack

ASP.NET 5 Buildpack
MIT License
92 stars 111 forks source link

use dotnet core #10

Open friism opened 8 years ago

tkrotoff commented 7 years ago

(Could not open an issue on your repo) I've tried you fork and got this with heroku logs:

heroku[web.1]: Starting process with command `ASPNETCORE_URLS='http://+:31719' dotnet ./app.dll -c Release`
app[web.1]: 
app[web.1]: Unhandled Exception: System.FormatException: The short switch '-c' is not defined in the switch mappings.

Sure the -c switch exists? see https://docs.microsoft.com/en-us/dotnet/articles/core/tools/dotnet

Edit: I'm using this Procfile instead and it works fine: web: dotnet ./app.dll --urls http://+:$PORT

The Release flag should be on the dotnet publish, something like: dotnet publish --output out --configuration Release, see https://docs.microsoft.com/en-us/dotnet/articles/core/tools/dotnet-publish

friism commented 7 years ago

Interesting, it doesn't complain when I test in a heroku run:

Microsoft.DotNet.PlatformAbstractions.dll              System.ComponentModel.Primitives.dll
~ $ dotnet app.dll -c Release
Hosting environment: Production
Content root path: /app
Now listening on: http://localhost:5000
Application started. Press Ctrl+C to shut down.
^CApplication is shutting down...
~ $ dotnet app.dll -c Debug
Hosting environment: Production
Content root path: /app
Now listening on: http://localhost:5000
Application started. Press Ctrl+C to shut down.
^CApplication is shutting down...

I think you're right though, it doesn't make sense here, I'll remove it.

tkrotoff commented 7 years ago

btw, you can remove $DNU_FLAGS from here, I believe it is a relic from the past :)

friism commented 7 years ago

@tkrotoff done, thx

tkrotoff commented 7 years ago

I prefer to document the error I've encountered here.

I had a hard time getting rid of the following error One or more errors occurred. (The process cannot access the file because it is being used by another process.) with friism/dotnet-buildpack. Of course this error did not happen locally or with a Docker image.

// ...
remote:    [0] multi react 76 bytes {12} [built]
remote:    [0] multi polyfills 40 bytes {11} [built]
remote:    [0] multi moment 40 bytes {9} [built]
remote:    [0] multi mobx 40 bytes {8} [built]
remote:     + 792 hidden modules
remote: Child html-webpack-plugin for "index.html":
remote:         + 3 hidden modules
remote: Child extract-text-webpack-plugin:
remote:         + 8 hidden modules
 66% 617/658 buiOne or more errors occurred. (The process cannot access the file because it is being used by another process.)
remote:  !     Push rejected, failed to compile .NET Core app.
remote:
remote:  !     Push failed
remote: Verifying deploy....
remote:
remote: !   Push rejected to [...].

I use webpack for the client side and my project.json looks like this:

"scripts": {
  "prepublish": [ "npm install", "npm run clean", "npm run build:client:prod" ],
  "postpublish": [ "dotnet publish-iis --publish-folder %publish:OutputPath% --framework %publish:FullTargetFramework%" ]
}

package.json:

"scripts": {
  "clean": "npm-run-all --parallel clean:*",
  "clean:server": "rimraf ./bin ./obj",
  "clean:client": "rimraf ./wwwroot/*",
  "build:client:prod": "cross-env NODE_ENV=production webpack -p --progress"
}

The problem was the --progress option passed to webpack. Without I don't get the error. Don't ask me why or how, after 2 days spent on this I have no clue.

tkrotoff commented 7 years ago

@friism

Your buildpack installs Grunt, Bower and Gulp globally.

This is not a good practice. These dependencies should be installed locally using package.json and managed with scripts or by calling node_modules/.bin/myclitool. (You are also doing this "mistake" in your docker example).

Otherwise what about the buildpack installing globally Webpack, Browserify, Rollup, Compass, SASS, ..., the next cool new command line tool? A buildpack should be agnostic and heroku-buildpack-nodejs for example is.

Btw Bower is kind of obsolete: 1, 2, 3.

Anyway, your buildpack works wonderfully, better than the others, thanks a lot for your work.

friism commented 7 years ago

@tkrotoff I'm not particularly happy about adding the javascript stuff either, but the problem is that almost all of the default .NET samples assume that they exist on the dev/build system. My docker sample is a good example: I didn't build it, I just copied sample code from the EntityFramework docs and tweaked them a little: https://github.com/aspnet/EntityFramework.Docs/tree/master/samples/core/GetStarted/AspNetCore/AspNetCore.NewDb

I could remove it from the buildpack, but then the buildpack would be able to build only a small subset of projects.

Also note that Bower and Grunt are only available when the buildpack is building, they're not added to the runtime slug. If you do heroku run bash and look around in the running app, there's no Javascript stuff slowing down app deployment, etc.

tkrotoff commented 7 years ago

@friism The Procfile provided by default is: web: ASPNETCORE_URLS='http://+:$PORT' dotnet ./app.dll

It should be web: ASPNETCORE_URLS='http://+:$PORT' dotnet ./app.dll --urls http://+:$PORT, see https://github.com/friism/dotnet-buildpack/pull/1

friism commented 7 years ago

@tkrotoff thanks, I've opened up for issues on the repo on my account.

mplacona commented 7 years ago

Tested this and it works fine. This should get merged!

mpdeimos commented 7 years ago

I can just second that. Works like a charm, I just had to specify a Procfile together with a .deployment file to target my app dll instead of the test dll: https://github.com/mpdeimos/strava-weather/blob/master/Procfile

kharlamov commented 7 years ago

Any way we can get some movement on all of these changes?