Fix picotool to work with p8 carts that use CRLF line endings

.gitattributes

@@ -0,0 +1,3 @@
+# test p8 cart files should not have their line endings changed on checkout
+# use 'binary' settings for p8 cart files to force this
+*.p8 binary

.github/workflows/python-app.yml

@@ -0,0 +1,64 @@
+# This workflow will install Python dependencies, run tests and lint with a single version of Python
+# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions
+
+name: Tests
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+
+permissions:
+  contents: read
+
+jobs:
+  test-ubuntu:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v3
+    - name: Set up Python 3.10
+      uses: actions/setup-python@v3
+      with:
+        python-version: "3.10"
+    - name: Install dependencies
+      run: |
+        pip install --upgrade pip
+        pip install -r pydevtools.txt
+        pip install flake8 pytest
+    - name: Lint with flake8
+      run: |
+        # stop the build if there are Python syntax errors or undefined names
+        flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
+        # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
+        flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
+    - name: Test with pytest
+      run: |
+        pytest
+
+  test-windows:
+
+    runs-on: windows-latest
+
+    steps:
+    - uses: actions/checkout@v3
+    - name: Set up Python 3.10
+      uses: actions/setup-python@v3
+      with:
+        python-version: "3.10"
+    - name: Install dependencies
+      run: |
+        pip install --upgrade pip
+        pip install -r pydevtools.txt
+        pip install flake8 pytest
+    - name: Lint with flake8
+      run: |
+        # stop the build if there are Python syntax errors or undefined names
+        flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
+        # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
+        flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
+    - name: Test with pytest
+      run: |
+        pytest

README.md

@@ -1,5 +1,7 @@
 # picotool: Tools and Python libraries for manipulating PICO-8 game files
 
