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

4.16: no longer possible to package invalid symlink #1159

Closed
edsantiago opened this issue Apr 1, 2020 · 6 comments
Closed

4.16: no longer possible to package invalid symlink #1159

edsantiago opened this issue Apr 1, 2020 · 6 comments

Comments

@edsantiago
Copy link

We have a package, buildah, that includes a symlink loop as part of its -tests subpackage:

lrwxrwxrwx. 1 esm esm   13 Oct 21 05:53 circular-link -> circular-link

This has packaged up just fine, up to and including rpm-4.15.1-2.fc32.1.x86_64

As of rpm-4.15.90-0.git14971.1.fc33.x86_64, rpmbuild fails with:

RPM build errors:
    Recognition of file "/root/rpmbuild/BUILDROOT/buildah-1.15.0-0.22.dev.git912ca5a.fc33.x86_64/usr/share/buildah/test/system/bud/dockerignore2/subdir/circular-link" failed: mode 120777 broken symbolic link to circular-link (Too many levels of symbolic links)

My lame guess is that c6ff614 is responsible.

I realize this is an obscure corner case and that in fact it may be desirable in most cases to catch this condition. This is, however, a regression and places us in a difficult situation wrt packaging our tests.

@cgwalters
Copy link
Contributor

This is, however, a regression and places us in a difficult situation wrt packaging our tests.

You can just ship a tarball of this stuff instead right?

@edsantiago
Copy link
Author

I'm sure we could find a workaround. It will be challenging because the same tests will need to work in CI as well as rpm-packaged, root and non-root. I will of course do that work if necessary. I filed this issue in hopes that the rpmbuild change was inadvertent/unintentional and could be reverted.

@soig
Copy link
Contributor

soig commented Apr 2, 2020 via email

@pmatilai
Copy link
Member

pmatilai commented Apr 2, 2020

Sorry but benefits catching what is broken by definition at build time outweighs the special case needs of any single project, by far.

The fact that its only caught the dependency generator is mostly a hint that we're lacking sanity checks someplace else.

@pmatilai
Copy link
Member

pmatilai commented Apr 2, 2020

I suppose this does deserve a mention in the release notes though - will add.

edsantiago added a commit to edsantiago/buildah that referenced this issue Apr 2, 2020
rpm 4.16 alpha in rawhide has broken our koji build:

   containers#2264

   rpm-software-management/rpm#1159

TL;DR we can no longer package the 'circular-symlink' file
used in tests. This PR removes that and other symlinks from
the git working tree, and reworks the test so it runs from
a temporary directory and creates all necessary symlinks.

Also, since the test as written was simply checking "did it
run" without actually testing anything, add a test to confirm
that the files added to the image are exactly what we expect.

Signed-off-by: Ed Santiago <santiago@redhat.com>
edsantiago added a commit to edsantiago/buildah that referenced this issue Apr 2, 2020
rpm 4.16 alpha in rawhide has broken our koji build:

   containers#2264

   rpm-software-management/rpm#1159

TL;DR we can no longer package the 'circular-symlink' file
used in tests. This PR removes that and other symlinks from
the git working tree, and reworks the test so it runs from
a temporary directory and creates all necessary symlinks.

Also, since the test as written was simply checking "did it
run" without actually testing anything, add a test to confirm
that the files added to the image are exactly what we expect.

Signed-off-by: Ed Santiago <santiago@redhat.com>
bors bot added a commit to containers/buildah that referenced this issue Apr 2, 2020
2267: Bump 1.14.6 r=rhatdan a=TomSweeneyRedHat

As the title says

2268: dockerignore tests : remove symlinks, rework r=rhatdan a=edsantiago

rpm 4.16 alpha in rawhide has broken our koji build:

   #2264

   rpm-software-management/rpm#1159

TL;DR we can no longer package the 'circular-symlink' file
used in tests. This PR removes that and other symlinks from
the git working tree, and reworks the test so it runs from
a temporary directory and creates all necessary symlinks.

Also, since the test as written was simply checking "did it
run" without actually testing anything, add a test to confirm
that the files added to the image are exactly what we expect.

Signed-off-by: Ed Santiago <santiago@redhat.com>

<!--
Thanks for sending a pull request!

Please make sure you've read and understood our contributing guidelines
(https://github.com/containers/buildah/blob/master/CONTRIBUTING.md) as well as ensuring
that all your commits are signed with `git commit -s`.
-->

#### What type of PR is this?

/kind other

#### What this PR does / why we need it:

Work around behavior change in rpmbuild

#### How to verify it

buildah will build in koji on rawhide (it currently does not)

#### Special notes for your reviewer:

#### Does this PR introduce a user-facing change?

```release-note
None
```



Co-authored-by: TomSweeneyRedHat <tsweeney@redhat.com>
Co-authored-by: Ed Santiago <santiago@redhat.com>
@pmatilai
Copy link
Member

pmatilai commented Apr 9, 2020

Comment added: rpm-software-management/rpm-web@1fddfd1

Closing, thanks for the report.

@pmatilai pmatilai closed this as completed Apr 9, 2020
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

No branches or pull requests

4 participants