patchFreeciv - Patches: patch #2829, Text data files with native line...

 
 
Show feedback again

patch #2829: Text data files with native line endings

Submitted by:  Marko Lindqvist <cazfi>
Submitted on:  Sun 24 Jul 2011 06:35:30 PM UTC  
 
Category: bootstrapPriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open
Planned Release: 

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Thu 18 Oct 2012 01:21:58 AM UTC, comment #7:

So it's about a year too late to comment on this (sorry), but:

> Notable conceptual change here would be that data files would
> be built.

I have to say that I'm a little uneasy with that as a solution to the line-ending problem -- it feels like a sledgehammer to crack a nut.
It'll be a bit of additional inconvenience during development (as already acknowledged in comment #4) -- nothing insurmountable if the dependencies are accurate, but the benefit doesn't obviously outweigh the cost, especially as for data files Freeciv is already line-ending-agnostic as of patch #2843.
(That doesn't catch READMEs etc, which I think it'll be more common for users to try to open, but Freeciv doesn't try and read those anyway.)

It feels to me like this (and similar things like compression) would perhaps better be part of "make install" rather than "make".
That does mean that the typical developer experience (running from source directory) diverges further from the "mission mode" (install and run), perhaps increasing the possibility of install-only bugs not spotted until too late.

The arguments change if we're doing more stuff as part of "building" text files, such as automatic substitutions.

Jacob Nevins <jtn>
Project Administrator
Sat 13 Oct 2012 02:57:34 AM UTC, comment #6:

- Replaced implicit pattern rules with static pattern rules. Former is GNU make extension. This requires that individual Makefiles explicitly list all the datafiles to convert.

(file #16692)

Marko Lindqvist <cazfi>
Project Administrator
Sun 13 May 2012 11:19:20 PM UTC, comment #5:

As current files will be considered only srcfiles and not final files, all of them must be renamed before this ticket is completed. I don't want to rush that massive rename operation that would be equal pain to revert if we later find out problems in the system.

Instead I want to go by three steps.
1) Add code to support newline settings (attached patch) Before datafiles are renamed, system sees them as final target files and happily concludes that there is no need to build anything.
2) Rename a couple of datafiles for testing purposes. These particular files will be built by the new system
3) Rename all the remaining files

Attached patch adds make rules to build .rsdata -> .ruleset, .tsdata -> .tilespec, .sdata -> .spec, .ssdata -> .soundspec, savdata -> .sav, and .servdata -> .serv
What these rules do is determined configure-time. By default srcfile is just copied as destination file. Configure option --enable-dataln can be used to change that so it makes either "dos2unix" or "unix2dos". At this time this must be given explicitly, automatic detection of the newline conversion needs is left to future tickets.

(file #15738)

Marko Lindqvist <cazfi>
Project Administrator
Sat 12 May 2012 09:28:54 PM UTC, comment #4:

Would it cause notable problems for any developer that ruleset data must be built after every srcfile modification? Currently one can just modify ruleset and start freeciv immediately to test out the changes (or even reload files to already running freeciv), but after this change one would need to 'make' in between ruleset src modification and their use.

When/if we decide to build srcfiles, we could make also other conversions from src to dest. Compressing the files comes to mind. Currently build process unconditionally gzips included scenarios, but no other file gets compressed with any method. We could compress some datafiles with best compression method that freeciv being built will support.

Marko Lindqvist <cazfi>
Project Administrator
Sat 12 May 2012 07:41:26 PM UTC, comment #3:

Notable conceptual change here would be that data files would be built. We currently have them in their final form in srcdir, but with this change we would have some sort of srcfile (game.ruleset.stxt) in srcdir which is then built to final file in builddir.

Last summer I said that this would make crosser builds impossible, as those builds depended on data-directory taken from version control to work as is. That's no longer the case. Reworked ("new style") crosser builds have data files gone through build system.

Marko Lindqvist <cazfi>
Project Administrator
Wed 27 Jul 2011 11:10:17 PM UTC, comment #2:

> I'd say having files with UNIX file endings on Windows is not a
> big problem, since they can be opened with WordPad.

True, but it's not a problem most Windows users are familiar with, I think -- a typical non-Unix-literate user will just double-click on the file, see text all munged together, and not know how to get a sensible display.

In the same vein, it would be useful if we shipped files in the Windows installer with names like README.txt rather than plain README (which Windows doesn't know what to do with).

However, implementing these suggestions is a can of worms if not done carefully.

Jacob Nevins <jtn>
Project Administrator
Sun 24 Jul 2011 10:48:07 PM UTC, comment #1:

I'd say having files with UNIX file endings on Windows is not a big problem, since they can be opened with WordPad. But Freeciv should be able to handle both types.

Christian Prochaska <cproc>
Project Administrator
Sun 24 Jul 2011 06:35:30 PM UTC, original submission:

In bug #18396 there is some discussion about always providing textual data files with native (dos / unix) line endings.

This ticket is for further discussion about the matter. Should that be part of general freeciv build system at all, or should we just let people making windows builds to resolve it somehow themselves.
If it will be part of main freeciv build process, preferably it should not introduce new requirement (unix2dos / dos2unix) for builds where it really is not needed, but configure woudl check for conversation tools only if it knows that they will be needed.

One cannot rust source tree to have certain kind of line endings without checking. Tarball has certain kind of line endings regardless where it's used. svn checkouts have native line endings.

Conversations (and lack of them) must be supported for both native and cross-compilations.

Marko Lindqvist <cazfi>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #16692:  DataNewlines-2.patch added by cazfi (10kB - text/x-diff)
file #15738:  DataNewlines.diff added by cazfi (10kB - text/plain)

 

Depends on the following items

Digest:
   patch dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jtn (Posted a comment)
  • -unavailable- added by cproc (Posted a comment)
  • -unavailable- added by cazfi (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 12 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 20 Mar 2013 09:52:22 PM UTCcazfiAssigned tocazfi=>None
    Wed 23 Jan 2013 07:41:54 PM UTCcazfiStatusReady For Test=>None
      Planned Release2.5.0=>
    Sat 13 Oct 2012 02:57:34 AM UTCcazfiAttached File-=>Added DataNewlines-2.patch, #16692
      StatusIn Progress=>Ready For Test
      Planned Release=>2.5.0
    Mon 02 Jul 2012 05:24:01 PM UTCcazfiStatusReady For Test=>In Progress
    Sun 13 May 2012 11:20:21 PM UTCcazfiDependencies-=>Depends on patch #3296
    Sun 13 May 2012 11:19:20 PM UTCcazfiAttached File-=>Added DataNewlines.diff, #15738
      CategoryNone=>bootstrap
      StatusNone=>Ready For Test
      Assigned toNone=>cazfi
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup