Show PKGBUILD diff when updating a package

[olejorgenb]

This would make it quicker for the paranoid to audit upgrades.

[pbrisbin]

Thanks for the request. I'm not sure I'll have time to tackle it soon. It seems like it'll be difficult since the PKGBUILD used to build the currently installed package is not readily available.

[LilyCathelineau]

@pbrisbin Hello! Forgive me if this is incorrect. I have been reading through the source code and thinking about how to implement this feature, and was wondering if the local PKGBUILD could be moved to a permanent directory specified during the set_defaults function after being downloaded to the temp_directory?

During the update process, the old PKGBUILD could be removed and then replaced with the new one. This could allow for archiving PKGBUILDs to compare at their current version. For the actual comparing process, you could download the PKGBUILD to the temporary directory, and compare it with the version in the permanent directory.

It also might be worth looking into .SRCINFO instead of PKGBUILD for parsing package information. I might be completely wrong on how to do this, so I am really sorry if I am.

[pbrisbin]

[I] was wondering if the local PKGBUILD could be moved to a permanent directory.

Seems like a reasonable way to do it. I would probably change the code to just use ~/.cache/aurget/$package/$version/PKGBUILD always (and not use a temporary directory at all). We could make this change first and ship it ahead of time as one increment.

Once that's established, it becomes pretty easy to implement a diff $cache/$package/{$old_version,$new_version}/PKGBUILD somewhere

I would probably let these build up, but provide some kind of --clean option that wipes the entire ~/.cache/aurget directory when used.

It also might be worth looking into .SRCINFO instead of PKGBUILD for parsing package information

I'm open to this if you provide more details on how it would work and what upside there is. However, I don't see any immediate need to do so.

[LilyCathelineau]

@pbrisbin That sounds like a good way to do it. 😄 The reason that I brought up a temp directory was because of the create_temp_directory function where PKGBUILDs are currently stored in, I didn't want to suggest removing code because I didn't know the full reason for it being added in that manner.

Also, for the --clean function it would make sense just to reuse the create_temp_directory function and just change temp_directory='/tmp/aurget' to temp_directory='~/.cache/aurget' and change when the function is called to only be used in the context of --clean.

For the .SRCINFO part, the idea of using it to parse metadata instead of using PKGBUILD is from a post by AladW. To my understanding the only benefit is that because the metadata is much more simple, and just contains the information in a key/value format, it is more reliable to parse, and would also allow for simplifying the parser.

[pbrisbin]

Also, for the --clean function it would make sense just

I generally avoid discussing actual in-code functions and variable names until we get to PR stage, where we can comment on concrete lines. That said though, I wouldn't want to leave functions or variables with "temp" in their names if they aren't actually dealing with a temporary directory.

To my understanding the only benefit is that because the metadata is much more simple, and just contains the information in a key/value format, it is more reliable to parse, and would also allow for simplifying the parser.

That benefit doesn't really apply to aurget because (for better or worse) we don't do our own parsing, we just source it into the shell environment. Parsing .SRCINFO would actually increase complexity compared to that, so there would have to be other upsides for me to consider such a change.

[LilyCathelineau]

I generally avoid discussing actual in-code functions and variable names until we get to PR stage Okay, sorry. 😢 If you want I can try and implement the feature. I am not as experienced in shell script as you are, but it seems like something that shouldn't be too hard to do. That benefit doesn't really apply to aurget because (for better or worse) we don't do our own parsing, we just source it into the shell environment. Okay, that makes sense.

[pbrisbin]

Please feel free to work on this! I don't know when I'd have to time to take it on, so PRs are probably the fasted bet.

My only recommendation would be to try and slice it up into very small changes (at least at first). Besides just reducing the risk of us shipping something broken, this will make sure you get review early on, and avoid writing a lot of code that I might not accept due to issues that could've been easily avoided.

[LilyCathelineau]

@pbrisbin Before I continue doing anything, could you take a look at the implementation of the --clean argument? https://github.com/LilyCathelineau/aurget/commit/b02a3971d665c60d9f843e034e0dec54bbeab78f I don't know if I properly implemented it based on the current structure of the project. (The function called at the end is in the first commit.)

[pbrisbin]

Can you open that as a PR? It's not complete enough to merge of course, but it'll be easier for me to review (and leave line comments) that way