Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cliquer - Update Upstream contact #9766

Closed
nathanncohen mannequin opened this issue Aug 19, 2010 · 31 comments
Closed

Cliquer - Update Upstream contact #9766

nathanncohen mannequin opened this issue Aug 19, 2010 · 31 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Aug 19, 2010

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

Component: packages: standard

Author: Nathann Cohen, David Kirkby

Reviewer: David Kirkby, Mitesh Patel

Merged: sage-4.5.3.rc0

Issue created by migration from https://trac.sagemath.org/ticket/9766

@nathanncohen nathanncohen mannequin added this to the sage-4.5.3 milestone Aug 19, 2010
@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 19, 2010

comment:2

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

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 19, 2010

comment:3

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

Sorryyyyy ! (SPKG updated)

Nathann

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 19, 2010

comment:4

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

  • Add the trac number (Cliquer - Update Upstream contact #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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 27, 2010

comment:5

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

Dave

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 27, 2010

comment:6

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

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 28, 2010

comment:7

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

Nathann

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 28, 2010

comment:8

Replying to @nathanncohen:

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 28, 2010

comment:9

Replying to @nathanncohen:

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

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 28, 2010

comment:10

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 28, 2010

comment:11

Replying to @nathanncohen:

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 28, 2010

Attachment: 9766.patch.gz

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 28, 2010

Reviewer: David Kirkby

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 28, 2010

Author: Nathann Cohen

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 30, 2010

comment:13

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?

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 30, 2010

comment:14

Replying to @qed777:

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?

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 30, 2010

comment:16

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 30, 2010

comment:17

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 30, 2010

comment:18

Replying to @qed777:

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 30, 2010

comment:20

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 31, 2010

Attachment: 9766-improved-SPKG.txt-information.patch.gz

Improve the historical accuracy of SPKG.txt

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 31, 2010

Changed author from Nathann Cohen to Nathann Cohen, David Kirkby

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 31, 2010

comment:21

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 31, 2010

comment:22

The package with the changes can be found at

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

Dave

@sagetrac-drkirkby sagetrac-drkirkby mannequin self-assigned this Aug 31, 2010
@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 31, 2010

comment:23

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

cliquer-1.2.p6$ hg stat
? ,

Could you remove the file?

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 31, 2010

comment:24

Replying to @qed777:

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Aug 31, 2010

comment:25

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.

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 31, 2010

comment:26

Thanks!

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 31, 2010

Changed reviewer from David Kirkby to David Kirkby, Mitesh Patel

@qed777

This comment has been minimized.

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 31, 2010

Merged: sage-4.5.3.rc0

@qed777 qed777 mannequin removed the s: positive review label Aug 31, 2010
@qed777 qed777 mannequin closed this as completed Aug 31, 2010
@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 4, 2010

comment:29

Replying to @sagetrac-drkirkby:

Replying to @qed777:

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

0 participants