feat: add amove

[CosmicToast]

Amove is a utility function for subpackage splitting.
It takes up to 2 arguments:
$1 is a relative directory (or file)
$2 (optional) is a regex-like format, as taken by find(1p)/-name

If only $1 is specified, it will move $1 from pkgdir to subpkgdir,
creating what is needed and cleaning up after itself appropriately.
If $2 is specified, $1 should be a directory (not checked - unneeded).
When $2 is specified and $1 is a directory, all files directly inside of
$1 that match the $2 namespec will be copied instead.

[CosmicToast]

Ping @maxice8 - directly related to #74 and similar.

[maxice8]

@ncopa

[ncopa]

return error instead of silently do nothing

[ncopa]

In general, i have been hesitant to add functions like this, because I want keep it easy to understand what is going on when you read the APKBUILDs. I don't want end up as gentoo with a ton of magic functions and you have no clue of what is actually going on behind the scene.

However, we have grown much and we need to make maintenance easier, and I think this will do so.

But we should probably think, what other a* functions do we need/want? It would be good to have a bigger picture though of it so we don't end up with a big mess down the road.

[CosmicToast]

I would say it should be a matter of how common something is.
If nearly every time an APKBUILD calls cmake, it passes some arguments (like -DCMAKE_INSTALL_PREFIX), perhaps that should be wrapped.
Similarly amove is intended to be used in practically every split function, and minimizes some common mistakes (like forgetting to create the subpkgdir locations, or forgetting to clean up afterwards).
By that logic, there's probably no need for an amake function, since make invocations are usually just make, unless something needs overriding.

[CosmicToast]

So is there anything left/needed to do here?
All noted concerns (so far) have been resolved.

[maxice8]

Looks good to me. Just need the big penguin to approve.

[ncopa]

I'm not sure I like the API. Wouldn't it be nicer with: amove DIR/PATTERN? for example:

  amove 'usr/lib/libfoo.so.*'

Something like:

amove() {
	local subdir=${1%/*}  # directory part
	local fpattern=${1##*/} # pattern
	...
	cd "$pkgdir"
	mkdir -p "$subpkgdir/$subdir"
	find "$subdir" ! -path "$subdir" -prune ! -type d -name "$fpattern" \
		-exec mv '{}' "$subpkgdir/$subdir/" \;
	...
}

But we may want be able to move dirs too: amove usr/lib/python3*

[ncopa]

I have doubts about the API. What do you think about having a single arg only, but takes a pattern:

amove() {
    ...
    cd "$pkgdir"
    local f
    for f in $1; do
        # only create dir if needed
        if [ "${f%/*}" != "$f" ]; then
            mkdir -p "$subpkgdir/${f%/*}"
        fi
        mv "$pkgdir"/$f "$subpkgdir/${f%/*}"
    done
    ...
}

then we can do things like:

    amove 'usr/lib/libfoo.so.*'
    amove 'usr/lib/python3*'

That way we don't need to have the second option variant at all.

[CosmicToast]

1. Why not do subdir + pattern?
   ->
   Because moving individual directories seamlessly is important too (think -openrc, just amove /etc/init.d && amove /etc/conf.d).
   Getting this working without a lot of extra complexity isn't worth it, in my opinion, though you're welcome to do it if you'd like (I can try to dig up and update the test suite I had written, if I can find it (it has been half a year by now)).
   Moving multiple directories can be done by a basic for loop (for dir in /usr/bin/python3*; do amove $dir; done).

2. The second comment
   ->
   I'm not entirely sure what's being suggested here, how's this better than the first variant?
   Further, the # only create dir if needed part worries me, what's the harm in mkdir -p $subpkgdir without anything following it?
   The harm of explicitly avoiding that is, of course, a corner case no one's thought of.

I'm not sure why the API is suddenly a problem a whole half year after the design phase, and almost 3 months into the review.
If someone wants to write a significantly different variant they remain free to do so, but after waiting 6 months (3 of which was before even submitting it, as per your personal request to wait until the 3.10 release before doing so) I'd rather not start the whole process over.
Hopefully that sentiment is understandable.

If you just don't want to merge this, you can just say so and close it.

[ncopa]

1. Why not do subdir + pattern?
   ->
   Because moving individual directories seamlessly is important too (think -openrc, just amove /etc/init.d && amove /etc/conf.d).
   Getting this working without a lot of extra complexity isn't worth it, in my opinion, though you're welcome to do it if you'd like (I can try to dig up and update the test suite I had written, if I can find it (it has been half a year by now)).

It would be great if you could dig up the tests.

What are the cornercases that you are trying to handle?

Moving multiple directories can be done by a basic for loop (for dir in /usr/bin/python3*; do amove $dir; done).

