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

Improve make dist to warn on building inside a git repository #1458

Closed
wants to merge 28 commits into from

Conversation

fasterit
Copy link
Member

No description provided.

(exit 1); \
else :; \
fi
@if grep 'PACKAGE_VERSION.*-g' '$(top_distdir)/configure'; then \
echo 'WARNING: You are building a dist from a git version. Better run make dist outside of a .git repo on a tagged release.'>&2; \
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't get what the warning is for. What's wrong with doing make dist inside a git repo?
For what I've seen is that make dist shouldn't touch anything in the source directory.

Copy link
Member Author

@fasterit fasterit Apr 18, 2024

Choose a reason for hiding this comment

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

There's nothing wrong with it per se, but it's likely intent you want tagged releases for a dist tarball and not your random git commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

@fasterit
The message isn't clear on what the user is supposed to do. It's not a bug when make dist can work on a development version of the package (you will get a tarball named htop-3.4.0-dev-3.3.0-95-g2acd62d.tar.gz as #1456 is now merged in main). The warning doesn't help anything to the user.

@BenBE BenBE added the build system 🔧 Affects the build system rather then the user experience label Apr 17, 2024
@BenBE BenBE added this to the 3.4.0 milestone Apr 17, 2024
Explorer09 and others added 26 commits May 17, 2024 20:48
It is inappropriate to use the 'fd' name for 'FILE*' variables.
POSIX file descriptiors are of type 'int' and are distinguished from
ISO C stream pointers (type 'FILE*').

Rename these variables to 'fp', which is a preferred naming in POSIX.
(Note that ISO C preferred the 'stream' name for the variables.)

No code changes.
Deprecate the use of 'st' and other names. The 'sb' name is often seen
in example codes in Linux man pages. (The 'statbuf' name is sometimes
also used but I choose 'sb' name because it's shorter.)

No code changes.
Only happens with LTO, -fanalyzer, and -fsanitize=address,leak
The use of CUSTOM_METERMODE value in meter default mode was a bad
design. There are no meter that really has a "custom" mode to work with
and currently that value serves as a useless placeholder that hides the
real default mode for a meter.

Replace CUSTOM_METERMODE in `defaultMode` of all meters with the real
intended default modes. Currently only CPU meters and MemorySwapMeter
used this, and their real defaults are BAR_METERMODE.

In Meter_setMode(), remove the special treatment of `defaultMode ==
CUSTOM_METERMODE`, Meter_setMode() still calls the `updateMode` function
for custom treatment when it's present for a meter class.

As CUSTOM_METERMODE is obsolete from `defaultMode`, the init functions
of CPU meters and MemorySwapMeter need to be adjusted to avoid
incomplete initialization (Meter.draw function bring NULL).

Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
Let the respective `MeterClass.updateMode` functions initialize the
meter mode instead. The `updateMode` function is always called after the
`MeterClass.init` function by `Meter_new()`.

This not only simplifies the init functions of meter subclasses, but
avoids the use of the global `Meter_modes` array when it's unnecessary.

Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
Meter mode ID 0 will now be reserved.
The Meter objects have their own 'h' properties.
Avoid access to the `Meter_modes` array.

Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
The Meter objects have their own 'h' properties.
Avoid access to the `Meter_modes` array.

Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
This section reordering is a prerequisite to privatizing the
`Meter_modes` array.
All client codes that access this global `Meter_modes` array have been
replaced in previous commits, thus it's no longer necessary to keep
this internal information (draw functions, default heights, etc.)
public. It was also a bad idea when the client codes need to avoid
accessing `Meter_modes[0]` (which is reserved and contains null
information) by themselves.

Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
This is a prerequisite to using MeterModeId type in more headers, to
avoid header dependency hell.
All uses of meter drawing "mode" numbers now have the type
`MeterModeId`, including the uses in structures and arrays.
`MeterModeId` is defined as `unsigned int`, as (currently) it doesn't
save any code size by defining it to any smaller type.

Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
`__STDC_VERSION__` will be defined as 202311L for C23.
Assume the CGROUP, CCGROUP, CONTAINER and SECATTR field value may
contain non-ASCII characters and use RichString_appendWide() to convert
the strings.

Patch suggested by Christian Göttsche (@cgzones)

Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
The warning message is
"array subscript has type 'char' [-Wchar-subscripts]"

Fix this by casting to 'unsigned char' before passing any character to a
`<ctype.h>` function.

Also add an assertion to RichString_writeFromAscii() to ensure the
characters in the string are all in ASCII range.

Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
Since \n is used internally by htop to split command lines, replace \n
with \r in command lines to not display them as space.

Merging the command string will not work, but newlines in commands
should be rather the exception.
The arguments to our mbrtowc(3) wrapper should not alias, since they
also must not for mbrtowc(3).

Might help some compilers optimizing the code.
When parsing an essential pid entry file like 'status' fails, we treat
the process as a short-lived one and skip adding it into the process
table.

This should be done before the process is added, as the goto label used
for error handling can free the process structure, thus causing an
use-after-free scenario.

Fixes: 22d25db ("Linux: detect container process by different PID namespace")
Closes: htop-dev#1455
@fasterit
Copy link
Member Author

Merged as 8dfe7ed

@fasterit fasterit closed this May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build system 🔧 Affects the build system rather then the user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants