On 07/09/13 20:34, Ashley Whetter wrote:
On 2013-09-07 07:40, Allan McRae wrote:
On 06/09/13 08:13, ashley@awhetter.co.uk wrote:
From: Ashley Whetter <ashley@awhetter.co.uk>
This time around I've split up amekpkg as per Allan's recommendation, and put each function into it's own file. There's a file at the base of each libmakepkg directory that imports that part of the library. (eg libmakepkg/download.sh imports libmakepkg/download/*.sh). Instead creating a file that imports every part of the library, I've instead put the for loop straight into makepkg.sh.in. Each libmakepkg file has it's own inclusion guard. Each libmakepkg copyright notice was deduced from the full set, dependent on what years git blame said that each line of a function had been written in.
I feel like the scripts Makefile is getting a bit cluttered. Even the number of .sh.in files is getting a bit big. This could be reduced by not using LIBRARY in every libmakepkg file and using relative imports instead, but this seems like even less of a good idea. I'm not really sure if/how we want to get rid of this issue.
OK - definitely too much splitting...
I mentioned this on IRC while I was doing it and I totally agree.
I wanted the download_foo functions to be in individual files are they are fairly substantial functions. But thinks like the message functions that are four lines do not need split.
So lets work on a layout before we go further. Here is a start: https://wiki.archlinux.org/index.php/User:Allan/Makepkg_Split
Allan
I disagree with util/source.sh going into util. The source array is part of a PKGBUILD so I think it might be worth having a separate libmakepkg/pkgbuild folder, and putting source.sh and pkgbuild.sh in it.
To clarify, this is what I put in util/source.sh: utils/source.sh - get_filepath() - get_filename() - get_url() - get_protocol() I consider these utility functions for extracting information from URLs. We could name the file utils/url.sh instead?
I definitely agree with grouping the download and extract functions together. It makes more sense, and adding a new VCS target will mean creating only one new file.
To move out the extraction functions we'll also need to address extract_sources calling the check_build_status function. I originally grouped check_build_status into a makepkg specific part of the library. We could put check_build_status into libmakepkg/makepkg.sh (or is this what libmakepkg/packaging.sh is for?).
I put packaging.sh there as a place to deal with the run_build, run_package etc functions.
I don't think makepkg.sh should go into the util folder because it's not a "generally useful" function, but specific to the functionality of makepkg. Also to quote something I said in my original grouping proposal "extract_sources to check_build_status is the only dependency between 'sources' and 'makepkg'. I feel like it's worth getting rid of this dependency somehow?".
The few lines dealing with updating the pkgver and checking whether the build needs to continue should be removed from the extract sources function. They really should just be put into the makepkg script itself right after the extract_sources function is called.
check_build_status also depends on get_full_version. "I think get_full_version is a PKGBUILD specific function, but run_function (in utils) depends on it to create the log file. Maybe logging stuff could be split into another [part of the] library?". check_build_status also depends on install_package. I originally put install_package in makepkg.sh/packaging.sh because although it was calling pacman, it was generating the location of the generated package file (something which is specific to makepkg) to pass onto pacman. So I didn't think it made sense to put it into dependencies.sh unless we passed it a list of package files to install. install_package depends on run_pacman. I still think it makes sense to put this into libmakepkg/util/dependencies.sh.
I would not get too carried away with what is makepkg specific. I highly doubt anything else will make use of this "library". As far as I am concerned, the whole reason for the split is to make the script more maintainable and allow for testing individual components. I see run_pacman as a utility function.
Minor side note: Are we calling util "utils" or "util"? I keep writing utils then correcting myself, but it should be plural as it's a set of utilities.
I us "util" because we have util.c in the pacman source. But I really do not care. BTW, feel free to edit that wiki page with your proposed layout. You can even put a different section for your version. It is a lot easier to visualise than describing with text. Allan