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

fix to be able to compile on Windows #13

Merged
merged 1 commit into from
Sep 25, 2016
Merged

Conversation

stunndard
Copy link

By default, code like this won't compile on Windows because of some missing definitions in Windows:

// daemonizing
    if config.Cfg.IsDaemon && runtime.GOOS == "linux" {
        logger.Log("Daemon mode, detaching from terminal...", logger.LOG_INFO)

        cntxt := &daemon.Context{
            PidFileName: config.Cfg.PidFile,
            PidFilePerm: 0644,
            //LogFileName: "log",
            //LogFilePerm: 0640,
            WorkDir: "./",
            Umask:   027,
            //Args:        []string{"[goicy sample]"},
        }

        d, err := cntxt.Reborn()
        if err != nil {
            logger.File(err.Error(), logger.LOG_ERROR)
            return
        }
        if d != nil {
            logger.File("Parent process died", logger.LOG_INFO)
            return
        }
        defer cntxt.Release()
        logger.Log("Daemonized successfully", logger.LOG_INFO)
    }

This change will use empty functions for Windows and will allow it to compile on Windows when creating cross-platform apps that use go-daemon on Linux, and don't use it for Windows. This change won't affect compilation on Linux in any way. I think go-daemon should follow the standard Golang naming convention which is used by Google:

For example:
your_package_linux.go
your_package_darwin.go
your_package_windows.go

You could see how it is implemented in standard os/signal package for example: https://github.com/golang/go/tree/master/src/os/signal

your_package_posix.go is not following this convention and gets picked up and compiled on Windows, which is wrong.

@sevlyar
Copy link
Owner

sevlyar commented Sep 25, 2016

You are right. I forgot about +build directive. Thank you for contributing!

@sevlyar sevlyar merged commit f84331e into sevlyar:master Sep 25, 2016
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.

2 participants