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

Expose more of the package directly + simplify + better composability #26

Open
thomasf opened this issue Sep 7, 2017 · 10 comments
Open

Comments

@thomasf
Copy link

thomasf commented Sep 7, 2017

Many of the structs and their methods should progbly be exported directly and have exported members.. The way the package is constructed makes it nearly impossible to use in many situations.

There is also a needless involvment with details which the API consumer can take care of like the splitting of maintainer into separate name + email..

the exprted addFile.. methods should have a varant which takes something like io.reader + os.fileinfo so that it's possible to stream data into the archive.. The same problem exists in the other direction for DebPkg.Write()..

To me it's unexpected that a New() function allocates file descriptors. Maybe Open() is a better name to signal what actually is going on.

This is a lot more natual and easier to work with:

	d := debpkg.Deb{
      Name: c.Name,
      Version: "1.2.3",
    }
    
    w, _ := d.Open(target)

  // not exactly sure about the signature of WriteXXX. but probably one of these:
    w.Data.WriteFile(name string, r io.Reader, size int64)
    w.Control.WriteFile(name string, r io.Reader, os.FileInfo)
    
    ...
	defer w.Close()


    

Than this:

	d := debpkg.New()
	defer d.Close()

	d.SetName(c.Name)
	d.SetVersion(c.Version)
    ...
@xor-gate
Copy link
Owner

xor-gate commented Sep 7, 2017

I totaly agree the API is currently not idiomatic (I did came from a C/C++ background, so it feels a bit like it). As we still not have hit a release I prefer to break the API before the first release. Thats why I asked you to have a look at it.

I have thought of a reader interface, but I would also like to have a non-reader function (just give the filenames).

It is indeed weird New opens files on disk, Open is more convenient that I/O is happening. I'm not sure how the standard library handles NewXXX vs OpenXXX.

@thomasf
Copy link
Author

thomasf commented Sep 7, 2017

The Control/Data/Deb structs can maybe be made usalbe together from outside the package and let the DebPkg be the simplified interface which does the higher level stuff like adding files by os pathnames to keep the core functionality minimal.

@xor-gate
Copy link
Owner

xor-gate commented Sep 7, 2017

Yes I agree. This would also be useful for packages/applications which read debian packages into those structs.

@thomasf
Copy link
Author

thomasf commented Sep 8, 2017

I have been thinking about this some more.. Maybe the tar/ar stuff can be handled by package consumers themselves and focus the API towards building/validating the control file/archive.. Thats the part where actual complexity... Having said that that maybe already exists here https://godoc.org/pault.ag/go/debian/control in any case that package seems to be the best Go struct representation of control that I have seen so far.

@xor-gate
Copy link
Owner

xor-gate commented Sep 8, 2017

We must think about this very good, I would like to have the debpkg project a complete package which should be able to read and write debian packages. So we need some subpackages with helpers (tar, ar, control, data, signing) to make it more easy. For a CI pipeline you should only care about a yaml (or other) file which does the trick, or even a custom builddeb.go script.

Probably we need to split the project into multiple packages and make it some sort of "framework". Where the debpkg package is the one with the least typing to create a debian package from a go application.

@xor-gate
Copy link
Owner

xor-gate commented Sep 8, 2017

I don't want to go the way it is a package which can only do some control file and archive I/O. It must be a complete solution and that is why we created this in the first place (current debian build system doesn't fit in well in a golang ci build pipeline).

@thomasf
Copy link
Author

thomasf commented Sep 8, 2017

Yes, we are talking about two different things.. Use as a package (from debpkg/drone-deb) and use as a tool (as debpkg/drone-deb). The use-as scenario should of course always be the complete thing.

@xor-gate
Copy link
Owner

xor-gate commented Sep 8, 2017

Exactly, you may feel free to submit a WIP/Draft PR where your propose how you want to see how it integrates with drone-deb to expose some internals. Because I'm not sure what you expect, and for us the debpkg package works just fine without exposing to much. Make sure you add some tests for code you add or change.

@xor-gate
Copy link
Owner

xor-gate commented Sep 8, 2017

Are you aware of package github.com/paultag/go-debian?

@xor-gate
Copy link
Owner

@thomasf I created a draft in #29 of this issue with improved API. But not finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants