Comment 1 Niels Breet 2008-07-23 12:26:55 UTC

I can confirm this bug.

The problem arises when a package with the exact same name is uploaded to the
incoming queue after being built on the autobuilder. Due to small differences
in file dates etc, the package can differ a few bytes. A reindex is needed to
fix the md5sums and filesizes.

We currently allow you to upload the same package version multiple times and
that is the root of the problem. I don't think that this is allowed in Debian
and so the tools aren't able to handle this case.

We can solve this problem in a few ways:

- Deny upload of a package when that exact version is already in extras-devel.
This can be done when processing the autobuilder queue.
- Reindex the repository after a package has been uploaded that had the same
version. (This is very IO intensive and causes a big delay)

I think the best solution is the first one, but I also see this forces more
work on the uploader. Although I think that Debian does the same.

What do you think?

Comment 2 Graham Cobb (reporter) 2008-07-23 13:03:22 UTC

Option 1 really does not work.  With the other bugs, it is often necessary to
upload the same package several times to get it to build successfully!

The most serious problem is that it is not unusual that the armel build works
but the i386 build fails due to a failure in installing a build dependency.  It
is necessary to upload the package again to try the build again.  However, this
can trigger the problem in this bug for the armel package.

Note that in this case it is exactly the same source package which is uploaded,
so there is no reason to change the version.  If you forced the submitter to
increment the version it would increase the time it takes to upload GPE to a
week instead of the day it is taking at the moment!

In the medium term maybe we should aim to implement the rule you suggest.  In
that time you could fix the network problem which causes build failures and we
could add a feature to MUD to easily rebuild a source package with just the
version incremented.  

In the short term maybe a web page to force a re-index would be a workround? 
Another short term workround might be to prevent the actual update to the
repository being made if the same version already exists (the build could
happen or not, as you wish but the key thing is that this occurring on an armel
build would not prevent the i386 build and update happening).

Comment 3 Niels Breet 2008-07-24 11:59:22 UTC

There is now a check in place which should reindex the repository within the
hour.

The queuemanager for the repository has an option to only allow new versions to
be put in the repository. So it will skip uploading to the repository (after
coming from the autobuilder) if the file is already there. One problem is: if
for example the armel package is already there, but the i386 isn't, it will
still skip the package.

I'll look into improving the queuemanager to be a bit smarter about this.

Comment 4 Niels Breet 2008-08-12 17:30:46 UTC

Change to default priority as required for Sprint.

(In reply to comment #2)
> Option 1 really does not work.  With the other bugs, it is often necessary to
> upload the same package several times to get it to build successfully!
> 
I tend to disagree with this, sorry. Uploading the same version to autobuilder
is OK, but uploading the same version to repository is bad thing. It's OK if
package isn't in repo yet and maintainer fixes building failures, but as soon
as package is there it means that it can be installed by users. Uploading the
same version would mean that different users would possibly have different
packages with the same version installed. This would confuse users, testers and
developers, because, for example, testers, would report problems maintainer
wouldn't actually know which package they're referring to.

So, I'm going to fix this by not allowing re-uploading the package with the
same version to repository after the build. Packages with the same version as
in repo will be built by autobuilder, but not uploaded to extras-devel.

I also have similar bug in garage tracker:
https://garage.maemo.org/tracker/index.php?func=detail&aid=2661&group_id=602&atid=2262
and I agreed with reporter about the way I'm going to fix it.

Comments?

Comment 6 Graham Cobb (reporter) 2008-09-29 15:47:30 UTC

You are right, of course.

Any chance you can do the check BEFORE the build actually runs, and reject the
build?  If not, please at least make sure the update rejection is reflected in
the logs and in the email sent to the submitter.

(In reply to comment #6)
> Any chance you can do the check BEFORE the build actually runs, and reject the
> build?  
I can do that. However, I'm not 100% convinced. Can you explain why we should
reject such a builds?

> If not, please at least make sure the update rejection is reflected in
> the logs and in the email sent to the submitter.
> 
Of course. It will be reflected there in any case. 
BTW, can you suggest proper log message, please?
I can only make something like 'package buildversion has not been uploaded to
the extras-devel repository because package repoversion is already there' :)

Comment 8 Graham Cobb (reporter) 2008-09-29 18:12:09 UTC

(In reply to comment #7)
> > Any chance you can do the check BEFORE the build actually runs, and reject the
> > build?  
> I can do that. However, I'm not 100% convinced. Can you explain why we should
> reject such a builds?

I only submit builds when I want them to be uploaded.  I just assumed any such
submission was a mistake and the sooner it was reported, the better.  If others
would find it useful to be able to do a "test" build by using the same version
number then I am not against allowing it.

But one advantage is that the build could be rejected if either of the targets
(i386 or armel) would not be uploaded.  This reduces the risk of having
different versions in the two repos.

> > If not, please at least make sure the update rejection is reflected in
> > the logs and in the email sent to the submitter.
> > 
> Of course. It will be reflected there in any case. 

Jolly good. I was worried that the upload may happen after the email is sent.

> BTW, can you suggest proper log message, please?
> I can only make something like 'package buildversion has not been uploaded to
> the extras-devel repository because package repoversion is already there' :)

That looks good.  You do need to deal with the fact that there are two
repositories. Also, I presume this would be a less-than-or-equal test -- we
can't upload the same or an earlier version?  So the message could be:

gpe-calendar 1.2.3 has not been uploaded to diablo extras-devel armel because
the same or newer version (gpe-calendar 1.3) is already there.

And I guess a positive log message (gpe-calendar 1.2.3 has been uploaded to
diablo extras-devel armel) would not be a bad idea in the cases where it works!
 Or do you not know at that point if it has actually worked (you just know it
doesn't fail this particular check)?

(In reply to comment #8)
> I only submit builds when I want them to be uploaded.  I just assumed any such
> submission was a mistake and the sooner it was reported, the better.  If others
> would find it useful to be able to do a "test" build by using the same version
> number then I am not against allowing it.
>
I see here two ways to go:
- check before the build and see what people would do
- ask them 

According to one of Python programming rules 'it's better to ask forgiveness
than permission' I'd go for the first way (I'm a Python guy :)), but if you
think I should ask first I'll try to ask.

> But one advantage is that the build could be rejected if either of the targets
> (i386 or armel) would not be uploaded.  This reduces the risk of having
> different versions in the two repos.
> 
In current implementation builder only uploads packages when all possible
architecture builds are OK, so this situation should not happen for
extras-devel. It can happen only in extras if maintainer forgets to promote all
packages to extras. I'm not sure autobuilder should check packages in extras at
all. It only works with extras-devel.

> > Of course. It will be reflected there in any case. 
> 
> Jolly good. I was worried that the upload may happen after the email is sent.
> 
Well, packages are uploaded before the email is sent, but it only means that
they're in incoming queue for extras-devel. It's job of queue-manager script to
pick them up from incoming queue, put them into repo and reindex it.
Autobuilder has nothing to do with it.
So, even if this check will be in place there will be a small possibility to
upload package with the same version for build if previous package haven't
reached the repo yet.

> > BTW, can you suggest proper log message, please?
> > I can only make something like 'package buildversion has not been uploaded to
> > the extras-devel repository because package repoversion is already there' :)
> 
> That looks good.  You do need to deal with the fact that there are two
> repositories. 
Hmm. Can you explain why? At the moment it only cares about extras-devel.
Promoting to extras is under maintainer's responcibility.

> Also, I presume this would be a less-than-or-equal test -- we
> can't upload the same or an earlier version?  So the message could be:
> 
> gpe-calendar 1.2.3 has not been uploaded to diablo extras-devel armel because
> the same or newer version (gpe-calendar 1.3) is already there.
>
This message is much better than mine, thank you. Of course the same or an
earlier version will be checked.

> And I guess a positive log message (gpe-calendar 1.2.3 has been uploaded to
> diablo extras-devel armel) would not be a bad idea in the cases where it works!
>  Or do you not know at that point if it has actually worked (you just know it
> doesn't fail this particular check)?
Sorry, I don't understand what do you mean by 'worked'. Currently autobuiler
aploads packages if all builds are successfull and I wasn't going to change
this.

Comment 10 Graham Cobb (reporter) 2008-09-30 02:13:15 UTC

(In reply to comment #9)
> According to one of Python programming rules 'it's better to ask forgiveness
> than permission' I'd go for the first way (I'm a Python guy :)), but if you
> think I should ask first I'll try to ask.

Don't ask.  We can always file bugs if we don't like it!

> In current implementation builder only uploads packages when all possible
> architecture builds are OK, so this situation should not happen for
> extras-devel. It can happen only in extras if maintainer forgets to promote all
> packages to extras. I'm not sure autobuilder should check packages in extras at
> all. It only works with extras-devel.

I agree that you don't do anything with extras -- that is a completely separate
problem.

But it is possible that a builder bug, or a manual action by a repository
maintainer to fix a problem, or something, could cause the armel and i386
repositories to be unsynchronised.  I think you need to check both and fail the
operation if **either** one contains the newer version.

Actually, couldn't you see exactly this if you first uploaded the package as
armel only and then reuploaded the same version but now claiming it is both
armel and i386?

> > That looks good.  You do need to deal with the fact that there are two
> > repositories. 
> Hmm. Can you explain why? At the moment it only cares about extras-devel.
> Promoting to extras is under maintainer's responcibility.

I meant armel and i386.  I.e. you have to tell the submitter in the log message
which binary directory (armel or i386) contains the incompatible version (just
in case they do not always contain the same versions).

> > And I guess a positive log message (gpe-calendar 1.2.3 has been uploaded to
> > diablo extras-devel armel) would not be a bad idea in the cases where it works!
> >  Or do you not know at that point if it has actually worked (you just know it
> > doesn't fail this particular check)?
> Sorry, I don't understand what do you mean by 'worked'. Currently autobuiler
> aploads packages if all builds are successfull and I wasn't going to change
> this.

I meant whether the repository upload had actually worked.  What I am really
suggesting is that if the version test does not fail, it would be useful to
include messages in the log saying the "successful" equivalent of the error
message we discussed above.  Something like:

gpe-calendar 1.2.3 has been queued for loading into diablo extras-devel armel.

gpe-calendar 1.2.3 has been queued for loading into diablo extras-devel i386.

(In reply to comment #10)
> I agree that you don't do anything with extras -- that is a completely separate
> problem.
> 
> But it is possible that a builder bug, or a manual action by a repository
> maintainer to fix a problem, or something, could cause the armel and i386
> repositories to be unsynchronised.  I think you need to check both and fail the
> operation if **either** one contains the newer version.
I'm going to check version of source package. Why it's needed to check binaries
at all?

It also makes sense to forbid moving only binary packages from extras-devel to
extras in promoter interface. Source package and binary packages for all
architectures have to be promoted at the same time. 

Another thing here is to analyze dependencies before moving and don't allow to
move packages if their dependencies are not going to be satisfied in extras. 

With this 2 features we can avoid a lot of issues with packages in extras.
You can create bugs or feature requests for promoter interface if you want.

> 
> Actually, couldn't you see exactly this if you first uploaded the package as
> armel only and then reuploaded the same version but now claiming it is both
> armel and i386?
> 
Would't check for source version be enough to catch all these issues?

> 
> I meant whether the repository upload had actually worked.  What I am really
> suggesting is that if the version test does not fail, it would be useful to
> include messages in the log saying the "successful" equivalent of the error
> message we discussed above.  Something like:
> 
> gpe-calendar 1.2.3 has been queued for loading into diablo extras-devel armel.
> gpe-calendar 1.2.3 has been queued for loading into diablo extras-devel i386.
> 
Looks good, thank you. I'll do that.

I implemented source version check. If the same version of source package is
already present in extras-devel package is rejected by autobuilder.

Graham, I'd appreciate if you check this out and say if this is enough to close
this bug.

Any news on this? Graham?

Comment 14 Niels Breet 2008-11-27 11:33:53 UTC

This seems to be fixed. I have had no reports about size mismatch errors for
quite some time.

Please reopen of this issue reappears.