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

Migrate github.com/docker/docker/pkg/symlink #54

Merged
merged 63 commits into from
Oct 29, 2020

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Oct 29, 2020

This migration was taken from https://github.com/moby/moby/commits/0b93c6e1315aae52638474476c9e91f2677d2e52/pkg/symlink

Strategy taken:

# install filter-repo (https://github.com/newren/git-filter-repo/blob/main/INSTALL.md)
brew install git-filter-repo

cd ~/projects

# create a temporary clone of docker
git clone https://github.com/docker/docker.git moby_symlink
cd moby_symlink

# create branch to work with
git checkout -b migrate_pkg_symlink

# remove all code, except for pkg/symlink
git filter-repo --path pkg/symlink

# go to the target github.com/moby/sys repository
cd ~/projects/moby-sys

# create a branch to work with
git checkout -b integrate_moby_symlink

# add the temporary repository as an upstream and make sure it's up-to-date
git remote add moby_symlink ~/projects/moby_symlink
git fetch moby_symlink

# merge the upstream code, rewriting "pkg/symlink" to "symlink"
git merge --allow-unrelated-histories --signoff -S -Xsubtree=pkg/ moby_symlink/migrate_pkg_symlink

After the above;

  • symlink: remove "github.com/docker/docker" import enforcement comments
  • symlink: add go.mod
  • Makefile: run tests on symlink module
  • symlink: add package doc

crosbymichael and others added 30 commits May 13, 2014 10:35
Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)
Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)
…d#5370

normally symlinks are created as either
ln -s /path/existing /path/new-name
or
cd /path && ln -s ./existing new-name

but one can create it this way
cd /path && ln -s existing new-name

this drives FollowSymlinkInScope into infinite loop

Docker-DCO-1.1-Signed-off-by: Lajos Papp <lajos.papp@sequenceiq.com> (github: lalyos)
ideally it should never reach it, but there was already multiple issues with infinite loop
at following symlinks. this fixes hanging unit tests

Docker-DCO-1.1-Signed-off-by: Lajos Papp <lajos.papp@sequenceiq.com> (github: lalyos)
use path.IsAbs() instead of checking if first char is '/'

Docker-DCO-1.1-Signed-off-by: Lajos Papp <lajos.papp@sequenceiq.com> (github: lalyos)
Docker-DCO-1.1-Signed-off-by: Tibor Vass <teabee89@gmail.com> (github: tiborvass)
Docker-DCO-1.1-Signed-off-by: Tibor Vass <teabee89@gmail.com> (github: tiborvass)
The doc (or src) says: The result is Cleaned.

http://golang.org/pkg/path/filepath/#Join

Signed-off-by: shuai-z <zs.broccoli@gmail.com>
The doc (or src) says: The result is Cleaned.

http://golang.org/pkg/path/filepath/#Join

Signed-off-by: shuai-z <zs.broccoli@gmail.com>
Docker-DCO-1.1-Signed-off-by: Cristian Staretu <cristian.staretu@gmail.com> (github: unclejack)
Signed-off-by: Tibor Vass <teabee89@gmail.com>
Docker-DCO-1.1-Signed-off-by: Cristian Staretu <cristian.staretu@gmail.com> (github: unclejack)
Signed-off-by: Tibor Vass <teabee89@gmail.com>
Docker-DCO-1.1-Signed-off-by: Cristian Staretu <cristian.staretu@gmail.com> (github: unclejack)
Signed-off-by: Tibor Vass <teabee89@gmail.com>
Signed-off-by: Tibor Vass <teabee89@gmail.com>
Signed-off-by: Andrew Page <admwiggin@gmail.com>
Signed-off-by: Tibor Vass <teabee89@gmail.com>
Signed-off-by: Tibor Vass <teabee89@gmail.com>
Signed-off-by: Tibor Vass <teabee89@gmail.com>
Signed-off-by: Andrew Page <admwiggin@gmail.com>
Signed-off-by: Tibor Vass <teabee89@gmail.com>
timwangdev and others added 18 commits December 31, 2015 13:07
Signed-off-by: Tim <timwangdev@gmail.com>
Signed-off-by: John Howard <jhoward@microsoft.com>
Signed-off-by: Jessica Frazelle <acidburn@docker.com>
Signed-off-by: Darren Stahl <darst@microsoft.com>
Signed-off-by: phineas <phin@phineas.io>
* additional
* ambiguous
* anonymous
* anything
* application
* because
* before
* building
* capabilities
* circumstances
* commit
* committer
* compresses
* concatenated
* config
* container
* container's
* current
* definition
* delimiter
* disassociates
* discovery
* distributed
* doesnotexist
* downloads
* duplicates
* either
* enhancing
* enumerate
* escapable
* exactly
* expect
* expectations
* expected
* explicitly
* false
* filesystem
* following
* forbidden
* git with
* healthcheck
* ignore
* independent
* inheritance
* investigating
* irrelevant
* it
* logging
* looking
* membership
* mimic
* minimum
* modify
* mountpoint
* multiline
* notifier
* outputting
* outside
* overridden
* override
* parsable
* plugins
* precedence
* propagation
* provided
* provides
* registries
* repositories
* returning
* settings
* should
* signals
* someone
* something
* specifically
* successfully
* synchronize
* they've
* thinking
* uninitialized
* unintentionally
* unmarshaling
* unnamed
* unreferenced
* verify