+[![tests](https://github.com/scottnm/picotool/actions/workflows/python-app.yml/badge.svg)](https://github.com/scottnm/picotool/actions/workflows/python-app.yml)
+
 [PICO-8](http://www.lexaloffle.com/pico-8.php) is a _fantasy game console_ by [Lexaloffle Games](http://www.lexaloffle.com/). The PICO-8 runtime environment runs _cartridges_ (or _carts_): game files containing code, graphics, sound, and music data. The console includes a built-in editor for writing games. Game cartridge files can be played in a browser, and can be posted to the Lexaloffle bulletin board or exported to any website.
 
 `picotool` is a suite of tools and libraries for building and manipulating PICO-8 game cartridge files. The suite is implemented in, and requires, [Python 3](https://www.python.org/). The tools can examine and transform cartridges in various ways, and you can implement your own tools to access and modify cartridge data with the Python libraries.

pico8/game/formatter/p8.py

@@ -3,6 +3,7 @@
 __all__ = [
     'P8Formatter',
     'InvalidP8HeaderError',
+    'InvalidP8VersionError',
     'InvalidP8SectionError',
     'P8IncludeNotFound',
     'P8IncludeOutsideOfAllowedDirectory',
@@ -24,8 +25,8 @@
 from ...music.music import Music
 
 HEADER_TITLE_STR = b'pico-8 cartridge // http://www.pico-8.com\n'
-HEADER_VERSION_RE = re.compile(br'version (\d+)\n')
-SECTION_DELIM_RE = re.compile(br'__(\w+)__\n')
+HEADER_VERSION_RE = re.compile(br'version (\d+)\r?\n')
+SECTION_DELIM_RE = re.compile(br'__(\w+)__\r?\n')
 INCLUDE_LINE_RE = re.compile(
     br'\s*#include\s+(\S+)(\.p8\.png|\.p8|\.lua)(\:\d+)?')
 PICO8_CART_PATHS = [
@@ -39,8 +40,22 @@
 class InvalidP8HeaderError(util.InvalidP8DataError):
     """Exception for invalid .p8 file header."""
 
+    def __init__(self, bad_header, expected_header):
+        self.bad_header = bad_header
+        self.expected_header = expected_header
+
+    def __str__(self):
+        return 'Invalid .p8: missing or corrupt header. Found "%s" Expected "%s"' % (self.bad_header, self.expected_header)
+
+
+class InvalidP8VersionError(util.InvalidP8DataError):
+    """Exception for invalid .p8 version header."""
+
+    def __init__(self, bad_version_line):
+        self.bad_version_line = bad_version_line 
+
     def __str__(self):
-        return 'Invalid .p8: missing or corrupt header'
+        return ('Invalid .p8: invalid version header. found "%s"' % self.bad_version_line)
 
 
 class InvalidP8SectionError(util.InvalidP8DataError):
@@ -68,12 +83,13 @@ class InvalidP8Include(util.InvalidP8DataError):
 
 def _get_raw_data_from_p8_file(instr, filename=None):
     header_title_str = instr.readline()
-    if header_title_str != HEADER_TITLE_STR:
-        raise InvalidP8HeaderError()
+    # use rstrip to normalize line endings
+    if header_title_str.rstrip() != HEADER_TITLE_STR.rstrip():
+        raise InvalidP8HeaderError(header_title_str, HEADER_TITLE_STR)
     header_version_str = instr.readline()
     version_m = HEADER_VERSION_RE.match(header_version_str)
     if version_m is None:
-        raise InvalidP8HeaderError()
+        raise InvalidP8VersionError(header_version_str)
     version = int(version_m.group(1))
 
     # (section is a text str.)

pico8/gfx/gfx.py

@@ -84,7 +84,8 @@ def from_lines(cls, lines, version):
         """
         datastrs = []
         for line in lines:
-            if len(line) != 129:
+            # Each line of the GFX section is 128 characters followed by either an LF or CRLF line ending
+            if len(line.rstrip()) != 128:
                 continue
 
             larray = list(line.rstrip())

pydevtools.txt

@@ -9,3 +9,4 @@ pytest
 coverage
 ipython
 rstcheck
+pypng

pytest.ini

@@ -1,2 +1,2 @@
 [pytest]
-addopts = --ignore=pyenv/ -n4
+addopts = --ignore=pyenv/

tests/pico8/game/game_test.py

@@ -197,7 +197,11 @@ def testFromP8File(self):
 
     def testInvalidP8HeaderErrorMsg(self):
         # coverage
-        str(p8.InvalidP8HeaderError())
+        str(p8.InvalidP8HeaderError('bad', 'expected'))
+
+    def testInvalidP8VersionErrorMsg(self):
+        # coverage
+        str(p8.InvalidP8VersionError('bad version'))
 
     def testInvalidP8SectionErrorMsg(self):
         # coverage
@@ -214,7 +218,7 @@ def testInvalidP8HeaderLineOne(self):
 
     def testInvalidP8HeaderLineTwo(self):
         self.assertRaises(
-            p8.InvalidP8HeaderError,
+            p8.InvalidP8VersionError,
             p8.P8Formatter.from_file,
             io.BytesIO(
                 INVALID_P8_HEADER_TWO +
@@ -349,6 +353,22 @@ def testToP8FileFromP8(self):
         p8.P8Formatter.to_file(orig_game, outstr)
         self.assertEqual(expected_game_p8, outstr.getvalue())
 
+    def testToP8FileFromP8WithCrlf(self):
+        test_cart_path = os.path.join(self.testdata_path, 'test_cart_crlf.p8')
+        with open(test_cart_path, 'rb') as fh:
+            orig_game = p8.P8Formatter.from_file(fh)
+        with open(test_cart_path, 'rb') as fh:
+            expected_game_p8 = fh.read()
+        outstr = io.BytesIO()
+        p8.P8Formatter.to_file(orig_game, outstr)
+
+        # It's not (yet) important for this tool to retain CRLF endings when building from a source p8 cart
+        # which uses CRLF endings. It's ok for the resulting generated cart to use LF endings.
+        expected_game_p8 = expected_game_p8.replace(b"\r", b"")
+        outstr = outstr.getvalue().replace(b"\r", b"")
+
+        self.assertEqual(expected_game_p8, outstr)
+
     def testToP8FileFromP8PreservesLabel(self):
         test_cart_path = os.path.join(
             self.testdata_path, 'test_cart_with_label.p8')
@@ -511,7 +531,7 @@ def testGetRootIncludePathLinuxCartRoot(self):
 
     def testGetRootIncludePathUnrecognizedRoot(self):
         cartpath = '/tmp/subdir/somecart.p8'
-        expected = '/tmp/subdir'
+        expected = os.path.abspath('/tmp/subdir')
         self.assertEqual(
             expected,
             p8.get_root_include_path(cartpath))