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

Deploy subcommand for CLI tool #23

Merged
merged 21 commits into from Mar 28, 2016

Conversation

Projects
None yet
4 participants
@ogoding
Contributor

ogoding commented Mar 17, 2016

Implements the deploy subcommand.

deploy cleans the relevant target directory, runs tests and builds the project. It then creates a deploy directory, zips all files (including the directory itself) in resources, and copies all dynamically linked libraries and binaries to deploy.

ogoding added some commits Mar 2, 2016

Added a test and deploy command. Test command just executes cargo's d…
…efault test command. Deploy command currently just creates a 'deployed' directory, runs tests, and compiles binary. Compression of resources directory and copying compiled binary features will be implemented next.
Added compression of resources folder. Currently just compresses all …
…files within the folder using the Deflate compression method. Improved the error handling but some work converting std::io::Error to &str still needs to happen.
Removed hard coded resources path in zip_dir and moved to a constant …
…string, same as the other hard coded strings.
Changed zip_dir to use walkdir crate to walk through resources direct…
…ory. Moved out create_dir code from setup_deploy_dir into it's own function, and fixed the clean stage of setup_deploy_dir to no longer crash when it contains a directory. Started on the function to copy correct binaries (game executable and any dylibs) to the deploy dir but is currently failing for unix binaries (due to no extension).
@ogoding

This comment has been minimized.

Show comment
Hide comment
@ogoding

ogoding Mar 17, 2016

Contributor

I was also planning on including the option of specifying the deploy directory, but I think that could wait for some time in the future.

Contributor

ogoding commented Mar 17, 2016

I was also planning on including the option of specifying the deploy directory, but I think that could wait for some time in the future.

White-Oak and others added some commits Mar 17, 2016

Changed build directory back to target/debug until there is a release…
… flag (--release). Fixed function for copying dynamic libraries and executable (by parsing Cargo.toml).
Added a test to test deploy subcmd. Added support for Cargo.toml file…
…s that do not have [[bin]] names generated (default amethyst new mygame Cargo.toml appears to be missing this). Fixed bug with copy_binaries function where it would fail due to binary generated when compiling tests.

@White-Oak White-Oak referenced this pull request Mar 22, 2016

Merged

New API for subcommands #24

@White-Oak

This comment has been minimized.

Show comment
Hide comment
@White-Oak

White-Oak Mar 22, 2016

Contributor

@ogoding #24 should allow to pass strings to subcommands, but will aso break your code, so you'll need to merge from master, if my PR is going to land (I hope it will).

Contributor

White-Oak commented Mar 22, 2016

@ogoding #24 should allow to pass strings to subcommands, but will aso break your code, so you'll need to merge from master, if my PR is going to land (I hope it will).

@ogoding

This comment has been minimized.

Show comment
Hide comment
@ogoding

ogoding Mar 22, 2016

Contributor

@White-Oak Ah, so now Vec<&str> are ArgMatches interchangeable for the commands? Cool. I'll have a look at merging from master when it lands 😄 I'll make deploy always just supply vec!["--release", "--color=always"] to test and build.

Contributor

ogoding commented Mar 22, 2016

@White-Oak Ah, so now Vec<&str> are ArgMatches interchangeable for the commands? Cool. I'll have a look at merging from master when it lands 😄 I'll make deploy always just supply vec!["--release", "--color=always"] to test and build.

@White-Oak

This comment has been minimized.

Show comment
Hide comment
@White-Oak

White-Oak Mar 22, 2016

Contributor

@ogoding those commands already supply color=always and instead of --release, you'll need to supply release to them (that's what they're looking for).

Contributor

White-Oak commented Mar 22, 2016

@ogoding those commands already supply color=always and instead of --release, you'll need to supply release to them (that's what they're looking for).

@ebkalderon

This comment has been minimized.

Show comment
Hide comment
@ebkalderon

ebkalderon Mar 22, 2016

Member

@White-Oak Pull request #24 looks pretty good to me. @ogoding, what are your thoughts? Whose PR should I merge first to minimize breakage?

Member

ebkalderon commented Mar 22, 2016

@White-Oak Pull request #24 looks pretty good to me. @ogoding, what are your thoughts? Whose PR should I merge first to minimize breakage?

@White-Oak

