[new major release] Test and release the noTomb branch
nxadm opened this issue · 5 comments
Branch: https://github.com/nxadm/tail/tree/noTomb
Tail depends on Tomb v1, a version of the library not updated updated in the last 5 years. The next version of Tomb, v2, is not API compatible with v1 and hasn't been updated for 2 years. A better solution is using the core coroutine workflow around context.Context. This branch implements this workflow and passes all the tests. However, because the API of Tomb v1 is very different to the one of context.Context, the changes in this branch need to be well tested in order to make sure no threading issues occur.
This branch also removes internal packages that resulted in many exported functions and structs. Most of these, probably all, were only useful within tail and their public nature was just a consequence of being in a package. This methods were removed from the public API. Also, the OpenFile function was removed from the API as it doesn't do anything special and is not related to tailing. The documentation was updated and public functions and structs were documented. Programs that use the "useful" API of previous releases will not be impacted:
- the variables DefaultLogger and DiscardingLogger.
- the structs Config, Line, Seekinfo and Tail.
- the functions TailFile, Tell, Stop and StopAtEOF.
This branch, once ready, will be released as a major release, v2.
Any update on this release? Would be great to get this finished.
@NWBY It's a little stalled due to lack of time, but certainly not dead. I wll deap dive in it soon. Thx.
First of all thanks for taking care of this project!
I'm trying to test out this branch, here are some ideas/thoughts I have
- You should be able to pass a context on the configuration and the TailFile function should be able to create a child context, this will make it so that the library can get signals through context from a parent process.
- Right now, I'm not sure if there's any option but to call the "TailFile" function, because it ends up calling
tailFileSync
, this makes some fields exposed by the Tail struct very much usless (the context and cancelfunc)
Also I've been hitting a different issue where the library (specifically the dependency fsnotify) "leaks" one go routine. This is because of the shared "inotifyTracker" that you can't really shut down and being a package level global, I've found that it's impossible to run a test twice (-count=2) if you use a library like https://github.com/uber-go/goleak to detect leaks.
Ideally I should be able to tell the library that I want to stop tailing a file and the library would end up shutting everything down down to the inotifyTracker. I also notice that you've made this one private (ideally you wouldn't need it but I do think it would be a good idea)
Let me know what you think!!
As you may have noticed, I have spent most of the time reviewing issues/PR from the original project instead of working on v2. This is because it looks like this repo is gradually being used as a drop-in replacement for the abandoned upstream (I am seeing more than 10000 clones per day and inclusion of this lib in Debian/Fedora/Ubuntu/RHEL etc). This makes me very careful in not breaking the lib while at the same time fixing "historical"bugs. There will certainly be a v2 (e.g. I marked 2 functions as deprecated), but the priority is somewhat lower.
This ticket will stay open to collect ideas about the next release.