Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify purpose of fetching #64

Closed
rogerbinns opened this issue Dec 29, 2013 · 11 comments
Closed

Clarify purpose of fetching #64

rogerbinns opened this issue Dec 29, 2013 · 11 comments
Assignees

Comments

@rogerbinns
Copy link
Owner

From rogerbinns on August 14, 2009 09:43:01

See http://www.sqlite.org/cvstrac/tktview?tn=4026

Original issue: http://code.google.com/p/apsw/issues/detail?id=64

@ghost ghost assigned rogerbinns Dec 29, 2013
@rogerbinns
Copy link
Owner Author

From jamilan on August 18, 2009 00:48:43

Could be this approach better than adding the defines to sqlite.c?

In setup.py
{{{
@@ -259,17 +259,16 @@ if fetch:
if '=' in part:
part=part.split('=', 1)
else:
part=(part, )
defs.append(part)

  •    op=open("sqlite3-fixed.c", "wt")
    
  •    op=open("config.h", "wt")
     for define in defs:
         op.write('#define %s %s\n' % tuple(define))
    
  •    op.write(open('sqlite3.c', 'rt').read())
     op.close()
    
  •    os.rename("sqlite3-fixed.c", "sqlite3.c")
     os.chdir("..")
    
  •    define_macros.append( ('_HAVE_SQLITE_CONFIG_H', '1') )
    

    We depend on every .[ch] file in src

    depends=[f for f in glob.glob("src/*.[ch]") if f!="src/apsw.c"]
    }}}

@rogerbinns
Copy link
Owner Author

From jamilan on August 18, 2009 01:12:14

Actually this will also work if not fetching.

In setup.py
{{{
@@ -264,20 +264,21 @@ if fetch:
op=open("config.h", "wt")
for define in defs:
op.write('#define %s %s\n' % tuple(define))
op.close()
os.chdir("..")

  •    define_macros.append( ('_HAVE_SQLITE_CONFIG_H', '1') )
    

    We depend on every .[ch] file in src

    depends=[f for f in glob.glob("src/*.[ch]") if f!="src/apsw.c"]

    We don't want assertions

    if "--debug" not in sys.argv:
    define_macros.append( ('NDEBUG', '1') )

+if not sys.platform in ('win32', 'win64'):

  • define_macros.append( ('_HAVE_SQLITE_CONFIG_H', '1') )

Look for amalgamation in our directory or in sqlite3 subdirectory

amalgamation=(
os.path.join(os.path.dirname(os.path.abspath(file)), "sqlite3.c"),
os.path.join(os.path.dirname(os.path.abspath(file)), "sqlite3", "sqlite3.c")
}}}

@rogerbinns
Copy link
Owner Author

From rogerbinns on August 24, 2009 04:46:50

I don't really want to go down this path as it is creeping featuritis, but could be
convinced otherwise :-) The current approach is very simple and will always work,
with the sole exception being people who use it instead of fetching stuff themselves
as in the associated SQLite ticket.

I do intend to add a new setup command (ie the proper way of doing things) so that
you would say something like "python setup.py fetch --sqlite build install". (Fetch
could also fetch genfkey and asyncvfs). There is no communication between commands
and the user could run them separately anyway. I still need to do experiments with
subclassing the builtin ones (eg build, build_ext) to what the scope is for
automatically adding flags etc. Ultimately my goal will be for things to be simple
and deterministic.

@rogerbinns
Copy link
Owner Author

From jamilan on September 23, 2009 02:58:03

We just update to the lastest code and here is an update patch, a cleaner one I must
say:

{{{
diff -r f1b6b05c5d76 setup.py
--- a/setup.py Mon Sep 21 00:14:18 2009 -0700
+++ b/setup.py Wed Sep 23 11:42:27 2009 +0200
@@ -25,6 +25,9 @@

This includes the functionality marked as experimental in SQLite 3.

Comment out the line to exclude them

define_macros.append( ('EXPERIMENTAL', '1') )
+
+if not sys.platform in ('win32', 'win64'):

  • define_macros.append( ('_HAVE_SQLITE_CONFIG_H', '1') )

End of customizations

@@ -204,12 +207,10 @@
else:
part=(part, )
defs.append(part)

  •            op=open("sqlite3-fixed.c", "wt")
    
  •            op=open("config.h", "wt")
             for define in defs:
                 op.write('#define %s %s\n' % tuple(define))
    
  •            op.write(open('sqlite3.c', 'rt').read())
             op.close()
    
  •            os.rename("sqlite3-fixed.c", "sqlite3.c")
             os.chdir("..")
         downloaded+=1
    

}}}

