Skip to content

Add init.lua to rockspec #46

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

Closed
wants to merge 1 commit into from
Closed

Add init.lua to rockspec #46

wants to merge 1 commit into from

Conversation

no1seman
Copy link

@no1seman no1seman commented Feb 1, 2022

Add lost init.lua to rockspec

@no1seman no1seman requested a review from Totktonada March 31, 2022 13:14
@Totktonada
Copy link
Member

Thank for pinging me. I'm aware of this pull request, but in fact we should reimplement the solution of the graphql.VERSION constant generation (see #29 and PR #41).

  1. tarantoolctl rocks pack checked out sources from the git tag. So the modified init.lua does not land into the .src.rock file.
  2. It does not work by construction for installing from the rockspec (without .src.rock).

The .src.rock for the 0.1.3 release was repacked manually (unzip, fix, zip).

I see the only solution: move from the builtin build type to make and call git describe --long --always inside the build phase.

@no1seman
Copy link
Author

no1seman commented Apr 2, 2022

@Totktonada Let's move to Cmake buildtype and pack all.rock instead src.rock?

@no1seman
Copy link
Author

no1seman commented Apr 2, 2022

closing this PR in favour of #50

@no1seman no1seman closed this Apr 2, 2022
@Totktonada Totktonada deleted the add_init branch April 2, 2022 08:33
Totktonada added a commit that referenced this pull request Apr 8, 2022
This reverts commit 0caab75.

I'm going to reimplement it using 'make' build type in the rockspec.

See [1] for details.

[1]: #46 (comment)

Follows up #29
Totktonada added a commit that referenced this pull request Apr 8, 2022
XXX: luarocks removes `.git` directory after cloning a repository. So
actually this approach only works when we're within our cloned
repository. So, questions:

1. We need to reprase comments acoordingly at least.
2. Whether it works with tarantoolctl rocks install in CI?

Now it is possible to obtain a version of the module using the
following code:

```lua
local graphql = require('graphql')
print(graphql.VERSION)
```

It does not work for cloned repository (without calling `tarantoolctl
rocks make`) -- the `graphql.VERSION` field will return `'unknown'`, but
it works for a user scenario: when the module is installed either from
.all.rock file or from the rockspec.

The short list of changes is the following.

- Upload all.rock instead of .src.rock for releases. I don't find a way
  to add a generated file into the .src.rock tarball using luarocks. It
  is not necessary, though: it is completely okay (and even preferable)
  to ship .all.rock for a pure Lua module.
- Upload scm-1 rockspec from CI.
- Moved from the `'builtin'` build type to `'make'` in the rockspec to
  generate the `graphql/VERSION.lua` file at the build stage.
- Added Make targets for luarocks with `graphql/VERSION.lua` file
  generation.
- Added `graphql/init.lua` file with `VERSION` field, which is filled
  from the generated `graphql/VERSION.lua` file if it is present.
  Otherwise it is filled with the `'unknown'` string.

And a couple of minor tweaks:

- Replaced `tarantoolctl rocks new_version --tag $TAG` with `sed` in the
  rockspec publication workflow, because the former strips comments and
  reformats the file. It is a thing of taste: personally I prefer to be
  able to diff rockspecs on the rock server in case of an incident.
- Use `ubuntu-latest` in rockspec publication workflow, because we have
  nothing Ubuntu version specific here. This way it requires less
  maintenance.

The publication workflow now almost the same as in expirationd, see [1].
As for me, it is good to have such code standartized.

This is reimplementation of the original #29 fix from PR #41. The key
points are the following.

- Usage the `'make'` build type allows to lower the logic down to
  luarocks level, so it works not only from CI.
- Complete the implementation by packing into .all.rock instead of
  .src.rock. Otherwise it does not work at all, see [2] for details.
- Generate the gitignored `graphql/VERSION.lua` file (don't touch
  `graphql/init.lua`) to don't leave the repository clone in a dirty
  state.

Also it is the alternative to PR #50, where the `'cmake'` build type was
proposed for the same purposes. The differences are the following.

- Use Make instead of CMake for the same of simplicity.
- Don't require tarantool-dev/devel package for building the rock or
  installing the package from the rockspec.

[1]: tarantool/expirationd#55
[2]: #46 (comment)

Follows up #29
Supersedes PR #50
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