Opened 8 years ago

Closed 8 years ago

#12087 closed enhancement (fixed)

Clean up devel/sage/.hgignore

Reported by: jdemeyer Owned by: jdemeyer
Priority: major Milestone: sage-4.8
Component: misc Keywords:
Cc: Merged in: sage-4.8.alpha3
Authors: Jeroen Demeyer Reviewers: Keshav Kini
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

There are lots of files in devel/sage/.hgignore which no longer exist.

Also, we should add MANIFEST which is created by sage -sdist.

Attachments (1)

12087_hgignore.patch (2.8 KB) - added by jdemeyer 8 years ago.

Download all attachments as: .zip

Change History (10)

Changed 8 years ago by jdemeyer

comment:1 follow-up: Changed 8 years ago by kini

There was a relevant thread (though without a lot of info) on sage-devel. I was concerned that these files could possibly exist in a modern Sage installation when some arcane SPKG or other is installed. Did you confirm that this is not the case for anything removed by your patch?

comment:2 in reply to: ↑ 1 ; follow-up: Changed 8 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Status changed from new to needs_review

Replying to kini:

Did you confirm that this is not the case for anything removed by your patch?

No, I did not try installing every optional and experimental package. Note also that I did not remove all non-existing files, only the ones where I could guess where they came from and which looked unlikely to re-appear (for example, I did not remove .git in case we ever change to use git). It's also not a big disaster if some file turns out to exist anyway, the only consequence is complaints from hg status.

comment:3 Changed 8 years ago by jdemeyer

  • Description modified (diff)
  • Owner changed from jason to jdemeyer

comment:4 in reply to: ↑ 2 Changed 8 years ago by kini

  • Reviewers set to Keshav Kini
  • Status changed from needs_review to positive_review

Well, for example sage-update actually cares about whether there are complaints from hg status, though this is silly in the first place, IMO. Anyway I see no real problem with this patch. If something breaks because of it, it shouldn't be breaking. I will give it a positive review.

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

Can you explain why some files are listed in .hgignore but are part of the distribution? For example, "PKG-INFO" (autogenerated somehow?), "pull", and "sage/rings/integer.h"? What about the 4.5 megabyte file sage/c_lib/.sconsign.dblite?

comment:6 Changed 8 years ago by kini

I think William was trying to make hg status tell him whether he had changed the Sage library code or not, rather than whether the working directory had any changes at all, which is what the usual meaning of hg status's output and return code is. See $SAGE_LOCAL/sage-test-new which implements sage -tnew and parses the output of hg status. So things that are not code are put in the ignore file.

comment:7 Changed 8 years ago by jhpalmieri

That sounds plausible; however, I would rather track all of these files and do more processing in sage-test-new — only test the file if its suffix is something appropriate. (This obviously would belong on another ticket.)

comment:8 in reply to: ↑ 5 Changed 8 years ago by jdemeyer

Replying to jhpalmieri:

Can you explain why some files are listed in .hgignore but are part of the distribution? For example, "PKG-INFO" (autogenerated somehow?), "pull", and "sage/rings/integer.h"? What about the 4.5 megabyte file sage/c_lib/.sconsign.dblite?

pull is no longer in .hgignore with my patch.

The .h files are auto-generated by Cython and should not be in the MANIFEST.

I have no idea about .sconsign.dblite

comment:9 Changed 8 years ago by jdemeyer

  • Merged in set to sage-4.8.alpha3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.