ci: go back to stricter checks #1012

maelle · 2023-12-18T11:15:57Z

Now I understand why everything was green on CI!

aviator-app · 2023-12-18T11:16:01Z

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.

See the real-time status of this PR on the Aviator webapp.

Use the Aviator Chrome Extension to see the status of your PR within GitHub.

krlmlr · 2023-12-18T12:27:51Z

What's there for me to review? Should we fix first?

maelle · 2023-12-18T12:38:12Z

I think it'd make sense to merge then rebase all PRs that work towards getting the build green again?

aviator-app · 2024-01-11T19:41:04Z

This pull request failed to merge: some CI status(es) failed. Remove the blocked label to re-queue.

Failed CI(s): Check ubuntu-20.04 (devel), Check ubuntu-20.04 (oldrel-2)

aviator-app · 2024-01-11T21:42:01Z

This pull request failed to merge: some CI status(es) failed. Remove the blocked label to re-queue.

Failed CI(s): Check ubuntu-20.04 (oldrel-2)

aviator-app · 2024-01-11T21:59:37Z

This pull request failed to merge: some CI status(es) failed. Remove the blocked label to re-queue.

Failed CI(s): Check windows-latest (release), Check ubuntu-20.04 (oldrel-3), Check ubuntu-20.04 (release), Check macOS-latest (release), Check ubuntu-20.04 (oldrel-1), Check ubuntu-20.04 (devel), Check ubuntu-20.04 (oldrel-2)

krlmlr

Thanks! Should be good now.

aviator-app · 2024-01-11T22:40:42Z

This pull request failed to merge: some CI status(es) failed. Remove the blocked label to re-queue.

Failed CI(s): Check macOS-latest (release)

krlmlr · 2024-01-11T23:53:46Z

@Antonov548: We now have compilation warnings on a few platforms only, e.g., on macOS:

  rinterface.c:6349:84: warning: variable 'c_outfile' is uninitialized when used here [-Wuninitialized]
  rinterface.c:8381:58: warning: variable 'c_instream' is uninitialized when used here [-Wuninitialized]
  rinterface.c:8441:59: warning: variable 'c_outstream' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10886:33: warning: variable 'c_a' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10886:38: warning: variable 'c_b' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10886:43: warning: variable 'c_eps' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10910:31: warning: variable 'c_a' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10910:36: warning: variable 'c_b' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10910:41: warning: variable 'c_eps' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10917:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11610:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11632:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11646:28: warning: variable 'c_igraph_errno' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11653:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11761:31: warning: variable 'version_string' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11762:31: warning: variable 'major' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11763:31: warning: variable 'minor' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11764:31: warning: variable 'subminor' is uninitialized when used here [-Wuninitialized]

Is this dead code? We can remove in rinterface.c and reconcile later, we have drift anyway.

There are more failures on Windows.

Can you please push to this branch?

Antonov548 · 2024-01-12T15:06:32Z

@Antonov548: We now have compilation warnings on a few platforms only, e.g., on macOS:

  rinterface.c:6349:84: warning: variable 'c_outfile' is uninitialized when used here [-Wuninitialized]
  rinterface.c:8381:58: warning: variable 'c_instream' is uninitialized when used here [-Wuninitialized]
  rinterface.c:8441:59: warning: variable 'c_outstream' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10886:33: warning: variable 'c_a' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10886:38: warning: variable 'c_b' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10886:43: warning: variable 'c_eps' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10910:31: warning: variable 'c_a' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10910:36: warning: variable 'c_b' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10910:41: warning: variable 'c_eps' is uninitialized when used here [-Wuninitialized]
  rinterface.c:10917:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11610:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11632:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11646:28: warning: variable 'c_igraph_errno' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11653:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11761:31: warning: variable 'version_string' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11762:31: warning: variable 'major' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11763:31: warning: variable 'minor' is uninitialized when used here [-Wuninitialized]
  rinterface.c:11764:31: warning: variable 'subminor' is uninitialized when used here [-Wuninitialized]

Is this dead code? We can remove in rinterface.c and reconcile later, we have drift anyway.

There are more failures on Windows.

Can you please push to this branch?

Would we like to keep synchronized generated code with functions.yml?

krlmlr · 2024-01-12T15:13:11Z

We reconcile at some point, yes, but for now, drift is fine. I want to move to autogenerating everything, any Stimulus changes might have to be reverted anyway. We can edit interface.c and aaa-auto.R directly until after the release. Michael Antonov ***@***.***> schrieb am Fr. 12. Jan. 2024 um 16:06:

…

@Antonov548 <https://github.com/Antonov548>: We now have compilation warnings on a few platforms only, e.g., on macOS: rinterface.c:6349:84: warning: variable 'c_outfile' is uninitialized when used here [-Wuninitialized] rinterface.c:8381:58: warning: variable 'c_instream' is uninitialized when used here [-Wuninitialized] rinterface.c:8441:59: warning: variable 'c_outstream' is uninitialized when used here [-Wuninitialized] rinterface.c:10886:33: warning: variable 'c_a' is uninitialized when used here [-Wuninitialized] rinterface.c:10886:38: warning: variable 'c_b' is uninitialized when used here [-Wuninitialized] rinterface.c:10886:43: warning: variable 'c_eps' is uninitialized when used here [-Wuninitialized] rinterface.c:10910:31: warning: variable 'c_a' is uninitialized when used here [-Wuninitialized] rinterface.c:10910:36: warning: variable 'c_b' is uninitialized when used here [-Wuninitialized] rinterface.c:10910:41: warning: variable 'c_eps' is uninitialized when used here [-Wuninitialized] rinterface.c:10917:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized] rinterface.c:11610:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized] rinterface.c:11632:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized] rinterface.c:11646:28: warning: variable 'c_igraph_errno' is uninitialized when used here [-Wuninitialized] rinterface.c:11653:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized] rinterface.c:11761:31: warning: variable 'version_string' is uninitialized when used here [-Wuninitialized] rinterface.c:11762:31: warning: variable 'major' is uninitialized when used here [-Wuninitialized] rinterface.c:11763:31: warning: variable 'minor' is uninitialized when used here [-Wuninitialized] rinterface.c:11764:31: warning: variable 'subminor' is uninitialized when used here [-Wuninitialized] Is this dead code? We can remove in rinterface.c and reconcile later, we have drift anyway. There are more failures on Windows. Can you please push to this branch? Would we like to keep synchronized generated code with functions.yml? — Reply to this email directly, view it on GitHub <#1012 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANJGSZPRLZECFYX2IUTSB3YOFGQJAVCNFSM6AAAAABAZLGH5KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBZGQ2TQNBXHE> . You are receiving this because your review was requested.Message ID: ***@***.***>

szhorvat · 2024-01-12T15:49:24Z

Beware that most of these "uninitialized when used here" warning indicate actual bugs! If the affected function is used, that's a problem we need to fix now. Hopefully none of them are used?

krlmlr · 2024-01-13T15:20:43Z

Now there's only Windows and macOS failing, for good reason, I think.

Antonov548 · 2024-01-13T15:29:44Z

Now there's only Windows and macOS failing, for good reason, I think.

Yes, checking

szhorvat · 2024-01-13T16:17:56Z

src/rinterface_extra.c

-    igraph_errorf("Expecting a scalar integer but received a vector of length %zu.",
-                    __FILE__, __LINE__, IGRAPH_EINVAL, (size_t) Rf_xlength(value));
+    igraph_errorf("Expecting a scalar integer but received a vector of length %d.",
+                    __FILE__, __LINE__, IGRAPH_EINVAL, Rf_xlength(value));


@Antonov548 Why did you make this change? %d is only for int, and the return type of Rf_xlength() is system-dependent (may not be compatible with int!), but always fits in size_t. I chose %zu and a cast to size_t on purpose.

zu is not compatible with some versions of compiler. Maybe good solution will be to use lu?

What specific compiler is it not compatible with?

zu is standard in both C99 and C++11. Could it be that the proper standard is not set? I'd be very surprised to see any still-relevant compiler not support it.

According to tests wit Godbolt, works with Clang 3.4 and GCC 4.8. igraph itself wouldn't support anything earlier.

@szhorvat https://stackoverflow.com/questions/68900199/how-to-get-mingw-gcc-to-recognize-the-zu-format-specifier-for-size-t

Hm, looks like there still no agreement for zu.

What that SO answer is saying is that it does actually work but MinGW's GCC incorrectly warns about it. That is very disappointing.

You should be aware that on Windows the long type is 32-bit wide. Using long here is actually wrong. Can't we just use -Wno-format specifically on Windows?

@Antonov548 Can you try to use PRIuPTR instead? Here is an example:

https://godbolt.org/z/jcWrvWo9b

PRIuPTR should be defined to a string that is the appropriate format specifier for uintptr_t (which should fit size_t). I'm curious to see what this is with MinGW.

Seems uintptr_t works well. Let's see in ci.
I guess it's better to leave all warning enabled.

Yes, it's better to leave warnings enabled on all platforms.

Someone just told me about the __USE_MINGW_ANSI_STDIO macro. Defining this to either 1 or 0 may help (unclear).

But if PRIuPTR works, let's just stick to that, as it is standard.

maelle · 2024-01-15T12:12:34Z

@Antonov548 thanks! What are the windows and sanitizer failures?

maelle · 2024-01-15T12:36:29Z

I see #1110

Antonov548 · 2024-01-15T14:42:12Z

@Antonov548 thanks! What are the windows and sanitizer failures?

On windows there is warning in vendor folder, precise in core igraph. Not sure why the are not ignored. I though that vendor folder is ignored.

maelle · 2024-01-15T14:55:21Z

True, ^vendor$ is listed in .Rbuildignore so they should be ignored 🤔

szhorvat · 2024-01-15T15:09:37Z

@Antonov548 As a general rule, never use long in igraph code. long has an unpredictable width, and will cause trouble on Windows (where it is 32-bit).

maelle requested a review from krlmlr December 18, 2023 11:15

aviator-app bot force-pushed the maelle-patch-1 branch 2 times, most recently from 4fa994a to 3a734ed Compare December 21, 2023 13:47

krlmlr force-pushed the maelle-patch-1 branch from 3a734ed to dfc7422 Compare December 29, 2023 20:35

krlmlr added this to the upgrade milestone Dec 29, 2023

krlmlr assigned maelle Jan 8, 2024

maelle force-pushed the maelle-patch-1 branch from dfc7422 to f7f8dcb Compare January 9, 2024 11:48

krlmlr added the mergequeue label Jan 11, 2024

aviator-app bot force-pushed the maelle-patch-1 branch from f7f8dcb to 970ed1e Compare January 11, 2024 19:26

aviator-app bot added the blocked label Jan 11, 2024

krlmlr force-pushed the maelle-patch-1 branch 3 times, most recently from 82fc1b3 to 0dfb8d1 Compare January 11, 2024 19:44

krlmlr removed the blocked label Jan 11, 2024

aviator-app bot force-pushed the maelle-patch-1 branch 2 times, most recently from 4d23cb7 to e3e43ce Compare January 11, 2024 21:18

aviator-app bot added the blocked label Jan 11, 2024

krlmlr removed the blocked label Jan 11, 2024

aviator-app bot added the blocked label Jan 11, 2024

krlmlr removed the blocked label Jan 11, 2024

krlmlr approved these changes Jan 11, 2024

View reviewed changes

aviator-app bot added the blocked label Jan 11, 2024

maelle force-pushed the maelle-patch-1 branch from e4ee229 to 323e63a Compare January 12, 2024 13:42

szhorvat reviewed Jan 13, 2024

View reviewed changes

maelle and others added 12 commits January 16, 2024 09:11

ci: go back to stricter checks

aae2c2d

Remove autogenerated dead code

167c46e

Fix documentation

6a8a0b8

Autogenerate cpp11 glue

3a29728

Use sh

444193f

Tweak Makevars

be4e0a9

fix: remove broken functions

d29a598

fix: remove one more

b01855d

fix: change print type

a3b6f86

fix: use long int in error

b0ad3ef

use unsigned long

0de8954