2. The second comment
   ->
   I'm not entirely sure what's being suggested here, how's this better than the first variant?

What I'm thinking is that instead of having the function take one or two options, it only takes one: amove PATTERN.

Further, the # only create dir if needed part worries me, what's the harm in mkdir -p $subpkgdir without anything following it?
The harm of explicitly avoiding that is, of course, a corner case no one's thought of.

It handles the cornercase when there are no / in the arg. For example amove etc.

I'm not sure why the API is suddenly a problem a whole half year after the design phase, and almost 3 months into the review.

If someone wants to write a significantly different variant they remain free to do so, but after waiting 6 months (3 of which was before even submitting it, as per your personal request to wait until the 3.10 release before doing so) I'd rather not start the whole process over.
Hopefully that sentiment is understandable.

If you just don't want to merge this, you can just say so and close it.

Sorry for taking long time. It took me a while to think if we want this in the first place and I concluded that we do. But I want sure that we do it the best possible way, because once this is introduced, it may be difficult or impossible to change the behavior. So I want make sure its done right.

And I am not convinced that two different behavior depending on the number of arguments is the best way to handle this. I believe we can cover all our needs with only one arg.

[CosmicToast]

Sorry for taking long time. It took me a while to think if we want this in the first place and I concluded that we do. But I want sure that we do it the best possible way, because once this is introduced, it may be difficult or impossible to change the behavior. So I want make sure its done right.

Okay.

And I am not convinced that two different behavior depending on the number of arguments is the best way to handle this. I believe we can cover all our needs with only one arg.

I believe this is a case of misunderstanding semantics - the behavior is (from the user's perspective) the same.
The first argument specifies the directory (working on individual files is a happy coincidence).
The second argument, if specified, narrows things down - instead of the entire directory, it only moves over things inside that directory that match the pattern.
I.e the first argument was never a pattern to begin with, but a selector.

It would be great if you could dig up the tests.

I did in fact manage to find them - https://minio.toast.cafe/shared/amove-tests.tgz
To run them, put an amove binary in path first (simply taking it out from the PR and adding #!/bin/sh at the top and amove "$@" at the bottom is sufficient).

What are the cornercases that you are trying to handle?

.a files are a significant one, but see the tests for further details.
Consider a hypothetical package for a lightweight language.
Modules are distributed as .a files, but there is also liblanguage.a.
A module is named libfoo.a.
Modules live in /usr/lib/language/modules.
Using a regular find(1) on .a files recursively would result in language-static also holding all of the modules.
Using a glob (/usr/lib/lib*.a) has awkward failure conditions because of how shell parsing works (would be better in zsh).
So I use find with a specially crafted prune directive to get -maxdepth 1 behavior in pure POSIX (just as an example).

Moving multiple directories can be done by a basic for loop (for dir in /usr/bin/python3*; do amove $dir; done).

What I'm thinking is that instead of having the function take one or two options, it only takes one: amove PATTERN.

I think this has a couple of downsides:

1. It is unintuitive. amove as-is now roughly emulates find, with -name stripped.
   Consider move some/dir *.a -> find some/dir -name *.a.
2. Splitting the pattern like that introduces possible edge cases on unexpected inputs (test suite would need to be significantly expanded) as well as adding more complexity.

As it is now, it's the "simplest possible building block" with as few side-effects as possible.
I'm not sure if an aesthetic improvement/preference (having a single arg) is really worth making the change.
If we do have single-arg, we might as well for arg in there to allow multiples (e.g amove /etc/init.d /etc/conf.d /usr/lib/*.a), which just pushes it further in that direction.

It handles the cornercase when there are no / in the arg. For example amove etc.

When I have time (likely later today) I'll expand the test suite and correct anything needed, posting a new comment.

[CosmicToast]

It handles the cornercase when there are no / in the arg. For example amove etc.

This was actually the original mode of operation. Using non-relative directories (e.g amove /etc) would actually fail.
Seeing as to how that might be confusing (and in the spirit of emulating find(1) as if in pkgdir), I changed the check for relative directories into a corrective statement (stripping the first / from $1 if it's present).

I also updated the test suite to check sanity re: / stripping. (and rebased)
The latest version is still available at the same url.

[ncopa]

Thanks for the test suite!

I have played around with the tests and with the amove function.

My conclusion is that it should be:

amove PATTERN...

That way we can do things like:

amove 'lib/lib*.a'
amove 'etc/*.d' # move both etc/conf.d and etc/init.d

If user actually wants find(1) behavior, he/she will have to invoke find(1) directly. That makes it more obvious what is going on:

cd "$pkgdir"
find usr -name '*.a' | xargs amove

[ncopa]

This was fixed with aa86438.

Thank you for your work on this, and thank you for your patience.