We are using the same procedure than the EXPERIMENTAL flag, and it does not modify
the sqlite3.c file. It may not be a perfect solution, but allow us to build the same
codebase both in windows and linux, no mather on which one we update apsw. ;)

Can we do something else to convince you?

@rogerbinns
Copy link
Owner Author

From rogerbinns on September 23, 2009 14:18:57

Doing what you want is relatively trivial from a code point of view. (I'd take a
slightly different approach to the .h filename to avoid issues.)

But what isn't clear to me is why you insist on using the APSW fetch functionality if
you want to store SQLite in your repository. Why aren't you fetching it yourself?
Even better you shouldn't be fetching the amalgamation but instead the source since
that will track history and allows you to build SQLite for any OMIT flags you want to
use.

I am mostly convinced to not modify the .c file for one reason - it makes it harder
to report SQLite issues due to line number mismatches. But that pretty much only
affects me.

@rogerbinns
Copy link
Owner Author

From jamilan on September 24, 2009 03:19:12

Our workflow for apsw/sqlite integrations is the following:

We update a priscine copy of apsw from mercurial and run the next command on it:
{{{
python setup.py fetch --all build --enable-all-extensions
}}}

The current configuration of apsw and sqlite works perfectly for us, and
since the sqlite team recommends using the amalgamation instead of the
source code, we are happy with that.

We do not need to track every change that happened on sqlite or apsw since
their respectives repositories have them for us. (In case we need to review them in
order to contribute back.)

On our repository (we are using git) we need a stable and operative apsw´s version,
so
after building, we run our tests and we double check if something is broken, fix it,
integrate new
functionality and remove old one, and then commit everything as a whole. (I do not
like bothering co-workers) ;)

When they update their sources, all they have to do is:
{{{
python setup.py build --enable-all-extensions
}}}

We know the proposed change may not fit everybody, but we found it easy to implement
and
work just fine on all the platforms we need. (It even works on WinCE out of the box)

But as always it is only a proposal, and we do not want to force anyone to use it. ;)

@rogerbinns
Copy link
Owner Author

From rogerbinns on September 24, 2009 03:29:55

You didn't answer my question about why you insist on using APSW's fetch
functionality when you could quite easily do the fetching yourself in any manner you
wish with absolute control. (BTW I will be modifying the fetch functionality to
error out if there is no checksum for a file overrideable with a flag. See also issue #70 .)

There is a problem of where to put the various HAVE_* definitions since Makefiles
aren't used. What I am most likely going to do is stop modifying sqlite3.c and
instead have a sqlite3config.h alongside it that is automagically included if present
(I don't like "config.h" since I can't be absolutely sure of the include path after
distutils has mucked around and there are many files with that name on my machine.)

Note that you could supply the HAVE_* defs to the setup.py build_ext command either
on the command line or by futzing with setup.cfg, but those approaches aren't
particularly fun and are prone to error.

@rogerbinns
Copy link
Owner Author

From jamilan on September 24, 2009 04:44:04

Well, the fetch functionality does the same as a manual download and it is automatic,
leaves the sources where they have to be, and we do not have to worry about doing it
as another step. It is one of the functionalities off apsw setup.py we really like
and abuse. :)

We do not care about the version of sqlite in use, since we always use it through
apsw,
so if the new fetch functionality grab the right sqlite´s version, it is ok for us.

We even compile our sqlite extensions over the sqlite fetch done by apsw and
everything work as expected.

The sqlite3config.h solution sound good, I will test it as soon as you place it on
mercurial.

@rogerbinns
Copy link
Owner Author

From rogerbinns on September 25, 2009 02:52:26

It is now committed to Mercurial as revision 12dc7e1efe The documentation (look in doc/build.rst) still says that setup.py modifies the files
and I am leaving that warning in there since that is the case for the async extension
and may again become true of the amalgamation.

Let me know if things work for you now so I can close this.

Status: Fixed

@rogerbinns
Copy link
Owner Author

From jamilan on September 28, 2009 02:15:21

Works for us, thanks!!

@rogerbinns
Copy link
Owner Author

From rogerbinns on September 28, 2009 02:17:24

Verified fixed by jamilan

Status: Verified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant