Building latest stable (1.57.0) with Go 1.17 produces warnings from ftp backend

[eharris]

$ go get github.com/rclone/rclone
go get: installing executables with 'go get' in module mode is deprecated.
        Use 'go install pkg@version' instead.
        For more information, see https://golang.org/doc/go-get-install-deprecation
        or run 'go help get' or 'go help install'.
# github.com/rclone/rclone/backend/ftp
../../pkg/mod/github.com/rclone/rclone@v1.57.0/backend/ftp/ftp.go:331:33: undefined: ftp.DialWithShutTimeout
../../pkg/mod/github.com/rclone/rclone@v1.57.0/backend/ftp/ftp.go:334:33: undefined: ftp.DialWithWritingMDTM
../../pkg/mod/github.com/rclone/rclone@v1.57.0/backend/ftp/ftp.go:507:16: c.IsGetTimeSupported undefined (type *ftp.ServerConn has no field or method IsGetTimeSupported)
../../pkg/mod/github.com/rclone/rclone@v1.57.0/backend/ftp/ftp.go:508:16: c.IsSetTimeSupported undefined (type *ftp.ServerConn has no field or method IsSetTimeSupported)
../../pkg/mod/github.com/rclone/rclone@v1.57.0/backend/ftp/ftp.go:509:16: c.IsTimePreciseInList undefined (type *ftp.ServerConn has no field or method IsTimePreciseInList)
../../pkg/mod/github.com/rclone/rclone@v1.57.0/backend/ftp/ftp.go:999:21: c.GetTime undefined (type *ftp.ServerConn has no field or method GetTime)
../../pkg/mod/github.com/rclone/rclone@v1.57.0/backend/ftp/ftp.go:1022:9: c.SetTime undefined (type *ftp.ServerConn has no field or method SetTime)

[ncw]

Hmm, go get seems to be ignoring the replace statement in the go.mod

If I try what the command suggests then I get

$ go install github.com/rclone/rclone@latest
go: downloading github.com/rclone/rclone v1.57.0
go install: github.com/rclone/rclone@latest (in github.com/rclone/rclone@v1.57.0):
	The go.mod file for the module providing named packages contains one or
	more replace directives. It must not contain directives that would cause
	it to be interpreted differently than if it were the main module.

Which is more explicit.

I don't understand exactly what that means though. It is discussed at length in this flamewar issue golang/go#44840

It is complaining about

rclone/go.mod

Line 5 in 4c93378

replace github.com/jlaffaye/ftp => github.com/rclone/ftp v1.0.0-210902f

To fix this I think we'd need to get rid of the replace line and use the rclone fork explicitly in the code.

I think this will fix both the go get and the go install problems.

@ivandeex do you think that will work?

If this fix is correct it should go in a point release.

[ivandeex]

It was reported a month ago here:

#5596 (comment)

The main reason is that the upstream library maintainer has been (and continues to be) unresponsive so we made a soft fork backed by replace directives in rclone's go.mod. If upstream won't respond in 5-7 months we'll consider hard-forking (replacing package preamble in forked go.mod) or just embed the source directly.

For now you can use the below flow instead of go install or go get:

$ git clone https://github.com/rclone/rclone
$ cd rclone
$ go install -v -ldflags="-s -w"

By the way, go get is generally known to be bad on modules due to backwards compatibility with pre-module times.

[ivandeex]

do you think that will work?

No. It's not enough. You also have to update go.mod in forked library.

[ncw]

do you think that will work?

No. It's not enough. You also have to update go.mod in forked library.

Gotcha: go.mod and all the references to github.com/jlaffaye/ftp would need to be changed to github.com/rclone/ftp

In the mean time our install instructions are broken

https://rclone.org/install/#install-from-source

