Skip to content

Remove the 'preload' option#1304

Merged
stevenengler merged 1 commit intoshadow:devfrom
stevenengler:remove-preloadpath
Apr 26, 2021
Merged

Remove the 'preload' option#1304
stevenengler merged 1 commit intoshadow:devfrom
stevenengler:remove-preloadpath

Conversation

@stevenengler
Copy link
Copy Markdown
Contributor

Always get the preload path from the rpath. Closes #1215.

@stevenengler stevenengler added Type: Maintenance Refactoring, cleanup, documenation, or process improvements Component: Main Composing the core Shadow executable labels Apr 26, 2021
@stevenengler stevenengler self-assigned this Apr 26, 2021
@stevenengler stevenengler requested a review from robgjansen April 26, 2021 15:30
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 26, 2021

Codecov Report

Merging #1304 (d26af59) into dev (7f49974) will increase coverage by 0.02%.
The diff coverage is 73.46%.

❗ Current head d26af59 differs from pull request most recent head a739efb. Consider uploading reports for the commit a739efb to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #1304      +/-   ##
==========================================
+ Coverage   55.26%   55.29%   +0.02%     
==========================================
  Files         142      142              
  Lines       20713    20688      -25     
  Branches     5058     5044      -14     
==========================================
- Hits        11448    11440       -8     
+ Misses       6151     6144       -7     
+ Partials     3114     3104      -10     
Flag Coverage Δ
tests 55.29% <73.46%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/main/core/support/configuration.c 47.42% <0.00%> (-1.14%) ⬇️
src/main/core/controller.c 62.41% <73.91%> (+2.17%) ⬆️
src/main/core/manager.c 65.16% <100.00%> (+1.52%) ⬆️
src/main/core/worker.c 77.23% <0.00%> (-0.97%) ⬇️
src/main/core/support/options.c 62.16% <0.00%> (-0.93%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f49974...a739efb. Read the comment docs.

Copy link
Copy Markdown
Member

@robgjansen robgjansen left a comment

Choose a reason for hiding this comment

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

Looks good overall!

Comment on lines +511 to +517
gchar* preloadPath = _controller_scanRPathForPreloadShim();
if (preloadPath != NULL) {
message("found %s at %s", PRELOAD_SHIM_LIB_STR, preloadPath);
} else {
critical("could not find %s in rpath", PRELOAD_SHIM_LIB_STR);
return 1;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unless there is some advantage to doing this in the controller that I'm missing, I think the right place to do it is in the manager. Eventually, when we enhance shadow to utilize multiple machines, we'll have one manager per machine but only one single controller per simulation. Each machine (i.e., manager) will have to scan the rpath separately.

I think we can just move all of the code over to manager, and initiate the rpath scan in manager_new?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved this over to the manager.

Always get the preload path from the rpath.
@stevenengler stevenengler merged commit 564a83f into shadow:dev Apr 26, 2021
@stevenengler stevenengler deleted the remove-preloadpath branch April 26, 2021 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Main Composing the core Shadow executable Type: Maintenance Refactoring, cleanup, documenation, or process improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants