Skip to content
This repository was archived by the owner on Jan 31, 2023. It is now read-only.

Conversation

@rwst
Copy link
Contributor

@rwst rwst commented May 9, 2014

fixes #18

@robertwb
Copy link
Collaborator

I don't think this is the behavior one wants--we should test as much as possible.

@rwst
Copy link
Contributor Author

rwst commented May 21, 2014

So, what's better, a ticket tested correctly when all dependencies are closed or a ticket tested incorrectly, i.e., false failure? Example:
http://patchbot.sagemath.org/log/16165/debian/wheezy/sid/x86_64/3.2.0-57-generic/selmer/2014-05-21%2007:32:58%20+0100

In another comment you were complaining that docbuilding would take too much time. Compare this with a complete useless doctest. I would guess factor 20.

You really should look closer at why patchbot reports failure, because under normal conditions it should be rare, but at the moment isn't.

@robertwb
Copy link
Collaborator

Yes, you should always look at any failures to understand why the failed (and hopefully fix it).

Ideally, the git hash at the tip of the branch should exactly describe the ticket in question. This is one of the advantages over posting patches--reduced ambiguity. If the dependencies are required, they (or at least the portion that is a true dependency) should be merged in.

I might go so far as to make it a warning on the trac page if there are dependencies that are declared but not fully merged in.

@rwst
Copy link
Contributor Author

rwst commented May 28, 2014

Following that demand would be impossible in case of a dependency on a pkg
upgrade.
On 28 May 2014 02:55, "Robert Bradshaw" [email protected] wrote:

Yes, you should always look at any failures to understand why the failed
(and hopefully fix it).

Ideally, the git hash at the tip of the branch should exactly describe the
ticket in question. This is one of the advantages over posting
patches--reduced ambiguity. If the dependencies are required, they (or at
least the portion that is a true dependency) should be merged in.

I might go so far as to make it a warning on the trac page if there are
dependencies that are declared but not fully merged in.


Reply to this email directly or view it on GitHubhttps://github.com//pull/29#issuecomment-44353966
.

@robertwb
Copy link
Collaborator

Only for optional packages (which are the exception). Standard package upgrades are git commits as well.

@rwst
Copy link
Contributor Author

rwst commented May 29, 2014

Really? For example http://trac.sagemath.org/ticket/16396 --- how do I as
ticket author put the tarball into a git commit? Up to now I thought the
tarball was downloaded by the webmaster and provided at the website, from
where it gets downloaded by build processes.

On Wed, May 28, 2014 at 10:14 PM, Robert Bradshaw
[email protected]:

Only for optional packages (which are the exception). Standard package
upgrades are git commits as well.


Reply to this email directly or view it on GitHubhttps://github.com//pull/29#issuecomment-44458353
.

@rwst
Copy link
Contributor Author

rwst commented May 29, 2014

However, I would agree that this patch is not ready simply because bailing out does not work. Patchbot will just try the same ticket again. See #42.

@robertwb
Copy link
Collaborator

The tarball itself is external, but strong reference is stored in git. See
http://git.sagemath.org/sage.git/commit/?id=cd12a2bfbea0cb98f48d69b009e7d3c8ccdae885

When you build with this commit merged it will see that it doesn't have
this version and download it (checking the hash) and install.

On Thu, May 29, 2014 at 12:07 AM, rwst [email protected] wrote:

Really? For example http://trac.sagemath.org/ticket/16396 --- how do I as
ticket author put the tarball into a git commit? Up to now I thought the
tarball was downloaded by the webmaster and provided at the website, from
where it gets downloaded by build processes.

On Wed, May 28, 2014 at 10:14 PM, Robert Bradshaw
[email protected]:

Only for optional packages (which are the exception). Standard package
upgrades are git commits as well.


Reply to this email directly or view it on GitHub<
https://github.com/robertwb/sage-patchbot/pull/29#issuecomment-44458353>
.


Reply to this email directly or view it on GitHubhttps://github.com//pull/29#issuecomment-44500867
.

@rwst
Copy link
Contributor Author

rwst commented May 29, 2014

Yes but from *where? There is no URL in the commit, so patchbot will only
know when the webmaster has published the tarball.

On Thu, May 29, 2014 at 5:45 PM, Robert Bradshaw
[email protected]:

The tarball itself is external, but strong reference is stored in git. See

http://git.sagemath.org/sage.git/commit/?id=cd12a2bfbea0cb98f48d69b009e7d3c8ccdae885

When you build with this commit merged it will see that it doesn't have
this version and download it (checking the hash) and install.

On Thu, May 29, 2014 at 12:07 AM, rwst [email protected] wrote:

Really? For example http://trac.sagemath.org/ticket/16396 --- how do I
as
ticket author put the tarball into a git commit? Up to now I thought the
tarball was downloaded by the webmaster and provided at the website,
from
where it gets downloaded by build processes.

On Wed, May 28, 2014 at 10:14 PM, Robert Bradshaw
[email protected]:

Only for optional packages (which are the exception). Standard package
upgrades are git commits as well.


Reply to this email directly or view it on GitHub<
https://github.com/robertwb/sage-patchbot/pull/29#issuecomment-44458353>

.


Reply to this email directly or view it on GitHub<
https://github.com/robertwb/sage-patchbot/pull/29#issuecomment-44500867>
.


Reply to this email directly or view it on GitHubhttps://github.com//pull/29#issuecomment-44547002
.

@robertwb
Copy link
Collaborator

It will download it from $SAGE_UPSTREAM just like a fresh build, see
https://github.com/sagemath/sage/blob/master/src/bin/sage-spkg . Correct,
if the webmaster has not uploaded the tarball yet it will fail (as it
probably should; the ticket is not yet ready to be merged).

On Thu, May 29, 2014 at 9:29 AM, rwst [email protected] wrote:

Yes but from *where? There is no URL in the commit, so patchbot will only
know when the webmaster has published the tarball.

On Thu, May 29, 2014 at 5:45 PM, Robert Bradshaw
[email protected]:

The tarball itself is external, but strong reference is stored in git.
See

http://git.sagemath.org/sage.git/commit/?id=cd12a2bfbea0cb98f48d69b009e7d3c8ccdae885

When you build with this commit merged it will see that it doesn't have
this version and download it (checking the hash) and install.

On Thu, May 29, 2014 at 12:07 AM, rwst [email protected]
wrote:

Really? For example http://trac.sagemath.org/ticket/16396 --- how do
I
as
ticket author put the tarball into a git commit? Up to now I thought
the
tarball was downloaded by the webmaster and provided at the website,
from
where it gets downloaded by build processes.

On Wed, May 28, 2014 at 10:14 PM, Robert Bradshaw
[email protected]:

Only for optional packages (which are the exception). Standard
package
upgrades are git commits as well.


Reply to this email directly or view it on GitHub<

https://github.com/robertwb/sage-patchbot/pull/29#issuecomment-44458353>

.


Reply to this email directly or view it on GitHub<
https://github.com/robertwb/sage-patchbot/pull/29#issuecomment-44500867>

.


Reply to this email directly or view it on GitHub<
https://github.com/robertwb/sage-patchbot/pull/29#issuecomment-44547002>
.


Reply to this email directly or view it on GitHubhttps://github.com//pull/29#issuecomment-44552327
.

@rwst
Copy link
Contributor Author

rwst commented May 29, 2014

So okay, putting aside all cases where we agree, I agree the rest could be
handled if this were implemented:

I might go so far as to make it a warning on the trac page if there are
dependencies that are declared but not fully merged in.

So where to open such a ticket, is that sagemath/trac-git?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

do not test tickets with open dependencies

2 participants