WIP: CHG: Use pycparser as a git submodule

[AlexLao512]

WIP: Needs more testing.

Use pycparser as submodule.

[JulianKemmerer]

IIUC this does not pin a version of pycparser - and would, as coded now, have people doing git init recursive clone stuff and would pull the latest pycparser code @Datavenia does this work with the direction/mechanism you were thinking for pinning versions?

[ghost]

versions?

We could add a pinned version to the pyproject.toml, this means people would need to use poetry as a user but it's probably the most extensible system.

[AlexLao512]

IIUC this does not pin a version of pycparser - and would, as coded now, have people doing git init recursive clone stuff and would pull the latest pycparser code @Datavenia does this work with the direction/mechanism you were thinking for pinning versions?

It actually does pin, there is a .gitmodules file with the URL to the repository with a release tag (version).

[AlexLao512]

Sorry, some mistakes above, the commit on the remote repo is pinned here. If you click through to the commit you will see the release tag.

If is in the submodule itself via commit hash and not in the .gitmodules branch = because the remote repo has a TAG on master, instead of a release branch and tag.

[AlexLao512]

versions?

We could add a pinned version to the pyproject.toml, this means people would need to use poetry as a user but it's probably the most extensible system.

I suggest we use a setup.py so pip can be used. pip is installed along with the users Python install, no additional pieces needed on the users end.

[JulianKemmerer]

It sounds like doing like this PR doing, and using git submodules to pull in pycparser at a certain commit point ~essentially having a copy of the repo inside pipelinec repo like now but the support git way - makes sense to do. I of course want to test it but sounds simple and easy.

And then after, in a separate PR/issue perhaps, is the question of 'proper' dependency/package management for python projects? And IIUC it has to be either or pip or poetry based management? Idk if it can be both easily?

But I am thinking this PR can go through sooner than deciding on / setting up pip/poetry with pycparser dependency etc

[AlexLao512]

Yea so this does half the cleanup required for moving to proper Python package management, the submodule part is trivial to remove later. If you want to test, I think this can be merged.

[JulianKemmerer]

Yeah testing I want to do is basic - does it still seem to parse c code and not crash kinda tests

Should have some time this week

[JulianKemmerer]

Wow I just realized you got the pycparser 2.18 tagged commit version that should exactly match what I had copied into the repo - tests should go smoothly :+1:

[JulianKemmerer]

IIUC

New repo clone will need to be git clone --recurse-submodules

And users pulling to update their repo need to one time update+init submodules

git pull
git submodule update --init --recursive

[JulianKemmerer]

@AlexLao512 can you change the temp work around locations in code to use the absolute path to the repo?

from utilities import REPO_ABS_DIR

# TODO: Temporarily import from submodule, remove this hack when we create a proper pipelinec setup.py
sys.path.append(REPO_ABS_DIR()+'/submodule/pycparser')

Otherwise depends on being in ./src/ as is on branch

---

If I wanted to make/suggest this change I would need to clone your fork of my repo w/ the branch - and then make another branch to act as a PR into your branch? :grimacing:

[JulianKemmerer]

Oh dope - I didnt know I could just commit and push to a repo owned by not my user (I guess forking my repo gives me access by default idk?)

So I think I can continue to make the changes need to the PR for full merge :+1: hopefully sooner than later...

[JulianKemmerer]

Something fucky is up

Trying the branch I get a C parsing related ~'nodes in a dictionary arent as expected' internal pipelinec sanity check error (tried examples/blink.c)

Weird...

C_AST_VAL_UNIQUE_KEY_DICT_MERGE Dicts aren't unique: {'BIN_OP_EQ[blink_c_l17_c6_e220]': <pycparser.c_ast.BinaryOp object at 0x7f341b2f3310>, 'TRUE_CLOCK_ENABLE_MUX[blink_c_l18_c1_af86]': <pycparser.c_ast.Compound object at 0x7f341b2e8180>, 'FALSE_CLOCK_ENABLE_MUX[blink_c_l25_c1_5b0e]': <pycparser.c_ast.Compound object at 0x7f341b2e8040>, 'UNARY_OP_NOT[blink_c_l20_c12_cefe]': <pycparser.c_ast.UnaryOp object at 0x7f341b4baa00>} {'BIN_OP_EQ[blink_c_l17_c6_e220]': <pycparser.c_ast.BinaryOp object at 0x7f341b2f3220>, 'TRUE_CLOCK_ENABLE_MUX[blink_c_l18_c1_af86]': <pycparser.c_ast.Compound object at 0x7f341b1cc7c0>, 'FALSE_CLOCK_ENABLE_MUX[blink_c_l25_c1_5b0e]': <pycparser.c_ast.Compound object at 0x7f341b1ccc40>, 'BIN_OP_PLUS[blink_c_l26_c5_c986]': <pycparser.c_ast.BinaryOp object at 0x7f341b2f3630>}
key BIN_OP_EQ[blink_c_l17_c6_e220]
v1 <pycparser.c_ast.BinaryOp object at 0x7f341b2f3310> blink_c_l17_c6_5b93
v2 <pycparser.c_ast.BinaryOp object at 0x7f341b2f3220> blink_c_l17_c6_72f9
Traceback (most recent call last):
  File "./src/pipelinec", line 109, in <module>
    parser_state = C_TO_LOGIC.PARSE_FILE(c_file)
  File "/home/julian/Desktop/AlexLao512/PipelineC/src/C_TO_LOGIC.py", line 9723, in PARSE_FILE
    parser_state.FuncLogicLookupTable = APPEND_FUNC_NAME_LOGIC_LOOKUP_TABLE(
  File "/home/julian/Desktop/AlexLao512/PipelineC/src/C_TO_LOGIC.py", line 10967, in APPEND_FUNC_NAME_LOGIC_LOOKUP_TABLE
    logic = C_AST_FUNC_DEF_TO_LOGIC(
  File "/home/julian/Desktop/AlexLao512/PipelineC/src/C_TO_LOGIC.py", line 8807, in C_AST_FUNC_DEF_TO_LOGIC
    body_logic = C_AST_NODE_TO_LOGIC(
  File "/home/julian/Desktop/AlexLao512/PipelineC/src/C_TO_LOGIC.py", line 1901, in C_AST_NODE_TO_LOGIC
    return C_AST_COMPOUND_TO_LOGIC(c_ast_node, prepend_text, parser_state)
  File "/home/julian/Desktop/AlexLao512/PipelineC/src/C_TO_LOGIC.py", line 5188, in C_AST_COMPOUND_TO_LOGIC
    item_logic = C_AST_NODE_TO_LOGIC(
  File "/home/julian/Desktop/AlexLao512/PipelineC/src/C_TO_LOGIC.py", line 1905, in C_AST_NODE_TO_LOGIC
    return C_AST_IF_TO_LOGIC(c_ast_node, prepend_text, parser_state)
  File "/home/julian/Desktop/AlexLao512/PipelineC/src/C_TO_LOGIC.py", line 5997, in C_AST_IF_TO_LOGIC
    true_logic.MERGE_COMB_LOGIC(false_logic)
  File "/home/julian/Desktop/AlexLao512/PipelineC/src/C_TO_LOGIC.py", line 762, in MERGE_COMB_LOGIC
    self.submodule_instance_to_c_ast_node = C_AST_VAL_UNIQUE_KEY_DICT_MERGE(
  File "/home/julian/Desktop/AlexLao512/PipelineC/src/C_TO_LOGIC.py", line 315, in C_AST_VAL_UNIQUE_KEY_DICT_MERGE

[AlexLao512]

Might be a good idea to diff the version you have in the repo and the tagged releases of pycparser.

[JulianKemmerer]

Hopefully I can get around to this in the next week or so :+1: , starting with the diff

[JulianKemmerer]

OK I think I know whats going on.

The version of the files I copied into my repo in ~2017/2018 some time was marked at version 2.18. And that is what was used for the submodule.

However, the files are marked as version 2.18 all the way through to 2.19. Meaning, I likely had copied the files after 2.18 release, including any differences accumulated since but before 2.19.

Anywhoo if you look here https://github.com/eliben/pycparser/commits/caa4c11ebb99ed5cf854dc6342b5352d5ff52686

The commit right before 2.19 is https://github.com/eliben/pycparser/commit/bcb5f9f3ae9d62df3f2b7ee058cebfee91be8791

I checked out that commit on top of this PR and it seems to be working fine in early tests...

[JulianKemmerer]

Perhaps it would be sane to try a version of this PR using exactly version 2.19 which differs by only the version bump commit :sweat_smile:

[JulianKemmerer]

Now thats the submodule is up to 2.19 again the last thing seems to be fixing the path append like

from utilities import REPO_ABS_DIR

# TODO: Temporarily import from submodule, remove this hack when we create a proper pipelinec setup.py
sys.path.append(REPO_ABS_DIR() + '/submodule/pycparser')

[JulianKemmerer]

Looking great so far, a test build running...

But I pushed another minor fix to the pipelinec main repo

How do I get that change onto this branch under test - you mentioned 'rebase' is what I want? - ill do some googling :smirk:

[AlexLao512]

Done. You don't actually have push access to my fork. I can pull changes from your forks master branch into my forks master branch, then I can rebase my forks branch onto my forks master branch (which is now in sync with yours).

[JulianKemmerer]

I have more testing to do than I anticipated

Using the new pycparser version I am getting designs with ~10% more LUTs

Trying to decide how to debug this or just accept the hit for now...