Fix now dev and other changes
#19
Conversation
The openssl installation was breaking `now dev` on any environment that isn't using `yum` as package manager. It seems sensible to remove it and instead projects that require it can add it (along with package management logic) into `build.sh`.
The replacement logic was breaking `now dev` as it was adding imports and the main function each time `now dev` was run. To mitigate this, the entrypoints now require the main function.
Other builders are using @now/build-utils `debug` function to log debug messages. It is hidden behind NOW_BUILD_DEBUG environment variable, so I have also matched that behaviour for cargo verbosity. Without enabling, cargo is now less verbose, however still shows compilation errors.
rustc does not support `[]` in crate names, hence cargo was failing to build path segments. Replacing brackets with underscores for naming fixes the issue.
This shaves off time as the installer no longer needs to download rustup and try to install. Instead it checks before downloading.
In order to allow usage of helper libraries across all folders Cargo allows specifying them via [lib]. This was removed on each build, previously which no longer happens. These changes however mean that Cargo is not as strictly controlled (now_lambda is not set to latest always, however the [[bin]] is overwritten). Cargo.toml is now also backed up and restored for builds, so that it does not get updated for the users source control management.
|
I verified this works, thanks! |
mike-engel
left a comment
mike-engel
left a comment
There was a problem hiding this comment.
@ekadas Thank you so much for working on this, and thanks for your patience on this review! This looks really good, and I'm thinking this constitutes a 1.0.0 rc 😄
As for pinning to verion 3, I think that's ok as well, especially since this will definitely be a major bump.
I have just one question around caching, otherwise I'd like to merge this soon.
| return cargoTomlPath; | ||
| } | ||
|
|
||
| export const getDefaultCache = ({ files, entrypoint }: BuildOptions) => { |
There was a problem hiding this comment.
Did Zeit change their caching mechanism, or are we just giving up since it never seemed to work?
There was a problem hiding this comment.
I belive they expect prepareCache for the caching and do not expect getDefaultCache, so this seemed to be redundant, as now-rust also exposes prepareCache. I could have missed it somewhere, but a quick search on zeit/now did not return results for getDefaultCache.
I think the caching could be improved though, as while locally caching seems to work, when deploying multiple functions the build seems to re-download and build cargo packages, so builds can take quite long. That does not really have a negative effect on the development experience and I don't think zeit's pricing model is based on build time, so I don't think it's anything urgent, just for the future.
|
@ekadas Thanks again for all your work on this. I changed my mind, and this is now available as |
3 participants
My goal with this PR was to fix
now dev(fixes #3 and #9). While doing that I've also aimed to provide feature parity for how the official builders work (I.e: debug logs, support for utility libraries, support for parameterized routes). Each commit should have a further description and rationale.I think one of the more controversial changes is fixing the version to 3 and removing the old Cargo.toml builld support (as per this comment). Deploying with other versions did not work for my test repository, although I've seen that the unit tests still worked, so I'm not sure if removing completely is the right thing. My rationale for doing so is that it provides more of a similar experience to the official builders and a major version bump in
now-rustshould not cause issues for dependencies. I also sawprobesin unit tests which is no longer documented nor does it work when runningnowfrom the fixture itself, so maybe unit tests have other behaviors to how the builder is meant to be consumed.Thanks for your work on this so far, let me know if you have any questions or comments.