using uintptr_t

b98dc71

maelle force-pushed the maelle-patch-1 branch from 9e8249a to b98dc71 Compare January 16, 2024 08:11

YAGNI

2da3305

krlmlr removed the blocked label Jan 16, 2024

aviator-app bot merged commit 4b5be17 into main Jan 16, 2024
14 checks passed

aviator-app bot deleted the maelle-patch-1 branch January 16, 2024 08:48

github-actions bot locked as resolved and limited conversation to collaborators Jan 16, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: go back to stricter checks #1012

ci: go back to stricter checks #1012

maelle commented Dec 18, 2023 •

edited

Loading

aviator-app bot commented Dec 18, 2023 •

edited

Loading

krlmlr commented Dec 18, 2023

maelle commented Dec 18, 2023

aviator-app bot commented Jan 11, 2024

aviator-app bot commented Jan 11, 2024

aviator-app bot commented Jan 11, 2024

krlmlr left a comment

aviator-app bot commented Jan 11, 2024

krlmlr commented Jan 11, 2024

Antonov548 commented Jan 12, 2024

krlmlr commented Jan 12, 2024 via email

szhorvat commented Jan 12, 2024

krlmlr commented Jan 13, 2024 •

edited

Loading

Antonov548 commented Jan 13, 2024

szhorvat Jan 13, 2024 •

edited

Loading

Antonov548 Jan 13, 2024

szhorvat Jan 13, 2024

szhorvat Jan 13, 2024

szhorvat Jan 13, 2024 •

edited

Loading

Antonov548 Jan 15, 2024

szhorvat Jan 15, 2024 •

edited

Loading

szhorvat Jan 15, 2024 •

edited

Loading

Antonov548 Jan 15, 2024

szhorvat Jan 15, 2024

maelle commented Jan 15, 2024

maelle commented Jan 15, 2024

Antonov548 commented Jan 15, 2024

maelle commented Jan 15, 2024

szhorvat commented Jan 15, 2024

ci: go back to stricter checks #1012

ci: go back to stricter checks #1012

Conversation

maelle commented Dec 18, 2023 • edited Loading

aviator-app bot commented Dec 18, 2023 • edited Loading

Current Aviator status

krlmlr commented Dec 18, 2023

maelle commented Dec 18, 2023

aviator-app bot commented Jan 11, 2024

aviator-app bot commented Jan 11, 2024

aviator-app bot commented Jan 11, 2024

krlmlr left a comment

Choose a reason for hiding this comment

aviator-app bot commented Jan 11, 2024

krlmlr commented Jan 11, 2024

Antonov548 commented Jan 12, 2024

krlmlr commented Jan 12, 2024 via email

szhorvat commented Jan 12, 2024

krlmlr commented Jan 13, 2024 • edited Loading

Antonov548 commented Jan 13, 2024

szhorvat Jan 13, 2024 • edited Loading

Choose a reason for hiding this comment

Antonov548 Jan 13, 2024

Choose a reason for hiding this comment

szhorvat Jan 13, 2024

Choose a reason for hiding this comment

szhorvat Jan 13, 2024

Choose a reason for hiding this comment

szhorvat Jan 13, 2024 • edited Loading

Choose a reason for hiding this comment

Antonov548 Jan 15, 2024

Choose a reason for hiding this comment

szhorvat Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

szhorvat Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

Antonov548 Jan 15, 2024

Choose a reason for hiding this comment

szhorvat Jan 15, 2024

Choose a reason for hiding this comment

maelle commented Jan 15, 2024

maelle commented Jan 15, 2024

Antonov548 commented Jan 15, 2024

maelle commented Jan 15, 2024

szhorvat commented Jan 15, 2024

maelle commented Dec 18, 2023 •

edited

Loading

aviator-app bot commented Dec 18, 2023 •

edited

Loading

krlmlr commented Jan 13, 2024 •

edited

Loading

szhorvat Jan 13, 2024 •

edited

Loading

szhorvat Jan 13, 2024 •

edited

Loading

szhorvat Jan 15, 2024 •

edited

Loading

szhorvat Jan 15, 2024 •

edited

Loading