Skip to content

Commit

Permalink
Remove sqlitebck dependency + windows is working, but not in runner +…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
harelba authored Oct 23, 2021
1 parent 5be7815 commit 2c5c6e5
Show file tree
Hide file tree
Showing 11 changed files with 65 additions and 61 deletions.
71 changes: 36 additions & 35 deletions .github/workflows/build-and-package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -397,23 +397,36 @@ jobs:
- name: Upload Linux Executable
uses: actions/[email protected]
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
Expand All @@ -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/[email protected]
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
Expand All @@ -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
Expand All @@ -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' }}
Expand All @@ -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
Expand All @@ -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/[email protected]"
with:
repo_token: "${{ secrets.GITHUB_TOKEN }}"
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/q.rb.brew-formula-template
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions QSQL-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).


4 changes: 2 additions & 2 deletions README.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
22 changes: 15 additions & 7 deletions bin/q.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
import six
import io
import json
import sqlitebck
import datetime
import hashlib

Expand All @@ -67,6 +66,7 @@
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):
Expand All @@ -76,12 +76,17 @@ def iprint(*args,**kwargs):
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
Expand Down Expand Up @@ -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:
Expand All @@ -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))
Expand Down Expand Up @@ -1374,8 +1379,10 @@ def get_last_modification_time_hash(self):
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:
Expand Down Expand Up @@ -1699,7 +1706,8 @@ def save_cache_to_disk_if_needed(self, disk_db_filename, table_creator):
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()

Expand Down
4 changes: 2 additions & 2 deletions pyoxidizer.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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"
)
Expand Down
4 changes: 0 additions & 4 deletions requirements-win-x86_64.txt

This file was deleted.

1 change: 0 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
six==1.11.0
flake8==3.6.0
setuptools<45.0.0
sqlitebck
3 changes: 1 addition & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
long_description_content_type="text/markdown",
author_email='[email protected]',
install_requires=[
'six==1.11.0',
'sqlitebck'
'six==1.11.0'
],
package_dir={"": "bin"},
packages=setuptools.find_packages(where="bin"),
Expand Down
1 change: 0 additions & 1 deletion test-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
pytest==6.2.2
flake8==3.6.0
six==1.11.0
sqlitebck
5 changes: 3 additions & 2 deletions test/test_suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 2c5c6e5

Please sign in to comment.