Arch Linux

Relevant: https://lists.archlinux.org/pipermail/aur-dev/2015-July/003676.html

Thanks for the link Dave :)

If I may sum up that discussion: people think this would be very useful, but some want to retain the possibility of making a commit that only modifies the PKGBUILD, thus nothing came of it.

Is there any reason why we could not add a "dummy" timestamp field to the .SRCINFO file, and thus require the .SRCINFO to be technically changed (in terms of its SHA1) for each commit, but possibly semantically unchanged?

I'm fine with adding the timestamp to the header of the .SRCINFO file (makepkg does this), but I'm reserving judgement on whether or not the hook should exist.

And instead of an Hook on server side and one optional on the client side, just create one for pre-commit on the client side which checks for PKGBUILD in staging, and if found run mksrcinfo.[0]

In my opinion transactions that don't require an update to the SRCINFO shouldn't create one


[0]Kinda:
SRC_PATTERN="PKGBUILD"
git diff --cached --name-only | if grep --quiet "$SRC_PATTERN"
then
mksrcinfo
git add .SRCINFO
fi

The downside with client-side hooks is that there is no way to enforce them, and not even a proper way of distributing or upgrading them. Thus I don't think they're much of an improvement on the current situation.

For the proposed server-side hook, I think it depends on being able to have that check just the final commit being pushed, since that would avoid requiring people to rewrite history. So that hook would print

"Error: .SRCINFO not present in latest commit. Please run mksrcinfo, commit the result and push again."

Checking for presence of .SRCINFO does not work. The file exists, even if it is not modified.

Another suggestion: Whenever a user pushes new revisions (either a single commit or multiple commits), make sure that *any* of the commits modifies .SRCINFO, i.e. the diff between the old head and the new head limited to .SRCINFO is non-empty. Add a command to the SSH interface to disable that check for the next push and maybe another option to disable the check completely for a user. Opinions?

> make sure that *any* of the commits modifies .SRCINFO, i.e. the diff between the old head and the new head limited to .SRCINFO is non-empty
This still leaves open the possibility of a mismatch between SRCINFO and PKGBUILD, e.g. given ordered commits 'A', 'B', and 'C', where A modifies the PKGBUILD, B modifies the .SRCINFO, and then C modifies the PKGBUILD again.

I'm not sure what level of detail you have access to in the pre-receive hook, but would it be possible to ensure that the latest commit modifying SRCINFO is as new or newer than the PKGBUILD? In other words, given ordered commits A and B where A modifies the PKGBUILD and B modifies the SRCINFO, accept this push. But, if A modifies the SRCINFO and B modifies the PKGBUILD, reject the push.

I think all this might be "misplaced." Have a warning (warning only, don't reject the push) if the .SRCINFO wasn't part of the commit, that might be helpful. But trying to enforce things further is probably doomed to fail: commit A modifies PKGBUILD, commit B update both .SRCINFO & PKGBUILD but only because the .SRCINFO was first regenerated, and then the PKGBUILD updated again, w/out updating of the .SRCINFO, and so the .SRCINFO is still "out of date" -- how do you expect to catch this?

A warning would be helpful, educating users and making a pre-commit hook available with explanations as to how to use it to ensure the .SRCINFO is always & automatically regenerated on every single commit is probably best; Just like with AUR3 one could do it manually (generate/edit the .SRCINFO/.AURINFO file), or use whatever the helper name was (mkaurball?) that was available (but not required).

Such a hook could even be part of pkgbuild-introspection, couldn't it, along side mksrcinfo?

> trying to enforce things further is probably doomed to fail
I'm sure there's plenty of ways to shoot yourself in the foot. It's probably a distinct improve on the current situation, though. Can you think of any edge cases where a "good" push might be rejected? That's much more interesting than occasionally allowing a "bad" push.

> Such a hook could even be part of pkgbuild-introspection, couldn't it, along side mksrcinfo?
This has exactly the same downsides as a git-based hook -- you can't force people to use it.

>Can you think of any edge cases where a "good" push might be rejected?
Sure, let me quote Doug Newgard from the mailing list thread[1]:

>There are plenty of changes that could be made to the PKGBUILD that wouldn't
>require a change to the .SRCINFO. Anything from formatting changes/cleanups,
>to restructuring functions, to switching to using install commands instead of
>cp, to abstracting out variables. You don't want to bump the pkgrel if they
>don't make any changes to the final package.

One could do such changes and want to share/push them, without updating the pkgrel/.SRCINFO yet. So the last commit would update the PKGBUILD only. I'm not sure those are edge cases, but I certainly consider them to be "good" pushes, that would be rejected.
Might be me, but I would also find it odd that such a commit be rejected if I try to push it alone/last, but when pushing it with another one afterwards that updates the .SRCINFO it now gets accepted.

>you can't force people to use it.
Sure, but how is that a problem? You can't force them to use mksrcinfo either, nor could you force them to use mkaurball before. My point was, I don't think it's good to try and force people to do things the way you want, because that might limit them for no good reasons, people might come up with ways to do things neither you or me is thinking of right now, that won't work even though perfectly legitimate.

Since the AUR relies on people providing the metadata, you need to trust they'll do so properly. Helping them achieving it is good, trying to enforce things however...
And if you really wanna do so, keep things simple: given that it's already required for .SRCINFO to always exist from day one, just extend it so that every single commit must include (add or update) .SRCINFO, else it's rejected. That's it. And have mksrcinfo add a timestamp à la makepkg, so it can always be done. Then a pre-commit hook can be provided alongside mksrcinfo, to ensure compliance easily. Sure, you can't force people to use that hook, or mksrcinfo, but you're never gonna do that anyways.

[1] https://lists.archlinux.org/pipermail/aur-dev/2015-July/003683.html

The point about changes which do not affect the .SRCINFO is moot -- mksrcinfo will timestamp the generated file.

> One could do such changes and want to share/push them, without updating the pkgrel/.SRCINFO yet.
I'm not sure why you would want to push these changes to the AUR. If you want to share them e.g. with another maintainer or a limited subset of users, but not have the general person use it yet, you don't push it to the AUR. You use github or wherever.

The git repo *on AUR* for a given package is actually a bit special: any commit that is the last commit before a push to the AUR is in some sense a release. This release should update the .SRCINFO, even if it is just for the timestamp. So, if I may sum up the current proposal:

1. Add a timestamp field to .SRCINFO that mksrcinfo will generate when it is run. Make it use UTC to avoid time zone issues.
2. Have a server-side hook on AUR that, when someone tries to push one or more commits, checks that at *least one of those commits* modifies the .SRCINFO file.
(If possible, have the hook verify that PKGBUILD has not been modified in a commit later in history than .SRCINFO)
3. If this check fails, deny the push, and tell the user to run mksrcinfo, commit the result and push again (the push will then succeed).

If you have an edge case where a valid commit would be denied in this model, please describe it. We should identify (and hopefully) fix as many problems/issues with this proposal as possible before someone (potentially) starts working on it.

>The point about changes which do not affect the .SRCINFO is moot -- mksrcinfo will timestamp the generated file.
I don't follow. Update the PKGBUILD in ways that don't affect the metadata, commit, push : rejected. Rejected unless you also use mksrcinfo to add .SRCINFO in the commit. So effectively you're forcing the file to be part of the commit, even when not required, only you don't admit it frankly, so as I said the same commit could be rejected or accepted depending on how you do your push...


>2. Have a server-side hook on AUR that, when someone tries to push one or more commits, checks that at *least one of those commits* modifies the .SRCINFO file.
Well, Dave will then argue that it "still leaves open the possibility of a mismatch between SRCINFO and PKGBUILD" due to ordering. So you could try to check for the order, or just require a .SRCINFO in every single commit.

So I had a look, and checking the order is quite straight-forward: the pre-receive hook (a python/bash/etc script) receives three arguments on stdin: sha1A, sha1B, refname. Here sha1A points to the first commit in the push and sha1B to the last. In the python script you can get the number of commits between them as N = git rev-list --count sha1A..sha1B. Then you iterate over the commits as sha1A~i for i from 0 to N-1. Check for which i .SRCINFO and PKGBUILD is modified, and Rob's your uncle.

What if I want to push a commit which only adds comments, or removes trailing whitespace from the PKGBUILD?

No problem, the proposal is to have a timestamp in .SRCINFO, so you edit PKGBUILD as you want, run mksrcinfo, commit and push. There is no problem if a changeset only does "trivial" changes like you mention.

Is the change in "Due in version" to 4.2.0 a sign that this proposal has been accepted? Moreover, is it a sign that someone has plans to implement it? Can we enable the hook on some repos for beta testing before it is put into production?

I am fine with displaying a warning when .SRCINFO isn't updated. Why do we need beta tests for that specific feature?

FYI, the latest release of mksrcinfo (coming soon to a mirror near you) will generate .SRCINFO files which look more like the ones makepkg produces, including a timestamp with the generation time.

Lukas: so the implementation will now display a warning but allow the commit to be pushed? That sounds fine and I agree that doesn't need a beta test. We were discussing having the server deny the push if it contained PKGBUILD changes in a more recent commit than .SRCINFO changes. That is more "intrusive" and is why I suggested a beta test.

Correct, we only show a warning. If that doesn't do the trick, we can still add more complicated logic as I suggested before.

First, sorry for the duplicate task; should have searched harder.

>the implementation will now display a warning but allow the commit to be pushed

This is insufficient. AUR should refuse the push request entirely.

If an initial push missing .SRCINFO goes through, warning or not, the packager then has to:
1. figure out what is wrong (websearch the warning, read some wiki pages, etc.).
2. correct their mistake (learn about .SRCINFO, wiki pages, etc)
3. comprehend and execute a complicated git procedure (which is not as simple as the wiki implies, I've had to do it--wiki pages, etc).
4. push again (with crossed fingers).

Much better to refuse any push request without updates to .SRCINFO, and return a clear error message.

Could AUR be made to refuse push requests with no commits that contain changes to .SCRINFO, or would it have to be on a per-commit basis?

If each commit must contain changes to .SRCINFO, then packagers may still need to reverse a commit from time to time.

The process is not simple, but seems less risky than "rebase" or "filter-branch".

Example commit "undo" http://stackoverflow.com/a/927386

"If an initial push missing .SRCINFO goes through..."

This doesn't happen, there's a separate check that requires every commit to include a .SRCINFO file.

Not sure how I double posted... oh well, repurposing as a reply.

>This doesn't happen

You should probably test that.

I have, repos that contain commits without a .SRCINFO file cannot be pushed.

I also gave it another try. You are correct, initial pushes without a .SCRINFO no longer go through.

How long has this been implemented? Have you seen the warning I'm about to remove from the Wiki?

I had to rebase a repository for that mistake only some days ago.

It's been there since the AUR switch to git. If you were able to rebase, the push you're talking about didn't go through as the AUR also reject non-fast-forward pushes.

I had to rebase--as specified in the wiki until moments ago--and also force, which was not specified in the wiki, but managed to fix the broken repository.

Anyway, it seems that no one will ever have to do that again, which is good news.

How about requiring changes to .SRCINFO for subsequent commits/push requests?

> Correct, we only show a warning. If that doesn't do the trick, we can still add more complicated logic as I suggested before.

Two years later and this issue is still widespread: many packages provide unreliable metadate through the RPC
interface. Any chance to refuse push requests without any .SRCINFO update?

In pacaur code, when untrustful metadata is detected, I went so far as to not implement workarounds and bail out instead, to incite (read: force) users to notify forgetful AUR maintainers.
This is rather annoying, especially when the metadata difference is minor or insignificant, but having higher quality RPC metadata takes priority over users/maintainers convenience imho.

> Two years later and this issue is still widespread: many packages provide unreliable metadate through the RPC interface. Any chance to refuse push requests without any .SRCINFO update?

We've already explained, numerous times, why that is not an issue. "many packages" aren't all that many, and they are edge cases, and the suggestion would force users who currently do sane things, to add pointless changes and be pointlessly inconvenienced. I'd much rather encourage people to use something like https://github.com/eli-schwartz/pkgbuilds

Arch Linux as a distribution is against the idea of needless complexity and babying users, so why should we baby AUR maintainers as a special exception to this general philosophy? Note that we've since decided that makepkg --printsrcinfo cannot, should not, and will not add timestamps to the srcinfo, as the only timestamps that matter are the ones in the git metadata. So users would *have* to bump the pkgrel even if they wanted to commit whitespace changes.

Especially since, as was already mentioned, any such hook would be unable to prove in any sort of reliable way whatsoever that the .SRCINFO was updated after the last PKGBUILD edit, as opposed to in between edits.

> when untrustful metadata is detected

Nice scare words there, too bad it isn't true. At the stage where you could choose to reject the metadata or recreate it with makepkg --printsrcinfo, the user has already read the PKGBUILD and trusted it. "trust" != "reliable", so stop trying to conflate the issue with security applicability.

> This is rather annoying, especially when the metadata difference is minor or insignificant, but having higher quality RPC metadata takes priority over users/maintainers convenience imho.

Yes, we know you're an annoying person who likes inciting people.

...

It's a shame your AUR helper isn't programmed well enough to use the already available workaround for VCS packages, on regular packages as well.

If you're not going to have the integrity to give an honest error message whenever the .SRCINFO is mismatched, rather than your current error message which only manifests when the epoch:pkgver-pkgrel is mismatched, then why should we believe the justification for your request here?

*Your own code brought as an example* is undermining your argument by flat-out refusing to do what you claim it is doing as a defense against the evils of forgetful maintainers!

Thank you for your answer, though I regret we can't have a simple well mannered conversation about RPC metadata - independently of any code soupe helper hate or personal attack.

> Nice scare words there
My choice of word was indeed poor here, I indeed thought about reliability and not security (see my first sentence).

There are cases where this lack of reliability *is* annoying (f.e. when maintainers add or remove dependencies in PKGBUILDs), which prevents a correct AUR dependency resolution using the RPC. This is honestly the only issue at hand here. Hey, I am the very first to recognize my code sucks, especially such old code that never has seriously been reworked/refactored as it should have been. Knowing if the RPC data could be improved in the first place is the first step to improve the situation.

I did miss the decision about removing/not adding the timestamp to .SRCINFO, so this is at least useful information.

Is it a personal attack if I attack the fact that you have declared your motivation for your code to be "I would like to incite AUR maintainers by punishing users and making sure the package refuses to install with pacaur, in the hope that pacaur users will complain on the AUR page until the maintainer updates the .SRCINFO"? Because I regard it as an aggressive overreach of what an AUR helper is meant to do?

Given the context of https://bbs.archlinux.org/viewtopic.php?pid=1734271#p1734271 I think it is quite obvious that
1) This "reliability" is completely ignored in the case of VCS packages.
2) This "reliability" was only added after makepkg removed the --pkg flag. The implication is that your original motivation was reliability of built package filenames, something you already had to work around for VCS packages, therefore it is not a case of you "not implement workarounds", it is a case of you absolutely implementing workarounds, and then special-casing those workarounds so they only get a ctivated in certain cases (which is additional code and additional effort you had to deploy in order to turn something that worked into something that threw an error message).
3) After being told about the issue, you completely ignored it and declared it is a feature, not a bug. This is exactly the opposite of "I am the very first to recognize my code sucks, especially such old code that never has seriously been reworked/refactored as it should have been".
4) You engage in personal attacks at least as much as I do. Maybe you should consider *why* I jump straight to personal attacks against you.
5) Somehow you ended up saying the statement '[...] pitiful when someone like Scimmia comments on the aur-requests and many times on AUR pages without actually being a TU and (let's call a cat a cat) using wording that is less than constructive, leading to users thinking he is a despicable TU unable to write a sentence without the "idiot" word' [...].

...

So all in all, I guess I can live with committing personal attacks in this discussion. If we're not going to have a productive conversation based on technical merits (because you regard pacaur as a political platform by which to impose your views rather than a tool for building AUR packages with a minimum of fuss and drama), then it's not like I'm interrupting anything anyway.

...

Also you haven't made any arguments in favor of enforcing a check, that weren't already argued against and rejected 2 years ago!

My apologies to anyone subscribed to this bug report. I did not expect it to turn into such a sterile conversation.
Since we all have better things to read, I am not answering further to this bug entry.