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

Remove const char* elements from cdef #126

Merged
merged 1 commit into from
Mar 27, 2019
Merged

Conversation

flacjacket
Copy link
Contributor

@flacjacket flacjacket commented Mar 24, 2019

When building the ffi module, there is a message printed noting that
string literals are ignored when building. I'm not sure if it is
possible to actually get at these value via the generated ffi module,
trying cairocffi.ffi.CAIRO_TAG_DEST or cairocffi.ffi.CAIRO_TAG_LINK
returns an error. These constants don't seem to be used internally, and
are accessible via the exported value as cairocffi.constants.TAG_DEST
or cairocffi.constants.TAG_LINK.

@flacjacket flacjacket changed the title Remove const char elements from cdef Remove const char* elements from cdef Mar 24, 2019
@liZe
Copy link
Member

liZe commented Mar 26, 2019

Hello, and thank you for this pull request.

These constants don't seem to be used internally, and
are accessible via the exported value as cairocffi.constants.TAG_DEST
or cairocffi.constants.TAG_LINK.

It's true. But the goal of mkconstants.py is to generate the whole constants.py file. If we remove CAIRO_TAG_DEST and CAIRO_TAG_LINK definitions from the headers, we remove TAG_DEST and TAG_LINK from the constants.py file generated by mkconstants.py.

In other words: if we merge this PR and call mkconstants.py, we remove TAG_DEST and TAG_LINK constants. That's not what we want 😄.

When building the ffi module, there is a message printed noting that
string literals are ignored when building.

I don't get this message. Do you get it when calling ffi_build.py or when installing with pip (or both)?

@flacjacket
Copy link
Contributor Author

When I run python cairocffi/ffi_build.py, I see:
/usr/lib/python3.7/site-packages/cffi/cparser.py:150: UserWarning: String literal found in cdef() or type source. String literals are ignored here, but you should remove them anyway because some character sequences confuse pre-parsing.
This is why I'm proposing to change these.

I get that is the goal of mkconstants, however I was not able to actually access these values via the generated ffi:

In [1]: import cairocffi

In [2]: cairocffi.cairo.CAIRO_ANTIALIAS_BEST
Out[2]: 6

In [3]: cairocffi.cairo.CAIRO_TAG_DEST
---------------------------------------------------------------------------
error                                     Traceback (most recent call last)
<ipython-input-3-cdc3caa45c13> in <module>
----> 1 cairocffi.cairo.CAIRO_TAG_DEST

error: symbol 'CAIRO_TAG_DEST' not found in library 'libcairo.so.2': /usr/lib64/libcairo.so.2: undefined symbol: CAIRO_TAG_DEST

In [4]: cairocffi.cairo.CAIRO_TAG_LINK
---------------------------------------------------------------------------
error                                     Traceback (most recent call last)
<ipython-input-4-431cc28517d3> in <module>
----> 1 cairocffi.cairo.CAIRO_TAG_LINK

error: symbol 'CAIRO_TAG_LINK' not found in library 'libcairo.so.2': /usr/lib64/libcairo.so.2: undefined symbol: CAIRO_TAG_LINK

The mkconstants script just manually puts these constants here, they are not actually parsed out of the header file, so I did play a bit with what is given in the cdef, and I was not able to come up with something valid to put in its place that would make these symbols accessible. My note about them not being used internally is just to note that there is no clear way to access them through the generated ffi via the current API.

This does not change the fact that these values are still generated up at the top of constants.py, on lines 4-6, should somebody want to make use of these constants:

TAG_DEST = b"cairo.dest"

TAG_LINK = b"Link"

so we are not totally removing these constants, we are just removing the instances of them that are inaccessible from the ffi and are generating cffi warnings.

@liZe
Copy link
Member

liZe commented Mar 26, 2019

When I run python cairocffi/ffi_build.py, I see:
/usr/lib/python3.7/site-packages/cffi/cparser.py:150: UserWarning: String literal found in cdef() or type source. String literals are ignored here, but you should remove them anyway because some character sequences confuse pre-parsing.

Oh. It's strange, I didn't get the error before, but I get it now.

The mkconstants script just manually puts these constants here, they are not actually parsed out of the header file

It's not fully true. The current code in master doesn't read them from cairo's headers, but it adds them to the source where other headers are. The constants are then generated from the source variable parsed by pycparser, just as other variables. The work is done by the visit_Decl method in PrintEnumsVisitor.

I get that is the goal of mkconstants, however I was not able to actually access these values via the generated ffi

It's true, it's a problem. As CAIRO_TAG_DEST is defined as a macro, we should should handle them as we handle enums (type used for other constants). Cffi doesn't handle macros and there's probably no way to make them available from cairocffi.cairo.

This does not change the fact that these values are still generated up at the top of constants.py, on lines 4-6

They're not anymore if I use your branch (or did I miss something?).

should somebody want to make use of these constants

Actually, they at least are used in WeasyPrint and work well.

When building the ffi module, there is a message printed noting that
string literals are ignored when building.  I'm not sure if it is
possible to actually get at these value via the generated ffi module,
trying `cairocffi.ffi.CAIRO_TAG_DEST` or cairocffi.ffi.CAIRO_TAG_LINK`
returns an error.  These constants don't seem to be used internally, and
are accessible via the exported value as `cairocffi.constants.TAG_DEST`
or `cairocffi.constants.TAG_LINK`.
@flacjacket
Copy link
Contributor Author

Ah, I see, I was thinking the lines that I was deleting was directly creating the constants.py file. I didn't realize that was the source that was being visited by the printing code. I have tweaked it so that we emit the constants into the top of the constants.py file, but that these constants do not wind up in the cdef'd source. I have also regenerated the constants.py off of the latest cairo release.

@liZe
Copy link
Member

liZe commented Mar 27, 2019

I have tweaked it so that we emit the constants into the top of the constants.py file, but that these constants do not wind up in the cdef'd source.

Thank you! It's probably the best way to do that. I'll try to see what we can do with CAIRO_PDF_OUTLINE_ROOT too.

I have also regenerated the constants.py off of the latest cairo release.

OK.

@liZe liZe merged commit e06187d into Kozea:master Mar 27, 2019
netbsd-srcmastr referenced this pull request in NetBSD/pkgsrc Apr 30, 2024
Version 1.7.0
.............

Released on 2024-04-27

* Drop Python 3.7 support, add Python 3.12 support
* `#221 <https://github.com/Kozea/cairocffi/pull/225>`_:
  Add environment variable to set folder where DLLs are installed on Windows
* `#225 <https://github.com/Kozea/cairocffi/pull/225>`_:
  Use Ruff instead of Flake8 and isort


Version 1.6.1
.............

Released on 2023-07-24

* `#217 <https://github.com/Kozea/cairocffi/issues/217>`_:
  Repair installation with PyInstaller


Version 1.6.0
.............

Released on 2023-06-12

**This version uses a new CFFI mode that may break your program.**

CairoCFFI now uses Flit for packaging and is also distributed as a Python
wheel.

Please test carefully and don’t hesitate to report issues before using it in
production.

* `#216 <https://github.com/Kozea/cairocffi/pull/216>`_:
  Use ABI-level in-line CFFI mode


Version 1.5.1
.............

Released on 2023-04-15

* `#212 <https://github.com/Kozea/cairocffi/issues/212>`_:
  Bring back XCB support during wheel generation


Version 1.5.0
.............

Released on 2023-03-17

* `#106 <https://github.com/Kozea/cairocffi/issues/106>`_,
  `#200 <https://github.com/Kozea/cairocffi/issues/200>`_:
  Fallback to manual PNG file creation on hardened systems
* `#210 <https://github.com/Kozea/cairocffi/pull/210>`_:
  Use pyproject.toml for packaging and remove other useless files


Version 1.4.0
.............

Released on 2022-09-23

* `#205 <https://github.com/Kozea/cairocffi/pull/205>`_:
  Use pikepdf to parse generated PDF
* `#171 <https://github.com/Kozea/cairocffi/pull/171>`_:
  Don’t use deprecated pytest-runner anymore


Version 1.3.0
.............

Released on 2021-10-04

* `2cd512d <https://github.com/Kozea/cairocffi/commit/2cd512d>`_:
  Drop Python 3.6 support
* `#196 <https://github.com/Kozea/cairocffi/pull/196>`_:
  Fix import `constants.py` import
* `#169 <https://github.com/Kozea/cairocffi/pull/169>`_:
  Add extra library name "cairo-2.dll"
* `#178 <https://github.com/Kozea/cairocffi/pull/178>`_:
  Workaround for testing date string with cairo 1.17.4
* `#186 <https://github.com/Kozea/cairocffi/pull/186>`_:
  Fix link in documentation
* `#195 <https://github.com/Kozea/cairocffi/pull/195>`_:
  Fix typo in documentation
* `#184 <https://github.com/Kozea/cairocffi/pull/184>`_,
  `a4fc2a7 <https://github.com/Kozea/cairocffi/commit/a4fc2a7>`_:
  Clean .gitignore


Version 1.2.0
.............

Released on 2020-10-29

* `#152 <https://github.com/Kozea/cairocffi/pull/152>`_:
  Add NumPy support
* `#143 <https://github.com/Kozea/cairocffi/issues/143>`_:
  Make write_to_png function work on hardened systems
* `#156 <https://github.com/Kozea/cairocffi/pull/156>`_:
  Use major version name to open shared libraries
* `#165 <https://github.com/Kozea/cairocffi/pull/165>`_:
  Don’t list setuptools as required for installation


Version 1.1.0
.............

Released on 2019-09-05

* `#135 <https://github.com/Kozea/cairocffi/pull/135>`_,
  `#127 <https://github.com/Kozea/cairocffi/pull/127>`_,
  `#119 <https://github.com/Kozea/cairocffi/pull/119>`_:
  Clean the way external libraries are found
* `#126 <https://github.com/Kozea/cairocffi/pull/126>`_:
  Remove const char* elements from cdef
* Support Cairo features up to 1.17.2
* Fix documentation generation


Version 1.0.2
.............

Released on 2019-02-15

* `#123 <https://github.com/Kozea/cairocffi/issues/123>`_:
  Rely on a recent version of setuptools to handle VERSION


Version 1.0.1
.............

Released on 2019-02-12

* `#120 <https://github.com/Kozea/cairocffi/issues/120>`_:
  Don't delete _generated modules on ffi_build import


Version 1.0.0
.............

Released on 2019-02-08

6 years after its first release, cairocffi can now be considered as stable.

* Drop Python 2.6, 2.7 and 3.4 support
* Test with Python 3.7
* Clean code, tests and packaging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants