Skip to content

Allow blank iniFile and paramFile parameters#557

Merged
gquerret merged 5 commits intoRiverside-Software:mainfrom
lievendf:lievend/AllowBlankIniAndParamFile
Oct 13, 2025
Merged

Allow blank iniFile and paramFile parameters#557
gquerret merged 5 commits intoRiverside-Software:mainfrom
lievendf:lievend/AllowBlankIniAndParamFile

Conversation

@lievendf
Copy link
Copy Markdown
Contributor

@lievendf lievendf commented Sep 2, 2025

In the same veign as previous PR, this allows to specify a default value of "" for iniFile/paramFile when creating a generic build.xml script.
Current workaround is to create an "empty.pf" and specify that one as default fallback.
For ini files it seems files not found are ignored, but we shouldn't log an "Unable to find INI file " + iniFile.getAbsolutePath() + " - Skipping attribute" message when the file path is blank.

For some reason this PR seems to include the already merged changes from previous PR, although I have based my new PR on the updated main branch...

Lieven De Foor and others added 5 commits August 4, 2025 14:24
Only add clientMode, cpstream, cpinternal, cpcoll, cpcase, numsep and numdec when they are not empty ("").
This allows to create a generic build script where these attributes are always specified.
Ignore whitespace only values
… these as not specified (null)

This allows for a generic build script that falls back to "" as default iniFile/paramFile.
Currently you need an "empty.pf" as workaround for paramFile, while iniFile ignores files not found apparently...

// Client mode
if (clientMode != null) {
if (clientMode != null && !clientMode.trim().isEmpty()) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Regarding all these not cases:

  1. when are you passing in parameters that have trailing or leading spaces?
    2, in which case wouldn't a simple > "" be a lot cleaner?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the only parameter where spaces have an impact is -param. That was part of the previous PR, and I know that this changed the behavior of PCT, but I think it's better this way. Let me know if that could have any negative impact on your builds.
There's no easy way to trim properties in PCT, so the extra spaces can be a problem.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wouldn't a simple > "" be a lot cleaner?

doesn't work in Java.
The idea is not to pass -cpinternal -cpstream something as startup parameters when cpInternal attribute contains only one space (as that will prevent the AVM from starting).

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.

Regarding all these not cases:

  1. when are you passing in parameters that have trailing or leading spaces?
    2, in which case wouldn't a simple > "" be a lot cleaner?

No, my intention for this PR is to be able to create a reusable generic build script that gets its property values from a project specific file, and have default values in the build script itself (for when the file does not contain specific property). That way you can call e.g. pct:compile and always provide values for a fixed set of parameters. Since you can't define a property value as null, the way to work around this is to define it as "" instead, but then this needs proper handling on the receiving (Java) side...

@gquerret gquerret merged commit 45a9704 into Riverside-Software:main Oct 13, 2025
@lievendf
Copy link
Copy Markdown
Contributor Author

@gquerret , it appears the change i introduced didn't have the desired effect, When passing in empty string Ant converts this to a File object pointing to the current/working directory. Fixing this probably needs more than the simple change I thought was needed...

@gquerret
Copy link
Copy Markdown
Contributor

Confirmed. I'll take care of the change.

@gquerret
Copy link
Copy Markdown
Contributor

gquerret commented Mar 4, 2026

@lievendf Fixed in main branch, will be included in next release

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.

3 participants