Fix related to the issue 746: not tested yet!

[thomas-z8]

Hello,

sorry I did not test the changes yet, but thought with a pull request you can already see a proposal for the changes;

I will look further to see if I can build and start eclipse cdt with the new changes, for ensuring eclipse cdt is not broken because of me!

Regards
Thomas

[thomas-z8]

Hello,

I tried to build eclipse cdt with the new changes but I encountered the following error (Build Failure Error):
"[ERROR] Failed to execute goal org.eclipse.tycho.extras:tycho-document-bundle-plugin:4.0.6:javadoc (eclipse-javadoc) on project org.eclipse.remote.doc.isv: Failed to run javadoc: Failed to generate toc file: $USER_HOME\cdt-main\git\cdt\remote\org.eclipse.remote.doc.isv\html\reference\api\element-list (The system cannot find the file specified) -> [Help 1]"

I looked in the eclipse-cdt/cdt github repository and actually the file reference\api\element-list ist not present in the github repository and therefore it is not present in my local system neither;

Can you maybe help me further with this problem?

Regards
Thomas

[jonahgraham]

Not sure why it didn't build for you - I see you may be on Windows (\ vs / in path) and perhaps there is something platform dependent in the build script. I kicked off the build on GitHub actions, so we should know in a little while where we are.

[jonahgraham]

This LGTM - build needs to complete and someone needs to run it to make sure UI still looks correct.

My only question/concern is if the id (e.g id="org.eclipse.cdt.codan.internal.checkers.BlacklistProblem") is stored in the user space anywhere? With other projects (e.g Jenkins) that have made such improvements, the UI has been updated, but internal IDs have remained as to not break users when they upgrade.

[jonahgraham]

The modifications in ForbiddenlistCheckerTest.java need to be formatted, as the lengths of variables changed, the line wrapping needs to change too. I have applied the formatting change in the latest commit 8afc2f3

[thomas-z8]

Yes concerning the id you are right I think, I tried to add two functions to the checker in Eclipse under "Window -> Preferences -> Code Analysis" and the functions in the list are added to the following .prefs file in the workspace: ws\.metadata\.plugins\org.eclipse.core.runtime\.settings\org.eclipse.cdt.codan.core.prefs
There is now 2 lines in this .prefs file by me with the two functions I put into the list:

org.eclipse.cdt.codan.internal.checkers.BlacklistProblem=Warning
org.eclipse.cdt.codan.internal.checkers.BlacklistProblem.params={launchModes\=>{RUN_ON_FULL_BUILD\=>true,RUN_ON_INC_BUILD\=>true,RUN_ON_FILE_OPEN\=>false,RUN_ON_FILE_SAVE\=>false,RUN_AS_YOU_TYPE\=>true,RUN_ON_DEMAND\=>true},suppression_comment\=>"@suppress(\\"Function or method is blacklisted\\")",blacklist\=>(test_forbidden_function_1,test_forbidden_function_2)}

So I think you are right and changing the id would break users who are already using this checker.
I will try to push a new commit soon to take back the id to its first value.

[thomas-z8]

Or is there maybe a way to handle that better, and to make the checker work further for the existing users which use the "blacklist" id, while new users would use the new "forbiddenlist" id instead? (a kind of backward compatibility?)
I would like to try to implement such backward compatibility mechanism, but unfortunately if I cannot build eclipse with the new changes I cannot test the changes.
I will try to understand how the build with Github actions work, maybe I will be able to test the changes if the Github build action succeeds?

[thomas-z8]

Sorry, I have seen that the "code cleanliness checks" failed because a service segment version bump is needed for the bundle "org.eclipse.cdt.codan.checkers", but I cannot find out how to do this currently.
Maybe you can provide me with informations or documentation about how to do this?
Thank you and Regards
Thomas

[thomas-z8]

The build failure "Code Cleanliness Checks" advises to read the following documentation for understanding how to bump the service segment version of the bundle, but as far as I can understand the documentation it is not mentioned in which file exactly the version of the bundle is actually saved:
https://wiki.eclipse.org/Version_Numbering#When_to_change_the_service_segment - which has been moved to:
https://github.com/eclipse-platform/eclipse.platform/blob/master/docs/VersionNumbering.md

I will try to look further but yes if you could me an advice here it would help me a lot :-) !

Concerning the backward compatibility problem that we discussed, eventually I could also make a first pull request wich would only change the wording in the front-end, in a first step, and then in a second time I could make a second pull request which would also change the ids, names and other properties (with backward compatibility for not breaking users which already use this checker)

[thomas-z8]

And maybe the word "deny" is here more appropriate than "forbid" or "forbidden", I am not english native speaker but yes I think "deny" is here more appropriate (sounds less definitive, a function could have once been "denied" but then was improved and could now be "allowed").

I will also try to commit new changes with "deny" instead of "forbidden".

[thomas-z8]

I renamed the branch "issue_746" into "issue_746_full_version" (this branch will contain all word replacements including id and names properties, and backward compatibility):
https://github.com/thomas-z8/cdt/tree/issue_746_full_version
And I will create a second branch "issue_746_lite_version" which will only contain the replacement for the wording in the front-end; Then I will first make a pull request for "issue_746_lite_version", and the full version will come later.

[jonahgraham]

in which file exactly the version of the bundle is actually saved:

The file you need to change is the MANIFEST.MF's version, something like this:

diff --git a/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF b/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF
index e7817f8..de4ec01 100644
--- a/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF
+++ b/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@
 Bundle-ManifestVersion: 2
 Bundle-Name: %Bundle-Name
 Bundle-SymbolicName: org.eclipse.cdt.codan.checkers;singleton:=true
-Bundle-Version: 3.5.500.qualifier
+Bundle-Version: 3.5.600.qualifier
 Bundle-Activator: org.eclipse.cdt.codan.checkers.CodanCheckersActivator
 Require-Bundle: org.eclipse.core.runtime,
  org.eclipse.core.resources,

[jonahgraham]

In response to #769 (comment):

I also created #791 which is to explain more fully the error message from the build and updates the link to the new location.