Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#9766 closed defect (fixed)

Cliquer - Update Upstream contact

Reported by: ncohen Owned by: drkirkby
Priority: major Milestone: sage-4.5.3
Component: packages: standard Keywords:
Cc: Merged in: sage-4.5.3.rc0
Authors: Nathann Cohen, David Kirkby Reviewers: David Kirkby, Mitesh Patel
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by mpatel)

The URL was missing from the SPKG.txt file, and CJ Fearnley requested it to be changed to work on a debian package of Sage.

Package at

http://boxen.math.washington.edu/home/kirkby/patches/cliquer-1.2.p6.spkg

Attachments (2)

9766.patch (1.4 KB) - added by drkirkby 8 years ago.
Mercurial patch to add contact information and generally clean up SPKG.txt
9766-improved-SPKG.txt-information.patch (1.6 KB) - added by drkirkby 8 years ago.
Improve the historical accuracy of SPKG.txt

Download all attachments as: .zip

Change History (31)

comment:1 Changed 8 years ago by ncohen

  • Status changed from new to needs_review

comment:2 Changed 8 years ago by drkirkby

  • Status changed from needs_review to needs_work

SPKG.txt does not list the change you made - instead it lists

== Changelog ==

=== cliquer-1.2.p6 (19 August 2010) ===
 * Fixed Trac #8279 to make the cliquer spkg work on Cygwin with the Sage library.

=== cliquer-1.2.p5 Mike Hansen (15 February 2010) ===
 * Fixed Trac #8279 to make the cliquer spkg work on Cygwin with the Sage library.

It seems you have just copied the previous entry and incremented the patch level.

You need to put your own name and date, along with the change you made - but I think you know that.

Also the commit message does not have the trac number in it.

You should take the opportunity to add the sections from SPKG.txt which are missing - namely:

== Dependencies ==

List the dependencies here

== Special Update/Build Instructions ==

List patches that need to be applied and what they do

See http://www.sagemath.org/doc/developer/producing_spkgs.html#creating-a-new-spkg

It wont take you long to find out the dependcies, and if there are no special build instructions, just put

== Special Update/Build Instructions ==
 * None

Dave

comment:3 Changed 8 years ago by ncohen

  • Status changed from needs_work to needs_review

Stupid copy/paste mistake... I was even doubting adding a line to the changelog in this case was necessary ^^;

Sorryyyyy ! (SPKG updated)

Nathann

comment:4 Changed 8 years ago by drkirkby

  • Status changed from needs_review to needs_work

That looks a bit better, but I would suggest a few changes.

  • Add the trac number (#9766) on the cliquer-1.2.p6 entry.
  • State the URL was added to SPKG.txt - at the moment we have no idea where it was added.
  • State you added the Special Update/Build Instructions and Dependencies to SPKG.txt which were previously missing.
  • I think it should be URL, not url, though that's a minor point.

I noticed there was no spkg-check file to run the self-tests, which the package does have. That needs addressing, but on another ticket, as it's quite separate. I created a ticket for that (#9767) and will address that at some point myself, if nobody beats me to it.

As such, it might be wise to put a note of this fact - one suggestion is below.

== Special Update/Build Instructions ==
 * TODO - Add an spkg-check file - see #9767

Once that is done, I expect to be able to give it a positive review.

Dave

comment:5 follow-up: Changed 8 years ago by drkirkby

Nathann, did you manage to finish this? The amount you need to do to get a positive review is trivial.

Dave

comment:6 in reply to: ↑ 5 ; follow-up: Changed 8 years ago by ncohen

Nathann, did you manage to finish this? The amount you need to do to get a positive review is trivial.

I have not had a "stable" internet connection for several weeks now (travelling -- I access internet through coffes, and when I am lucky in the hotels I stay in if they have a connection), and I will not be able to update this spkg until at least one week and a half. Sorry for that.

On the technical side David, we have known for a long time that we view development very differently. I try not to forget that you want Sage to be a "professionnal" software, with all the necessary -- what I call -- administration (standard procedures for modification of the code, correct documentation, supporting many platforms, changelogs, etc...). Even though it very often seems "too much" for my way of doing, I have two things to admit :

  • You think Sage will be improved through all this, and your intent is good
  • It requires a lot of work, which you often do yourself

In the end, even though I have a different way to work, it sounds like we are both trying to work on the same piece of (great) software, as efficiently as we can. This is why I am asking this question to a fellow developper :

There are necessary things in all this administration, to ensure that everything stays correct (doctests, spkg-checks, ...), or documented, or logged. But don't you think somethings goes really wrong when it takes 2 weeks, 2 persons, and 30 minutes or 1 hour of cumulated worktime to add an url to a file ?

How do you think we could simplify these things ? I am confident any mean you could name would never harm Sage's reliability.

I promise you will have this spkg updated with the modifications you requested as soon as I have a -- real -- internet connection. I may even be able to find a way to send it tomorrow ! :-)

Nathann

comment:7 follow-up: Changed 8 years ago by ncohen

  • Status changed from needs_work to needs_review

Wonderful airport with a free wifi, and no filter on port 80... Packages updated ! I hope you will like this version :-)

Nathann

comment:8 in reply to: ↑ 6 Changed 8 years ago by drkirkby

Replying to ncohen:

Nathann, did you manage to finish this? The amount you need to do to get a positive review is trivial.

I have not had a "stable" internet connection for several weeks now

Sorry to hear that. I often lose mine from home, and it really annoying. Particularly if I have a chess game scheduled as part of a team. Failure to play lets the whole team down, so I have on several occasions made a 90 mile round-trip to go to my fathers, use his internet, then come home. There's not even a local internet cafe here.

On the technical side David, we have known for a long time that we view development very differently.

Yes.

I try not to forget that you want Sage to be a "professionnal" software, with all the necessary -- what I call -- administration (standard procedures for modification of the code, correct documentation, supporting many platforms, changelogs, etc...). Even though it very often seems "too much" for my way of doing, I have two things to admit :

I think if Sage is ever going to be a viable alternative to the commercial products, it needs to get more professional in its approach. As Tim Daily points out in that recent thread on sage-devel, if things are not documented properly, then whole sections of code may need to be swapped out as they are unmaintainable. This is very close to the case with SYMPOW.

  • You think Sage will be improved through all this, and your intent is good
  • It requires a lot of work, which you often do yourself

In the end, even though I have a different way to work, it sounds like we are both trying to work on the same piece of (great) software, as efficiently as we can. This is why I am asking this question to a fellow developper :

There are necessary things in all this administration, to ensure that everything stays correct (doctests, spkg-checks, ...), or documented, or logged. But don't you think somethings goes really wrong when it takes 2 weeks, 2 persons, and 30 minutes or 1 hour of cumulated worktime to add an url to a file ?

Yes, I do. It is a waste of peoples time.

How do you think we could simplify these things ? I am confident any mean you could name would never harm Sage's reliability.

  • Sage has a Developers Guide. It's not that large, and I feel that anyone developing for Sage should look at the sections which are relevant to their work. Minh in particular has put a lot of effort into improving Sage's documentation. Let's hope hope his, and others efforts to improve documentation are not wasted, because people can't be bothered to read them. I think you would have to agree it's a waste of time creating documentation if it's not read.
  • Had the original author set up SPKG.txt properly, as documented in the section on creating a new spkg, CJ Fearnley would not have needed to request the information for Debian. So first and foremost, whoever wrote the SPKG.txt file in the first place, failed to follow the documentation, and so has caused
    • CJ Fearnley to write you an email
    • You to create a ticket
    • Me to review it.
    • The release manager to merge the package
    • You to write to CJ Fearnley to advise him the package is now corrected.
  • Next, had the reviewer for the cliquer package done their job properly, they would have spotted the authors omissions,
  • Finally, had you have taken a bit more care, and updated your SPKG.txt to actually reference the ticket, and not copy/past the previous entry, I would probably have given it a positive review immediately, though probably put a note like "it could be useful if you added the missing sections", and leave it up to your judgment whether you did that. Most developers will make minor changes like that. But your entry was confusing, so it needed to be changed.
  • Once your entry needed to be changed, it did not seem unreasonable that you add the missing sections at the same time.

I promise you will have this spkg updated with the modifications you requested as soon as I have a -- real -- internet connection. I may even be able to find a way to send it tomorrow ! :-)

Nathann