I really think go install should work with a replace directive in a go.mod file :-(

So how to fix - update instructions? Or go full fork?

[ivandeex]

go install [.] from the project directory will work.

go install https://github.com/rclone/rclone and go get https://github.com/rclone/rclone will fail.

I don't think it's not the last case when an upstream turns sour.

I'm for updating the docs to leave rclone team more freedom.

git clone https://github.com/rclone/rclone
cd rclone
go build / go install / whatever

However the hard fact is it will break some workflows out there in the wild.
Building rclone from source in one line will not be possible.

@ncw Quickly glancing through previous requests submitted by @eharris I conclude that one of his workflows is dependent on one-line build so I guess he will not be happy.

[eharris]

I don't depend on a one line build, although that would be nice. But I do kind of depend on being able to automate the build/update for the latest stable code without having to manually specify what tag that requires. (see #5811).

[ivandeex]

Here is possible 3-line replacement for go install xxx@latest

latest_tag=$(basename $(curl -sLo/dev/null -w"%{url_effective}" https://github.com/rclone/rclone/releases/latest))
git clone --depth 1 -b $latest_tag https://github.com/rclone/rclone
cd rclone && go install

[ivandeex]

Here is golang issue requesting to relax behaviors of go install and go get
golang/go#44840 (comment)

[ivandeex]

Here is PowerShell replacement

$latest_tag=($(curl -sLo/dev/null -w"%{url_effective}" https://github.com/rclone/rclone/releases/latest) -split "/")[-1]
git clone --depth 1 -b $latest_tag https://github.com/rclone/rclone
cd rclone; go install

[ivandeex]

I just received a github notification that upstream patches were accepted by jlaffaye.
This means we don't need the go.mod hack until we plan new ftp fixes.
I will roll back the replace directive hack in a month.
After that go get and go install will work as documented.
cc @ncw

[ivandeex]

There are however requests to support rare FTP cases:

• non-standard CLNT command
  https://forum.rclone.org/t/use-ftp-in-rclone-with-filename-encode-by-shift-jis-gbk/27514/3
• file transfer based on something not yet investigated rather than REST command
  https://forum.rclone.org/t/rclone-with-bunnycdn/27439/13
• broken server which advertises MFMT but breaks on it (quirk flag --ftp-disable-mfmt)
  https://forum.rclone.org/t/option-to-retry-a-single-failed-file-instead-of-the-whole-sync/27571/2

If (1) users submit it as feature requests here on github (2) we allocate time to implement,
we are going to wait for upstream again :)))))))))))

[ivandeex]

We might go the following way as a short term solution:

• rollback go.mod "replace" hack as per Building latest stable (1.57.0) with Go 1.17 produces warnings from ftp backend #5810 (comment)
• have users open feature requests
• create a feature branch ftp which will accumulate new fixes/features independent of master branch
• open a series of pull requests against the branch with supporting ftp library patches on the dev branch of rclone/ftp
• open separate pull requests to cherry pick patches from ftp when patches from dev get accepted upstream

I'm not happy about it thou. Too much hassle. Higher odds of mistake.
The alternative is to:

• have users open ftp feature requests
• turn rclone/ftp into hard fork, modify backend code appropriately
• maintain rclone master against the fork, add supporting library fixes on fork
• backport library fixes upstream paced to suite their maintainer, stop worrying about dependency or diverging

@ncw thoughts?

[ivandeex]

Third alternative:

• modify go install build docs, replace go install by the snippet above
• keep rclone/ftp the soft fork
• maintain rclone master against the fork, add supporting library fixes on fork
• backport library fixes upstream like above

[ncw]

I would like to make go install work, which means either use the upstream or use a hard fork. So I don't think the soft fork alternatives work for me.

To hard fork rclone/ftp the only thing we need to do is

• drop replace from rclone's go.mod
• s|github.com/jlaffaye/ftp|github.com/rclone/ftp| in rclone's backend/ftp code
• s|github.com/jlaffaye/ftp|github.com/rclone/ftp| in rclone/ftp's go.mod

The fact that the ftp library doesn't have any sub modules makes keeping that one patch up to date quite easy I think.

So we could then make master on rclone/ftp have our hard fork patch in, plus any patches, but we could still submit pull requests from there as the code won't have diverged (only in the go.mod).

I think that would work, and would play nice with upstream and also play nice with go install unless I forgot something.

Merging code in from upstream could then be done with a merge commit or with a rebase and force push if we don't care about downstream git users.

This probably needs a bit more thought but it is a very soft hard fork as essentially we'd only be carrying forward the patch to rclone/ftp's go.mod.

The other alternative is for you to volunteer to help maintain jlaffaye/ftp so you can merge your own patches.

[vlavorini]

Hi, still no updates on this?

[ncw]

I'd like to make sure this is fixed for 1.58.0 using one of the methods above!

@ivandeex do you know if your patches have been merged upstream yet?

[ncw]

I would like to make go install work, which means either use the upstream or use a hard fork. So I don't think the soft fork alternatives work for me.

To hard fork rclone/ftp the only thing we need to do is

• drop replace from rclone's go.mod
• s|github.com/jlaffaye/ftp|github.com/rclone/ftp| in rclone's backend/ftp code
• s|github.com/jlaffaye/ftp|github.com/rclone/ftp| in rclone/ftp's go.mod

I have done this now, so that go get github.com/rclone/rclone will work from 1.58 onwards.

In due course I expect we will revert to upstream ftp, but this will fix the main issue.

Lessons learnt

• never make a release with a replace directive in go.mod
• when forking upstreams, change the go.mod to point to the new location so a replace line isn't necessary

I've merged this to master now which means it will be in the latest beta in 15-30 minutes and released in v1.58