Yay: Clean build discards source files needed for diff

[Magotchi]

Affected Version

yay v7.887.r4.g77a5cf5

Issue

If clean build is chosen for an AUR package, its package source files are discarded, and then if the diff is shown for the package, every line of every source file is shown as having been added.

Steps to reproduce

1. Install an AUR package with yay.
2. Update the package in the AUR.
3. Install the package with yay, but choose to do a clean build and to view a diff.

[Morganamilo]

Right now the clean build wipes out the package then the diffing is done. I'll see about swapping this around, doesn't seem to hard to do.

[Patricol]

A related improvement that would accomplish this as a side effect would be diffing between two commits in the remote repo; the newest and the one matching the version of the package currently installed. That'd prevent PKGBUILDS etc. from needing to be re-read every time .cache is wiped.

[Patricol]

I might do this; first time working with go though.

Looks like the relevant code is in showPkgbuildDiffs in install.go; anything I should know, @Morganamilo? (I don't see any contribution guidelines.)

[Jguer]

@Patricol mostly keep your code gofmt clean, avoid pulling in more third party dependencies ( unless it is truly justified) and in the case of showPkgbuildDiffs return errs instead of handling them would be my observations. I think the code changes needed will be small enough.

[Morganamilo]

I've though about doing this but didn't really go for it because you would need to keep an extra file around tracking the commit hashes. But ultimately I don't think it matters that much. I believe pikaur does exactly that if you need any inspiration.

Also there's a somewhat unofficial feature where you can change a pkgbuild, commit it back and as long as there are no conflicts yay will happily merge it with the aur and keep your changes. There's still one big problem where the diffs go from HEAD to HEAD@{upstream}, meaning your changes are always shown to be taken out even though this does not actually happen.

There's this repo https://github.com/Morganamilo/aurlib where I experimented with some stuff and got that fixed. It took a more aurutils like approach and dumped the diffs to files and showed them through vimf.

I never got around to implementing any of it proper. You can take a look at that too if you want to.

[Patricol]

Thinking about it more, can it be argued that that information could belong in the config folder?

I think it would make sense, from the perspective of "these are the PKGBUILDS(etc.) that I have read and approved, only show me diffs of them in the future."

For a user who reads the PKGBUILD(etc.) files or diffs in full every update; it makes more sense for the record of that to be in .config rather than .cache.

Of course it also doesn't belong in "$XDG_CONFIG_HOME/yay/config.json", but maybe something like "$XDG_CONFIG_HOME/yay/diffs-read.json"?

[Patricol]

@Jguer @Morganamilo Thoughts?
I don't want to move forward in that direction without approval; it'll impact how I design the implementation.

[Morganamilo]

I don't really see why you'd consider this belonging in config. This file would be caching the last commit we have reviewed. The user does not create it nor modify it and if it is deleted then it's a minor inconvenience.

[Patricol]

@Morganamilo very good points, I'll try to better explain the perspective.

if it is deleted then it's a minor inconvenience

The 'arch-way'* is to read all PKGBUILDs before installing things from the AUR. This is so inconvenient that few people do it; even people who can understand their contents. Diffs make it a lot more manageable. Assuming the user is doing things the 'arch-way', and actually reading PKGBUILDs, losing diffs can be extremely inconvenient.

The user does not create it nor modify it

Many useful config files work that way; including those for command-line utilities.
htop specifically advises against manually editing its config file.

This file would be caching the last commit we have reviewed.

95% sure we're on the same page with this sentence, but just to be sure: I'd intend it to store the last commit that the user has accepted as good, via the same interactive confirmation that already exists in yay.

Another point towards this being a convenient file to keep is when (re)installing arch. Being faced with reading all of the PKGBUILDs and related files for 30+ programs in the middle of a (re)install is very inconvenient; especially knowing that there's a strong chance nothing has changed since they were last reviewed.

*Not implying the 'arch-way' means right way; the 'arch-way' involves not using AUR helpers. Just pointing out that the ideal and supported user behavior involves reading all changes.

[Patricol]

Another point would be to think about what the concept of "cache" is.

Clearing a cache should normally not meaningfully impact the behavior of a program.

Generally it only impacts the performance of the program; as the program must spend time rebuilding whatever was in the cache.

In the case of yay's current functionality, clearing the cache meaningfully impacts the program's user-facing behavior; in a way that is very inconvenient for a small subset of users.

It might be easier to think of this as "it doesn't belong in .cache" rather than "it belongs in .config".

[Morganamilo]

As far as XDG is concerned it says config is for user specific config files and cache is for non essential data.

Although what is a config file is not really defined so really I don't know.

Clearing a cache should normally not meaningfully impact the behaviour of a program.

That also makes sense to me but is that actually defined anywhere in the XDG specification?

Anyway, I don't really know, but it's such a minor thing whatever you work on that's easy enough to be changed right at the end.

[Magotchi]

As far as XDG is concerned it says config is for user specific config files and cache is for non essential data.

In Arch, when a user has reviewed the entirety of a PKGBUILD, determined that it's safe to execute its contents, and indicated so in a program which remembers that choice, which can be quite the time-consuming action, I certainly wouldn't call that "non essential data". If I clear my cache or reinstall Arch, data that represents moderately difficult work I've performed should not be lost. Whether it's considered configuration data or something else, it should at least be considered important and something to be kept.

[JuniorJPDJ]

Are you still working on this?
It's annoying and makes me actually not to read new PKGBUILDs.

[Patricol]

Got a new job, still settling in.
It's on my radar, but somewhere after buying furniture etc.
The issue of handling manual modifications made it more complicated; as did figuring out go for the first time. Definitely not complex overall though.

[Morganamilo]

Wait, no. Even though we switched to using a ref for tracking, deleting the repo will still delete the ref. This should be closed when cleaning it changed to git clean instead of deleting the repo.

[JuniorJPDJ]

:<<

[JuniorJPDJ]

Wait, no. Even though we switched to using a ref for tracking, deleting the repo will still delete the ref. This should be closed when cleaning it changed to git clean instead of deleting the repo.

Or just saving refs out of git tree and then showing diff only from this ref.

[mPiotrek]

Also there's a somewhat unofficial feature where you can change a pkgbuild, commit it back and as long as there are no conflicts yay will happily merge it with the aur and keep your changes. There's still one big problem where the diffs go from HEAD to HEAD@{upstream}, meaning your changes are always shown to be taken out even though this does not actually happen.

I am trying to get a hang of how does this 'unofficial feature' exactly work. I've seen it referenced a few times on some forums. Why would one's changes be gone in the diff between HEAD and HEAD@{upstream}? If the user edited the build files and committed them to the local repository, the HEAD should point to the commit with changes, shouldn't it?

I believe that if Yay were to track installed packages PKGBUILDs' versions – e.g. remember commit hashes or something, it would belong either to the $XDG_CONFIG_HOME as it is a configuration (of the user's system) or to the $XDG_DATA_HOME as it does not configure Yay but is part of the data on which Yay is working on.

On the other hand, if Yay were to fully support custom patches it would have to preserve the modifications. The most reasonable way is probably to track it in git, right? And having drastically different implementation for regular diffs is definitely sub-optimal but could be argued for from the disk usage perspective. (Still, we could just remove the history I guess.)
Here I am pretty sure that $XDG_DATA_HOME fits much better since a git repo doesn't look like a config. Or maybe it is, idk. The reasoning from before still applies I think.
But doesn't this feature deserve a separate issue? Explicitly supporting custom patches is not a part of 'non-discarding sources for diffs'.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[mPiotrek]

I believe it should be agreed whether or not Yay is responsible for tracking the changes in the build instructions. Then it could be either closed or marked as intended feature.

Letting this close automatically does not really solve anything.

[JuniorJPDJ]

@mPiotrek but yay offers show diffs functionality, which just doesn't work when using clean build

[mPiotrek]

@JuniorJPDJ Yeah, this would suggest that it is an intended feature. But right now it is partially broken, or at least does not play nice with the rest.

I also would like to see it done in a persistent way, but it would not be surprising if the maintainers decided they don't have the resources to make it work this way.