This comment has been minimized.

Show comment
Hide comment
@White-Oak

White-Oak Mar 22, 2016

Contributor

@ebkalderon #24 is going to be merged first, since @ogoding 's work is dependent on this.

Contributor

White-Oak commented Mar 22, 2016

@ebkalderon #24 is going to be merged first, since @ogoding 's work is dependent on this.

@ebkalderon

This comment has been minimized.

Show comment
Hide comment
@ebkalderon

ebkalderon Mar 22, 2016

Member

@ogoding All right, #24 has been merged. Go ahead and rebase! If you need any assistance, I'm sure we'd all be happy to help.

Member

ebkalderon commented Mar 22, 2016

@ogoding All right, #24 has been merged. Go ahead and rebase! If you need any assistance, I'm sure we'd all be happy to help.

@ogoding

This comment has been minimized.

Show comment
Hide comment
@ogoding

ogoding Mar 23, 2016

Contributor

@ebkalderon Merging it now, just trying to work out how to call/import the new methods from the deploy Cmd struct at the moment.

Contributor

ogoding commented Mar 23, 2016

@ebkalderon Merging it now, just trying to work out how to call/import the new methods from the deploy Cmd struct at the moment.

Merge branch 'master' from ebkalderon/amethyst_tools into impl-deploy…
…-subcmd. Resolving conflicts with src/cli/subcmds/deploy.rs
@ogoding

This comment has been minimized.

Show comment
Hide comment
@ogoding

ogoding Mar 23, 2016

Contributor

@ebkalderon Should be merged without issue now! Will try to finish off the conversion from std;:io::Error to &str and then it'll all be finally done! 😄

Contributor

ogoding commented Mar 23, 2016

@ebkalderon Should be merged without issue now! Will try to finish off the conversion from std;:io::Error to &str and then it'll all be finally done! 😄

@LucioFranco

This comment has been minimized.

Show comment
Hide comment
@LucioFranco

LucioFranco Mar 23, 2016

Member

@ogoding I have some work done with that. Would you like me to give you a small PR that removes the tryio! macro so we can just use try! everywhere and not have that borrow issue.

Member

LucioFranco commented Mar 23, 2016

@ogoding I have some work done with that. Would you like me to give you a small PR that removes the tryio! macro so we can just use try! everywhere and not have that borrow issue.

src/cli/subcmds/deploy.rs
+impl AmethystCmd for Cmd {
+ /// Compresses and deploys the project as a distributable program.
+ fn execute<I: AmethystArgs>(matches: &I) -> cargo::CmdResult {
+ let cargo_args = vec!["--release"];

This comment has been minimized.

@White-Oak

White-Oak Mar 23, 2016

Contributor

Once again, build and test subcommands look for release argument, not --release.

@White-Oak

White-Oak Mar 23, 2016

Contributor

Once again, build and test subcommands look for release argument, not --release.

This comment has been minimized.

@ogoding

ogoding Mar 23, 2016

Contributor

Oh whoops, sorry @White-Oak. I'll fix that now.

@ogoding

ogoding Mar 23, 2016

Contributor

Oh whoops, sorry @White-Oak. I'll fix that now.

@ogoding

This comment has been minimized.

Show comment
Hide comment
@ogoding

ogoding Mar 23, 2016

Contributor

@LucioFranco That'd be great, thanks. Hopefully you don't have any conflicts, but I suspect you might.

Contributor

ogoding commented Mar 23, 2016

@LucioFranco That'd be great, thanks. Hopefully you don't have any conflicts, but I suspect you might.

@LucioFranco

This comment has been minimized.

Show comment
Hide comment
@LucioFranco

LucioFranco Mar 23, 2016

Member

@ogoding I am almost ready with the new error system. Expect a PR within the next 20. (Hopefully) :)

Member

LucioFranco commented Mar 23, 2016

@ogoding I am almost ready with the new error system. Expect a PR within the next 20. (Hopefully) :)

LucioFranco and others added some commits Mar 23, 2016

Merge branch 'impl-deploy-subcmd' of github.com:ogoding/amethyst_tool…
…s into impl-deploy-subcmd

Conflicts:
	src/cli/subcmds/deploy.rs
	src/cli/subcmds/new.rs
