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

Fix #8524 - CPack packages missing the vast majority of files #8525

Merged
merged 2 commits into from
Feb 11, 2021

Conversation

jmarrec
Copy link
Contributor

@jmarrec jmarrec commented Feb 10, 2021

Pull request overview

cf https://cmake.org/pipermail/cmake/2017-February/065033.html

Started with git bisect, but it didn't prove terribly useful.

(py39)julien@EnergyPlus2 ((247efc8b0f...) %|BISECTING)$ git bisect log
git bisect start
# good: [d6cf92a2a7dc8ca3489d39792c4315f6f3bea957] Merge pull request #8470 from energy-plus/global-state-mixedAir
git bisect good d6cf92a2a7dc8ca3489d39792c4315f6f3bea957
# bad: [5eba847e594e32952faddc2fb9c9c224f3ca45bb] Try to fix Mac package now since brew cask install is disabled
git bisect bad 5eba847e594e32952faddc2fb9c9c224f3ca45bb
# bad: [ce501834287fc66d760650c5369dcfd0672bf3fd] Merge pull request #8275 from NREL/sam_pvwatts
git bisect bad ce501834287fc66d760650c5369dcfd0672bf3fd
# bad: [b0b1246f26b4e0f2ebce5e30b026ab598e366439] Merge remote-tracking branch 'origin/develop' into sam_pvwatts
git bisect bad b0b1246f26b4e0f2ebce5e30b026ab598e366439
# skip: [0772c6f8a1836321d1f8819bea8f22cb9b498358] fixing cmake policy
git bisect skip 0772c6f8a1836321d1f8819bea8f22cb9b498358
# skip: [148f7e7bb310d9ed0721031788a1aa2a2f92349c] paring down what is built in ssc
git bisect skip 148f7e7bb310d9ed0721031788a1aa2a2f92349c
# bad: [bd5dc502e96f656681c26df97be19b5eb9635dd0] updating patches.md for ssc so we remember how to update it later.
git bisect bad bd5dc502e96f656681c26df97be19b5eb9635dd0
# skip: [063662f931411f68e4914f22fd3fb8227c623a73] Merge remote-tracking branch 'origin/develop' into sam_pvwatts
git bisect skip 063662f931411f68e4914f22fd3fb8227c623a73
# bad: [63d2eeb2d1157cb51b2c2d50a541ab4ddf6289ee] updating ioref
git bisect bad 63d2eeb2d1157cb51b2c2d50a541ab4ddf6289ee
# skip: [85378217cf4d545b57a0c18885c44bf3ee0aaf2a] setting SAMAPI_EXPORT to off
git bisect skip 85378217cf4d545b57a0c18885c44bf3ee0aaf2a
# bad: [63fcfaa789a14fa61f2e8de216f6153726c41d1e] fixing shading bug and adding shading output
git bisect bad 63fcfaa789a14fa61f2e8de216f6153726c41d1e
# skip: [224e5d80c626499a504450ba955958759b666ec4] Merge commit '0c7663494d5d92686b4c627392f2e238b4ba2b3b' into sam_pvwatts
git bisect skip 224e5d80c626499a504450ba955958759b666ec4
# good: [a5805bc565d142f789b4fc22de7c8d14fa33e94c] Merge remote-tracking branch 'origin/develop' into sam_pvwatts
git bisect good a5805bc565d142f789b4fc22de7c8d14fa33e94c
# skip: [e0ad0272a8e7a774fdc2ed5317aa6ac46ec216ab] Merge commit '8fff8048908775cdc531a32f130f1efba089f85e' into sam_pvwatts
git bisect skip e0ad0272a8e7a774fdc2ed5317aa6ac46ec216ab
# skip: [cf92b7fa307bb58f9f2e9310018270ea7c3acf1f] Merge remote-tracking branch 'origin/develop' into sam_pvwatts
git bisect skip cf92b7fa307bb58f9f2e9310018270ea7c3acf1f
# good: [7fbf86d640918ae0e73c3c75e9d34825c7d54c67] Merge remote-tracking branch 'origin/develop' into sam_pvwatts
git bisect good 7fbf86d640918ae0e73c3c75e9d34825c7d54c67
# skip: [4c7b5996bd5c021147fe7fd65fd9d5bdc5d1f609] fixing cmake for ssc and adjusting pvwatts test
git bisect skip 4c7b5996bd5c021147fe7fd65fd9d5bdc5d1f609
# skip: [56f0291f856ba9d7e1ea7c15ee56433706bc862f] fixing indentation
git bisect skip 56f0291f856ba9d7e1ea7c15ee56433706bc862f
# skip: [8fff8048908775cdc531a32f130f1efba089f85e] Squashed 'third_party/ssc/' changes from 8cd75564c5..41e46913b1
git bisect skip 8fff8048908775cdc531a32f130f1efba089f85e
# skip: [bb5ac7d423e72a7be724648980a871e93eec4fdb] changing cmake_minimum_required to 3.10 for ssc
git bisect skip bb5ac7d423e72a7be724648980a871e93eec4fdb
# skip: [7f8d22e97da3d2604f15dae5ce5961f1d2f702fc] Merge remote-tracking branch 'origin/develop' into sam_pvwatts
git bisect skip 7f8d22e97da3d2604f15dae5ce5961f1d2f702fc
# skip: [0fd10822e39209dad8dbb217139caa5500149292] fixing ssc cmake options
git bisect skip 0fd10822e39209dad8dbb217139caa5500149292
# skip: [0c7663494d5d92686b4c627392f2e238b4ba2b3b] Squashed 'third_party/ssc/' changes from 45ff11cd1e..8cd75564c5
git bisect skip 0c7663494d5d92686b4c627392f2e238b4ba2b3b
# bad: [708862fa60fedd3eca875cc8ca5de3a76551d04a] adding empty ssc_equation_entry to avoid an empty ssc_equantion_table array
git bisect bad 708862fa60fedd3eca875cc8ca5de3a76551d04a
# skip: [247efc8b0f4253ecc68724c5ed9d91ee69d3d271] Merge remote-tracking branch 'origin/develop' into sam_pvwatts
git bisect skip 247efc8b0f4253ecc68724c5ed9d91ee69d3d271
# only skipped commits left to test
# possible first bad commit: [708862fa60fedd3eca875cc8ca5de3a76551d04a] adding empty ssc_equation_entry to avoid an empty ssc_equantion_table array
# possible first bad commit: [0fd10822e39209dad8dbb217139caa5500149292] fixing ssc cmake options
# possible first bad commit: [bb5ac7d423e72a7be724648980a871e93eec4fdb] changing cmake_minimum_required to 3.10 for ssc
# possible first bad commit: [148f7e7bb310d9ed0721031788a1aa2a2f92349c] paring down what is built in ssc
# possible first bad commit: [063662f931411f68e4914f22fd3fb8227c623a73] Merge remote-tracking branch 'origin/develop' into sam_pvwatts
# possible first bad commit: [85378217cf4d545b57a0c18885c44bf3ee0aaf2a] setting SAMAPI_EXPORT to off
# possible first bad commit: [4c7b5996bd5c021147fe7fd65fd9d5bdc5d1f609] fixing cmake for ssc and adjusting pvwatts test
# possible first bad commit: [e0ad0272a8e7a774fdc2ed5317aa6ac46ec216ab] Merge commit '8fff8048908775cdc531a32f130f1efba089f85e' into sam_pvwatts
# possible first bad commit: [8fff8048908775cdc531a32f130f1efba089f85e] Squashed 'third_party/ssc/' changes from 8cd75564c5..41e46913b1
# possible first bad commit: [cf92b7fa307bb58f9f2e9310018270ea7c3acf1f] Merge remote-tracking branch 'origin/develop' into sam_pvwatts
# possible first bad commit: [56f0291f856ba9d7e1ea7c15ee56433706bc862f] fixing indentation
# possible first bad commit: [0772c6f8a1836321d1f8819bea8f22cb9b498358] fixing cmake policy
# possible first bad commit: [224e5d80c626499a504450ba955958759b666ec4] Merge commit '0c7663494d5d92686b4c627392f2e238b4ba2b3b' into sam_pvwatts
# possible first bad commit: [0c7663494d5d92686b4c627392f2e238b4ba2b3b] Squashed 'third_party/ssc/' changes from 45ff11cd1e..8cd75564c5
# possible first bad commit: [7f8d22e97da3d2604f15dae5ce5961f1d2f702fc] Merge remote-tracking branch 'origin/develop' into sam_pvwatts
# possible first bad commit: [247efc8b0f4253ecc68724c5ed9d91ee69d3d271] Merge remote-tracking branch 'origin/develop' into sam_pvwatt

