Bug 90355 – [CMAKE] Items of IDL list should be absolute path.

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Ryuan Choi]

Because items of IDL list is relative path now, we append ${WEBCORE_DIR} before using it.

I think that items of IDL lists should be absolute path
in order to simplify and generalize related codes which uses IDL list.

[Ryuan Choi]

Created attachment 150352 [details]
Patch

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 150352 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150352&action=review

Sorry, I didn't really understand your rationale for this change in https://bugs.webkit.org/show_bug.cgi?id=90258#c3 or in this bug. It makes more sense to just adjust the ADD_CUSTOM_COMMAND calls somehow than do this hack (there shouldn't be a need for some files to be specified as absolute paths and other files not in the CMake files).

> Source/WebCore/PlatformBlackBerry.cmake:331
> -FOREACH (_idl ${WebCore_CPP_IDL_FILES})
> -    SET(IDL_FILES_LIST "${IDL_FILES_LIST}${WEBCORE_DIR}/${_idl}\n")
> -ENDFOREACH ()
> +STRING(REPLACE ";" "\n" IDL_FILES_LIST "${WebCore_CPP_IDL_FILES}")

This looks wrong; you're treating the list as a string here. Is there any particular reason to change this code?

[Ryuan Choi]

(In reply to comment #2)
> (From update of attachment 150352 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150352&action=review
> 
> Sorry, I didn't really understand your rationale for this change in https://bugs.webkit.org/show_bug.cgi?id=90258#c3 or in this bug. It makes more sense to just adjust the ADD_CUSTOM_COMMAND calls somehow than do this hack (there shouldn't be a need for some files to be specified as absolute paths and other files not in the CMake files).

OK. I will close this.
changing working directory is better.
thank you.

> > Source/WebCore/PlatformBlackBerry.cmake:331
> > -FOREACH (_idl ${WebCore_CPP_IDL_FILES})
> > -    SET(IDL_FILES_LIST "${IDL_FILES_LIST}${WEBCORE_DIR}/${_idl}\n")
> > -ENDFOREACH ()
> > +STRING(REPLACE ";" "\n" IDL_FILES_LIST "${WebCore_CPP_IDL_FILES}")
> 
> This looks wrong; you're treating the list as a string here. Is there any particular reason to change this code?

As my curiosity, is list string which contains semicolons?
I thought that this is the simplest way to make IDL_FILES_LIST if we don't need to insert ${WEBCORE_DIR}

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #3)
> > > Source/WebCore/PlatformBlackBerry.cmake:331
> > > -FOREACH (_idl ${WebCore_CPP_IDL_FILES})
> > > -    SET(IDL_FILES_LIST "${IDL_FILES_LIST}${WEBCORE_DIR}/${_idl}\n")
> > > -ENDFOREACH ()
> > > +STRING(REPLACE ";" "\n" IDL_FILES_LIST "${WebCore_CPP_IDL_FILES}")
> 
> As my curiosity, is list string which contains semicolons?
> I thought that this is the simplest way to make IDL_FILES_LIST if we don't need to insert ${WEBCORE_DIR}

According to CMake's documentation for the LIST() command, "A  list  in  cmake is a ; separated group of strings" (so that code isn't really wrong). This might save a few lines compared to the original loop, but makes the meaning of the code less clear.

[Ryuan Choi]

(In reply to comment #4)
> (In reply to comment #3)
> > > > Source/WebCore/PlatformBlackBerry.cmake:331
> > > > -FOREACH (_idl ${WebCore_CPP_IDL_FILES})
> > > > -    SET(IDL_FILES_LIST "${IDL_FILES_LIST}${WEBCORE_DIR}/${_idl}\n")
> > > > -ENDFOREACH ()
> > > > +STRING(REPLACE ";" "\n" IDL_FILES_LIST "${WebCore_CPP_IDL_FILES}")
> > 
> > As my curiosity, is list string which contains semicolons?
> > I thought that this is the simplest way to make IDL_FILES_LIST if we don't need to insert ${WEBCORE_DIR}
> 
> According to CMake's documentation for the LIST() command, "A  list  in  cmake is a ; separated group of strings" (so that code isn't really wrong). This might save a few lines compared to the original loop, but makes the meaning of the code less clear.

Thank you for your kind explanation.
I agree with you.