Skip to content

Revert "Merge pull request #1439 from ltouati/fsnotify" #1508

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

Merged
merged 1 commit into from
Jan 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,3 @@
[[constraint]]
name = "github.com/bmatcuk/doublestar"
revision = "v1.1.1"

[[constraint]]
name = "github.com/rjeczalik/notify"
version = "0.9.2"
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ $(BUILD_DIR)/$(PROJECT): $(BUILD_DIR)/$(PROJECT)-$(GOOS)-$(GOARCH)
cp $(BUILD_DIR)/$(PROJECT)-$(GOOS)-$(GOARCH) $@

$(BUILD_DIR)/$(PROJECT)-%-$(GOARCH): $(GO_FILES) $(BUILD_DIR)
GOOS=$* GOARCH=$(GOARCH) CGO_ENABLED=1 go build -ldflags $(GO_LDFLAGS) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) -o $@ $(BUILD_PACKAGE)
GOOS=$* GOARCH=$(GOARCH) CGO_ENABLED=0 go build -ldflags $(GO_LDFLAGS) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) -o $@ $(BUILD_PACKAGE)

%.sha256: %
shasum -a 256 $< > $@
Expand All @@ -81,7 +81,7 @@ test:

.PHONY: install
install: $(GO_FILES) $(BUILD_DIR)
GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=1 go install -ldflags $(GO_LDFLAGS) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) $(BUILD_PACKAGE)
GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=0 go install -ldflags $(GO_LDFLAGS) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) $(BUILD_PACKAGE)

.PHONY: integration
integration: install $(BUILD_DIR)/$(PROJECT)
Expand Down
2 changes: 1 addition & 1 deletion cmd/skaffold/app/cmd/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func NewCmdDev(out io.Writer) *cobra.Command {
}
AddRunDevFlags(cmd)
cmd.Flags().BoolVar(&opts.TailDev, "tail", true, "Stream logs from deployed objects")
cmd.Flags().StringVar(&opts.Trigger, "trigger", "polling", "How are changes detected? (polling, manual or notify)")
cmd.Flags().StringVar(&opts.Trigger, "trigger", "polling", "How are changes detected? (polling or manual)")
cmd.Flags().BoolVar(&opts.Cleanup, "cleanup", true, "Delete deployments after dev mode is interrupted")
cmd.Flags().StringArrayVarP(&opts.Watch, "watch-image", "w", nil, "Choose which artifacts to watch. Artifacts with image names that contain the expression will be watched only. Default is to watch sources for all artifacts")
cmd.Flags().IntVarP(&opts.WatchPollInterval, "watch-poll-interval", "i", 1000, "Interval (in ms) between two checks for file changes")
Expand Down
2 changes: 1 addition & 1 deletion docs/content/en/docs/references/cli/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ Flags:
--skip-tests Whether to skip the tests after building
--tail Stream logs from deployed objects (default true)
--toot Emit a terminal beep after the deploy is complete
--trigger string How are changes detected? (polling, manual or notify) (default "polling")
--trigger string How are changes detected? (polling or manual) (default "polling")
-w, --watch-image stringArray Choose which artifacts to watch. Artifacts with image names that contain the expression will be watched only. Default is to watch sources for all artifacts
-i, --watch-poll-interval int Interval (in ms) between two checks for file changes (default 1000)

Expand Down
4 changes: 2 additions & 2 deletions hack/check-docs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
cp docs/content/en/docs/references/cli/index_header docs/content/en/docs/references/cli/_index.md
go run cmd/skaffold/man/man.go >> docs/content/en/docs/references/cli/_index.md

readonly CLI_CHANGES=`git status -s | grep "docs/" | grep -x vendor | wc -l`
readonly CLI_CHANGES=`git status -s | grep "docs/" | wc -l`

if [ $CLI_CHANGES -gt 0 ]; then
echo "You have skaffold command changes but haven't generated the CLI reference docs. Please run hack/check-docs.sh and commit the results!"
exit 1
fi

readonly DOCS_CHANGES=`git diff --name-status master | grep "docs/" | grep -x vendor | wc -l`
readonly DOCS_CHANGES=`git diff --name-status master | grep "docs/" | wc -l`

if [ $DOCS_CHANGES -gt 0 ]; then
echo "There are $DOCS_CHANGES changes in docs, testing site generation..."
Expand Down
95 changes: 13 additions & 82 deletions pkg/skaffold/watch/triggers.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,20 @@ package watch

import (
"bufio"
"context"
"fmt"
"io"
"os"
"strings"
"sync/atomic"
"time"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/rjeczalik/notify"
"github.com/sirupsen/logrus"
)

// Trigger describes a mechanism that triggers the watch.
type Trigger interface {
Start(context.Context) (<-chan bool, error)
Start() (<-chan bool, func())
WatchForChanges(io.Writer)
Debounce() bool
}
Expand All @@ -44,11 +41,7 @@ func NewTrigger(opts *config.SkaffoldOptions) (Trigger, error) {
switch strings.ToLower(opts.Trigger) {
case "polling":
return &pollTrigger{
interval: time.Duration(opts.WatchPollInterval) * time.Millisecond,
}, nil
case "notify":
return &fsNotifyTrigger{
interval: time.Duration(opts.WatchPollInterval) * time.Millisecond,
Interval: time.Duration(opts.WatchPollInterval) * time.Millisecond,
}, nil
case "manual":
return &manualTrigger{}, nil
Expand All @@ -59,7 +52,7 @@ func NewTrigger(opts *config.SkaffoldOptions) (Trigger, error) {

// pollTrigger watches for changes on a given interval of time.
type pollTrigger struct {
interval time.Duration
Interval time.Duration
}

// Debounce tells the watcher to debounce rapid sequence of changes.
Expand All @@ -68,30 +61,27 @@ func (t *pollTrigger) Debounce() bool {
}

func (t *pollTrigger) WatchForChanges(out io.Writer) {
color.Yellow.Fprintf(out, "Watching for changes every %v...\n", t.interval)
color.Yellow.Fprintf(out, "Watching for changes every %v...\n", t.Interval)
}

// Start starts a timer.
func (t *pollTrigger) Start(ctx context.Context) (<-chan bool, error) {
func (t *pollTrigger) Start() (<-chan bool, func()) {
trigger := make(chan bool)

ticker := time.NewTicker(t.interval)
ticker := time.NewTicker(t.Interval)
go func() {
for {
select {
case <-ticker.C:
trigger <- true
case <-ctx.Done():
ticker.Stop()
}
<-ticker.C
trigger <- true
}
}()

return trigger, nil
return trigger, ticker.Stop
}

// manualTrigger watches for changes when the user presses a key.
type manualTrigger struct{}
type manualTrigger struct {
}

// Debounce tells the watcher to not debounce rapid sequence of changes.
func (t *manualTrigger) Debounce() bool {
Expand All @@ -103,78 +93,19 @@ func (t *manualTrigger) WatchForChanges(out io.Writer) {
}

// Start starts listening to pressed keys.
func (t *manualTrigger) Start(ctx context.Context) (<-chan bool, error) {
func (t *manualTrigger) Start() (<-chan bool, func()) {
trigger := make(chan bool)

var stopped int32
go func() {
<-ctx.Done()
atomic.StoreInt32(&stopped, 1)
}()

reader := bufio.NewReader(os.Stdin)
go func() {
for {
_, _, err := reader.ReadRune()
if err != nil {
logrus.Debugf("manual trigger error: %s", err)
}

// Wait until the context is cancelled.
if atomic.LoadInt32(&stopped) == 1 {
return
}
trigger <- true
}
}()

return trigger, nil
}

// notifyTrigger watches for changes with fsnotify
type fsNotifyTrigger struct {
interval time.Duration
}

// Debounce tells the watcher to not debounce rapid sequence of changes.
func (t *fsNotifyTrigger) Debounce() bool {
// This trigger has built-in debouncing.
return false
}

func (t *fsNotifyTrigger) WatchForChanges(out io.Writer) {
color.Yellow.Fprintln(out, "Watching for changes...")
}

// Start Listening for file system changes
func (t *fsNotifyTrigger) Start(ctx context.Context) (<-chan bool, error) {
// TODO(@dgageot): If file changes happen too quickly, events might be lost
c := make(chan notify.EventInfo, 100)

// Watch current directory recursively
if err := notify.Watch("./...", c, notify.All); err != nil {
return nil, err
}

trigger := make(chan bool)
go func() {
timer := time.NewTimer(1<<63 - 1) // Forever

for {
select {
case e := <-c:
logrus.Debugln("Change detected", e)

// Wait t.interval before triggering.
// This way, rapid stream of events will be grouped.
timer.Reset(t.interval)
case <-timer.C:
trigger <- true
case <-ctx.Done():
timer.Stop()
}
}
}()

return trigger, nil
return trigger, func() {}
}
9 changes: 2 additions & 7 deletions pkg/skaffold/watch/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,8 @@ func (w *watchList) Register(deps func() ([]string, error), onChange func(Events

// Run watches files until the context is cancelled or an error occurs.
func (w *watchList) Run(ctx context.Context, out io.Writer, onChange func() error) error {
ctxTrigger, cancelTrigger := context.WithCancel(ctx)
defer cancelTrigger()

t, err := w.trigger.Start(ctxTrigger)
if err != nil {
return errors.Wrap(err, "unable to start trigger")
}
t, cleanup := w.trigger.Start()
defer cleanup()

changedComponents := map[int]bool{}

Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/watch/watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestWatch(t *testing.T) {

// Watch folder
watcher := NewWatcher(&pollTrigger{
interval: 10 * time.Millisecond,
Interval: 10 * time.Millisecond,
})
err := watcher.Register(folder.List, folderChanged.call)
testutil.CheckError(t, false, err)
Expand Down
10 changes: 0 additions & 10 deletions vendor/github.com/rjeczalik/notify/AUTHORS

This file was deleted.

21 changes: 0 additions & 21 deletions vendor/github.com/rjeczalik/notify/LICENSE

This file was deleted.

53 changes: 0 additions & 53 deletions vendor/github.com/rjeczalik/notify/debug.go

This file was deleted.

9 changes: 0 additions & 9 deletions vendor/github.com/rjeczalik/notify/debug_debug.go

This file was deleted.

9 changes: 0 additions & 9 deletions vendor/github.com/rjeczalik/notify/debug_nodebug.go

This file was deleted.

Loading