Be more clear in --help what the value of --log is supposed to be

[lgommans]

Currently, --help says:

...
   --listen string        listen address (default ":8000")
   --log string           log HTTP requests in the combined log format
   --max-size int         the maximum size of the repository in bytes
...

I looked through the documentation and even gave the source code a glance to find what this "string" is supposed to be, which options exist? I just want it to dump the URI or something, whatever the syntax is for that in this combined format, why is an argument even required and can't I use some default?

Some %s-like guesses later and not seeing any log messages at all upon doing requests to the server, I moved on and did an ls for unrelated reasons, finding that it had created my attempts as filenames. Aha, it has to be a filename! It doesn't simply log to stdout!

This pull request hopefully makes that more clear for those who come after me. I found it hard to find good phrasing without making it overly long. I'd actually rather replace the word "string" with "filename" (or just "file") and leave the rest as-is, but I'm not sure if that's possible without overcomplicating things.

The proposed change would appear as:

...
   --listen string        listen address (default ":8000")
   --log string           write HTTP requests in the combined log format to the specified filename
   --max-size int         the maximum size of the repository in bytes
...

The --no-verify-upload option is the only one that is still (significantly) longer, but it's also a bit of an exception so that's not a great measure.

(If a changelog entry is needed for this change, just let me now. I've marked it as not applicable in the checklist because it seems too trivial for an entry.)

Checklist

• [x] I have enabled maintainer edits for this PR
• n/a I have added tests for all changes in this PR
• n/a I have added documentation for the changes (in the manual)
• n/a There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
• [x] I have run gofmt on the code in all commits
• [x] All commit messages are formatted in the same style as the other commits in the repo
• [x] I'm done, this Pull Request is ready for review

[mirabilos]

Huh, so a filename is expected… I was wondering what the string was supposed to be…

… but it doesn’t work with a filename either:

error: open /dev/stdout: permission denied

[lgommans]

That's a special file, try a regular file that you have permissions to create and write to?

For me, though, it also works with this special /dev "file": rest-server --log /dev/stdout --no-auth --path /tmp/test and then curl localhost:8000 (default port) shows up in the stdout whereas without --log /dev/stdout it does not.

[mirabilos]

@lgommans I need it on stdout though, so it can be combined with the rest of the output into the service manager’s logging (and timestamping and rotating) system (here: DJB dæmontools)

A separate actual file is no option.

(Ideally, they’d allow - for stdout.)

[lgommans]

Might it be that you're not running it from bash or a similar shell and /dev/stdout thus does not exist? I think it's some special file that bash somehow emulates (I'm not sure if I remember this correctly).

If that's the case, maybe run it with bash -c 'rest-server --...', else I don't really have an idea how to solve it. Might be that you have to contribute a patch to rest-server such that - is stdout; or it might be that these djb tools need a patch to work with log files or fifos or so.

(The above occurred to me while I was actually typing that I can't reproduce your issue so I wasn't sure how to further help without further information. If this missing stdout 'device' isn't it, then you'll have to give more info.)

[mirabilos]

The link does exist, but I suspect that, as soon as stdout is piped to somewhere, the opening fails, because:

l-wx------ 1 resticd resticd 64 Apr 17 23:42 1 -> pipe:[41829]

(from /proc/$pid/fd/)

I guess it opens for more than appending?

[mirabilos]

Hm, no, it isn’t even that.

nobody@cafe:/ $ /tmp/rest-server --log /dev/stdout --no-auth --path /tmp/test
Data directory: /tmp/test
Authentication disabled
error: open /dev/stdout: permission denied
1|nobody@cafe:/ $ /tmp/rest-server --log /dev/fd/1 --no-auth --path /tmp/test
Data directory: /tmp/test
Authentication disabled
error: open /dev/fd/1: permission denied
1|nobody@cafe:/ $ ls -l /dev/stdout /dev/fd/1
lrwx------ 1 nobody nogroup 64 Apr 17 23:45 /dev/fd/1 -> /dev/pts/0
lrwxrwxrwx 1 root   root     4 Apr 14 00:01 /dev/stdout -> fd/1
nobody@cafe:/ $ ls -l /dev/fd
lrwxrwxrwx 1 root root 13 Apr 14 00:01 /dev/fd -> /proc/self/fd

[lgommans]

I guess it opens for more than appending?

--log sets the server.Log option. When searching for use of server.Log, it shows up in mux.go#L95. The next line calls into a function defined earlier in the file and there the next line contains the file open options.

The code base is not that large. I often have trouble navigating established (read: large) projects in languages which I'm not very familiar with, but this project is imo quite accessible so I figured I'd show how I approached this :). At any rate, the call looks like this:

os.OpenFile(s.Log, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)

Since that works fine for me, I'm not sure if/how this helps, though. Maybe you know more about this.

[mirabilos]

Hrm. I don’t program in issue9 myself, but from a C programmer’s PoV that call “probably” looks good. Not sure whether the issue9 runtime or the Linux kernel throw sticks between its spokes.

The easiest way would be to specially handle an argument of - of course. Since rest-server isn’t packaged in Debian anyway I can then just upgrade to a new release…

[fd0]

I don’t program in issue9 myself, but from a C programmer’s PoV that call “probably” looks good. Not sure whether the issue9 runtime or the Linux kernel throw sticks between its spokes.

Please leave passive-aggressive comments about unrelated decisions you do not agree with out of the issue tracker. Thanks for understanding!

[mirabilos]

--log - was added in https://github.com/restic/rest-server/pull/217 (thanks!)