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

tools/docheck: fix for non-GNU OSes #12240

Closed
wants to merge 2 commits into from
Closed

Conversation

smlng
Copy link
Member

@smlng smlng commented Sep 16, 2019

Contribution description

This PR provides 2 fixes to run ./dist/tools/doccheck/check.sh on non-GNU (Linux) OSes such as FreeBSD. First it allows to overwrite the make command through the environment, which is needed bc FreeBSD provides non-GNU make by default and thus RIOT must use gmake instead.
And second, there was also an encoding error in generate-changelog.py bc on FreeBSD if no encoding is set for open it defaults to ascii but RIOT uses utf-8.

Testing procedure

Run make static-test or ./dist/tools/doccheck/check.sh on your (favourite) OS and it should still work as usual.

Issues/PRs references

#3392

@smlng smlng added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 16, 2019
@smlng smlng requested review from miri64 and aabadie September 16, 2019 09:15
@smlng smlng added the Area: tools Area: Supplementary tools label Sep 16, 2019
RIOT required GNU make for its build system. However not all OSes
provide GNU make by default, e.g. FreeBSD - on such system `gmake`
must be used instead. This PR allows to override the `make` command
used.
This fixes an encoding problem in the python helper script
generate-changelog.py by setting it to `utf-8`. On FreeBSD
the default seems to be `ascii` which resulted in an error.
@smlng
Copy link
Member Author

smlng commented Sep 16, 2019

@cladmi can you have a look on this one please? maybe you have a better way/idea on how to override make with gmake on non-GNU OSes such as FreeBSD? The idea is that the script can be invoked via make static-test but also standalone. Problem is that FreeBSD comes with a non-GNU make which raises errors when used with RIOT Makefiles.

@cladmi
Copy link
Contributor

cladmi commented Sep 16, 2019

For the python change, I do not think this is currently a reasonable idea.

Most of the things in RIOT assume that python is executed within an UTF-8 environment.
#11691

The issue you mention here seems like the FreeBSD used for testing, is not configured with an "adequate" locale as open uses the system default one

https://docs.python.org/3/library/functions.html#open

@cladmi
Copy link
Contributor

cladmi commented Sep 16, 2019

Also, almost everything tells to call 'make' directly, does it not make sense to ask to have a PATH where 'make' is 'gmake' ?
Changing this is in theory "possible", but not sure it is realistic or worth the effort of adding the indirection globally.

There is of course lots of false positive in this output but you get the idea:

git grep 'make' '**' ':!doc' ':!**/*.c' ':!**/*.h' | wc -l
1065

@smlng
Copy link
Member Author

smlng commented Sep 16, 2019

@cladmi you're right, setting locales to utf-8 stuff fixes that - will drop the 2nd commit, okay?

@smlng
Copy link
Member Author

smlng commented Sep 16, 2019

thx for pointing that out.

@smlng
Copy link
Member Author

smlng commented Sep 16, 2019

regarding your second comment: to me it would make sense to use ${MAKE} in all Makefiles, scripts and so on, to achieve easy reconfiguration of make command.

@cladmi
Copy link
Contributor

cladmi commented Sep 16, 2019

So also, documentation, README, examples should mention use 'make' or 'gmake' ?
And packages and external tools compilers updated to use $(MAKE) too if they use it directly.

I don't find it worth the effort for a platform that is not used.
Just ask to configure the system accordingly.

Windows users also have some setup to do anyway.

@smlng
Copy link
Member Author

smlng commented Sep 16, 2019

Just ask to configure the system accordingly.

so you mean by setting an alias?

@smlng
Copy link
Member Author

smlng commented Sep 16, 2019

mhm, makes sense to

@smlng
Copy link
Member Author

smlng commented Sep 16, 2019

closing, thx @cladmi for discussing this. alias make=gmake works for me.

@smlng smlng closed this Sep 16, 2019
@cladmi
Copy link
Contributor

cladmi commented Sep 16, 2019

I did not thought about putting an alias, I tend to have my '~/.bin' in the path where I do "script" aliases, but alias should work perfectly.

@smlng
Copy link
Member Author

smlng commented Sep 17, 2019

I did not thought about putting an alias, I tend to have my '~/.bin' in the path where I do "script" aliases, but alias should work perfectly.

yep, using ~/bin is even better, more versatile and also was the only option to get it work with FreeBSD as a Jenkins Slave.

@cladmi
Copy link
Contributor

cladmi commented Sep 24, 2019

was the only option to get it work with FreeBSD as a Jenkins Slave.

When you have a non interactive ssh session, you must manually source the environment with . /etc/profile if you want your environment to be included.
This should work to have the alias defined too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants