magiconair / properties

Java properties scanner for Go
BSD 2-Clause "Simplified" License
323 stars 77 forks source link

FTBFS: test failure: "circular reference" #14

Closed onlyjob closed 8 years ago

onlyjob commented 8 years ago

As reported in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=830727 magiconair/properties fails test on x86_64 as follows:

  === RUN   Test
  2016/07/10 22:26:08 circular reference
  exit status 1
  FAIL  github.com/magiconair/properties    0.004s
onlyjob commented 8 years ago

I've traced this problem to dodgy _TestLoadExpandedFile_ test which is sensitive to USER environment variable. Test succeeds if I set USER=nobody.

magiconair commented 8 years ago

Thanks and sorry for the late reply. I'll have a look. I should be able to make the test independent of the actual environment.

onlyjob commented 8 years ago

Thanks. :)

magiconair commented 8 years ago

@onlyjob: Could you test whether that fixes the issue?

Also, may I ask

  1. why you are packaging the source code of this repository?
  2. why are you decoupling the vendored in dependency gopkg.in/check.v1? The whole point of the Go dependency management is to not do this to avoid the transitive dependency problem. What if the gopkg.in/check.v1 version you are vendoring is is incompatible with the one my code needs?
onlyjob commented 8 years ago

Yes, b3f6dd5 fixed the problem. Thanks. :)

why you are packaging the source code of this repository?

This library is a dependency of other libraries that are part of dependency chain of Docker, rkt, fleet, kubernetes and others. Personally I'm not much involved into packaging of this particular library but I assume that by "you" you probably meant "Debian"...

why are you decoupling the vendored in dependency gopkg.in/check.v1?

We are shipping reusable Golang libraries separately, aiming at stable releases, properly tested on many architectures. In large coherent software repository like Debian's it is not viable to maintain multiple private versions of a library or even simple bugfix would turn into maintenance/backporting nightmare let alone security support...

The whole point of the Go dependency management is to not do this to avoid the transitive dependency problem.

IMHO this is where we have to compensate for broken Golang concept that thrown away 20+ years of best practice in software engineering...

What if the gopkg.in/check.v1 version you are vendoring is is incompatible with the one my code needs?

Since we are un-vendoring dependency libraries your concern is perfectly valid. If gopkg.in/check.v1 break API then there might be some FTBFS or other problems, tests failures, etc. We will have to identify the problem and submit a bug asking you to update your vendored libraries... ;)

Decay of vendored libraries is a practical problem from maintenance prospective. How do you get incentive to update vendored libraries and track changes in them? Any update may require an effort from you. It is much easier to track changes and problems in standalone package. On this instance our QA/CI people produced bug report that would hardly reach you otherwise because when your library is vendored its tests suite is hardly ever run... I hope it make sense.

magiconair commented 8 years ago

Well, one could also argue that you are trying to make Go behave the way it was not intended to be used. It is the responsibility of the kubernetes, rkt and Docker maintainers to ensure that their setup works not Debians. It is also my responsibility that dependencies for my library work.

If you want to build kubernetes you get their source tree and build it. It should have everything in it. With this approach you now force everybody to be on - or at least to be compatible - with the same version whether they want to or not.

Whether Go has thrown away 20+ years of best practice or is using a different approach is debatable IMO. For my part, I am quite happy that I can control the dependencies of my project and that this isn't someone elses responsibility. It also relieves me of bug reports stemming from incompatibilities introduced by this process.

Updating vendored libraries should require an effort from me since I - the maintainer - can decide best whether an upgrade of a dependency has an impact or not.

However, if you want to go through the trouble of unsplitting this then so be it. Things that work on a smaller scale may not work at a larger scale and it depends on how committed a maintainer is.

I can always re-evaluate this if tickets are opened as a result of this.

onlyjob commented 8 years ago

It is the responsibility of the kubernetes, rkt and Docker maintainers to ensure that their setup works not Debians.

Not quite. They are simply not in position to cover everything. How many architectures you test your software on? In practice Debian always build software for more architectures than original developers. You also overestimate ability of sophisticated projects to track changes in their dependencies (direct ones as well as indirect ones). Docker depends on more than 160+ libraries. How would you track changes in long dependency chains?

Besides we've already seen abuse of vendoring practice when software A depends on libs B and C, where C depends on different version of B and all those are vendored and built into resulting executable with more than one version of B...

Think of security support. How can you provide one if different versions of a library are scattered everywhere and built privately? It is basically the same problem as (universally condemned) static linking.

Think of benefits of coherent native software repositories in GNU/Linux distributions.

I don't have time to explain all this. Please have a look at https://wiki.debian.org/UpstreamGuide

Thanks.

onlyjob commented 8 years ago

One more thing: neither Docker nor any other Golang binary I've seen runs test suites of their dependency libraries. In Debian, we do. And we know how many bugs (and how often) are vendored with outdated libs that need update which is conveniently neglected due to ignorance or false rationale... Vendoring only gives you convenience of not updating for library transitions when it may be required... For example when update of your dependency library fixes problem on architecture you are not building your application for...

magiconair commented 8 years ago

Look, I get the differences and as I've said here:

Things that work on a smaller scale may not work at a larger scale and it depends on how committed a maintainer is.

yobert commented 8 years ago

This discussion is a huge philosophical problem. I see Fedora making RPMs out of go library source code and they have the same struggles. Go developers (myself included) tend to hate the old ways of doing software. I don't think there is an easy answer here. The role of maintainer grew out of developers not having users in their best interest, especially when a user does not want to upgrade the way the developer thinks they should.

magiconair commented 7 years ago

@yobert @onlyjob As a result of this discussion I've removed the dependency on gopkg.in/gocheck.v1 in #15 which is in v1.7.1 and later.

yobert commented 7 years ago

Sorry if my comment was confusing. (Re-reading it, it's quite poorly worded.) What I mean is, Fedora does the same thing as Debian and creates packages out of Go source code for dependencies. It's a very common thing for a package maintainer to break up build dependencies into packages matching the rest of the ecosystem.

This really isn't any different than how a package maintainer would approach if you put libpng16.so directly into your project. They would decouple that and make it match the rest of the ecosystem. Go just feels extra strange in this case, because go libraries are distributed as source code, not as compiled code.