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

api: update generator for Tarantool v3 #4

Merged

Conversation

DerekBum
Copy link
Contributor

Updated generation of constants (and tests) for Tarantool v3. Unfortunately, new generator does not support Tarantool version < v3, due to some naming differences for enums.

Part of tarantool/go-tarantool#337

@DerekBum DerekBum requested a review from oleg-jukovec October 19, 2023 13:03
example_test.go Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
flag.go Outdated Show resolved Hide resolved
generate.sh Outdated Show resolved Hide resolved
keys.go Outdated Show resolved Hide resolved
type_test.go Show resolved Hide resolved
@oleg-jukovec
Copy link
Collaborator

As result, do we have incompatible changes?

@oleg-jukovec
Copy link
Collaborator

oleg-jukovec commented Oct 19, 2023

Unfortunately, new generator does not support Tarantool version < v3, due to some naming differences for enums.

It is expected:

The code generated from Tarantool code. Code generation is only supported for
an actual commit/release. We do not have a goal to support all versions of
Tarantool with a code generator.

@DerekBum DerekBum force-pushed the DerekBum/no-gh-update-generator-for-tarantool-v3 branch from 573120b to 407e2bb Compare October 21, 2023 01:12
@DerekBum
Copy link
Contributor Author

DerekBum commented Oct 21, 2023

As result, do we have incompatible changes?

Yes. For example, current iproto_raft_key enum was named iproto_raft_keys in the 2.11.0. Other than that, I think, there are no incompatible changes. But still the code wont work for 2.11.0 because some of the grep calls will fail (they fail in case nothing is matched). There is a way to fix this by passing something nice as a default for their result, I tried that with no result (but with not so much effort).

@DerekBum DerekBum force-pushed the DerekBum/no-gh-update-generator-for-tarantool-v3 branch 2 times, most recently from 317afae to c2bb250 Compare October 21, 2023 01:22
@oleg-jukovec
Copy link
Collaborator

oleg-jukovec commented Oct 21, 2023

As result, do we have incompatible changes?

Yes. For example, current iproto_raft_key enum was named iproto_raft_keys in the 2.11.0. Other than that, I think, there are no incompatible changes. But still the code wont work for 2.11.0 because some of the grep calls will fail (they fail in case nothing is matched).

We are only interested in the names of the constants and their values in the Go code. Were there any incompatible changes?

I noticed only that ER_UNUSED is deleted now. It doesn't seems like a problem (but should be noticed in the changelog).

generate.sh Outdated Show resolved Hide resolved
@DerekBum DerekBum force-pushed the DerekBum/no-gh-update-generator-for-tarantool-v3 branch 2 times, most recently from bc2f584 to b1974a0 Compare October 21, 2023 21:50
@DerekBum
Copy link
Contributor Author

DerekBum commented Oct 21, 2023

As result, do we have incompatible changes?

Yes. For example, current iproto_raft_key enum was named iproto_raft_keys in the 2.11.0. Other than that, I think, there are no incompatible changes. But still the code wont work for 2.11.0 because some of the grep calls will fail (they fail in case nothing is matched).

We are only interested in the names of the constants and their values in the Go code. Were there any incompatible changes?

I noticed only that ER_UNUSED is deleted now. It doesn't seems like a problem (but should be noticed in the changelog).

Oh, ok. There are no other incompatible changes, other than that. ER_UNUSED do not exist. Error code 248 belongs to the new ER_DEFAULT_VALUE_TYPE.

Changelog updated.

Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the patch! I would like to clarify the changelog a little more.

CHANGELOG.md Outdated Show resolved Hide resolved
@DerekBum DerekBum force-pushed the DerekBum/no-gh-update-generator-for-tarantool-v3 branch 2 times, most recently from 756308c to e8398c7 Compare October 22, 2023 09:35
@oleg-jukovec
Copy link
Collaborator

Oh, ok. There are no other incompatible changes, other than that. ER_UNUSED do not exist. Error code 248 belongs to the new ER_DEFAULT_VALUE_TYPE.

It sounds like good news, we don’t need to bump the major version, it will be enough to release 0.2.0. And we could release 1.0.0 after Tarantool 3.0.

@oleg-jukovec oleg-jukovec requested review from 0x501D and removed request for better0fdead October 23, 2023 10:27
grep -oP "[\t ]+[A-Z0-9_]+[\t ]+=[\t ]+[\-0-9xa-f <]+" | \
awk '{val=$1;$1=$2="";printf("\t\t{iproto.%s, \"%s\",%s},\n", val, val, $0)}'
grep -oP "[\t ]*[A-Z0-9_]+[\t ]+=[\t ]+[A-Z_]*[\-0-9xa-f <\+]+" | \
awk '{val=$1;$1=$2="";gsub(/[A-Z][A-Z0-9_]+/,"int(iproto.&)",$0);
Copy link
Member

@0x501D 0x501D Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why not using only awk match? e.g.:
awk '/match/ {code}'

Copy link
Contributor Author

@DerekBum DerekBum Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it can be done only by using awk. We need to leave only matched string (but not the whole line it is in). In the current code grep -o is doing that, and its output we pass to the awk.

For example (https://github.com/tarantool/tarantool/blob/ddaa5a320ee8d8d1009e07bbf6b8fe5cb4acbb45/src/box/iterator_type.h#L64C2-L64C71): from the string
ITER_EQ = 0, /* key == x ASC order */
we need to get ITER_EQ = 0 and refactor it to the {iproto.ITER_EQ, "ITER_EQ", 0},.

# arg1 - name of the define, containing the prefix.
# arg2 - path to a C code.
function read_prefix {
read_define $1 $2 | grep -oP "\) .+?#" | \
Copy link
Member

@0x501D 0x501D Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: all this can be done with one call to awk

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated (last sed from this function now merged with awk, but I left the grep to use only the matched strings, not the whole lines).

Updated generation of constants (and tests) for Tarantool `v3`.
Unfortunately, new generator does not support Tarantool
version < `v3`, due to some naming differences for enums.

Part of tarantool/go-tarantool#337
@DerekBum DerekBum force-pushed the DerekBum/no-gh-update-generator-for-tarantool-v3 branch from e8398c7 to d13758c Compare October 23, 2023 13:13
@oleg-jukovec oleg-jukovec merged commit cb78944 into master Oct 25, 2023
@oleg-jukovec oleg-jukovec deleted the DerekBum/no-gh-update-generator-for-tarantool-v3 branch October 25, 2023 10:31
@oleg-jukovec oleg-jukovec mentioned this pull request Dec 26, 2023
oleg-jukovec added a commit that referenced this pull request Dec 26, 2023
Overview

    The first stable release with IPROTO constansts exported from
    Tarantool 3.0.

Breaking changes

    `ER_UNUSED` with error code 248 renamed to
    `ER_DEFAULT_VALUE_TYPE` (#4).

New features

    Constants from Tarantool 3.0.0 (#4).
oleg-jukovec added a commit that referenced this pull request Dec 26, 2023
Overview

    The first stable release exports IPROTO constansts from
    Tarantool 3.0.

Breaking changes

    `ER_UNUSED` with error code 248 renamed to
    `ER_DEFAULT_VALUE_TYPE` (#4).

New features

    Constants from Tarantool 3.0.0 (#4).
oleg-jukovec added a commit that referenced this pull request Dec 26, 2023
Overview

    The first stable release exports IPROTO constants from
    Tarantool 3.0.0.

Breaking changes

    `ER_UNUSED` with error code 248 renamed to
    `ER_DEFAULT_VALUE_TYPE` (#4).

New features

    Constants from Tarantool 3.0.0 (#4).
oleg-jukovec added a commit that referenced this pull request Dec 27, 2023
Overview

    The first stable release exports IPROTO constants from
    Tarantool 3.0.0.

Breaking changes

    `ER_UNUSED` with error code 248 renamed to
    `ER_DEFAULT_VALUE_TYPE` (#4).

New features

    Constants from Tarantool 3.0.0 (#4).
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.

3 participants