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

Hopefully, fix the long-standing problem with h2root #15915

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

couet
Copy link
Member

@couet couet commented Jun 24, 2024

Using the C option (meaning C I.O) in hropen seems to fix the issue on Mac.

@couet couet requested a review from dpiparo June 24, 2024 13:06
@couet couet self-assigned this Jun 24, 2024
@couet couet requested a review from pcanal as a code owner June 24, 2024 13:06
Copy link

github-actions bot commented Jun 24, 2024

Test Results

    13 files      13 suites   2d 23h 41m 39s ⏱️
 3 029 tests  3 029 ✅ 0 💤 0 ❌
33 856 runs  33 856 ✅ 0 💤 0 ❌

Results for commit 4f15d91.

♻️ This comment has been updated with latest results.

@hahnjo
Copy link
Member

hahnjo commented Jun 25, 2024

As far as I can tell, this shouldn't make a difference because HROPEN defaults to to C unless F is specified:

IF (INDEX(CHOPT,'F').EQ.0) THEN
IC = MIN(LENOCC(CHOPT)+1,8)
CHOPT(IC:IC) = 'C'
ENDIF

@couet
Copy link
Member Author

couet commented Jun 27, 2024

@hahnjo it made on on MacOS. (long debug)

@@ -373,6 +373,7 @@ SUBROUTINE HRFILE(LUN,CHDIR,CHOPT)
ENDIF
10 CHMAIL='//'//CHTOP(NCHTOP)
CALL HCDIR(CHMAIL,' ')

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

@dpiparo dpiparo closed this Sep 9, 2024
@dpiparo dpiparo reopened this Sep 9, 2024
@hahnjo
Copy link
Member

hahnjo commented Sep 9, 2024

my comment from #15915 (comment) still stands: based on the code, there should be no change in behavior because C is already the default.

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.

4 participants