Remove sqlitebck dependency + windows is working, but not in runner + other fixes (#284)

for some reason q.exe works well after an MSI installation on Windows, but not working well on the runner doing a sanity, in terms of dependencies on sqlite3 extension.

11 files changed, 65 insertions, 61 deletions

diff --git a/.github/workflows/build-and-package.yaml b/.github/workflows/build-and-package.yaml
index f723786..2c55051 100644
--- a/.github/workflows/build-and-package.yaml
+++ b/.github/workflows/build-and-package.yaml
@@ -304,8 +304,13 @@ jobs:
 
         mkdir brew
 
+        export BRANCH_NAME=${{ github.event.pull_request.head.ref }}
+
         # TODO temp, since template rendering action doesn't work in mac
-        cat .github/workflows/q.rb.brew-formula-template | sed 's/{{ .Q_VERSION }}/3.1.0-beta/g' > ./brew/q.rb
+        cat .github/workflows/q.rb.brew-formula-template | sed 's/{{ .Q_VERSION }}/3.1.0-beta/g' | sed "s/{{ .Q_BRANCH_NAME }}/${BRANCH_NAME}/g" > ./brew/q.rb
+
+        echo "Resulting formula:"
+        cat ./brew/q.rb
 
         brew install --display-times --formula --build-bottle --verbose ./brew/q.rb
         brew test ./brew/q.rb
@@ -352,10 +357,8 @@ jobs:
 
         echo "select sum(c1),count(*) from data_stream_stdin" | sqlite3 test.sqlite
 
-  # TODO Windows build/test/package flow is running, but q executable is still not running well, due to pyox+sqlite3 issue
   build-windows:
     runs-on: windows-latest
-    if: ${{ false }}
     steps:
     - name: Checkout
       uses: actions/checkout@v2
@@ -380,9 +383,6 @@ jobs:
       run: |
         set -e -x
 
-        # Hack to overcome the fact that apsw doesn't have a registered wheel for Windows
-        cp requirements-win-x86_64.txt requirements.txt
-
         pyoxidizer build --release
 
         export Q_EXECUTABLE=./build/x86_64-pc-windows-msvc/release/install/q
@@ -397,23 +397,36 @@ jobs:
     - name: Upload Linux Executable
       uses: actions/upload-artifact@v1.0.0
       with:
-        name: win-q
+        name: win-q.exe
         path: packages/windows/win-q.exe
 
-  test-windows:
+  not-really-test-windows:
     needs: build-windows
     runs-on: windows-latest
     steps:
     - name: Checkout
       uses: actions/checkout@v2
-    - name: Fail deliberately - No tests on Windows
+    - name: Install Python for Testing
+      uses: actions/setup-python@v2
+      with:
+        python-version: '3.8'
+        architecture: 'x64'
+    - name: Download Windows Executable
+      uses: actions/download-artifact@v2
+      with:
+        name: win-q.exe
+    - name: Not-Really-Test Windows
+      shell: bash
       continue-on-error: true
       run: |
-        echo "Tests are not actually running on the Windows Executable itself. Only the packaging is later tested"
-        exit 1
+        echo "Tests are not compatible with Windows (path separators, tmp folder names etc.). Only a sanity wil be tested"
+
+        chmod +x ./win-q.exe
+
+        seq 1 10000 | ./win-q.exe -c 1 "select sum(c1),count(*) from -" -S some-db.sqlite
 
   package-windows:
-    needs: [create-man, test-windows]
+    needs: [create-man, not-really-test-windows]
     runs-on: windows-latest
     steps:
     - name: Checkout
@@ -439,24 +452,23 @@ jobs:
       run: |
         set -e -x
 
-        # Hack to overcome the fact that apsw doesn't have a registered wheel for Windows
-        cp requirements-win-x86_64.txt requirements.txt
-
         pyoxidizer build --release msi_installer
 
         find ./ -ls
 
-        export Q_MSI=./build/x86_64-pc-windows-msvc/release/msi_installer/q-text-as-data-3.1.0-beta.msi
+        # TODO Windows versions do not support the -beta postfix
+
+        export Q_MSI=./build/x86_64-pc-windows-msvc/release/msi_installer/q-text-as-data-3.1.0.msi
         chmod 755 $Q_MSI
 
         mkdir -p packages/windows/
-        cp $Q_MSI packages/windows/q-text-as-data-3.1.0-beta.msi
+        cp $Q_MSI packages/windows/q-text-as-data-3.1.0.msi
 
     - name: Upload Windows MSI
       uses: actions/upload-artifact@v1.0.0
       with:
-        name: q-text-as-data-3.1.0-beta.msi
-        path: packages/windows/q-text-as-data-3.1.0-beta.msi
+        name: q-text-as-data-3.1.0.msi
+        path: packages/windows/q-text-as-data-3.1.0.msi
 
   test-windows-packaging:
     needs: package-windows
@@ -467,12 +479,12 @@ jobs:
     - name: Download Windows Package
       uses: actions/download-artifact@v2
       with:
-        name: q-text-as-data-3.1.0-beta.msi
+        name: q-text-as-data-3.1.0.msi
     - name: Test Install of MSI
       continue-on-error: true
       shell: powershell
       run: |
-        $process = Start-Process msiexec.exe -ArgumentList "/i q-text-as-data-3.1.0-beta.msi -l* msi-install.log /norestart /quiet" -PassThru -Wait
+        $process = Start-Process msiexec.exe -ArgumentList "/i q-text-as-data-3.1.0.msi -l* msi-install.log /norestart /quiet" -PassThru -Wait
         $process.ExitCode
         gc msi-install.log
 
@@ -481,14 +493,14 @@ jobs:
       continue-on-error: true
       shell: powershell
       run: |
-        $process = Start-Process msiexec.exe -ArgumentList "/u q-text-as-data-3.1.0-beta.msi /norestart /quiet" -PassThru -Wait
+        $process = Start-Process msiexec.exe -ArgumentList "/u q-text-as-data-3.1.0.msi /norestart /quiet" -PassThru -Wait
         $process.ExitCode
         exit $process.ExitCode
 
   perform-prerelease:
     # We'd like artifacts to be uploaded regardless of tests succeeded or not,
     # this is why the dependency here is not on test-X-packaging jobs
-    needs: [package-linux-deb, package-linux-rpm, package-mac]
+    needs: [package-linux-deb, package-linux-rpm, package-mac, package-windows]
     runs-on: ubuntu-latest
     # TODO Push to master will now pre-release as well, until things stabilize
     # if: ${{ github.event_name == 'pull_request' }}
@@ -513,8 +525,7 @@ jobs:
           artifacts/**/*
 
   perform-release:
-    # TODO Windows is not here so users won't be confused by seeing an MSI (it's still not production-grade, you need to have sqlite3 dll in the path)
-    needs: [test-mac-packaging, test-deb-packaging, test-rpm-packaging]
+    needs: [test-mac-packaging, test-deb-packaging, test-rpm-packaging, test-windows-packaging]
     runs-on: ubuntu-latest
     # Disabled on purpose for now - Changing the beta release to a real one will be done manually until everything stabilizes
     # and then this will be reinstated
@@ -525,16 +536,6 @@ jobs:
       uses: actions/download-artifact@v2
       with:
         path: artifacts/
-    - name: Delete Windows Artifacts so they're not part of the release for now
-      run: |
-        set -x -e
-
-        echo "Deleting windows artifacts so they're not part of the release - windows is not fully ready"
-
-        set +e
-        rm -vf artifacts/*.msi
-        rm -vf artifacts/win-q.exe
-        set -e
     - uses: "marvinpinto/action-automatic-releases@v1.2.1"
       with:
         repo_token: "${{ secrets.GITHUB_TOKEN }}"
diff --git a/.github/workflows/q.rb.brew-formula-template b/.github/workflows/q.rb.brew-formula-template
index c60fb43..15b77d0 100644
--- a/.github/workflows/q.rb.brew-formula-template
+++ b/.github/workflows/q.rb.brew-formula-template
@@ -4,8 +4,8 @@
 class Q < Formula
   desc "Run SQL directly on CSV or TSV files"
   homepage "https://harelba.github.io/q/"
-  # Building directly from master for now, eventually it will be tag-based so the version tag will be downloaded
-  url "https://github.com/harelba/q/archive/master.tar.gz"
+  # Using branch name for pre-releases, for tagged releases this would be the version tag, and not "version" part will be needed
+  url "https://github.com/harelba/q/archive/{{ .Q_BRANCH_NAME }}.tar.gz"
   version "{{ .Q_VERSION }}"
 
   # Removed for now, until everything is finalized
diff --git a/QSQL-NOTES.md b/QSQL-NOTES.md
index 48fa328..6ef7e2b 100644
--- a/QSQL-NOTES.md
+++ b/QSQL-NOTES.md
@@ -20,7 +20,7 @@ time q -c 1 "select sum(c1),count(*) from myfile.csv" -C readwrite
 500000500000 1000000
 q -c 1 "select sum(c1),count(*) from myfile.csv" -C readwrite  3.96s user 0.08s system 99% cpu 4.057 total
 
-# Now run with `-C read`. The query will run from the cache file and not the original. Change the query and run it several times, to notice the difference in speed.
+# Now run with `-C read`. The query will run from the cache file and not the original. Change the query and run it several times, to notice the difference in speed. As the file gets bigger, the difference will be much more noticable
 $ time q -c 1 "select sum(c1),count(*) from myfile.csv" -C read
 500000500000 1000000
 q -c 1 "select sum(c1),count(*) from myfile.csv" -C read  0.17s user 0.05s system 94% cpu 0.229 total
@@ -137,11 +137,12 @@ If you are such a user, and this decision hurts you considerably, please ping me
 
 
 # Installation of the new beta release
-For now, only Linux RPM, DEB and Mac OSX are supported. Almost made the Windows version work, but there's some issue there, and the windows executable requires some external dependencies which I'm trying to eliminate.
+For now, only Linux RPM, DEB, Mac OSX and Windows are supported. Packages for additional Linux Distros will be added later (it should be rather easy now, due to the use of `fpm`).
 
 The beta OSX version is not in `brew` yet, you'll need to take the `macos-q` executable, put it in your filesystem and `chmod +x` it.
 
-DEB/RPM are working well, although for some reason showing the q manual (`man q`) does not work for Debian, even though it's packaged in the DEB file. I'll get around to fixing it later.
+Note: For some reason showing the q manual (`man q`) does not work for Debian, even though it's packaged in the DEB file. I'll get around to fixing it later. If you have any thoughts about this, please drop me a line.
 
 Download the relevant files directly from [The Beta Release Assets](https://github.com/harelba/q/releases/tag/v3.1.0-beta).
 
+
diff --git a/README.markdown b/README.markdown
index 40f3b67..e0bc585 100644
--- a/README.markdown
+++ b/README.markdown
@@ -39,11 +39,11 @@ The current production version `2.0.19` installation is extremely simple.
 Instructions for all OSs are [here](http://harelba.github.io/q/#installation). 
 
 ### Installation of the new beta release
-For now, only Linux RPM, DEB and Mac OSX are supported. Almost made the Windows version work, but there's some issue there, and the windows executable requires some external dependencies which I'm trying to eliminate.
+For now, only Linux RPM, DEB, Mac OSX and Windows are supported. Packages for additional Linux Distros will be added later (it should be rather easy now, due to the use of `fpm`).
 
 The beta OSX version is not in `brew` yet, you'll need to take the `macos-q` executable, put it in your filesystem and `chmod +x` it. 
 
-DEB/RPM are working well, although for some reason showing the q manual (`man q`) does not work for Debian, even though it's packaged in the DEB file. I'll get around to fixing it later.
+Note: For some reason showing the q manual (`man q`) does not work for Debian, even though it's packaged in the DEB file. I'll get around to fixing it later. If you have any thoughts about this, please drop me a line.
 
 Download the relevant files directly from [The Beta Release Assets](https://github.com/harelba/q/releases/tag/v3.1.0-beta).
 
diff --git a/bin/q.py b/bin/q.py
index a71fec4..3d6ece7 100755
--- a/bin/q.py
+++ b/bin/q.py
@@ -56,7 +56,6 @@ import math
 import six
 import io
 import json
-import sqlitebck
 import datetime
 import hashlib
 
@@ -67,6 +66,7 @@ long = int
 unicode = six.text_type
 
 DEBUG = bool(os.environ.get('Q_DEBUG', None)) or '-V' in sys.argv
+SQL_DEBUG = False
 
 if DEBUG:
     def xprint(*args,**kwargs):
@@ -76,12 +76,17 @@ if DEBUG:
         print(datetime.datetime.utcnow().isoformat()," INFO ",*args,file=sys.stderr,**kwargs)
 
     def sqlprint(*args,**kwargs):
-        print(datetime.datetime.utcnow().isoformat(), " SQL ", *args, file=sys.stderr, **kwargs)
+        pass
 else:
     def xprint(*args,**kwargs): pass
     def iprint(*args,**kwargs): pass
     def sqlprint(*args,**kwargs): pass
 
+if SQL_DEBUG:
+    def sqlprint(*args,**kwargs):
+        print(datetime.datetime.utcnow().isoformat(), " SQL ", *args, file=sys.stderr, **kwargs)
+
+
 def get_stdout_encoding(encoding_override=None):
     if encoding_override is not None and encoding_override != 'none':
        return encoding_override
@@ -1320,7 +1325,7 @@ def normalize_filename_to_table_name(filename):
         filename = filename[:-7]
     elif filename.lower().endswith('.sqlite3'):
         filename = filename[:-8]
-    return filename.replace("-","_dash_").replace(".","_dot_").replace('?','_qm_').replace("/","_slash_").replace("\\","_backslash_")
+    return filename.replace("-","_dash_").replace(".","_dot_").replace('?','_qm_').replace("/","_slash_").replace("\\","_backslash_").replace(":","_colon_").replace(" ","_space_").replace("+","_plus_")
 
 def validate_content_signature(original_filename, source_signature,other_filename, content_signature,scope=None,dump=False):
     if dump:
@@ -1331,7 +1336,7 @@ def validate_content_signature(original_filename, source_signature,other_filenam
         scope = []
     for k in source_signature:
         if type(source_signature[k]) == OrderedDict:
-            return validate_content_signature(original_filename, source_signature[k],other_filename, content_signature[k],scope + [k])
+            validate_content_signature(original_filename, source_signature[k],other_filename, content_signature[k],scope + [k])
         else:
             if k not in content_signature:
                 raise ContentSignatureDataDiffersException("%s Content Signatures differ. %s is missing from content signature" % (s,k))
@@ -1374,8 +1379,10 @@ class DelimitedFileReader(object):
         if self.atomic_fns is None or len(self.atomic_fns) == 0:
             return "data stream-lmt"
         else:
-            x = ",".join(map(str,[os.stat(x).st_mtime_ns for x in self.atomic_fns]))
-            return hashlib.sha1(six.b(x)).hexdigest()
+            x = ",".join(map(lambda x: ':%s:' % x,[os.stat(x).st_mtime_ns for x in self.atomic_fns]))
+            res = hashlib.sha1(six.b(x)).hexdigest() + '///' + x
+            xprint("Hash of last modification time is %s" % res)
+            return res
 
     def open_file(self):
         if self.external_f:
@@ -1699,7 +1706,8 @@ class MaterializedDelimitedFileState(MaterializedState):
     def _store_qsql(self, source_sqlite_db, disk_db_filename):
         xprint("Storing data as disk db")
         disk_db_conn = sqlite3.connect(disk_db_filename)
-        sqlitebck.copy(source_sqlite_db.conn,disk_db_conn)
+        with disk_db_conn:
+            source_sqlite_db.conn.backup(disk_db_conn)
         xprint("Written db to disk: disk db filename %s" % (disk_db_filename))
         disk_db_conn.close()
 
diff --git a/pyoxidizer.bzl b/pyoxidizer.bzl
index 88ce10a..7727033 100644
--- a/pyoxidizer.bzl
+++ b/pyoxidizer.bzl
@@ -13,7 +13,7 @@ def make_exe():
     policy.set_resource_handling_mode("classify")
     policy.resources_location = "in-memory"
     policy.resources_location_fallback = "filesystem-relative:Lib"
-    policy.allow_in_memory_shared_library_loading = True
+    policy.allow_in_memory_shared_library_loading = False
 
     python_config = dist.make_python_interpreter_config()
 
@@ -61,7 +61,7 @@ def make_msi(exe):
         # The name of your application.
         "q-text-as-data",
         # The version of your application.
-        "2.1.0",
+        "3.1.0",
         # The author/manufacturer of your application.
         "Harel Ben-Attia"
     )
diff --git a/requirements-win-x86_64.txt b/requirements-win-x86_64.txt
deleted file mode 100644
index 69d4961..0000000
--- a/requirements-win-x86_64.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-six==1.11.0
-flake8==3.6.0
-setuptools<45.0.0
-# Turns out it would not be possible to use apsw without a major change all around, so we'd need to be able to compile sqlitebck on Windows as well
diff --git a/requirements.txt b/requirements.txt
index 9661caf..3ad7d2b 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,3 @@
 six==1.11.0
 flake8==3.6.0
 setuptools<45.0.0
-sqlitebck
diff --git a/setup.py b/setup.py
index ead9127..c260e58 100644
--- a/setup.py
+++ b/setup.py
@@ -19,8 +19,7 @@ setup(
     long_description_content_type="text/markdown",
     author_email='harelba@gmail.com',
     install_requires=[
-        'six==1.11.0',
-        'sqlitebck'
+        'six==1.11.0'
     ],
     package_dir={"": "bin"},
     packages=setuptools.find_packages(where="bin"),
diff --git a/test-requirements.txt b/test-requirements.txt
index bb72fa5..7e995ba 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -1,4 +1,3 @@
 pytest==6.2.2
 flake8==3.6.0
 six==1.11.0
-sqlitebck
diff --git a/test/test_suite.py b/test/test_suite.py
index 4f6aea3..f722e8f 100755
--- a/test/test_suite.py
+++ b/test/test_suite.py
@@ -6,9 +6,10 @@
 # Prefer end-to-end tests, running the actual q command and testing stdout/stderr, and the return code.
 # Some utilities are provided for making that easy, see other tests for examples.
 #
-# Don't forget to use the Q_EXECUTABLE instead of hardcoding the q command line. This will be used in the near future
-# in order to test the resulting binary executables as well, instead of just executing the q python source code.
+# Q_EXECUTABLE env var can be used to inject the path of q. This allows full e2e testing of the resulting executable
+# instead of just testing the python code.
 #
+# Tests are compatible with Linux and OSX (path separators, tmp folder, etc.).
 
 from __future__ import print_function