@aidanhs
We are trying to register docker daemon as a command. Like any other docker command, we should pass options after the command (daemon). So this should work sudo docker daemon -g ./graph -p ./d.pid

@aidanhs The problem we have now, is users specifying commands that are only appropriate for the --daemon on other commands, and the tool silently allowing it. Users expect something to happen, and have no idea why it did not. -d needs to die.

I absolutely agree that -d needs to die. I was planning on making a PR to warn users about the bad arguments you mention @rhatdan since this PR got stalled somewhat last time.

However, it's pretty easy to retain full backwards compat for a few releases - in the loop I've commented on, if you see a -d then remove it and insert daemon at position 1 in os.Args list. That would support the example I gave.

Alternatively, drop support for -d entirely. Please don't leave it the current state where it may or may not work (and possibly will fail in a cryptic way because the -d -> daemon transform is silent), depending on whether you had the foresight to give the -d arguments before anything else.

@aidanhs It works as long as we pass the arguments at the right place.
If you pass the argument before the daemon keyword. It takes it as a flag, and since it is not a flag (it fails), but an argument to the command daemon. it should be passed after the command, not before it.

We are not completely removing -d / --daemon to give users some head ups, as this command is deprecated and will be removed in near future. I have seen this before with other docker commands.

Jenkins failed on docker rmi .... docker rmi requires a minimum of 1 argument. Don't think it's related to the PR

@shishir-a412ed Change the patch to put the daemin to the front and @aidanhs will be happy and we will be backwards compatible.

@thaJeztah Yes that is strange, possible jenkens is a little hosed up.

I think you need a rebase it is failing on context errors https://jenkins.dockerproject.com/job/Docker-PRs2/4/console

@jfrazelle I just rebased 2 mins ago.

you can see the logs that are failing in the link

I think what's missing in the PR is updates to the completion scripts.

According to this: #10416 (comment), updating the fish completions can be updated almost automatically.

The bash completions (found here) need to be updated manually. You could ask @albers if he's willing to assist, but I think it's best to first see if the maintainers are willing to approve the PR first.

Not sure about the zsh completions, I think those have been neglected a bit.

If this is going to be accepted, I'd like to take over the bash completion part. @thaJeztah Thanks for pointing me to this.

@shishir-a412ed Thanks for your contribution!

I also prefer having docker daemon instead of docker -d, however we cannot simply remove docker -d overnight. docker -d will have to live for a while. I'm okay to have a deprecation warning message for docker -d. Please don't make changes to your PR until we have decided, we wouldn't want to waste your time if we were to decide otherwise.

@tiborvass should this be labeled as "proposal"?

WRT backwards compatibility (also see the discussion above) I think that's almost in place, but help/usage probably changes, yes.

@shishir-a412ed @thaJeztah Oh my bad, I tested with -d -D and saw:

INFO[0000] -d is deprecated. Please use daemon command
flag provided but not defined: -D

I assumed there was no backwards compat. Seems like it's just a bug in how the PR handles flags the old way. Also almost all the tests are failing, docker run -d is not working.

Anyway, I'll mark this as UX and it needs design approval.

@tiborvass I can take a look at those errors, and also work on backward compatibility issue.
However as you suggested, the first step is for you guys to make a decision on whether this is something docker wants as part of their product line ??

A quick decision on this would be really helpful as this is pending for a long time. PR 9255.

@tiborvass

1. The reason why docker -d -D was not working, because -D (debug flag) was never part of Daemon Config struct. I have fixed this, you can test now.
2. I have fixed the docker run -d issue.
3. I have also fixed the backward compatibility issue. So now we can pass -d/--daemon at any position.

These were small changes, I don't mind fixing it. However we would really appreciate if we can get this change upstream. Let me know what you guys think ?

Shishir

@aidanhs @rhatdan I have made the requested changes.

@rhatdan
adding some testing code to validate the behaviour now.

Will look closer tomorrow, but looks generally good to me (at least shuffle and checkDaemon do, which are what I'm mainly interested in).

@tiborvass any updates on this? I have added the test cases as well.

Thanks @shishir-a412ed I'll carry this PR and #11418 to get it in asap.

@tiborvass can you clarify? Are you going to make this a PR as part of something else? Is there anywhere to track progress?

Just not quite sure what's happened to this PR...

@aidanhs What is means is that we want the feature, but not in its current form: @tiborvass will take on @shishir-a412ed and make the required changes to get it merged.

@icecrime Can you please elaborate a little bit ?
Would you be using this PR's code as part of another PR ?
Are you going to use the same strategy (shuffling for maintaining the backward compatibility, segregating daemon and global flags into two different flagsets {I assume segregation is something we all want} ) ? or you plan on solving this with a completely different approach ?

I'm mainly interested in how I can follow progress, given @tiborvass probably has quite a few other things on his plate as well.

@tiborvass @crosbymichael @jfrazelle Has this patch been moved somewhere in a new pull request that we can view? Usually when you guys do this you refer to a new pull request where we can comment and continue to contribute.

We have been working on this pull request for over a year and just having it dissapear is not supporting the community.

I think @tiborvass is working on a new PR but its not been submitted yet.

@duglin I agree it's a bit confusing though to "carry" without opening a new PR; it makes people nervous. Perhaps just opening a placeholder "WIP" PR would have kept people at ease :)

Any update? Two week silence?

@tiborvass
Any updates on this? It's been a month since you closed this and #11418.
I have put in a lot of effort on this one, and it is a little demotivating and demoralizing to see all my effort go in vain.
how can I be sure that the next PR I spend my time and effort on would not become like this one?

/cc @crosbymichael @rhatdan

I agree, this is VERY contributor unfriendly. To say you are taking over a patch set and then going radio silent...

I sincerely apologize to @shishir-a412ed and everyone on this thread, this was 100% my fault. I'll make sure not to do this again.

Here's the PR #13771 I just opened, it was dependent on #13636 that just got merged. I welcome your feedback there on #13771. Thanks