Skip to content
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

libstore: ssh-ng: forward the default setOptions #2994

Closed
wants to merge 1 commit into from

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented Jul 17, 2019

Without these options it's not possible to forward build timeout options
for example. The TODO is probably applicable to all the remote stores.

Without these options it's not possible to forward build timeout options
for example. The TODO is probably applicable to all the remote stores.
@edolstra
Copy link
Member

But this also causes options like the sandbox or remote build configuration to be forwarded to the SSH host. In particular, forwarding the builders option would be quite bad because it could cause infinite build forwarding.

@zimbatm
Copy link
Member Author

zimbatm commented Jul 17, 2019

Isn't that argument true for all the remote stores? I think it would be best if the RemoteStore::setOptions would filter those options then so we get unified capabilities.

In practice I used --builders ssh-ng:// and didn't notice the infinite build forwarding issue so maybe the option isn't forwarded already.

@LnL7
Copy link
Member

LnL7 commented Jul 17, 2019

See #1946.

Sending everything makes ssh-ng unusable except if the source and target machines have the exact same sandbox-paths since that includes a store path. Especially problematic if the target is another platform.

@edolstra
Copy link
Member

Note that RemoteStore is a misnomer. It was actually originally not remote at all: it's the local store accessed via the daemon. So in that case having the same configuration makes sense.

@zimbatm
Copy link
Member Author

zimbatm commented Jul 17, 2019

Thanks, I will be loading all of the context tomorrow

@zimbatm
Copy link
Member Author

zimbatm commented Jul 18, 2019

Yeah that approach is not going to work.

  1. the protocol behind setOptions needs to be re-thought so only a subset of the options can be passed
  2. the options need to be classified so only build options are forwarded (eg: the various timeout options, sandbox-enable, ...). Or pass them to the remote builder URI as query string options like eelco suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants