Wolox / carthage_cache

A tool that allows to cache Carthage/Build folder in Amazon S3.
MIT License
173 stars 27 forks source link

FTPS Support #41

Open Lutzifer opened 7 years ago

Lutzifer commented 7 years ago

I have begun implementing support for using ftps instead of aws. The branch can be found here: https://github.com/Lutzifer/carthage_cache/tree/ftps_support It is not quite ready yet for a PR, as host, username and password are currently set via env variables.

So I just wanted to reach out if you are interested in this kind feature, before putting more effort in the config stuff.

guidomb commented 7 years ago

Hi! Thanks for reaching out. I don't use ftps and I'm a little bit reluctant to add features I wouldn't use. My main concern being that I won't be able to maintain it. I'll have to take a look at the final PR but judging by the code in your fork I see that you added double-bag-ftps. I'm not sure how stable / maintained is that dependency. I wouldn't want to be in the situation of having to maintain that dependency as well, although is quite small and single purpose.

It's hard for me to judge if this is a feature carthage_cache users would want. I'll have to think a little bit more about this. I'll have an answer by next week.

If I decide to accept this feature, Would you be willing to be a maintainer?

Thanks again for raising this issue and I hope you can understand my hesitation.

Lutzifer commented 7 years ago

Sure, I can understand the hesitation.

Seems like ruby 2.4 has support for ftps out of the box, i will look into that. Perhaps I can use your library as the base for a fastlane plugin that manages the carthage cache via ftps, which would keep the code decoupled from this repo and adds the functionality and the fastlane plugin I want to have in one single piece of code

guidomb commented 7 years ago

That sounds like a very goods option. Let me know if you want to move forward with that option because made we can modify carthage_cache to make it easier for such extension to be developed.

Lutzifer commented 7 years ago

This was less work then expected (due to the architecture you used).

A first version, which adds actions for AWS and ftps via fastlane actions can be found here:

https://github.com/num42/fastlane-plugin-carthage_cache_ftps

It is a bit hacky how I create the application though:

When I use this initializer:

  def initialize(project_path, verbose, config, repository: AWSRepository, terminal: Terminal, swift_version_resolver: SwiftVersionResolver)
      @terminal = terminal.new(verbose)
      @archiver = Archiver.new
      @config = Configurator.new(@terminal, project_path, config).config
      clazz = @config.read_only? ? HTTPRepository : repository
      @repository = clazz.new(@config.bucket_name, @config.hash_object[:aws_s3_client_options])
      @project = Project.new(project_path, CACHE_DIR_NAME, @terminal, @config.tmpdir, swift_version_resolver.new)
    end

I have to reuse the structure for AWS to transport my values:

        host = params.values[:host]
        remote_subfolder = params.values[:subfolder]
        username = params.values[:username]
        password = params.values[:password] || PasswordHelper.new.password(host, username)

        config = {
                  bucket_name: remote_subfolder,
                  aws_s3_client_options: {
                    region: host,
                    aws_access_key_id: username,
                    secret_access_key: password,
                    access_key_id: "bla"
                    }
                  }

        local_path = params.values[:local_path]
        application = CarthageCache::Application.new(local_path, true, config, repository: FTPRepository)

If we could change the initializer to simply pass through the config object to the clazz.new call if the repository param is not AWSRepository this would be a lot easier to understand on my side (and no change in behaviour in the original code).