Added @LucioFranco's improvement to the Display trait implementation …
…for CmdError so that string errors are displayed correctly. Added an error message/check and failure for when the resources directory is missing. Also added println!(string) calls to display when testing phase and build phase of the deploy process begin.
Moved missing resources directory error message into a more appropria…
…te place. Added --clean argument to deploy command to force ./target/release to be cleaned before building.
@ogoding

This comment has been minimized.

Show comment
Hide comment
@ogoding

ogoding Mar 23, 2016

Contributor

@ebkalderon PR is finally all done! Let me know if there is anything else that I should improve.

Contributor

ogoding commented Mar 23, 2016

@ebkalderon PR is finally all done! Let me know if there is anything else that I should improve.

@ebkalderon

This comment has been minimized.

Show comment
Hide comment
@ebkalderon

ebkalderon Mar 23, 2016

Member

Wonderful job! I'll try it out on my laptop tomorrow.

Member

ebkalderon commented Mar 23, 2016

Wonderful job! I'll try it out on my laptop tomorrow.

@ebkalderon

This comment has been minimized.

Show comment
Hide comment
@ebkalderon

ebkalderon Mar 24, 2016

Member

@ogoding The generated resources.zip file structure seems incorrect when deploying an empty project on my laptop.

Currently it results in:

  • resources.zip
    • resources/ (folder)
      • entities (zero-length file)
      • prefabs (zero-length file)
      • config.yml (file)
      • input.yml (file)
    • resources (zero-length file)

It should be:

  • resources.zip
    • entities/ (folder)
    • prefabs/ (folder)
    • config.yml (file)
    • input.yml (file)
Member

ebkalderon commented Mar 24, 2016

@ogoding The generated resources.zip file structure seems incorrect when deploying an empty project on my laptop.

Currently it results in:

  • resources.zip
    • resources/ (folder)
      • entities (zero-length file)
      • prefabs (zero-length file)
      • config.yml (file)
      • input.yml (file)
    • resources (zero-length file)

It should be:

  • resources.zip
    • entities/ (folder)
    • prefabs/ (folder)
    • config.yml (file)
    • input.yml (file)
@ogoding

This comment has been minimized.

Show comment
Hide comment
@ogoding

ogoding Mar 24, 2016

Contributor

@ebkalderon hmm, I'll have a look at it. Probably that resources file is due to walkdir using the given path as the first file/folder it iterates.

Contributor

ogoding commented Mar 24, 2016

@ebkalderon hmm, I'll have a look at it. Probably that resources file is due to walkdir using the given path as the first file/folder it iterates.

Fixed bugs with compressing resources where it would be deflated to r…
…esources/ instead of into current folder ./ and where directories would be turned into zero-length files during compression process.
@ogoding

This comment has been minimized.

Show comment
Hide comment
@ogoding

ogoding Mar 26, 2016

Contributor

@ebkalderon Just pushed a fix for those compression issues. Turns out that rust doesn't include a / character at the end of a directory's path. Also WalkDir starts in the current directory so I had to change it back to fs::read_dir(path) and use WalkDir for any sub directories in resources/.

Contributor

ogoding commented Mar 26, 2016

@ebkalderon Just pushed a fix for those compression issues. Turns out that rust doesn't include a / character at the end of a directory's path. Also WalkDir starts in the current directory so I had to change it back to fs::read_dir(path) and use WalkDir for any sub directories in resources/.

@ebkalderon

This comment has been minimized.

Show comment
Hide comment
@ebkalderon

ebkalderon Mar 28, 2016

Member

@ogoding Testing it now...

Member

ebkalderon commented Mar 28, 2016

@ogoding Testing it now...

@ebkalderon

This comment has been minimized.

Show comment
Hide comment
@ebkalderon

ebkalderon Mar 28, 2016

Member

@ogoding It works perfectly! Thank you so much for your effort and patience. I think this pull request is finally ready to be merged.

Member

ebkalderon commented Mar 28, 2016

@ogoding It works perfectly! Thank you so much for your effort and patience. I think this pull request is finally ready to be merged.

@ebkalderon ebkalderon merged commit d1f26a9 into amethyst:master Mar 28, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ogoding ogoding deleted the ogoding:impl-deploy-subcmd branch Mar 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment