Use git clone for pkgbuild downloading #297
Conversation
Would you use different cache directories for
I used to The main drawback imo with
I don't think optimizations are needed when cloning AUR repos. They're very small after all. Interesting idea on |
|
I agree merge is probably the way to go. If a PKGBUILD has been edited, the changes should probably be kept between updates. They can always clean build and grab a fresh copy if they want to discard them. Stashing seems like a good idea too. Too bad it caused some black magic errors. And yeah optimisation does seem a little pointless compared to the built package sizes. |
|
@AladW P.S. With the way the table is ordered now, what's the weight of optional entries? I believe it's considered worse than green even though I think there's nothing wrong with user choice. Maybe consider the default value instead? optional, default yes = green |
|
I'll try and benchmark as well, but operating only on the existing knowledge in this thread I'm beginning to flip flop on this:
Other stuff though:
|
|
My mentality is always give users a choice. Don't ever just take things away without good reason cough desktop icons. I'm happy for git to become the default setting though. |
|
I think having tar as a backup and git default is the best option. Simply if git breaks (which is does in silly ways) you can keep using tar, but git has still many convenient features you porbably want to have available by default.
I've proposed a while back to use a green entry if the helper supports |
|
I would have thought the point of git clone was just to use the git endpoint over the tarball one. Even though git provides diff functionality in a very convenient way diffs themselves are not a git thing and could easily be implemented in the tarball method. P.S. thanks for rewording the wiki I'm not so good with words. |
|
Yes I have similar doubts, hence I didn't make any changes on that regard but left it open for discussion. In any event, to me the sorting is After all, a yellow entry either means that something of note isn't enabled by default (like helpers that build without inspection, but have a flag to disable the build step), or that a behavior is either partially implemented or deviates from the norm in some way. Red means something is either impossible at all or is just plain broken. Perhaps these things should be clarified in the Note that mentions how sorting is done. |
I agree with this but having optional with a good default as green. Or have optional as yellow but have it act as green in the sorting.
That sounds like you agree with my first point.
Honestly I think helpers that are insecure by default, even if there is a flag to secure it, should be red. Similarly the opposite should be true. If they are secure by default but have a flag to operate in insecure mode then it should be green. While I could let it slide on git clone or whatever, being insecure is the worst offence. Users are probably more likely to use the default settings than anything else, especially when they start using a helper. |
|
User choice sounds good, as long as the fastest is the default I'm happy. |
|
Alright I'll work on this following what I said in the original description as well as have git be the default. Version stuff was worth mentioning but out of scope for this PR. |
Oh, that's how it has always been. At least aurutils has always supported both tar and git downloads but git is the default so it gets a green entry. Yaourt also supports both, but it uses tar by default so it gets a yellow one. So it already matches what you proposed in #297 (comment).
Well, the description of the secure column (much like the pacman wrap column) says "by default". I don't think you'll get much opposition if you wanted to propose those helpers who have a yellow entry in Secure to get a red entry instead though. |
|
Oh I didn't realise it was already like this. Aurutils just said yes instead of optional so I assumed it was git only. Maybe that should be explained better. |
|
I think it should be extended slightly to explain that "Yes" does not mean the opposite feature is unavailable. e.g. Yes to git clone does not imply no to tarball. |
|
I'm not sure how to word this or if it makes sense to add this to the general note, but perhaps we can reword the description of the "git clone" column where it's most relevant. (I was never fond of the arbitrary "uses git because tar is deprecated" since there's no harm in supporting tar and upstream hasn't mentioned they would actually remove this functionality.) For now I've just removed the reference to tar and focused on git. https://wiki.archlinux.org/index.php?title=AUR_helpers&diff=515208&oldid=515201 |
|
I wasn't sure how to word it myself, why I just gave an example instead. I think it's good as it is now, nice job. |
|
Git clone for abs stuff is a little more annoying but still doable. I'm not sure weather it's worth the effort to implement it. Could just use tarball only for abs stuff. asp should be used for most abs stuff anyway, Yay's implementation is rather basic. I've though about extending |
|
Doing something like It's possible to do the clone then move and delete files to get the standard layout, but that then removes the main benefits of cloning from git. Pulling wouldn't work, diffs wouldn't work unless you compare against the old pkgbuild's location. For this reason I think it is best to no support git on ABS packages, leave it up to asp. |
| } | ||
|
|
||
| return nil | ||
| } else if err != nil { |
There was a problem hiding this comment.
No need for this else (and the next one), since the blocks end with a return.
There was a problem hiding this comment.
True true, I'll change it in a bit.
| @@ -526,3 +526,15 @@ func passToMakepkg(dir string, args ...string) (err error) { | |||
| } | |||
| return | |||
| } | |||
|
|
|||
| func passToGit(dir string, _args ...string) (err error) { | |||
There was a problem hiding this comment.
Maybe pick a different name for this _args variable? I skimmed quickly through the code and didn't find other variables named similarly (starting with an underline).
There was a problem hiding this comment.
I would rather not have to have two separate variables at all like in the other passTo* functions but it didn't work like that. If you've got a better method or name feel free to suggest one.
ef7040f
to
45a5c6c
Compare
fb3b2a4
to
e396982
Compare
Use git clone over tarballs for pkgbuild downloading during -S. This option can still be toggled using the config flags. The config option for selecting clone or tarball will be overiden if an existing package is cached. The method used to download the package perviously will be used regardless of the config.
|
I think this is about ready, you can debate weather or not we should show the git output but that's about it. You wanna give this a final test yourself @Jguer before merging? |
|
Tested, looking good, apparently I murdered the merge a bit though |
|
Well I have no idea what you did but it looks like you merged. Close this? |
|
You can now change |
|
Nope gotta wait for a new release first. |
Yeah, sorry. |
|
Speaking of, the release is now. @AladW If you would be so kind. |
|
Thanks! |
|
All |
|
Somewhat offtopic, but I've also added a discussion to lessen restriction on what an "active" helper is, as well as move out some entries from the main comparison table. Feel free to make comments. https://wiki.archlinux.org/index.php/Talk:AUR_helpers#Effectiveness_of_the_.22inactive.22_table |
Use git clone for pkgbuild downloading
This is a restart of #130 given that PR has not gone anywhere and @simon04 has still not answered any of my questions related to the PR such as:
I decided it was better to start over. This PR also uses some of the code from #130.
This PR in its current state is mainly for discussion on how to implement things. Currently all this PR does is replace the download method during
yay -Sto usegit. This allows easy benchmarks, just checkout between the two versions and test for yourself.Testing
My testing was done against
ros-lunar-desktop-fullwhich on my machine wants to download 251 Aur packages.Time taken to download all the packages assuming none exist in cache:
3:261:43If all packages in cache and up to date then Yay will not try to redownload them but if using
--redownloadallthen the results are as follows:3:26(The same download is done, the time does not change)1:10(it only needs to pull instead of clone)This shows git to actually be faster than the tarball downloads.
Features
The features I plan to implement before merging are:
Use git clone in -G for ABSIdeas that may be implemented:
@versioning: When installing AUR packages. For exampleyay -S yay@1e01eafwould build the package from that revision of the PKGBUILD.<>=versioning: Pacman already supports using these during-Sto ensure the package you want fits in the range you specify. What I purpose is slightly different. Yay will find the first latest commit that matches the version you specify. Soyay -S yay<4will install the first PKGBUILD with a pkgver of less than 4.Discussion
This PR is mostly here for discussion on how to implement certain features.
Migrating between tarball and git
My current idea of this is to only respect the config option if the cache does not already exist. If it does exist check for the .git directory. If that exists use the git method no matter the config. If it does not use tarball no matter the config.
Updating the cache
When we have a cached package and the PKGBUILD updated how should we go about pulling changes? The tarball method is simple and just unpacks the tar into whats already there. Overwriting files that exist in the tar and in cache but leaving untracked files alone.
Doing a simple git pull could cause complications if there are any user made changes. Currently this PR resets to head then pulls which will undo any user made changes. #130 uses a similar method where they fetch then reset to upsteam master. I do not know id there are any real differences between the two.
Alternatively we could just do a pull by itself. If there are merge conflicts it would be the user's job to resolve them.
Pacman wrap
using
<>=behaviour different to Pacman might be frowned upon.maybe combine it with
@yay -S yay@7655bd9yay -S yay@=3.505yay -S yay@<4Optimisation
This implementation is very basic, there's a couple of possible tricks that could be done to speed things up or reduce disk space but I have not tested them.
git gcShould probably be run after every pull.--singlebranchcould be used to only clone master, although most AUR packages probably only have master, I am unsure if the AUR would even allow you to push a different branch.--depth 1Is an obvious one but would not allow the user to then checkout specific commits if they wished to. Also possible features such as@and<>=versioning would not work with this. It might be possible to run--depth 1then run--unshallowwhen needed.