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

Fixes bin/trace so it doesn't dirty the repo when autogen runs #295

Merged
merged 2 commits into from
Feb 3, 2021

Conversation

derobins
Copy link
Member

No description provided.

bin/trace Outdated
@@ -207,7 +207,7 @@ $Source = "";
# Maximum length of H5TRACE macro line
# If the ColumnLimit in .clang-format is changed, this value will need to be updated
#
my $max_trace_macro_line_len = 110;
my $max_trace_macro_line_len = 109;
Copy link
Member Author

Choose a reason for hiding this comment

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

This line length keeps bin/trace and bin/format_source from fighting over long TRACE macros

Copy link
Member

Choose a reason for hiding this comment

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

I feel like there's a +1/-1 that's not respected somewhere then?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, but I'm unexcited about digging through a rat's nest of Perl to figure out where it's going wrong. I'm just going to turn the formatter off for that one line.

@@ -470,7 +470,7 @@ for $file (@ARGV) {
$file_args = 0;

# Ignore some files that do not need tracing macros
unless ($file eq "H5FDmulti.c" or $file eq "src/H5FDmulti.c" or $file eq "H5FDstdio.c" or $file eq "src/H5FDstdio.c") {
unless ($file eq "H5FDmulti.c" or $file eq "src/H5FDmulti.c" or $file eq "H5FDstdio.c" or $file eq "src/H5FDstdio.c" or $file eq "src/H5TS.c") {
Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing in H5TS.c has a TRACE macro and this is easier than screwing with bin/trace to get it to stop messing with the file.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a good solution. Those other files are the "public" examples and H5TS.c is not.

Copy link
Member Author

Choose a reason for hiding this comment

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

H5TS has no calls that require the TRACE macro and I have zero interest in debugging the trace script to fix its processing.

@derobins
Copy link
Member Author

Why is this failing the format checks?

@derobins
Copy link
Member Author

ARGH. NOW IT'S FIGHTING OVER ANOTHER FILE.

Copy link
Contributor

@qkoziol qkoziol left a comment

Choose a reason for hiding this comment

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

I don't think this is the correct direction to go. The bin/trace script needs to be updated, not hacks in other files.

@derobins
Copy link
Member Author

@qkoziol The bin/trace issues are due to your checkins, so if you want to do a better job at fixing the problems, have at it. I'm just tired of having to engineer around this nonsense. autogen.sh should NEVER dirty the repo on a fresh checkout. Allowing that is insanity.

@derobins
Copy link
Member Author

Between this and all the bad format decisions clang-format makes, I'm of a mind to get rid of the formatter altogether.

@qkoziol
Copy link
Contributor

qkoziol commented Jan 29, 2021

@qkoziol The bin/trace issues are due to your checkins, so if you want to do a better job at fixing the problems, have at it. I'm just tired of having to engineer around this nonsense. autogen.sh should NEVER dirty the repo on a fresh checkout. Allowing that is insanity.

Yeah, I can look at bin/trace and try to get it to stop arguing with clang-format. Especially since it's the component that we have actual control over...

@lrknox
Copy link
Collaborator

lrknox commented Feb 3, 2021

I'm going to merge this to stop continuing issues with other PRs. This PR makes just a few changes which can be reverted when a better solution is available.

@lrknox lrknox merged commit eac0599 into HDFGroup:develop Feb 3, 2021
@derobins derobins deleted the minor/fix_bin_trace_madness branch July 28, 2021 16:45
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