I believe I have spoke to you about the cost of fixing bugs. Basically, the earlier a bug is caught, the less expensive it it to fix. In the case of Sage, we are primarily talking about peoples time. Had the documentation error been picked up early, a lot less of peoples time would have been wasted.

That's why I've tried to get over to you the point that you should spend you time stopping the bugs in the first place, as it's less time consuming to do that, than it is to fix bugs when they are reported.

Dave

comment:9 in reply to: ↑ 7 ; follow-up: Changed 8 years ago by drkirkby

  • Status changed from needs_review to positive_review

Replying to ncohen:

Wonderful airport with a free wifi, and no filter on port 80... Packages updated ! I hope you will like this version :-)

Nathann

Yes, that looks fine. You should have put the patch on the trac server, but I will do that for you. I'm giving it positive review.

As a matter of interest, are you aware of any reason the package should not be updated to the latest, which is 1.2.1? If not, I'll update the package version at the same time as adding the test code to #9767.

Dave

comment:10 in reply to: ↑ 9 ; follow-up: Changed 8 years ago by ncohen

Yes, that looks fine. You should have put the patch on the trac server, but I will do that for you. I'm giving it positive review.

Thanks ! But why do you want a patch to be independently uploaded when it is contained in the spkg file ?

As a matter of interest, are you aware of any reason the package should not be updated to the latest, which is 1.2.1? If not, I'll update the package version at the same time as adding the test code to #9767.

Yes. The reason is that I ignored a new version had been released, and that no one beside me was expected to pay attention to this. It's past time I sent an email to the original developpers though, they may not even know their software is used in Sage. I will also ask them to drop me a line when they update their code if they happen to think of it :-)

Nathann

comment:11 in reply to: ↑ 10 Changed 8 years ago by drkirkby

Replying to ncohen:

Yes, that looks fine. You should have put the patch on the trac server, but I will do that for you. I'm giving it positive review.

Thanks ! But why do you want a patch to be independently uploaded when it is contained in the spkg file ?

It is how ever other .spkg gets updated. The patch is kept on the trac server. It makes it much easier for a review to see what he is reviewing, and for anyone else to look back and see the changes which were made on the ticket.

As a matter of interest, are you aware of any reason the package should not be updated to the latest, which is 1.2.1? If not, I'll update the package version at the same time as adding the test code to #9767.

Yes. The reason is that I ignored a new version had been released, and that no one beside me was expected to pay attention to this. It's past time I sent an email to the original developpers though, they may not even know their software is used in Sage. I will also ask them to drop me a line when they update their code if they happen to think of it :-)

Nathann

I'll update the package then. There needs to be a Solaris specific change too, as the libraries are not being created optimally on Solaris.

Dave

Changed 8 years ago by drkirkby

Mercurial patch to add contact information and generally clean up SPKG.txt

comment:12 Changed 8 years ago by drkirkby

  • Authors set to Nathann Cohen
  • Reviewers set to David Kirkby

comment:13 follow-up: Changed 8 years ago by mpatel

Since we build Cliquer in Sage with make instead of with SCons (see #9804), should we include SCons in the dependency list in SPKG.txt? If we do, should we add a note that SCons is not a Cliquer dependency in Sage?

comment:14 in reply to: ↑ 13 ; follow-up: Changed 8 years ago by drkirkby

Replying to mpatel:

Since we build Cliquer in Sage with make instead of with SCons (see #9804), should we include SCons in the dependency list in SPKG.txt?

No, we should not. I'll create a new patch and provide a new package in a minute. It wont take me long to delete one line.

If we do, should we add a note that SCons is not a Cliquer dependency in Sage?

I don't know where you would add a note that SCons is not a Cliquer dependency. Where would you propose to add such a note?

comment:15 Changed 8 years ago by drkirkby

  • Status changed from positive_review to needs_work

comment:16 follow-up: Changed 8 years ago by mpatel

Perhaps under "Special Update/Build? Instructions" or in the relevant "Changelog" note?

comment:17 Changed 8 years ago by drkirkby

  • Status changed from needs_work to positive_review

I've put a package which removes the SCons from SPKG.txt. I also thought it wise to set the dependency to "None", since that's what used on every other package I've seen, though strictly "Base" is more accurate, I think it's also more confusing.

http://boxen.math.washington.edu/home/kirkby/patches/cliquer-1.2.p6.spkg

I'm considering this a reviewer patch, so are just going to mark it positive again.

Dave

comment:18 in reply to: ↑ 16 Changed 8 years ago by drkirkby

Replying to mpatel:

Perhaps under "Special Update/Build? Instructions" or in the relevant "Changelog" note?

I'm not sure how best to do this. It would seem sensible that it was documented under the particular version where the dependency was removed, but I don't know how to find that out.

I wonder if this package ever had such a dependancy? The upstream source code does not. Nathann created the package, and I don't think he knows anything about SCons, so I doubt he would have used it. You know mercurial better than me - perhaps you can see if there's a record of SCons being removed.

Dave

comment:19 Changed 8 years ago by drkirkby

  • Status changed from positive_review to needs_info

comment:20 Changed 8 years ago by drkirkby

It looks like there was a SConstript file at one point in time:

drkirkby@hawk:/tmp/cliquer-1.2.p6/.hg$ ggrep -r SCons *
store/fncache:data/SConstruct.i

Now how do I find it when it was removed?

Dave

Changed 8 years ago by drkirkby

Improve the historical accuracy of SPKG.txt

comment:21 Changed 8 years ago by drkirkby

  • Authors changed from Nathann Cohen to Nathann Cohen, David Kirkby
  • Status changed from needs_info to needs_review

I found out that the call to 'scons' was removed in change set 4 by Minh. I've added that information to that entry.

I've marked it as needing review again - perhaps you can look over it Mitesh.

Dave

comment:22 Changed 8 years ago by drkirkby

  • Owner changed from tbd to drkirkby

The package with the changes can be found at

http://boxen.math.washington.edu/home/kirkby/patches/cliquer-1.2.p6.spkg

Dave

comment:23 follow-up: Changed 8 years ago by mpatel

Positive review, except for a stray file with the name "," (a comma):

cliquer-1.2.p6$ hg stat
? ,

Could you remove the file?

comment:24 in reply to: ↑ 23 Changed 8 years ago by drkirkby

Replying to mpatel:

Positive review, except for a stray file with the name "," (a comma):

cliquer-1.2.p6$ hg stat
? ,

Could you remove the file?

Sure. I don't know how that got there. It has a date in 2032 too - only 22 years ahead in time.

drkirkby@hawk:/tmp/cliquer-1.2.p6$ ls -l ,
-rw-r--r--   1 drkirkby staff       2032 Aug 31 01:15 ,

I've uploaded the new .spkg, without the file with a comma in its name.

Dave

comment:25 Changed 8 years ago by drkirkby

I see the 2032 was the size of the file, not the date!

I must have created it myself somehow.

Anyway, it has gone now.

comment:26 Changed 8 years ago by mpatel

  • Reviewers changed from David Kirkby to David Kirkby, Mitesh Patel
  • Status changed from needs_review to positive_review

Thanks!

comment:27 Changed 8 years ago by mpatel

  • Description modified (diff)

comment:28 Changed 8 years ago by mpatel

  • Merged in set to sage-4.5.3.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:29 in reply to: ↑ 14 Changed 8 years ago by leif

Replying to drkirkby:

Replying to mpatel:

Since we build Cliquer in Sage with make instead of with SCons (see #9804), should we include SCons in the dependency list in SPKG.txt?

No, we should not. I'll create a new patch and provide a new package in a minute. It wont take me long to delete one line.

If we do, should we add a note that SCons is not a Cliquer dependency in Sage?

I don't know where you would add a note that SCons is not a Cliquer dependency. Where would you propose to add such a note?

I think it's worth adding a note (in parentheses) to the Dependencies section that some package['s ordinary build] does not depend on xy especially if (e.g) the upstream contains source files for / files (usually) to be processed by xy, or in cases where the package previously depended on some package(s), but no longer does. (And even if there erroneously was some dependency listed in spkg/standard/deps.)

Typical candidates for xy are Python, Perl, lex/flex, yacc/bison etc. (Autotools should not be listed.)

Also, some packages only do not depend on some others (e.g. libraries) in Sage just because of the way we configure, patch or build / install them. IMHO as well worth a note.

Note: See TracTickets for help on using tickets.