Skip to content
This repository has been archived by the owner on Jan 31, 2022. It is now read-only.

[hotfix] no tag failure #7

Merged
merged 2 commits into from
Mar 6, 2020
Merged

Conversation

jsturdy
Copy link
Contributor

@jsturdy jsturdy commented Mar 5, 2020

As reported in cms-gem-daq-project/reedmuller-c#3, the script was failing to correctly parse the git info if a tag was not present, when using an older version of git
This was due to the fact that the shebang was set to fail on error, although the underlying issue had been fixed in the past.
This PR changes the shebang to remove the fail on error such that if no tag is present, a version of 0.0.0 is assumed, and a release of 0.0.X, where X is the number of commits since the initial commit, is made.

Additionally, updates to the README have been made to better describe some of the scripts and variables, as well as adding an example section for the local installation use case.

##### Move to `ctp7_modules` work area
```sh
cd /path/to/ctp7_modules
export PETA_STAGE=/path/to/local/installation/opt/gem-peta-stage
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this targeting legacy only releases? In the templated version of the ctp7_modules we are getting rid of $PETA_STAGE and exporting it here doesn't seems to be a proper move...

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 hotfix is general, the documentation update is for the new framework.
Point me to an issue where removal of PETA_STAGE is discussed and I can rework this, however, PETA_STAGE is currently used by all templated compatible code building (it is just not required to be set by the developer except in certain cases)

Copy link

@lpetre-ulb lpetre-ulb left a comment

Choose a reason for hiding this comment

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

Fixes the main issue. Not sure that the compilation method and variable names PETA_STAGE and PETA_PATH are the best, but it is okay for now.

* `PackagePath` is the base directory of the package, defaults to `$(ProjectPath)`
* `INSTALL_PREFIX` : where to install the project, defaults to `/`, may be overridden for non-system installation
* `INSTALL_PATH` : base directory of the installed project, defaults to `/opt/$(Project)`, **should not be overridden**
* `PETA_PATH` : base directory where cross-compilation libraries will be installed on the host, defaults to `/opt/gem-peta-stage`, **should not be overridden**

Choose a reason for hiding this comment

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

Why shouldn't PETA_PATH be overridden? Isn't it supposed the base directory for all boards filesystem roots?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because both PETA_PATH and INSTALL_PATH are intentionally designed to only provide a fixed directory structure within the package, while INSTALL_PREFIX is the thing that gives freedom of where to put this predefined package structure.
If you have a suggestion on wording to make this more clear, please do

Choose a reason for hiding this comment

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

Ok, it makes senses. I currently use PETA_PATH as PETA_STAGE because... it works.


* `PETA_STAGE` is the location where the compiler will look for headers and libraries when cross-compiling, defaults to `$(PETA_PATH)`, may be overridden if developing features in multiple projects simultaneously.

Choose a reason for hiding this comment

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

How should PETA_STAGE be overridden when working with multiple backends?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In an appropriate future update to mfZynq.mk, the flags and INSTALL_PATH would be wrapped inside target specific conditionals

diff --git a/mfZynq.mk b/mfZynq.mk
index 286d030..ddefba7 100644
--- a/mfZynq.mk
+++ b/mfZynq.mk
@@ -1,13 +1,13 @@
-PETA_STAGE?=$(PETA_PATH)/$(TARGET_BOARD)
+PETA_STAGE?=$(PETA_PATH)

 CFLAGS= -fomit-frame-pointer -pipe -fno-common -fno-builtin \
        -Wall -std=c++14 \
        -march=armv7-a -mfpu=neon -mfloat-abi=hard \
        -mthumb-interwork -mtune=cortex-a9 \
        -DEMBED -Dlinux -D__linux__ -Dunix -fPIC \
-       --sysroot=$(PETA_STAGE)
+       --sysroot=$(PETA_STAGE)/$(TARGET_BOARD)

-LDFLAGS+=--sysroot=$(PETA_STAGE)
+LDFLAGS+=--sysroot=$(PETA_STAGE)/$(TARGET_BOARD)

 INSTALL_PATH=/mnt/persistent/$(ShortProject)

Thus, the only developer change would be to point PETA_STAGE to the location where the compile time dependencies would live.

Choose a reason for hiding this comment

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

Ok, maybe in a future update. Currently, I have to set PETA_PATH (i.e., PETA_STAGE?=$(PETA_PATH)/$(TARGET_BOARD)) to get the desired behavior.

@jsturdy jsturdy merged commit 019d1f8 into feature/common-ci Mar 6, 2020
@jsturdy jsturdy deleted the hotfix/no-tag-failure branch March 6, 2020 08:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants