heroku-buildpack-crystal icon indicating copy to clipboard operation
heroku-buildpack-crystal copied to clipboard

Change app executable to allow app folder

Open danielpclark opened this issue 8 years ago • 3 comments

It's easy to abstract away code to what you're familiar with. In one of my projects I modeled the directory structure after Rails. This build pack won't allow for a folder to be named /app/ since the executable /app exists (or vice versa). Can you please change the executable to be named /app.ex?

I don't think any other changes are necessary. Path is important so the executable should remain where it is.

I've tried thinking of another name for app for a folder to model after Rails, but it really loses the familiarly organized structure. Best thing I can think of is for the buildpack to add an extension to the executable.

danielpclark avatar Mar 29 '17 23:03 danielpclark

I think it makes sense and the PR looks good, but I'd like others to 👍 before merging. @bcardiff, WDYT?

mverzilli avatar Mar 30 '17 14:03 mverzilli

What about leaving the binary in ./bin/app?

I think that could be a better change because for new crystal apps the shard.yml will include a targets description (and we could use shard build --production command to build all the targets).

If there is only one file in ./bin that is the one that could be used. That will decouple the name structure.

So if the change is to ./bin/app it will solve your exposed issue and will prepare things for using shard.yml targets.

bcardiff avatar Mar 30 '17 14:03 bcardiff

@bcardiff That's a great idea but currently Crystal's Macro system only allows relative path files to be set by absolute path (with the possible exception of the current directory). So moving the executable will change the relation to the files it's mapped to.

In the case of web rendered content the template language ECR is a macro method so all absolute paths are fixed to the strings given in the build. So this would be a breaking change.

~Lastly the default behavior on anyone's system is for the executable to be built in the current directory so at the moment it has consistency. This change would require code to have two builtin configurations based on current environment.~

~To move forward with adding an executable in the bin directory I would recommend either a shell wrapper or a symbolic link put there and then update the README as to when the change will go into effect (and~ it should be a major version change as per semantic versioning).


EDIT: Forgive me I read the "targets description" after writing this.

I do think the target in the shards spec is a good way to go.

danielpclark avatar Mar 30 '17 20:03 danielpclark