Signed-off-by: Josh Soref <jsoref@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Changes most references of syscall to golang.org/x/sys/
Ones aren't changes include, Errno, Signal and SysProcAttr
as they haven't been implemented in /x/sys/.

Signed-off-by: Christopher Jones <tophj@linux.vnet.ibm.com>

[s390x] switch utsname from unsigned to signed

per golang/sys@33267e0
char in s390x in the /x/sys/unix package is now signed, so
change the buildtags

Signed-off-by: Christopher Jones <tophj@linux.vnet.ibm.com>
[project] Switch most syscalls to golang.org/x/sys
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Oliver Reason <oli@overrateddev.co>

Update LICENSE.APACHE

Signed-off-by: Oliver Reason <oli@overrateddev.co>

Update LICENSE.BSD

Signed-off-by: Oliver Reason <oli@overrateddev.co>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Steps taken:

    # install filter-repo (https://github.com/newren/git-filter-repo/blob/main/INSTALL.md)
    brew install git-filter-repo

    cd ~/projects

    # create a temporary clone of docker
    git clone https://github.com/docker/docker.git moby_symlink
    cd moby_symlink

    # create branch to work with
    git checkout -b migrate_pkg_symlink

    # remove all code, except for pkg/symlink
    git filter-repo --path pkg/symlink

    # go to the target github.com/moby/sys repository
    cd ~/projects/moby-sys

    # create a branch to work with
    git checkout -b integrate_moby_symlink

    # add the temporary repository as an upstream and make sure it's up-to-date
    git remote add moby_symlink ~/projects/moby_symlink
    git fetch moby_symlink

    # merge the upstream code, rewriting "pkg/symlink" to "symlink"
    git merge --allow-unrelated-histories --signoff -S -Xsubtree=pkg/ moby_symlink/migrate_pkg_symlink

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

I manually marked the DCO test to pass (see #53 (comment))

@dims
Copy link

dims commented Oct 29, 2020

LGTM 🚢 🚀 !!

@thaJeztah
Copy link
Member Author

When working on this PR, I recalled there's some things (not for this module alone) we need to address around the licensing. I noticed that while the repository has a top-level LICENSE in place, go submodules effectively act as individual repositories, which means that the individual modules don't have a license.

I think we need a LICENSE and NOTICE in each of the submodules, and while we're at it, add license headers to individual files (as is done in containerd/containerd), and probably change copyright to The Moby authors (or something along those lines) going forward.

I'll create a tracking issue for that.

@thaJeztah
Copy link
Member Author

ping @kolyshkin @cpuguy83 PTAL

@cpuguy83
Copy link
Member

LGTM

@cpuguy83 cpuguy83 merged commit 1bc8673 into moby:master Oct 29, 2020
@thaJeztah thaJeztah deleted the integrate_moby_symlink branch October 29, 2020 19:32
@@ -405,3 +355,12 @@ func TestFollowSymlinkNoLexicalCleaning(t *testing.T) {
t.Fatal(err)
}
}

func mkTempDir(t *testing.T) (string, func()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go 1.15 adds (*testing.TB).TempDir() which would be easier to use and won't require a cleanup.

But we'll have to wait until at least go 1.17 to use it :-\

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, nice; didn't know about the TempDir(), but we can start using CleanUp(). I did a PR for that in moby/term#21. That repo was still testing against go 1.13 for backward compatibility, but I see this repo only tests go 1.14 and go 1.15, so I can make a similar change here

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.