Skip to content

Conversation

@Jalakas
Copy link
Contributor

@Jalakas Jalakas commented Nov 28, 2025

Package Details

Maintainer: @jow-

Description:
Explicitly pass $(TARGET_LDFLAGS) to the build system to resolve MIPS linker errors related to missing -fPIC.

Fixes:

<artificial>:(.text+0x5130): relocation R_MIPS16_26 against `__stack_chk_fail' cannot be used when making a shared object; recompile with -fPIC
openwrt/staging_dir/toolchain-mips_24kc_gcc-15.2.0_musl/lib/gcc/mips-openwrt-linux-musl/15.2.0/../../../../mips-openwrt-linux-musl/bin/ld: non-dynamic relocations refer to dynamic symbol strcpy
openwrt/staging_dir/toolchain-mips_24kc_gcc-15.2.0_musl/lib/gcc/mips-openwrt-linux-musl/15.2.0/../../../../mips-openwrt-linux-musl/bin/ld: failed to set dynamic section sizes: bad value
collect2: error: ld returned 1 exit status

Build Testing Details

  • OpenWrt Version: SNAPSHOT
  • OpenWrt Target/Subtarget: ath79
  • OpenWrt Device: tplink_tl-wr842n-v3

Copy link
Member

@pprindeville pprindeville left a comment

Choose a reason for hiding this comment

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

LGTM

@hnyman
Copy link
Contributor

hnyman commented Nov 29, 2025

@Jalakas
Seems to fail a (new) CI check for commit title to be max 50 chars...
And that blocks the actual CI compile tests :-(

Please shorten the title.

@hnyman
Copy link
Contributor

hnyman commented Nov 30, 2025

Still fails. Likely one too many char. The new CI is really strict...
(50 is soft limits and 60 is the hard limit)

Checking subject:
lm-sensors: pass TARGET_LDFLAGS to fix shared library linking
[pass] Commit subject line format seems OK
[fail] Commit subject line is longer than 50 characters (is 61)
lm-sensors: pass TARGET_LDFLAGS to fix shared library linking

@BKPepe
I feel that this PR is one good example why I do not like the recent change that formalities check blocks the actual build tests. It would be good to first see if it builds ok, and then the title could be manually shorted at merging.

@Jalakas
Copy link
Contributor Author

Jalakas commented Nov 30, 2025

Likely one too many char.
[fail] Commit subject line is longer than 50 characters (is 61)
50 is soft limits and 60 is the hard limit

So, what is the real limit then, 50 or 60?

Couldn't this be more openly mentioned in https://github.com/openwrt/packages/blob/master/CONTRIBUTING.md

@hnyman
Copy link
Contributor

hnyman commented Nov 30, 2025

So, what is the real limit then, 50 or 60?

Hard to find. I only found out it yesterday...
The new check is recently authored by @GeorgeSapkin and confusingly always reports the 50 as the limit, although 60 is apparently the real hard limit.

openwrt/actions-shared-workflows@842b7b6

https://github.com/openwrt/actions-shared-workflows/blob/842b7b6f0e35378c92479f6c73cbff04d5a0b7cb/.github/scripts/check_formalities.sh#L89-L101

MAX_SUBJECT_LEN_HARD=60
MAX_SUBJECT_LEN_SOFT=50
...
	# Check subject length first for hard limit which results in an error and
	# otherwise for a soft limit which results in a warning. Show soft limit in
	# either case.

Explicitly pass $(TARGET_LDFLAGS) to the build system
to resolve MIPS linker errors related to missing -fPIC.

Signed-off-by: Anari Jalakas <[email protected]>
@BKPepe
Copy link
Member

BKPepe commented Nov 30, 2025

why I do not like the recent change that formalities check blocks the actual build tests. It would be good to first see if it builds ok, and then the title could be manually shorted at merging.

@hnyman I understand your point, however there's always a "but"...
What I'm seeing now is that waiting for the formality check takes too long:
image
And it's still waiting to be picked up by the runner. If we implement it as you suggest, the total time would increase even more, because preparing the test environment for each target and building packages (after the formality check passes) takes considerable time. The formality check itself can be resolved within a few minutes in ideal conditions—the most common issue is a missing Signed-off-by, and after fixing it and force pushing, it passes and can proceed to package compilation. :-)

If we allow it as in your example:

  • The formality check fails, packages get compiled anyway, then after fixing the formality check, packages would be compiled again (even after merge)—this just wastes runner resources and time.

This was previously discussed in #27592 and also in openwrt/actions-shared-workflows#55

@BKPepe
Copy link
Member

BKPepe commented Nov 30, 2025

Likely one too many char.
[fail] Commit subject line is longer than 50 characters (is 61)
50 is soft limits and 60 is the hard limit

So, what is the real limit then, 50 or 60?

Couldn't this be more openly mentioned in https://github.com/openwrt/packages/blob/master/CONTRIBUTING.md

This is explained in https://openwrt.org/submitting-patches , I see too many pages for new contributors.
Will check the soft and hard limit and it is a good idea to add it in there.

@GeorgeSapkin
Copy link
Member

GeorgeSapkin commented Nov 30, 2025

I don't want to derail the whole PR with OT, but I'll reply to some of the comments.

First of all there are the submissions guidelines, mentioned in the contribution guidelines:

As per the submissions guidelines:

commit subject
...

  • must be less than 50 characters long

So, what is the real limit then, 50 or 60?

Hard to find. I only found out it yesterday...
The new check is recently authored by @GeorgeSapkin and confusingly always reports the 50 as the limit, although 60 is apparently the real hard limit.

So the real limit is 50. Making it a soft limit with a warning, and error at 60 is to get a tiny bit of leeway when things are slightly off for good reasons. We can enforce the real limit as per the above guidelines, of course.

I didn't write the submissions guidelines, but I do feel it should be something that members of the project should encourage. The overall idea behind the checks is to improve the consistency and quality of the commit messages, since quite a few contributors don't follow the basic formatting, i.e. don't read either of the guidelines, despite the check in the template.

Suggestions are welcome in the actions repo (please cc me).

@hnyman hnyman merged commit 6469f06 into openwrt:master Nov 30, 2025
11 checks passed
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.

5 participants