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

Select filenames based on predicate instead of glob() #365

Merged
merged 3 commits into from
Apr 5, 2018

Conversation

joakim-hove
Copy link
Contributor

Fixes: #364

Pre un-WIP checklist

  • Statoil tests pass locally

@bjornjensen
Copy link
Contributor

Tested on Windows.
Crash in util_fstat() because fileno is -1. This happens when called from util_file_difftime() because file1 argument has the value "D:\project\CeeSol\Data\reek_ensemble_demo\scratch\wbr\1_r001_reek\realization-1\iter-0\eclipse\model\."
I think this is because the pattern now used in stringlist_select_files() is "*"

@joakim-hove
Copy link
Contributor Author

Tested on Windows. [...]

Thank you for the detailed testing - that is very valuable; fix coming later today.

@joakim-hove
Copy link
Contributor Author

@bjornjensen : Will you give it another spin?

@joakim-hove joakim-hove force-pushed the filename-predicates branch 2 times, most recently from f7a312a to 022a002 Compare March 22, 2018 09:28
@bjornjensen
Copy link
Contributor

bjornjensen commented Mar 22, 2018

Assert fails on Windows.

Here both list size is bigger than 1 and file_type is ECL_DATA_FILE:
ecl_file_enum file_type = ecl_util_get_file_type( stringlist_iget( filelist , 0 ) , NULL , NULL); if ((stringlist_get_size( filelist ) > 1) && (file_type != ECL_SUMMARY_FILE)) util_abort("%s: internal error - when calling with more than one file - you can not supply a unified file - come on?! \n",__func__);

Call stack:
ecl_sum_data_fread__(ecl_sum_data_struct * data, __int64 load_end, const stringlist_struct * filelist) Line 984 C ecl_sum_data_fread(ecl_sum_data_struct * data, const stringlist_struct * filelist) Line 1042 C ecl_sum_fread_data(ecl_sum_struct * ecl_sum, const stringlist_struct * data_files, bool include_restart) Line 183 C ecl_sum_fread(ecl_sum_struct * ecl_sum, const char * header_file, const stringlist_struct * data_files, bool include_restart) Line 205 C ecl_sum_fread_alloc(const char * header_file, const stringlist_struct * data_files, const char * key_join_string, bool include_restart) Line 255 C

@bjornjensen
Copy link
Contributor

There was also a typo in stringlist.c line 807. Capital N in cFileName.

@joakim-hove
Copy link
Contributor Author

Hmmmm; a bit in the blind here. I have fixed the typo and added a printf() debug statement. Could you please rerun - and post the stdout message.

@bjornjensen
Copy link
Contributor

bjornjensen commented Mar 22, 2018

I tried to modify code locally by using the predicate in the Windows code, like in the Linux code.

WIN32_FIND_DATA file_data;
  HANDLE          file_handle;
  char * pattern  = util_alloc_filename( path , "*", NULL );

  file_handle = FindFirstFile( pattern , &file_data );
  if (file_handle != INVALID_HANDLE_VALUE) {
    do {
      if (util_string_equal(file_data.cFileName, "."))
        continue;

      if (util_string_equal(file_data.cFileName, ".."))
        continue;

      if (predicate && !predicate(file_data.cFileName))
          continue;

      char * full_path = util_alloc_filename( path , file_data.cFileName , NULL);
      stringlist_append_owned_ref( names , full_path );
    } while (FindNextFile( file_handle , &file_data) != 0);
    FindClose( file_handle );
  }
  free( pattern );

Test succeeded on Windows!

@joakim-hove
Copy link
Contributor Author

I tried to modify code locally by using the predicate in the Windows code, like in the Linux code.

Of course - what was I thinking? That was the whole point of the exercise .... fix coming a bit later today.

@bjornjensen
Copy link
Contributor

Now my tests on Windows are successful.

@joakim-hove joakim-hove force-pushed the filename-predicates branch 3 times, most recently from 4cc9583 to be2f6b1 Compare March 22, 2018 16:04
@joakim-hove
Copy link
Contributor Author

@bjornjensen : could you give this another spin on Windows; it was surprisingly painful to get all tests green.

@joakim-hove joakim-hove force-pushed the filename-predicates branch 2 times, most recently from b57e15c to d92136f Compare March 22, 2018 16:32
@joakim-hove
Copy link
Contributor Author

ping

@bjornjensen
Copy link
Contributor

In function ecl_smspec_load_restart() the variable smspec_header is null which leads to an exception in util_same_file(). It seems that the reason is because the path argument to ecl_util_alloc_exfilename() is "\D:\project\CeeSol\Data\norneRSTtest" which is not a valid path on Windows.

@joakim-hove
Copy link
Contributor Author

In function ecl_smspec_load_restart() the variable smspec_header is null ...

Thank you!

@joakim-hove
Copy link
Contributor Author

One more spin?

@bjornjensen
Copy link
Contributor

Works nicely.

@joakim-hove
Copy link
Contributor Author

Works nicely.

Thank you - at last!

@joakim-hove joakim-hove merged commit f3e2954 into equinor:master Apr 5, 2018
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.

2 participants