Skip to content

Fix #104, Short term build fix for implicit endian functions#106

Merged
astrogeco merged 3 commits intonasa:integration-candidatefrom
skliper:fix104-cmdutil-endian-hotfix
Jun 24, 2020
Merged

Fix #104, Short term build fix for implicit endian functions#106
astrogeco merged 3 commits intonasa:integration-candidatefrom
skliper:fix104-cmdutil-endian-hotfix

Conversation

@skliper
Copy link
Contributor

@skliper skliper commented Jun 23, 2020 •
edited
Loading

Describe the contribution
Quick temporary fix for implicit declaration of endian functions on some systems (RH/CentOs). Note the real fix will be #95, custom implementation of these functions and removal of the messy defines.

Fix #104

Testing performed
Built cleanly on CentOS (confirmed build error before change and error gone after)

Expected behavior changes
No build errors on CentOS

System(s) tested on

  • Hardware: Intel i5, WSL
  • OS: CentOS 7.6.1810
  • Versions: bundle w/ this change

Additional context
#95 is real fix, this is a quick fix to avoid build errors

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC

@skliper skliper added bug Something isn't working CCB:FastTrack CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jun 23, 2020
@skliper skliper added this to the 2.2.0 milestone Jun 23, 2020
jphickey
jphickey requested changes Jun 23, 2020
View reviewed changes
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Note that the original symbol _BSD_SOURCE_ (trailing underscore) was wrong, it is just _BSD_SOURCE. That might be why it didn't work.

Also glibc says you shouldn't directly set internal __USE_BSD symbol - this is computed by features.h based on _BSD_SOURCE.

Recommend to remove __USE_BSD and _BSD_SOURCE_. Or I suppose it can be just merged as-is if we are confident that this is just a short term thing.

@skliper
Copy link
Contributor Author

skliper commented Jun 23, 2020

Note that the original symbol _BSD_SOURCE_ (trailing underscore) was wrong, it is just _BSD_SOURCE. That might be why it didn't work.

Also glibc says you shouldn't directly set internal __USE_BSD symbol - this is computed by features.h based on _BSD_SOURCE.

Recommend to remove __USE_BSD and _BSD_SOURCE_. Or I suppose it can be just merged as-is if we are confident that this is just a short term thing.

Thanks for looking into it. I'll update and retest.

@skliper skliper force-pushed the fix104-cmdutil-endian-hotfix branch from 8d2e2f9 to 84c1756 Compare June 23, 2020 17:22
@skliper
Copy link
Contributor Author

skliper commented Jun 23, 2020

Retested on both Ubuntu and CentOS w/ no build errors

jphickey
jphickey approved these changes Jun 23, 2020
View reviewed changes
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Great, this is cleaner!

@astrogeco astrogeco changed the base branch from master to integration-candidate June 24, 2020 13:14
@astrogeco astrogeco merged commit ddee8c8 into nasa:integration-candidate Jun 24, 2020
@astrogeco astrogeco mentioned this pull request Jun 24, 2020
Merged
@astrogeco astrogeco added CCB-20200624 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jun 24, 2020
@skliper skliper deleted the fix104-cmdutil-endian-hotfix branch February 1, 2021 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jphickey jphickey jphickey approved these changes

Assignees

No one assigned

Labels

bug Something isn't working CCB:FastTrack

Projects

None yet

Milestone

Bootes

Development

Successfully merging this pull request may close these issues.

2.1.11 fails to build on RHEL 7.8

3 participants

@skliper @jphickey @astrogeco