I dissected every cmakelist.txt until I found this block that looked fishy.

macro (install)
endmacro ()
add_subdirectory(jsoncpp)
macro (install)
_install(${ARGV})
endmacro(install)

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@jmarrec jmarrec added Defect Includes code to repair a defect in EnergyPlus DoNotPublish Includes changes that shouldn't be reported in the changelog labels Feb 10, 2021
@jmarrec jmarrec added this to the EnergyPlus 9.5.0 IOFreeze milestone Feb 10, 2021
@jmarrec jmarrec self-assigned this Feb 10, 2021
Comment on lines +143 to +145

# Skip install() commands for jsoncpp
add_subdirectory(jsoncpp EXCLUDE_FROM_ALL)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fix itself looks pretty simple, remove the override of the install command.

(An alternative to EXCLUDE_FROM_ALL is to modify the jsoncpp/CMakeLists.txt and comment out the install calls)

…nging in sam / SSC) was permanently overriding every `install()` command and producing an almost empty package

cf https://cmake.org/pipermail/cmake/2017-February/065033.html
@Myoldmopar
Copy link
Member

Not waiting on integration debug, thanks @jmarrec !

@Myoldmopar Myoldmopar merged commit ce25923 into develop Feb 11, 2021
@Myoldmopar Myoldmopar deleted the 8524_CPack_Regression branch February 11, 2021 16:59
@jmarrec
Copy link
Contributor Author

jmarrec commented Feb 12, 2021

The SSC stuff is still broken on mac though.

 +++ AutoDocs: Completed processing output variable audit: processed 4000 calls
PYTHON: Collecting Python dynamic library
PYTHON: Fixing up Python Dependencies on Mac
-- warning: embedded item does not exist '/libenergyplusapi.9.4.0.dylib'
-- 
warning: cannot resolve item '@executable_path/libenergyplusapi.9.4.0.dylib'
  possible problems:
    need more directories?
    need to use InstallRequiredSystemLibraries?
    run in install tree instead of build tree?
-- 
warning: cannot resolve item '@rpath/ssc.dylib'
  possible problems:
    need more directories?
    need to use InstallRequiredSystemLibraries?
    run in install tree instead of build tree?
-- warning: gp_resolved_file_type non-absolute file '@rpath/ssc.dylib' returning type 'other' -- possibly incorrect
-- 
warning: cannot resolve item '@rpath/ssc.dylib'
  possible problems:
    need more directories?
    need to use InstallRequiredSystemLibraries?
    run in install tree instead of build tree?
-- 
warning: cannot resolve item '@rpath/ssc.dylib'
  possible problems:
    need more directories?
    need to use InstallRequiredSystemLibraries?
    run in install tree instead of build tree?
-- warning: gp_resolved_file_type non-absolute file '@rpath/ssc.dylib' returning type 'other' -- possibly incorrect
CPack: -   Install component: WeatherData
CPack: - Run preinstall target for: ExpandObjects
CPack: - Install project: ExpandObjects []
CPack: -   Install component: Datasets
CPack: -   Install component: Documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defect Includes code to repair a defect in EnergyPlus DoNotPublish Includes changes that shouldn't be reported in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CPack packages missing the vast majority of files
6 participants