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

Revert "ci macos: add workaround for msgpack-c" #2

Closed
wants to merge 29 commits into from

Conversation

otegami
Copy link
Owner

@otegami otegami commented Apr 26, 2024

The following error is raised when building the Groonga.
ref: https://github.com/otegami/groonga/actions/runs/9613079483/job/26515053568

 ../libtool: line 7756: cd: lib: No such file or directory
libtool:   error: cannot determine absolute directory name of 'lib'

The cause of this error

This error is raised because msgpack-c libraries cannot be found at /lib.
The msgpack-c libraries are located in /usr/local/lib although the pkg-config shows the library's location is /lib like the following.

   MessagePack:           yes
    CFLAGS:              -Iinclude
    LIBS:                -Llib -lmsgpack-c

ref: https://github.com/Homebrew/homebrew-core/blob/ef06c790bcf1b96731418f66dfacae6aab615b46/Formula/m/msgpack.rb#L26-L39

How to resolve this error

We have to reflect the right path to pkg-config files and locate the libraries there.
The following changes can resolve this issue in the above way.

I tried to use this change in my GitHub Actions and resolved this issue too.
Now the pkg-config shows the right path we expected at the GitHub Actions' configuration step.

   MessagePack:           yes
    CFLAGS:              -I/usr/local/include
    LIBS:                -L/usr/local/lib -lmsgpack-c

@otegami otegami force-pushed the remove-workaround-for-msgpack-c branch from 9becdba to bac2acc Compare May 20, 2024 04:36
@otegami otegami force-pushed the remove-workaround-for-msgpack-c branch from bac2acc to 2048af7 Compare June 3, 2024 12:51
@otegami otegami force-pushed the remove-workaround-for-msgpack-c branch from 2048af7 to 2107ab3 Compare June 13, 2024 12:46
@otegami otegami force-pushed the remove-workaround-for-msgpack-c branch 6 times, most recently from e92d523 to 42a3874 Compare June 21, 2024 13:35
otegami and others added 20 commits July 1, 2024 11:07
…s image (groonga#1807)

ref: groonga#1806

This change solves the following error on CI.
Because Docker build process does not automatically detect the correct
platform for the image. As a result, the platform information defaults
to the host platform. So in this case, platform infromation wasn't as a
arm64.

To resolve this issue, we need to specify the platform explicitly in the
Docker build command.
ref: https://docs.docker.com/build/building/multi-platform/

Here is the error log:


https://github.com/groonga/groonga/actions/runs/9705591558/job/26788067581#step:9:42

```
------
#3 [internal] load metadata for docker.io/arm64v8/almalinux:8
#3 ERROR: no match for platform in manifest: not found
------
Dockerfile:2
--------------------
   1 |     ARG FROM=almalinux:8
   2 | >>> FROM ${FROM}
   3 |
   4 |     ARG DEBUG
--------------------
ERROR: failed to solve: arm64v8/almalinux:8: failed to resolve source metadata for docker.io/arm64v8/almalinux:8: no match for platform in manifest: not found
rake aborted!#1 [internal] load build definition from Dockerfile
```

---------

Co-authored-by: Horimoto Yasuhiro <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
It should be defines at close to grn_hook_entry.
Before:
1. Remove paths of the target object
2. Remove metadata of the target object

After:
1. Remove metadata of the target object
2. Remove paths of the target object

The before order may cause "there is metadata but path doesn't exist".
delete_source_hook() is only used when the target object is
removed. So we can simplify like this. If we want to use this when the
target object isn't removed, we may want to change this
implementation or create another one.
Before:
1. Save hooks of sources
2. Save sources of the index column

After:
1. Save sources of the index column
2. Save hooks of sources

If Groonga is crashed between 1. and 2. with the before order, there
are hooks of sources but index column doesn't have sources. In WAL
recovery process, index column will be regenerated by removing the
index column and create a new index column. In the "removing the index
column" process, hooks of sources aren't removed. Because the removing
process finds hooks of sources by sources of the index column. If
hooks of sources are remained, they break something because they are
dangling references.

If we use the after order, dangling hooks of sources aren't remained.
This reverts commit 264be86.

If there are path but aren't metadata, we can't remove the
remained path.
We needs the following steps on removing an object:
1. Remove paths for the object
2. Remove metadata for the object

If Groonga is crashed between 1. and 2., WAL recovery process can't
determine whether the object should be recovered or removed.

This creates "#{DB}.#{id}.removing" path before 1. and removes it
after 2. If there is the path, WAL recovery process removes the object
and the path instead of trying recovering the object.
komainu8 and others added 9 commits July 3, 2024 13:08
GitHub: fix groongaGH-1810

"grn_fopen()" is defined as "_fsopen()" in Windows.

We can specify "shflag" in the third argument of "_fsopen()". We use
"shflg" in "_fsopen()".

We need to include "share.h" to use "shflag".
See:
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fsopen-wfsopen?view=msvc-160

However, db.c does not include "share.h".
So, "error C2065: '_SH_DENYNO': undeclared identifier" occur.

This problem only occur in Groonga for Windows.
Because "fopen()" does not have "shflag".
("grn_fopen()" is defined "fopen()" in Linux.)
GitHub: fix mroonga/mroonga#732

If we don't add ".", we may remove too much objects. For example:

Tables:
* table1
* table1.column
* table12

remove_columns_raw("table1") will remove table1.column and table12.
This reverts commit 8275463.
This problem is fixed by the following commit.
- ref: msgpack/msgpack-c@68cc50a
@otegami otegami force-pushed the remove-workaround-for-msgpack-c branch from b7f2473 to 92753bb Compare July 3, 2024 13:03
@otegami
Copy link
Owner Author

otegami commented Aug 25, 2024

It's already been resolved.

@otegami otegami closed this Aug 25, 2024
@otegami otegami deleted the remove-workaround-for-msgpack-c branch August 25, 2024 12:14
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